On Fri, Nov 6, 2015 at 12:33 PM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

2015-11-06 16:17 GMT+01:00 Mariano Martinez Peck <marianopeck@gmail.com>:

...

Hi guys,

I have a very large kind of "rule engine" in which rules are written in smalltalk. These rules are saved as domain objects (imagine a rule kind of object) which end up having a closure. The code of those closure most of the time receives an argument and send messages to it. I want to add automatic method rename for rules. Let's say i have plenty of rules sending the message (rule) #price and now I want to refactor all senders to actually send #lastPrice.

I am taking a look to RBRenameMethodRefactoring and friends. But I wonder if there is something specifically for managing blocks rather than methods/classes?

Just write a RBParseTreeRewriter rule with specific elements to only activate inside blocks.

Thanks Thierry, that helped me to get started. Note that all my closures are clean (#isClean answering true, that is, they are all self contained), and I also have the 'string' of them. So I have no problem to just re generate them :) So this kind of worked: *| rewriter tree oldSeletor newSelector |* *"next: build the method-like string out of our closure strings"* *tree := RBParser parseMethod: 'DoIt ^ [:proxy | * * proxy at: #oldSelector.* * proxy oldSelector. * * proxy oldSelectorAnotherMethod.* * proxy do: ''oldSelectorAndSomething''.* * "a comment with #oldSelector "* * ]'.* *oldSeletor := #oldSelector. * *newSelector := #newSelector.* *oldSeletor numArgs > 0 ifTrue: [ self error: 'For the moment we only support renaming selectors without arguments'].* *rewriter := RBParseTreeRewriter replaceLiteral: oldSeletor with: newSelector. * *rewriter * * replace: '``@object ' , oldSeletor* * with: '``@object ' , newSelector.* *rewriter executeTree: tree. * *tree newSource* And that answers: *'DoIt ^ [:proxy | * * proxy at: #newSelector.* * proxy newSelector. * * proxy oldSelectorAnotherMethod.* * proxy do: ''oldSelectorAndSomething''.* * "a comment with #oldSelector "* * ]'*

Or a cascade: a RBParseTreeSearcher which matches blocks; on each block node you activate a rewriter.

However, there is an issue in rewriting blocks contents, because it supposes that the method defining the block is recompiled; it's significantly harder to make changes to the code of live blocks (change the bytescode itself? What if the block has multiples instances?).

Thierry

...

If not, I think my easiest path is to automatically compile dummy/temporal classes/methods from the rules, perform the refactor, then move source from methods to block closures, and finally remove created classes.

Any pointer is appreciated.

best,

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

Hi Mariano, Le 09/11/2015 01:13, Mariano Martinez Peck a écrit :

On Fri, Nov 6, 2015 at 1:59 PM, Mariano Martinez Peck <marianopeck@gmail.com <mailto:marianopeck@gmail.com>> wrote:

...

Hi Thierry,

I am trying to achieve a similar case like the one I commented in this post but I am unable to find the way. Previously I wanted to replace say the literal #oldSelector to #newSelector. Now, I need to use regular expressions. I mean, I would need to find matches of '*oldSelector*' and do the replace with 'newSelector'. For example, the search may find '*pre*oldSelector*post*' and I want it to be replaced by '*pre*newSelector*post'. *And of course, I don't know in advance what pre and post strings there could be.

What you need to do there is to use a match block after the @selector `{:node | node selector matches: '.*oldSelector.*'} Which would mean that your patterns become: rewriter replace: '``@object `oldSelector {:node | node selector matches: '.*oldSelector.*'}' with: '``@object `newSelector {:node | node selector }'. Wait, not entirely sure. I think you need to add a dictionary in the block to give the with: argument the pre and post string. And I'm not sure about the regular expression above as well. I need to have access to the SmaCC code source, so I'll answer to you a bit later.

Also..do you know where can I find some more info about RB? For example, if I read this:

rewriter replace: '``@object ' , oldSeletor with: '``@object ' , newSelector.

Where can I read that ``@object means XXX .. ?

There is a first level of explanation in the Pharo for the Enterprise book; but, yes the pattern language is fairly complex. Are you going to Smalltalks? John Brant is there and will give a talk on RB and SmaCC (two talks, I believe). Thierry

Thanks in advance!

Or a cascade: a RBParseTreeSearcher which matches blocks; on each block node you activate a rewriter.

However, there is an issue in rewriting blocks contents, because it supposes that the method defining the block is recompiled; it's significantly harder to make changes to the code of live blocks (change the bytescode itself? What if the block has multiples instances?).

Thierry

If not, I think my easiest path is to automatically compile dummy/temporal classes/methods from the rules, perform the refactor, then move source from methods to block closures, and finally remove created classes.

Any pointer is appreciated.

best,

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

2015-11-09 8:07 GMT+01:00 stepharo <stepharo@free.fr>:

mariano did you try the tool developed by mark rizun?

I believe what he's looking for is past Mark tool capabilities for now. Mark tool is something to invest on, certainly. But a small state of the art study on 'generalising refactorings' and related wouldn't hurt.

because I always fail to get the expression correct. We spent some times trying to document the pattern language but to us it is not really coherent.

I just asked John and he described a lot ;)

Jean-Christophe was expert in parse tree rewriting and he got lost.

I think RB is the oldest and probably the most powerful refactoring rewriting engine out there one can have a free access to (excluding general program transformation frameworks, but I consider those to be different beasts). Research afterwards decided that it should be considered a subset of model transformations and I think the know-how on such engines was lost. It doesn't help that it wasn't well published (the same goes with the DMS toolkit).

There is a chapter in PharoInProgress.

We are prototyping alternatives because we need better tools.

And for that, I'd say you're in for a lot of investment to beat Don Roberts and John Brant (not forgetting they were not alone at that time). Note that an extension of Mark tool would make new users and power users able to benefit immensely from the power of RB, and bring true progress on the usability of refactoring engines. Side note: John Brant is taking about RB and SmaCC at Smalltalks: use that occasion to ask questions! Thierry

On Mon, Nov 9, 2015 at 4:07 AM, stepharo <stepharo@free.fr> wrote:

mariano did you try the tool developed by mark rizun? because I always fail to get the expression correct.

I wasn't aware of this tool. I am checking it right now. Unfortunately, the smalltalk hub page has no documentation. So I must search a bit first.

We spent some times trying to document the pattern language but to us it is not really coherent. Jean-Christophe was expert in parse tree rewriting and he got lost.

There is a chapter in PharoInProgress.

I could not find it. Could you please send me the link? Thanks!

We are prototyping alternatives because we need better tools.

Stef

Le 9/11/15 06:38, Thierry Goubier a écrit :

Hi Mariano,

...

Le 09/11/2015 01:13, Mariano Martinez Peck a écrit :

...

On Fri, Nov 6, 2015 at 1:59 PM, Mariano Martinez Peck <marianopeck@gmail.com <mailto:marianopeck@gmail.com>> wrote:

...

...

Hi Thierry,

I am trying to achieve a similar case like the one I commented in this post but I am unable to find the way. Previously I wanted to replace say the literal #oldSelector to #newSelector. Now, I need to use regular expressions. I mean, I would need to find matches of '*oldSelector*' and do the replace with 'newSelector'. For example, the search may find '*pre*oldSelector*post*' and I want it to be replaced by '*pre*newSelector*post'. *And of course, I don't know in advance what pre and post strings there could be.

What you need to do there is to use a match block after the @selector

`{:node | node selector matches: '.*oldSelector.*'}

Which would mean that your patterns become:

rewriter replace: '``@object `oldSelector {:node | node selector matches: '.*oldSelector.*'}' with: '``@object `newSelector {:node | node selector }'.

Wait, not entirely sure. I think you need to add a dictionary in the block to give the with: argument the pre and post string. And I'm not sure about the regular expression above as well.

I need to have access to the SmaCC code source, so I'll answer to you a bit later.

...

Also..do you know where can I find some more info about RB? For example, if I read this:

rewriter replace: '``@object ' , oldSeletor with: '``@object ' , newSelector.

Where can I read that ``@object means XXX .. ?

There is a first level of explanation in the Pharo for the Enterprise book; but, yes the pattern language is fairly complex.

Are you going to Smalltalks? John Brant is there and will give a talk on RB and SmaCC (two talks, I believe).

Thierry

Thanks in advance!

...

Or a cascade: a RBParseTreeSearcher which matches blocks; on each block node you activate a rewriter.

However, there is an issue in rewriting blocks contents, because it supposes that the method defining the block is recompiled; it's significantly harder to make changes to the code of live blocks (change the bytescode itself? What if the block has multiples instances?).

Thierry

If not, I think my easiest path is to automatically compile dummy/temporal classes/methods from the rules, perform the refactor, then move source from methods to block closures, and finally remove created classes.

Any pointer is appreciated.

best,

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

On Mon, Nov 9, 2015 at 6:07 AM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

2015-11-09 6:38 GMT+01:00 Thierry Goubier <thierry.goubier@gmail.com>:

...

Hi Mariano,

Le 09/11/2015 01:13, Mariano Martinez Peck a écrit :

...

On Fri, Nov 6, 2015 at 1:59 PM, Mariano Martinez Peck <marianopeck@gmail.com <mailto:marianopeck@gmail.com>> wrote:

...

...

Hi Thierry,

I am trying to achieve a similar case like the one I commented in this post but I am unable to find the way. Previously I wanted to replace say the literal #oldSelector to #newSelector. Now, I need to use regular expressions. I mean, I would need to find matches of '*oldSelector*' and do the replace with 'newSelector'. For example, the search may find '*pre*oldSelector*post*' and I want it to be replaced by '*pre*newSelector*post'. *And of course, I don't know in advance what pre and post strings there could be.

What you need to do there is to use a match block after the @selector

`{:node | node selector matches: '.*oldSelector.*'}

Which would mean that your patterns become:

rewriter replace: '``@object `oldSelector {:node | node selector matches: '.*oldSelector.*'}' with: '``@object `newSelector {:node | node selector }'.

OK, thanks for the explanation.

...

Wait, not entirely sure. I think you need to add a dictionary in the block to give the with: argument the pre and post string. And I'm not sure about the regular expression above as well.

I need to have access to the SmaCC code source, so I'll answer to you a bit later.

Ok, here is a first try that make a change pre and post included:

| tree | RBParseTreeRewriter new replace: '`@object `oldSelector `{:node | node selector asString matchesRegex: ''.*oldSelector.*''}' with: '`@object newSelector'; executeTree: (tree := RBParser parseMethod: 'DoIt ^ [:proxy | proxy at: #oldSelector. proxy preoldSelector. proxy oldSelectorAnotherMethod. proxy do: ''oldSelectorAndSomething''. "a comment with #oldSelector " ]'). tree newSource

But it does not replace yet the right selector.

Yes, this one replaced both #oldSelector and #preoldSelector to #newSelector right?

For that, I will recreate a message send:

| tree | RBParseTreeRewriter new replace: '`object `oldSelector `{:node | node selector asString matchesRegex: ''.*oldSelector.*''} `@args' with: '`{RBMessageNode receiver: `object selector: (`oldSelector selector copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol arguments: `@args}'; executeTree: (tree := RBParser parseMethod: 'DoIt ^ [:proxy | proxy at: #oldSelector. proxy preoldSelector. proxy oldSelectorAnotherMethod. proxy do: ''oldSelectorAndSomething''. "a comment with #oldSelector " ]'). tree newSource

Thanks Thierry for your efforts. Very much appreciated. I tried the above but still did not work. I executed the very same example of yours (I pasted it) but the #newSource was: DoIt ^ [:proxy | proxy at: #oldSelector. proxy preoldSelector. proxy oldSelectorAnotherMethod. proxy do: 'oldSelectorAndSomething'. "a comment with #oldSelector " ] I am trying in Pharo 4.0. And you?

Now, I haven't tested with arguments on the message.

Yes..that's a different story. But for the time being, I can live without that :)

Look how you can use pattern variables inside the blocks :)

This is very cool and extremely powerfull.

Yes, I saw it. I wasn't aware of that. Super powerful but I think it has some learning curve to master it, right?

Thierry

...

...

Also..do you know where can I find some more info about RB? For example, if I read this:

rewriter replace: '``@object ' , oldSeletor with: '``@object ' , newSelector.

Where can I read that ``@object means XXX .. ?

There is a first level of explanation in the Pharo for the Enterprise book; but, yes the pattern language is fairly complex.

Are you going to Smalltalks? John Brant is there and will give a talk on RB and SmaCC (two talks, I believe).

Thierry

Thanks in advance!

...

Or a cascade: a RBParseTreeSearcher which matches blocks; on each block node you activate a rewriter.

However, there is an issue in rewriting blocks contents, because it supposes that the method defining the block is recompiled; it's significantly harder to make changes to the code of live blocks (change the bytescode itself? What if the block has multiples instances?).

Thierry

If not, I think my easiest path is to automatically compile dummy/temporal classes/methods from the rules, perform the refactor, then move source from methods to block closures, and finally remove created classes.

Any pointer is appreciated.

best,

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

Pharo 4.0 as well. I made a mistake in my example :(, so I'm adding more test cases to check ;).

| tree | RBParseTreeRewriter new replace: '``@object `oldSelector `{:node | node selector asString matchesRegex: ''.*oldSelector.*''}' with: '`{RBMessageNode receiver: ``@object selector: (`oldSelector copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol arguments: #()}'; executeTree: (tree := RBParser parseMethod: 'DoIt ^ [:proxy | proxy at: #oldSelector. proxy preoldSelector. self preoldSelector oldSelectorPost. proxy preoldSelector: proxy. proxy oldSelectorAnotherMethod. proxy do: ''oldSelectorAndSomething''. "a comment with #oldSelector " ]'). tree newSource.

Thanks, that worked much better! Here you have a much larger example/test: | rewriter tree oldSeletor newSelector | "closure method sourceCode" tree := RBParser parseMethod: 'DoIt ^ [:proxy | | oldSelector oldSelectorsuffix prefixoldSelector prefixoldSelectorSuffix | oldSelector := 1. oldSelectorsuffix := 1. prefixoldSelector := 1. prefixoldSelectorSuffix := 1. proxy at: #oldSelector. proxy at: #oldSelectorSuffix. proxy at: #prefixoldSelector. proxy at: #prefixoldSelectorSuffix. proxy oldSelector. proxy oldSelectorSuffix. proxy prefixoldSelector. proxy prefixoldSelectorSuffix. proxy do: ''oldSelector''. proxy do: ''oldSelectorSuffix''. proxy do: ''prefixoldSelector''. proxy do: ''prefixoldSelectorSuffix''. "a comment with #oldSelector " "a comment with #oldSelectorSuffix " "a comment with #prefixoldSelector " "a comment with #prefixoldSelectorSuffix " ]'. oldSeletor := #oldSelector. newSelector := #newSelector. rewriter := RBParseTreeRewriter new replace: '``@object `oldSelector `{:node | node selector asString matchesRegex: ''.*oldSelector.*''}' with: '`{RBMessageNode receiver: ``@object selector: (`oldSelector copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol arguments: #()}'. rewriter executeTree: tree. tree newSource I think I only miss the literal ones (for example proxy at: #oldSelector and the rest) which previously were catched by " RBParseTreeRewriter replaceLiteral: oldSeletor with: newSelector. " I guess I can apply a similar logic for with the regex. I will see if I can came up with that. If you happen to know how ;););) Yes, I will go to Smalltalks and I will give a talk. So I will ask him if there are remaining questions! BTW, later on today I will send you an screenshot to show you where you helped me :) Thanks!!! -- Mariano http://marianopeck.wordpress.com

Thierry, I have a last question. My last requirement will basically be to find ANY node that would match the regex *oldSelector* and rename it to *newSelector* being that.. a literal string, a literal symbol, a message send, a tempVar name, a comment, etc... I guess there is a simpler way that defining rewrite for each type of node right? I can always do a "(methodSource substrings detect: [:each | each matches: '*oldSelector'] ifNone: [#()] ) each2: [:each2 | copReplace. .... ] but RB is probably better. Any clues in this last requirement? On Mon, Nov 9, 2015 at 11:01 AM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

2015-11-09 14:45 GMT+01:00 Mariano Martinez Peck <marianopeck@gmail.com>:

...

...

Pharo 4.0 as well. I made a mistake in my example :(, so I'm adding more test cases to check ;).

| tree | RBParseTreeRewriter new replace: '``@object `oldSelector `{:node | node selector asString matchesRegex: ''.*oldSelector.*''}' with: '`{RBMessageNode receiver: ``@object selector: (`oldSelector copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol arguments: #()}'; executeTree: (tree := RBParser parseMethod: 'DoIt ^ [:proxy | proxy at: #oldSelector. proxy preoldSelector. self preoldSelector oldSelectorPost. proxy preoldSelector: proxy. proxy oldSelectorAnotherMethod. proxy do: ''oldSelectorAndSomething''. "a comment with #oldSelector " ]'). tree newSource.

Thanks, that worked much better!

Here you have a much larger example/test:

| rewriter tree oldSeletor newSelector | "closure method sourceCode" tree := RBParser parseMethod: 'DoIt ^ [:proxy | | oldSelector oldSelectorsuffix prefixoldSelector prefixoldSelectorSuffix | oldSelector := 1. oldSelectorsuffix := 1. prefixoldSelector := 1. prefixoldSelectorSuffix := 1. proxy at: #oldSelector. proxy at: #oldSelectorSuffix. proxy at: #prefixoldSelector. proxy at: #prefixoldSelectorSuffix. proxy oldSelector. proxy oldSelectorSuffix. proxy prefixoldSelector. proxy prefixoldSelectorSuffix. proxy do: ''oldSelector''. proxy do: ''oldSelectorSuffix''. proxy do: ''prefixoldSelector''. proxy do: ''prefixoldSelectorSuffix''. "a comment with #oldSelector " "a comment with #oldSelectorSuffix " "a comment with #prefixoldSelector " "a comment with #prefixoldSelectorSuffix " ]'. oldSeletor := #oldSelector. newSelector := #newSelector.

rewriter := RBParseTreeRewriter new replace: '``@object `oldSelector `{:node | node selector asString matchesRegex: ''.*oldSelector.*''}' with: '`{RBMessageNode receiver: ``@object selector: (`oldSelector copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol arguments: #()}'.

rewriter executeTree: tree. tree newSource

I think I only miss the literal ones (for example proxy at: #oldSelector and the rest) which previously were catched by " RBParseTreeRewriter replaceLiteral: oldSeletor with: newSelector. "

Yes, I didn't focus on this one. Didn't understood why it was there :)

Something like, for the match: '`#aLiteral {:node | node value isSymbol and: [node value matchesRegex: '.*oldSelector.*'] }'

I think it could be simplified; the #copyReplaceAll: probably works just all well on all message sends.

RBParseTreeRewriter new replace: '``@object `oldSelector' with: '`{RBMessageNode receiver: ``@object selector: (`oldSelector copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol arguments: #()}';

...

I guess I can apply a similar logic for with the regex. I will see if I can came up with that. If you happen to know how ;););)

Yes, I will go to Smalltalks and I will give a talk. So I will ask him if there are remaining questions! BTW, later on today I will send you an screenshot to show you where you helped me :)

Cool! Lucky you. Have fun :)

Thierry

...

Thanks!!!

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

Thierry, Sorry to bother again. Unfortunately, RB does not work out of the box in GemStone .... only the formatter part. grrr. Anyway, I am trying to make it work, and it's failing in this easy example: *| tree rewriter |* *tree := RBParser parseMethod: 'DoIt ^ [:proxy | proxy at: #oldSelector. ]'.* *rewriter := RBParseTreeRewriter new. * *rewriter * * replace: ('`#oldSelector `{:node | node value isSymbol and: [node value matchesRegex: ''.*oldSelector.*''] }')* * with: ('`{RBLiteralNode value: (`#oldSelector value copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol }').* *rewriter executeTree: tree. * *rewriter tree newSource* And the problem is the line: *with: ('`{RBLiteralNode value: (`#oldSelector value copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol }').* which gets translated to: [ :aDictionary | RBLiteralNode value: ((*self* lookupMatchFor: '`#oldSelector' in: aDictionary) value copyReplaceAll: 'oldSelector' with: 'newSelector') asSymbol ] The problem in GemStone is that that "self" is binded to nil and not to the receiver instance (RBPatternBlockNode). And so I get a Nil dnu.. *And I don't know how that closure is finally generated.* The message node: *(self lookupMatchFor: '`#oldSelector' in: aDictionary) * Its generated in: RBPatternBlockNode >> constructLookupNodeFor: aString in: aRBBlockNode | argumentNode | argumentNode := RBLiteralNode value: aString. ^RBMessageNode receiver:* (RBVariableNode named: 'self')* selector: #lookupMatchFor:in: arguments: (Array with: argumentNode with: aRBBlockNode arguments last) Any ideas? thanks in advance, On Mon, Nov 9, 2015 at 2:39 PM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

Le 09/11/2015 17:38, Mariano Martinez Peck a écrit :

...

Thierry, I have a last question. My last requirement will basically be to find ANY node that would match the regex *oldSelector* and rename it to *newSelector* being that.. a literal string, a literal symbol, a message send, a tempVar name, a comment, etc... I guess there is a simpler way that defining rewrite for each type of node right? I can always do a "(methodSource substrings detect: [:each | each matches: '*oldSelector'] ifNone: [#()] ) each2: [:each2 | copReplace. .... ]

I think that if it is really everywhere, then it may works better to just do text replace (methodSource copyReplaceAll: 'oldSelector' with: 'newSelector').

But remember that it will touch everything (pragmas, varNames, comments, etc.)

but RB is probably better.

...

It gives you more control to what you will change. But then you will have to write rewrites for all types of nodes (around 6 or 7?).

Any clues in this last requirement?

...

Well, one thing which may be interesting would be to consider encapsulating the rewrites in refactoring commands, and bring them as a composite change, so that you could launch a rewrite, recover all those method changes, apply (execute) them, check the result with the possibility of undoing them.

But I'm not familiar with that part of RB. I do consider it could become significant with EPICEA. It has been a long term project to have AltBrowser make all coding actions as refactoring commands so that EPICEA could log them properly, but its only partly there (and has been for years now)... and EPICEA is not yet here as well (but could be soon).

Thierry

-- Mariano http://marianopeck.wordpress.com

On Sat, Nov 21, 2015 at 7:07 AM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

Hi Mariano,

Le 20/11/2015 20:58, Mariano Martinez Peck a écrit :

...

Thierry,

Sorry to bother again. Unfortunately, RB does not work out of the box in GemStone .... only the formatter part. grrr.

There is maybe a message to send to the maintainer of RB in Gemstone :)

Thanks Thierry. Yes, I talked with Dale and the only "official" usage/port of RB is actually the formatter ;)

Anyway, I am trying to make it work, and it's failing in this easy example:

...

/| tree rewriter |/ /tree := RBParser parseMethod: 'DoIt ^ [:proxy | proxy at: #oldSelector. ]'./ /rewriter := RBParseTreeRewriter new./ /rewriter / /replace: ('`#oldSelector `{:node | node value isSymbol and: [node value matchesRegex: ''.*oldSelector.*''] }')/ /with: ('`{RBLiteralNode value: (`#oldSelector value copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol }')./ /rewriter executeTree: tree./ /rewriter tree newSource/ And the problem is the line:

/with: ('`{RBLiteralNode value: (`#oldSelector value copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol }')./

which gets translated to:

[ :aDictionary | RBLiteralNode value: ((*self* lookupMatchFor: '`#oldSelector' in: aDictionary) value copyReplaceAll: 'oldSelector' with: 'newSelector') asSymbol ]

The problem in GemStone is that that "self" is binded to nil and not to the receiver instance (RBPatternBlockNode). And so I get a Nil dnu.. *And I don't know how that closure is finally generated.*

The message node:

/(*self* lookupMatchFor: '`#oldSelector' in: aDictionary) /

Its generated in:

RBPatternBlockNode >> constructLookupNodeFor: aString in: aRBBlockNode | argumentNode | argumentNode := RBLiteralNode value: aString.

^RBMessageNode receiver:* (RBVariableNode named: 'self')* selector: #lookupMatchFor:in: arguments: (Array with: argumentNode with: aRBBlockNode arguments last)

Any ideas?

Yes. I think it is possible to work around it by using the dictionary itself, storing the node selector in it in the replace: pattern and retrieving it in the with: pattern.

rewriter replace: ('`#oldSelector `{:node :dic | (node value isSymbol and: [node value matchesRegex: ''.*oldSelector.*'']) ifTrue: [dic at: #selector put: node value. true] ifFalse: [false] }') with: ('`{:dic | RBLiteralNode value: ((dic at: #selector) copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol }').

Wow, that's very cool. I wasn't aware that I could pass arguments (like the dict in this case). This is powerful. Thank you RB Jedi.

Now, maybe correcting RB in Gemstone is something to do, as well.

Yes. So with the dict workaround you said, it DID work, but when I continue a little more and I found out where was the closure being generated, and that is RBPatternBlockNode >> #createBlockFor: And yeah, the way it was being generated the closure ("source evalauted") was binding "self" to nil. I found a way to make it bind the real self and that worked too. So...time to fork RB for GemStone and commit the fix :) BTW, do you image other cases like this? Thanks! Regards,

Thierry

thanks in advance,

...

On Mon, Nov 9, 2015 at 2:39 PM, Thierry Goubier <thierry.goubier@gmail.com <mailto:thierry.goubier@gmail.com>> wrote:

Le 09/11/2015 17:38, Mariano Martinez Peck a écrit :

Thierry, I have a last question. My last requirement will basically be to find ANY node that would match the regex *oldSelector* and rename it to *newSelector* being that.. a literal string, a literal symbol, a message send, a tempVar name, a comment, etc... I guess there is a simpler way that defining rewrite for each type of node right? I can always do a "(methodSource substrings detect: [:each | each matches: '*oldSelector'] ifNone: [#()] ) each2: [:each2 | copReplace. .... ]

I think that if it is really everywhere, then it may works better to just do text replace (methodSource copyReplaceAll: 'oldSelector' with: 'newSelector').

But remember that it will touch everything (pragmas, varNames, comments, etc.)

but RB is probably better.

It gives you more control to what you will change. But then you will have to write rewrites for all types of nodes (around 6 or 7?).

Any clues in this last requirement?

Well, one thing which may be interesting would be to consider encapsulating the rewrites in refactoring commands, and bring them as a composite change, so that you could launch a rewrite, recover all those method changes, apply (execute) them, check the result with the possibility of undoing them.

But I'm not familiar with that part of RB. I do consider it could become significant with EPICEA. It has been a long term project to have AltBrowser make all coding actions as refactoring commands so that EPICEA could log them properly, but its only partly there (and has been for years now)... and EPICEA is not yet here as well (but could be soon).

Thierry

-- Mariano http://marianopeck.wordpress.com

-- Mariano http://marianopeck.wordpress.com

...

Yes. I think it is possible to work around it by using the dictionary itself, storing the node selector in it in the replace: pattern and retrieving it in the with: pattern.

rewriter replace: ('`#oldSelector `{:node :dic | (node value isSymbol and: [node value matchesRegex: ''.*oldSelector.*'']) ifTrue: [dic at: #selector put: node value. true] ifFalse: [false] }') with: ('`{:dic | RBLiteralNode value: ((dic at: #selector) copyReplaceAll: ''oldSelector'' with: ''newSelector'') asSymbol }').

Wow, that's very cool. I wasn't aware that I could pass arguments (like the dict in this case). This is powerful.

Yes, quite significant. Also the fact the dictionary is cleared for each match (so that it can be reused for the next match in the same ast).

awesome.

Thank you RB Jedi.

...

I've had the guidance of John Brant (and spent some time over the SmaCC code where he uses it)...

Now, maybe correcting RB in Gemstone is something to do, as well.

...

Yes. So with the dict workaround you said, it DID work, but when I continue a little more and I found out where was the closure being generated, and that is RBPatternBlockNode >> #createBlockFor: And yeah, the way it was being generated the closure ("source evalauted") was binding "self" to nil. I found a way to make it bind the real self and that worked too. So...time to fork RB for GemStone and commit the fix :)

You're now the maintainer of RB for GemStone! Congratulations :)

ahhahahaha. You know what is funny? I run all AST-Tests-Core from Pharo and NONE halt on #createBlockFor: ... I even run all those from Refactoring-Tests-Core. *So..it seems there is not tests that covers the usage of #createBlockFor: :(*

BTW, do you image other cases like this?

...

You mean storing those RB rules? I try to collect them if I see some (or participate to), or remember where I see them.

No, I mean the compilation of block closures that would try to bind "self". Anyway, I will search if I find more.

I used to know where the RBRewriteTree chapter was in the Pharo books, but it seems to have disappeared, so I have recreated a copy that I'll try to update.

Thanks, please share it with me when you made it because I could not find it weeks ago.

Thanks!

...

You're welcome. You are giving opportunities to improve which is what I need (need to practice, need to practice, need to practice...).

ahahahahahha :)

I have ideas for Mark Rizun's tool, but I don't have the time to work on them.

Thierry

Thanks ! -- Mariano http://marianopeck.wordpress.com

On Tue, Nov 24, 2015 at 3:04 AM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

Hi Mariano,

Le 22/11/2015 23:31, Mariano Martinez Peck a écrit :

...

...

...

You know what is funny? I run all AST-Tests-Core from Pharo and NONE halt on #createBlockFor: ... I even run all those from Refactoring-Tests-Core. *So..it seems there is not tests that covers the usage of #createBlockFor: :(*

Ok. I'll have a scan of all the RB tests and add additional tests then. There is an issue John pointed to Nicolai and me that needs solving as well.

OK. Great. Once you do it let me know and I will see if I can port and make them work for GemStone.

BTW, do you image other cases like this?

...

You mean storing those RB rules? I try to collect them if I see some (or participate to), or remember where I see them.

No, I mean the compilation of block closures that would try to bind "self". Anyway, I will search if I find more.

Thanks.

I used to know where the RBRewriteTree chapter was in the Pharo

...

books, but it seems to have disappeared, so I have recreated a copy that I'll try to update.

Thanks, please share it with me when you made it because I could not find it weeks ago.

My fork of the Pharo for the enterprise book has it:

https://github.com/ThierryGoubier/PharoForTheEnterprise-english

But I don't have a CI building it yet.

Thanks! I will read it today! -- Mariano http://marianopeck.wordpress.com