Feature/3156 lombok utility class #3306
Conversation
…Class' into feature/3156_Lombok_UtilityClass
do not transform to UtilityClass iff non-static fields or methods are present
|
WIP (sommer camp) together with @lukas-poos-openvalue |
rewrite-java/src/test/java/org/openrewrite/java/LombokUtilityClassTest.java
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public TreeVisitor<?, ExecutionContext> getVisitor() { | ||
| return new ChangeVisitor(); |
There was a problem hiding this comment.
You could look at using Precondition.check here to first verify that a class is a utility class candidate, before applying the visitor that adds the annotation.
|
Looking good already; thanks for getting started on this! Since you still have TODO items in the code I only posted some quick suggestions. Let me know when you'd like a more thorough review. |
|
nice work! looks good to me besides a few comments from Tim and todo items. |
| @Nested | ||
| class ShouldApplyLombokUtility implements RewriteTest { | ||
|
|
||
| @Test | ||
| void givenOneStaticMethod() { | ||
| rewriteRun( | ||
| recipeSpec -> recipeSpec.recipe(new LombokUtilityClass()), | ||
| java( |
There was a problem hiding this comment.
Suggest to
- override defaults to define the recipe only once;
spec.recipe(Recipe)calls can be removed from all other tests too - nested classes don't have a to also implement
RewriteTest
| @Nested | |
| class ShouldApplyLombokUtility implements RewriteTest { | |
| @Test | |
| void givenOneStaticMethod() { | |
| rewriteRun( | |
| recipeSpec -> recipeSpec.recipe(new LombokUtilityClass()), | |
| java( | |
| @Override | |
| public void defaults(RecipeSpec spec) { | |
| spec.recipe(new LombokUtilityClass()); | |
| } | |
| @Nested | |
| class ShouldApplyLombokUtility { | |
| @Test | |
| void givenOneStaticMethod() { | |
| rewriteRun( | |
| java( |
| * TODO: Check the following criteria: | ||
| * - Lombok in dependencies + | ||
| * - All methods of class are static + | ||
| * - No instances of given class + | ||
| * - All static attributes are final + | ||
| * <p> | ||
| * TODO: Perform the transformation: | ||
| * - Add the annotation + | ||
| * - Remove static from all attributes and methods + | ||
| * <p> | ||
| * TODO: Features to consider: | ||
| * - Transformation: Add Lombok config if not present + supported configuration options for utility class | ||
| * - Transformation: Replace instantiation with static calls to methods --> na | ||
| * - Anonymous classes ??? | ||
| * - Reflection ??? |
There was a problem hiding this comment.
These comments and the failing test give me pause in reviewing this further; could you tell us your plans on these?
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.openrewrite.java; |
There was a problem hiding this comment.
| package org.openrewrite.java; | |
| package org.openrewrite.java.lombok; |
Put this in a lombok sub package? Maybe? @timtebeek thoughts?
There was a problem hiding this comment.
It might fit in best with https://github.com/openrewrite/rewrite-migrate-java/tree/main/src/main/java/org/openrewrite/java/migrate/lombok , since we've rearchitectured openrewrite/rewrite a bit to have less recipes in there, and more in specific modules
|
As indicated I suggest we move this over to the lombok package on rewrite-migrate-java. I didn't want to just copy over the content here, as we use Git history to give credit to our contributors. Would you mind opening a PR there @coiouhkc ? |
|
I am a big confused on the state here? |
This PR was opened against openrewrite/rewrite, but is a better fit for openrewrite/rewrite-migrate-java, given that we already have a few other Lombok recipes there. |
What's changed?
What's your motivation?
Closes openrewrite/rewrite-migrate-java#512
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Checklist
./gradlew licenseFormat