Hi everyone, does anyone have a better knowledge about replacing nodes in AST? Because what I saw is that there are methods like replaceSourceWith: that can be sent to an AST node with another AST node as a parameter. But this is not enough to get a new source code. Because when you ask the AST for newSource it generates a new source by taking replacements into account, parses the new source, compares it with itself and in case the new parse tree is equal it returns the new source. So itâs not enough to just use replaceSourceWith: but you also have to replace the current node with a new one it AST. Now the question is: am I doing something wrong, or we have strange API because we need to replace both source and node, replacing only one of them does not work⦠Uko
2017-01-23 14:48 GMT+01:00 Yuriy Tymchuk <yuriy.tymchuk@me.com>:
Hi everyone,
does anyone have a better knowledge about replacing nodes in AST? Because what I saw is that there are methods like replaceSourceWith: that can be sent to an AST node with another AST node as a parameter. But this is not enough to get a new source code. Because when you ask the AST for newSource it generates a new source by taking replacements into account, parses the new source, compares it with itself and in case the new parse tree is equal it returns the new source. So itâs not enough to just use replaceSourceWith: but you also have to replace the current node with a new one it AST.
Now the question is: am I doing something wrong, or we have strange API because we need to replace both source and node, replacing only one of them does not workâ¦
The API is strange : ) The nodeMapping and replacements are used by the tree rewriter. And the way the treerewriter works, it does not rewrite a AST-Tree but generates a new method by compiling the returned value, and it works on the returned value, the new/reformatted source text, not on a changed AST-Tree.
From the rewriter point of view (and some other parts of RB), the AST is more like a immutable structure. Only since when we started to use the RB-AST for the compiler, we started to use the AST for other purposes
(remember when we got into trouble, because some lint rules from critic browser were modifying the AST and we used that modified source as the method source, although the actual method source didn't changed )
Uko
On 01/23/2017 07:48 AM, Yuriy Tymchuk wrote:
Hi everyone,
does anyone have a better knowledge about replacing nodes in AST? Because what I saw is that there are methods like replaceSourceWith: that can be sent to an AST node with another AST node as a parameter. But this is not enough to get a new source code. Because when you ask the AST for newSource it generates a new source by taking replacements into account, parses the new source, compares it with itself and in case the new parse tree is equal it returns the new source. So itâs not enough to just use replaceSourceWith: but you also have to replace the current node with a new one it AST.
Now the question is: am I doing something wrong, or we have strange API because we need to replace both source and node, replacing only one of them does not workâ¦
This code was created so that some refactoring could be performed without having to format the entire parse tree. For example, converting direct variable access to use accessors should be able to rewrite the code without having to reformat. When you perform the replaceWith: on a node or the rewriter performs a node replacement, it calls the replaceMethodSource: method. That method calls the replaceSourceWith: method (replaceSourceWith: is a "private" method). The replaceSourceWith: methods test the source/replacement nodes for certain patterns. If the replacement fits the condition of the pattern, then it adds the appropriate text edit operations that should transform the old source into the new source. If the replaceSourceWith: doesn't recognize what is being done, then it dispatches the replaceSourceFrom: message to the replacement node. Currently, this handles cases were any section of code is being replaced with a variable or literal. Anyway, after all replacements are performed, it gets the new source like you explain. It performs a sanity check on the edited source. If it isn't parse tree equivalent to the rewritten tree, then it assumes that something went wrong in a replaceSourceWith: or replaceSourceFrom: method and uses the formatted code of the rewritten tree instead. John Brant
I think that we hacked too much :). I like how ParsetreeRewriter is working, but I think that it would be nice to have some API to say: ok, I have this node, now I want to replace it (maybe it should generate a new changed tree), so the pattern syntax would be more of a nice interface instead of a single entry point. Anyway, I also think that this substitution without reformatting is really cool. I need to double check it, because I think that many uses do not use auto fix from lint rules because it reformats their code. Cheers. Uko
On 23 Jan 2017, at 16:28, John Brant <brant@refactoryworkers.com> wrote:
On 01/23/2017 07:48 AM, Yuriy Tymchuk wrote:
Hi everyone,
does anyone have a better knowledge about replacing nodes in AST? Because what I saw is that there are methods like replaceSourceWith: that can be sent to an AST node with another AST node as a parameter. But this is not enough to get a new source code. Because when you ask the AST for newSource it generates a new source by taking replacements into account, parses the new source, compares it with itself and in case the new parse tree is equal it returns the new source. So itâs not enough to just use replaceSourceWith: but you also have to replace the current node with a new one it AST.
Now the question is: am I doing something wrong, or we have strange API because we need to replace both source and node, replacing only one of them does not workâ¦
This code was created so that some refactoring could be performed without having to format the entire parse tree. For example, converting direct variable access to use accessors should be able to rewrite the code without having to reformat.
When you perform the replaceWith: on a node or the rewriter performs a node replacement, it calls the replaceMethodSource: method. That method calls the replaceSourceWith: method (replaceSourceWith: is a "private" method). The replaceSourceWith: methods test the source/replacement nodes for certain patterns. If the replacement fits the condition of the pattern, then it adds the appropriate text edit operations that should transform the old source into the new source. If the replaceSourceWith: doesn't recognize what is being done, then it dispatches the replaceSourceFrom: message to the replacement node. Currently, this handles cases were any section of code is being replaced with a variable or literal.
Anyway, after all replacements are performed, it gets the new source like you explain. It performs a sanity check on the edited source. If it isn't parse tree equivalent to the rewritten tree, then it assumes that something went wrong in a replaceSourceWith: or replaceSourceFrom: method and uses the formatted code of the rewritten tree instead.
John Brant
2017-01-23 17:00 GMT+01:00 Yuriy Tymchuk <yuriy.tymchuk@me.com>:
I think that we hacked too much :). I like how ParsetreeRewriter is working, but I think that it would be nice to have some API to say: ok, I have this node, now I want to replace it (maybe it should generate a new changed tree), so the pattern syntax would be more of a nice interface instead of a single entry point.
Anyway, I also think that this substitution without reformatting is really cool. I need to double check it, because I think that many uses do not use auto fix from lint rules because it reformats their code.
Yes, I also hate extract temp/method because it reformats original method
On 01/23/2017 10:00 AM, Yuriy Tymchuk wrote:
I think that we hacked too much :). I like how ParsetreeRewriter is working, but I think that it would be nice to have some API to say: ok, I have this node, now I want to replace it (maybe it should generate a new changed tree), so the pattern syntax would be more of a nice interface instead of a single entry point.
Anyway, I also think that this substitution without reformatting is really cool. I need to double check it, because I think that many uses do not use auto fix from lint rules because it reformats their code.
The RB works at the AST level, so you always need to replace a node. If you want to specify your own custom source when doing a replacement, you could create a method like this: RBProgramNode>>replaceWith: aNode andSource: aString | method | parent ifNil: [ self error: 'This node doesn''t have a parent' ]. method := self methodNode. method notNil ifTrue: [ method map: self to: aNode ]. aNode parent: self parent. self addReplacement: (RBStringReplacement replaceFrom: self start to: self stop with: aString). parent replaceNode: self withNode: aNode Then you could use it with something like this: | source method | source := 'method ^1'. method := RBParser parseMethod: source. method body statements first value replaceWith: (RBParser parseExpression: '2 - 1') andSource: '2 - 1'. method newSource Here, the "-" and "1" should appear on separate lines. However, if you gave invalid source, you get the formatted version: | source method | source := 'method ^1'. method := RBParser parseMethod: source. method body statements first value replaceWith: (RBParser parseExpression: '2 - 1') andSource: '2 - 2'. method newSource Here the "2 - 2" expression in the replacement source, isn't equal to the "2 - 1" expression node that was provided so it uses the formatted code of the parse tree. John Brant
participants (4)
-
Denis Kudriashov -
John Brant -
Nicolai Hess -
Yuriy Tymchuk