Fix variables from being renamed with a property #7186
Conversation
| @@ -69,16 +71,21 @@ class GD_CORE_API ExpressionPropertyReplacer | |||
| void OnVisitNumberNode(NumberNode& node) override {} | |||
| void OnVisitTextNode(TextNode& node) override {} | |||
| void OnVisitVariableNode(VariableNode& node) override { | |||
| if (isParentTypeAVariable) { | |||
| // Do nothing, it's a variable. | |||
| if (node.child) node.child->Visit(*this); | |||
There was a problem hiding this comment.
Is it useful to explore the node's children if this one is __ (I'm not sure what it means)
There was a problem hiding this comment.
Yes, because of expression like: MyVariable.MyChild[MyProperty].
There was a problem hiding this comment.
In other words, we must continue to explore because properties could be used in children (becuase a variable node can have sub variables, or bracket accessors, which allows to enter a sub-expression inside the brackets ([MyProperty]. [1 + MyProperty]...), and this sub-expression must be explored to rename the property that could be there.
| auto& propertiesContainersList = | ||
| projectScopedContainers.GetPropertiesContainersList(); | ||
|
|
||
| // Match the potential *new* name of the property, because refactorings are | ||
| // done after changes in the variables container. | ||
| projectScopedContainers.MatchIdentifierWithName<void>( |
There was a problem hiding this comment.
I don't understand why this "MatchIdentifierWithName" is not doing its job? If it's a variable, it should match it (and so do nothing), no?
There was a problem hiding this comment.
I think isParentTypeAVariable is still required because when a node is an identifier, we don't want to replace it if the expression is a variable parameter even if the variable doesn't exist.
MatchIdentifierWithName doesn't know about the expected type for the current node. This is why isParentTypeAVariable is needed.
| "MyExtension::GetVariableAsNumber(MyVariable.MyChild[MyRenamedProperty])"); | ||
| } | ||
|
|
||
| SECTION("(Events based behavior) property not renamed (in variable parameter)") { |
There was a problem hiding this comment.
This is the test for it.
No description provided.