Contribute to JDT cleanup actions

[vogella]

Hello, I just found your plug-in and I'm already in love with it. Thanks for this awesome work.

Would you be interested in moving parts of your plug-in to JDT cleanup actions? I think this would be highly beneficial for all Eclipse users.

P.S. I'm one of the Eclipse developers and project leads. See https://projects.eclipse.org/projects/eclipse.platform/who

[vogella]

Btw. I'm starting to use your tool now to improve the Eclipse code. See for example: https://git.eclipse.org/r/c/144024/ and https://git.eclipse.org/r/c/144029/

:-)

[strkkk]

@vogella sounds interesting. What refactorings are most useful? Is there some guide how to add new JDT cleanup actions?

Btw, thank you for your site and tutorials, they were very useful for me.

[Fabrice-TIERCELIN]

It's the best future we hope for this plugin! We don't know if we will be here in a decade. This sounds like immortality :)

The current development version has transformed the plugin as a CleanUp Extension. It should ease the migration. There is still one limitation: in this mode, AutoRefactor only does 1 pass. I think I know how to fix it but it's not already done.

[vogella]

Awesome. One of our employees recently added a cleanup action to JDT so we should be able to help here. :-)

Maybe let's start with "Combine try catch blocks"?

Fabrice TIERCELIN [email protected] schrieb am Fr., 14. Juni 2019, 20:13:

It's the best future we hope for this plugin! We don't know if we will be here in a decade. This sounds like immortality :)

The current development version has transformed the plugin as a CleanUp Extension. It should ease the migration. There is still one limitation: in this mode, AutoRefactor only does 1 pass. I think I know how to fix it but it's not already done.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JnRouvignac/AutoRefactor/issues/386?email_source=notifications&email_token=AABCFBTOZPU6V37U54XX2TTP2PNS3A5CNFSM4HYFPID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXSERI#issuecomment-502211141, or mute the thread https://github.com/notifications/unsubscribe-auth/AABCFBS2ASGU36IICJHO4JTP2PNS3ANCNFSM4HYFPIDQ .

[Fabrice-TIERCELIN]

Do you mean UseMultiCatchRefactoring.java rule? Push negation down rule seems to be easier to integrate. The code is more simple, there is less dependency and it's useful.

I think we should check the policy:

1. Some rules erase some comments (not Push negation down). Perhaps, we should had a check and abort the refactoring if a comment will be lost.
2. Some rules need little cleaning from the other rules (Push negation down for instance) like removing useless parenthesis. The user doesn't see it as all the rules are running together but it can be a problem if we pick up one rule. Perhaps we need to rework the rules to make them self sufficient.
3. Some rules may trigger what I call zombie code (not Push negation down). Zombie code is a code that is dead because of an error above, but it can be triggered if the bug is fixed. For example, the Equals on constant rather than on variable rule avoids some null pointers, which should be a good thing, but in some case the null pointer is caught and it disables a zombie code. I think we should process the concerned rules later.

[vogella]

Remove useless blocks would also be a very nice addition, opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=548324 for that.

[vogella]

Testing now Push negation down.

[vogella]

By the way, should we start direct communication? If you agree, please drop me an email at [email protected]

[vogella]

"Push negation down" does not have one hit in the eclipse.platform code base so I think this is not the most valuable clean for Eclipse users.

I had lots of:

1.) contains vrs. indexOf cases 2.) Remove useless block 3.) Use try-with-resource 4.) Use String.contains 5.) Use multi-catch 6.) Remove empty ifs 7.) Remove unnecessary local variables before return statement 8.) All in one method, rather than loop

Do you have a convert to method reference refactoring? That would also be beneficial.

[Fabrice-TIERCELIN]

LambdaRefactoring.java converts a lambda expression into a method reference if possible.

[vogella]

What is the next step?

Would you like start contributing cleanup actions / quickfixes to JDT? For this we use Gerrit, if you have issues with this process, I can help with that. JDT is not very active in terms of reviewing community contributions but I could ask the Redhat team to review. If you start this, I think it would be great if you would gain JDT committership at some point in time to improve these cleanups directly.

[Fabrice-TIERCELIN]

You said "One of our employees recently added a cleanup action to JDT". Can I see the diff?

Another question: is cleanup action multi-pass? I mean:

Given

boolean reducedValue = !!!!true;

When

Cleanup is applied

Then

• A single pass get this:

boolean reducedValue = !!!false;

• A multi-pass get this:

boolean reducedValue = true;

A multi-pass process can reevaluate an already changed part of code. It's coded in AutoRefactor. If it is not the case in cleanup, I think it would be a good idea to implement it.

[jjohnstn]

Hi Fabrice, I have recently been made a JDT committer. I can help if you have code to be reviewed or have questions though I am relatively new and am still learning myself.

[vogella]

Jeff, can you give the link to your ; cleanup Gerrit?

jjohnstn [email protected] schrieb am Di., 25. Juni 2019, 18:37:

Hi Fabrice, I have recently been made a JDT committer. I can help if you have code to be reviewed or have questions though I am relatively new and am still learning myself.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JnRouvignac/AutoRefactor/issues/386?email_source=notifications&email_token=AABCFBR4DWNZ63UFFHSIYS3P4JCURA5CNFSM4HYFPID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYQ27LY#issuecomment-505524143, or mute the thread https://github.com/notifications/unsubscribe-auth/AABCFBQH6OQYSRPJMAPCQSTP4JCURANCNFSM4HYFPIDQ .

