[Experiment] Automatic rewriting of sends to deprecated methods

Hi,

Small experiment:

1) add this method to Object:

deprecated: anExplanationString rule: aRule
        | builder ast rewriteRule method |
        builder := RBCompositeRefactoryChange named: 'deprecation'.
        method := thisContext sender sender method.
        ast := method ast copy.
        rewriteRule := RBParseTreeRewriter new replace: aRule key with: aRule value.

        (rewriteRule executeTree: ast) ifTrue: [
         builder compile: rewriteRule tree formattedCode in: method methodClass classified: method protocol ].
        builder execute.

2) to see it in action, add it to #ifNotNilDo:  in UndefinedObject:

ifNotNilDo: aBlock
        "Please use #ifNotNil: instead"
        self
                deprecated: 'Please use #ifNotNil: instead'
                rule: '`@receiver ifNotNilDo: `@statements'->  '`@receiver ifNotNil: `@statements'.
        ^ self


—> open a browser and see how it fixes code magically.

The only problem is that it does a whole-method refactoring while it should only affect the node
that triggered the deprecation. Else there could be wrong transformations if the same selector
is used but only one of the implementations need to be rewritten, while if we are able to rewrite
the exact sender, we can do it fully automatically.

        Marcus

As far as I'm concerned, it's not based on my tool.

But it can be integrated with Rewrite Tool ;), it’s not so easy for everyone to write down a rewrite rule from scratch.

No, just plain old ParseTreeRewriter.. but integrating Mark’s tool would be very nice as a next step.

But the tool of mark is a ui tool so why would you want to get a tool to define a rule in your case?

In the future if mark and camille come up with a better rewrite engine then of course we will use it.

No, just plain old ParseTreeRewriter.. but integrating Mark’s tool would be very nice as a next step.

On 30 Aug 2015, at 14:34, Peter Uhnák <[hidden email]> wrote:

Is this based on Mark's Rewrite Tool?

On Sun, Aug 30, 2015 at 10:27 AM, Marcus Denker <[hidden email]> wrote:

Hi,

Small experiment:

1) add this method to Object:

deprecated: anExplanationString rule: aRule
        | builder ast rewriteRule method |
        builder := RBCompositeRefactoryChange named: 'deprecation'.
        method := thisContext sender sender method.
        ast := method ast copy.
        rewriteRule := RBParseTreeRewriter new replace: aRule key with: aRule value.

        (rewriteRule executeTree: ast) ifTrue: [
         builder compile: rewriteRule tree formattedCode in: method methodClass classified: method protocol ].
        builder execute.

2) to see it in action, add it to #ifNotNilDo:  in UndefinedObject:

ifNotNilDo: aBlock
        "Please use #ifNotNil: instead"
        self
                deprecated: 'Please use #ifNotNil: instead'
                rule: '`@receiver ifNotNilDo: `@statements'->  '`@receiver ifNotNil: `@statements'.
        ^ self


—> open a browser and see how it fixes code magically.

The only problem is that it does a whole-method refactoring while it should only affect the node
that triggered the deprecation. Else there could be wrong transformations if the same selector
is used but only one of the implementations need to be rewritten, while if we are able to rewrite
the exact sender, we can do it fully automatically.

        Marcus

> On 31 Aug 2015, at 08:41, stepharo <[hidden email]> wrote:
>
> But the tool of mark is a ui tool so why would you want to get a tool to define a rule in your case?
>

Yes, it would be nice to have a tool for creating the rules. But then, one can use Mark’s tool as is
for this already now for creating the rewrite rule and then copy-and-paste it.
An integration could make that even easier, but for me this is good enough for now.

> In the future if mark and camille come up with a better rewrite engine then of course we will use it.
>
Yes.

        Marcus

>
> The only problem is that it does a whole-method refactoring while it should only affect the node
> that triggered the deprecation. Else there could be wrong transformations if the same selector
> is used but only one of the implementations need to be rewritten, while if we are able to rewrite
> the exact sender, we can do it fully automatically.
>

I think using #sourceNodeExecuted we could do it like this:

