[Refactoring] Driver improvement for temp to instance variable #18965
Conversation
…aro into refactoring/newdriver
src/Refactoring-Core/RBTemporaryToInstanceVariableRefactoring.class.st
Outdated
Show resolved
Hide resolved
|
|
||
| ^ ReVariablesNotReadBeforeWrittenCondition new | ||
| subtree: parseTree; | ||
| variables: temporaryVariableName; |
There was a problem hiding this comment.
Why variables: tempVarName and not variable? or variables: { tempVarName }
| { #category : 'transforming' } | ||
| RBTemporaryToInstanceVariableRefactoring >> privateTransform [ | ||
|
|
||
| self removeTemporaryOfClass: class. |
There was a problem hiding this comment.
I do not really undersyand what is removeTemporyOfClass: doing so it is difficult for me to get further.
Trying to understand the logic: I think that there were something we should keep.
If one of the subclass got the same instance variable that the temp we want to convert, then we remove it from that class and defines it in the root class.
So why removing this logic.
Because now if a subclass has the variable with the same name we force the guy to do a pullUp.
May be this is better this way the developer has to see it.
src/Refactoring-Core/ReMultipleMethodsDontReferToTempVarsCondition.class.st
Outdated
Show resolved
Hide resolved
| #package : 'Refactoring-DataForTesting', | ||
| #tag : 'ForConvertingVariables' | ||
| } | ||
|
|
There was a problem hiding this comment.
We should add the scenario where a subclass has the same name as the temp that we want to convert.
|
|
||
| { #category : 'execution' } | ||
| ReConvertTemporaryToInstanceVariableDriver >> handleBreakingChanges [ | ||
|
|
There was a problem hiding this comment.
In latest Pharo you can remove this method because it is done in a different way in the superclass.
Ducasse
added
the
Status: Need more work
|
This test has a failing test that was not there before I think testExternalBasicToolsDependencies MacOSX64.System.Dependencies.Tests.SystemDependenciesTest Given Collections do not match! additions : #('Calypso-SystemTools-QueryBrowser') missing: #() |
3 participants
Implemented fully a driver for ReTemporaryToInstanceVariableRefactoring (which was not used actually, now is) with also new driver tests that covers all the changes