Add ChangeTaskToTasksRegister recipe
What's changed?
This PR introduces a new Gradle recipe, ChangeTaskToTasksRegister, which changes eager task creations
task exampleTask(type: ExampleType) to lazy registrations tasks.register("exampleTask", ExampleType).
What's your motivation?
- Closes #5333. This change aligns with modern Gradle best practices for improved build performance by utilizing lazy task registration.
Anything in particular you'd like reviewers to focus on?
- The handling of task names (both identifiers and string literals).
- The preservation of task type expressions and configuration closures.
- Whether any logic in the recipe could be further simplified or if any defensive checks are redundant.
- The test suite for any duplicated test cases or any missing edge cases that should be covered.
Checklist
- [X] I've added unit tests to cover both positive and negative cases
- [X] I've read and applied the recipe conventions and best practices
- [X] I've used the IntelliJ IDEA auto-formatter on affected files
Thanks a lot @jhl221123 ! I've tagged @Jenson3210 to also have a look as I'm out much of the coming week. Seems like a good addition! Have you tried this against any projects already?
Not necessary for this current PR, but an optimization from a code reduction standpoint would be to use the a single JavaVisitor because everything reduces down to a J.MethodInvocation. Equally, the GroovyTemplate and KotlinTemplate should work here nicely once a target code template has been selected.
Thanks for the feedback @timtebeek, @shanman190! I've pushed the updates you suggested.
Have you tried this against any projects already?
Not yet. If there's anything else I can do on my end, please let me know!
Hi @Jenson3210, I've updated the implementation based on the discussion and added more comprehensive tests. It should be ready for another review. Thanks!
As I am on a 3 weeks leave, responses will be a bit late. Feel free to take this further with anyone else if I fail to answer in a timely manner. Great to see the recipe progressing this way. I think, from my perspective, there are 2 things still open (of which I think, based on @shanman190 comment, only the first one is necessary before merging):
- Printing should/could be avoided.
- https://github.com/openrewrite/rewrite/pull/5492#issuecomment-2952739041
Locally this test is green, but here OOM. 🤔
This failing test is the only test that is using the static fromRuntimeClasspath method. This method scans entire RuntimeClassPath (250Mb). Together with the GradleParser (classpath taking 200Mb) and the GroovyParser (using 300Mb), I locally see a consistent usage of 850Mb in total.
Reworked the test to use a different recipe scanning part which reduces the need of scanning all other recipes also