deprecated: anExplanationString rule: aRule
        | builder rewriteRule method context node |
        builder := RBCompositeRefactoryChange named: 'deprecation'.
        method := thisContext sender sender method.
        context :=  thisContext sender sender.
        node := context sourceNodeExecuted.
        rewriteRule := RBParseTreeRewriter new replace: aRule key with: aRule value.
        (rewriteRule executeTree: node copy)
                ifTrue: [
                        node replaceWith: rewriteRule tree.
                        builder compile: method ast formattedCode in: method methodClass classified: method protocol ].
        builder execute.


And a question is if we need #formattedCode… all refactoring normally try to update white-space information.
Need to check.

        Marcus

Yes to create rules. I thought that when the rule was executed and I was
confused.


Le 31/8/15 08:46, Marcus Denker a écrit :

even simpler:

deprecated: anExplanationString rule: aRule
        |  rewriteRule method context node |
        context :=  thisContext sender sender.
        method := context method.
        node := context sourceNodeExecuted.
        rewriteRule := RBParseTreeRewriter new replace: aRule key with: aRule value.
        (rewriteRule executeTree: node) ifFalse: [ ^self ].
       
        node replaceWith: rewriteRule tree.
        method methodClass compile: method ast formattedCode classified: method protocol.


It seems to work but I have not tested it too much.

        Marcus

I do not get why you want to link the rewrite engine with deprecation at runtime.
We should not have such dependencies. To me it is insane.

Stef

On Mon, Aug 31, 2015 at 8:41 AM, stepharo <[hidden email]> wrote:

But the tool of mark is a ui tool so why would you want to get a tool to define a rule in your case?

You can use the UI to create the rule, but the application of the rule can be scripted.

 

Yes, it would be nice to have a tool for creating the rules. But then, one can use Mark’s tool as is
for this already now for creating the rewrite rule and then copy-and-paste it.
An integration could make that even easier, but for me this is good enough for now.

RewriteTool creates classes with the rules, so I don't see the need for copy-pasting.

Why not just

self deprecated: anExplanationString rule: aRuleClassGeneratedByRewriteTool

self

> On 01 Sep 2015, at 08:30, stepharo <[hidden email]> wrote:
>
> I do not get why you want to link the rewrite engine with deprecation at runtime.
> We should not have such dependencies. To me it is insane.
>


It can be made pluggable, of course. I don’t want to have that there in a way that it requires
the refactoring engine to be present all the time.

If it’s there —> transform. If not —> do what it does now.

But just think how much this simplifies the experience of people loading old code into a new
Pharo version: it just keeps running and fixes itself! It’s that kind of things we need to be
able to evolve the system.

Imagine, this is a functionality (I think) no other system has!

        Marcus

Le 1 sept. 2015 à 08:35, Marcus Denker a écrit :
I will not like a feature where it transforms my code without asking.
I would prefer that a diff browser pops up and asks me if changes are ok. Automatic rewrite of code often forgot to check some specific cases and you end with bugs in your code if there is no review.
The idea is great but the choice of changing the code or not has to be in the user hands.

Yes, I want to have a dialog as a default, too. It should have the possibility to review,
to do your own change *and* to continue and turn off the dialog for future transforms
of this session.

But it is good to know that people don’t think this is that important. The result is that I will
deprecate things with far less guilt using the current scheme ;-)

        Marcus

Le 1 sept. 2015 à 09:37, Marcus Denker a écrit :
perfect :)

> But it is good to know that people don’t think this is that important. The result is that I will
> deprecate things with far less guilt using the current scheme ;-)

with the support you will provide for deprecations, for sure, you will be able to deprecate a lot more things. People will care less because there will be support to convert their code.

Yes!

        Marcus

On 01-09-15 08:35, Marcus Denker wrote:
> But just think how much this simplifies the experience of people loading old code into a new
> Pharo version: it just keeps running and fixes itself! It’s that kind of things we need to be
> able to evolve the system.

This is a crucial step to support continuous improvement indeed.
Just-in-time repeatable refactoring, reducing the cost of change

Stephan