[Refactoring] Fix generating accessors variable slots bug #18777
Conversation
When generating getter and setter for an instance variable on class side, they were generated on instance side. Therefore, I made the distinction between those two instance variable types when configuring the refactoring
|
I will do the tests but for now this works |
| } | ||
|
|
||
| { #category : 'testing' } | ||
| ClyClassDefinitionContext >> isInstanceSide [ |
There was a problem hiding this comment.
I found this one strange.
And I also wondered why we could not use metaLevelOf: even if I do not like the name.
There was a problem hiding this comment.
metaLevelOf: is for classes I think and there we want to check it for variables
|
It is because in the execute method we can have different contexts, so using isInstanceSide simplify the API for all of them, in there indeed: |
Ducasse
added
the
Status: Need more work
|
Alexis can you run the refactoring tests locally because it will give you useful feedback. |
|
|
||
|
|
||
| (ReGenerateAccessorsDriver new | ||
| targetClass: variables first definingClass; |
There was a problem hiding this comment.
The #definingClass actually gives you the correct class side object for class side variables.
You can detect the class side by:
variables first definingClass isInstanceSideNo need to analyze the toolContext at all. Variables are first class objects with the full information.
There was a problem hiding this comment.
It is shame that new driver system is based on raw string names for variables
There was a problem hiding this comment.
The goal is to make it easy to use for scripting.
When you call it in a playground it's not nice to have to get variable objects.
But maybe we can add some API to configure the driver with real variables and then the driver gets the name and side of the variable for the refactoring?
4 participants
When generating getter and setter for an instance variable on class side, they were generated on instance side. Therefore, I made the distinction between those two instance variable types when configuring the refactoring
Fixes #18771