AST node replacement API

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

AST node replacement API

Uko2
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
Reply | Threaded
Open this post in threaded view
|

Re: AST node replacement API

Nicolai Hess-3-2


2017-01-23 14:48 GMT+01:00 Yuriy Tymchuk <[hidden email]>:
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

Reply | Threaded
Open this post in threaded view
|

Re: AST node replacement API

John Brant-2
In reply to this post by Uko2


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

Reply | Threaded
Open this post in threaded view
|

Re: AST node replacement API

Uko2
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 <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|

Re: AST node replacement API

Denis Kudriashov

2017-01-23 17:00 GMT+01:00 Yuriy Tymchuk <[hidden email]>:
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
Reply | Threaded
Open this post in threaded view
|

Re: AST node replacement API

John Brant-2
In reply to this post by Uko2
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