[jjohnstn]

With regards to single-pass vs multi-pass, the cleanups are not recursively checked, however, cleanups should be smart (e.g. I wrote a redundant semi-colon cleanup that will remove multiple extraneous trailing semi-colons at once and not just one at a time). Your example above could be calculated to a single change. An example where this doesn't work is removing unused private members/fields where a private field is being used by an unused private method. First cleanup removes the unused method, second cleanup will remove the unused field.

My gerrit change for redundant semi-colons can be found at:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=539437

[Fabrice-TIERCELIN]

@vogella, here we are, I have made a first contribution. I searched the simplest useful rule. I have complete the cleanup that removes the unnecessary modifiers. Now it will also remove the abstract modifier on interface : Remove_abstract_modifier_on_interface.patch

Now, I don't know how I'm supposed to contribute. Can I create an entry in Eclipse bugzilla or you create it?

[Fabrice-TIERCELIN]

I have been able to test the feature on Eclipse but I haven't been able to run the unitary tests. Maybe you can do it.

[vogella]

@Fabrice-TIERCELIN awesome. I'm already in summer vacation, so it would be great if you can open a bug directly. Please use "UI" and the following link: https://bugs.eclipse.org/bugs/enter_bug.cgi?product=JDT

This requires a bugzilla user.

We use Gerrit for code contributions, as for the setup, please see https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html#exericse-eclipse-user-creation-and-gerrit-server-configuration

The setup usually only takes 10 minutes in our hackerthons, we do from time to time in Hamburg.

For the Gerrit, please add Jeff (Jeff Johnston from Redhat) as reviewer. Review times in JDT can be very long, adding Jeff (or Roland Grunberg if Jeff is not available) as reviewer should result in faster review.

I'm happy to help with any Gerrit questions, important rule is to use the same Change-Id if you want to update the review. Eclipse Staging View has nice Gerrit support and the ammend option can be used to update Gerrits.

[vogella]

Gerrit will run the test automatically, no need for you to do it

Fabrice TIERCELIN [email protected] schrieb am Mo., 8. Juli 2019, 08:19:

I have been able to test the feature on Eclipse but I haven't been able to run the unitary tests. Maybe you can do it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JnRouvignac/AutoRefactor/issues/386?email_source=notifications&email_token=AABCFBWOO2Q367HO2RD7QYLP6LL7RA5CNFSM4HYFPID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZMCR5I#issuecomment-509094133, or mute the thread https://github.com/notifications/unsubscribe-auth/AABCFBUHIXUECVUENXVWO4LP6LL7RANCNFSM4HYFPIDQ .

[vogella]

Awesome @Fabrice-TIERCELIN, thanks for https://bugs.eclipse.org/bugs/show_bug.cgi?id=549242. Any plans to contribute more cleanups?

[Fabrice-TIERCELIN]

I'm preparing the migration of Autoboxing and Unboxing.

[akurtakov]

@Fabrice-TIERCELIN thanks for doing it - would you please watch https://twitter.com/ZhekaKozlov/status/1135506701438857217 . I'm interested whether there are other of your cleanup actions that can help us achieve parity (boxing/unboxing should be one of them).

[Fabrice-TIERCELIN]

@akurtakov What is this tool? What is the list of the features?

[PyvesB]

Awesome work, looking forward to seeing more features getting migrated! :+1:

[Fabrice-TIERCELIN]

Hi @vogella, do you know how your Jenkins is configured to feature the build SUCCESS as green? On my Jenkins, it appears in blue...

Or do you know who knows?

[PyvesB]

Hi @vogella, do you know how your Jenkins is configured to feature the build SUCCESS as green? On my Jenkins, it appears in blue...

Or do you know who knows?

You'll have to install the Green Balls plugin. :wink:

[Fabrice-TIERCELIN]

OK, thanks @PyvesB.

[Fabrice-TIERCELIN]

Awesome work, looking forward to seeing more features getting migrated! 👍

@PyvesB, @akurtakov and @vogella, don't support the coder, support the reviewer. The migration time is estimated to 6 years and it's mostly review time.

Or be a merger 😺

[vogella]

Eclipse ist currently in release freeze but we should open master soon again. And I assume this means reviews will continue.

Fabrice TIERCELIN [email protected] schrieb am Sa., 7. Sep. 2019, 15:58:

Awesome work, looking forward to seeing more features getting migrated! 👍

@PyvesB https://github.com/PyvesB, @akurtakov https://github.com/akurtakov and @vogella https://github.com/vogella, don't support the coder, support the reviewer. The migration time is estimated to 6 years and it's mostly review time https://bugs.eclipse.org/bugs/show_bug.cgi?id=550394.

Or be a merger 😺

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JnRouvignac/AutoRefactor/issues/386?email_source=notifications&email_token=AABCFBRGIATQE6OJW3IZKVDQIOXSDA5CNFSM4HYFPID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6EZNFI#issuecomment-529110677, or mute the thread https://github.com/notifications/unsubscribe-auth/AABCFBUZPIUW2IEDF5LKNR3QIOXSDANCNFSM4HYFPIDQ .

[Fabrice-TIERCELIN]

OK, what do you prefer, what will be faster:

• One rule at a time?
• Several rules in several tickets?
• Several rules in one ticket?
• Another solution...?