refactor: OpenRewrite best practices (#406)

* refactor: OpenRewrite best practices Use this link to re-run the recipe: https://app.moderne.io/recipes/builder/KG5ME0uWf?organizationId=ZDI4ZjE4NTAtNTg0Yi00Zjk1LWFkZDgtOGQzY2ZiMzgwNmM4 Co-authored-by: Moderne <team@moderne.io> * DontOverfetchDto examples and clean up of unused types * Add two more example values * Use `GH_PAT_ACTIONS_READ` in comment-pr.yml * Revert "DontOverfetchDto examples and clean up of unused types" This reverts commit cf691de. * Only add examples to DontOverfetchDto * Expect to clean up unused import * Disable RemoveIllegalSemicolonsTest for now * Apply suggestions from code review --------- Co-authored-by: Moderne <team@moderne.io>
openrewrite · Feb 4, 2024 · 329e8f8 · 329e8f8
1 parent b04b55e
commit 329e8f8
Show file tree

Hide file tree

Showing 10 changed files with 27 additions and 26 deletions.
diff --git a/.github/workflows/comment-pr.yml b/.github/workflows/comment-pr.yml
@@ -11,4 +11,6 @@ on:
 # Since this pull request has write permissions on the target repo, we should **NOT** execute any untrusted code.
 jobs:
   post-suggestions:
-    uses: openrewrite/gh-automation/.github/workflows/comment-pr.yml@main
+    uses: openrewrite/gh-automation/.github/workflows/comment-pr.yml@main
+    secrets:
+      GH_PAT_ACTIONS_READ: ${{ secrets.GH_PAT_ACTIONS_READ }}
diff --git a/src/main/java/org/openrewrite/java/migrate/CastArraysAsListToList.java b/src/main/java/org/openrewrite/java/migrate/CastArraysAsListToList.java
@@ -52,8 +52,8 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
 
     private static class CastArraysAsListToListVisitor extends JavaVisitor<ExecutionContext> {
         @Override
-        public J visitTypeCast(J.TypeCast typeCast, ExecutionContext executionContext) {
-            J j = super.visitTypeCast(typeCast, executionContext);
+        public J visitTypeCast(J.TypeCast typeCast, ExecutionContext ctx) {
+            J j = super.visitTypeCast(typeCast, ctx);
             if (!(j instanceof J.TypeCast) || !(((J.TypeCast) j).getType() instanceof JavaType.Array)) {
                 return j;
             }

diff --git a/src/main/java/org/openrewrite/java/migrate/DontOverfetchDto.java b/src/main/java/org/openrewrite/java/migrate/DontOverfetchDto.java
@@ -41,11 +41,13 @@
 public class DontOverfetchDto extends Recipe {
 
     @Option(displayName = "DTO type",
-            description = "The fully qualified name of the DTO.")
+            description = "The fully qualified name of the DTO.",
+            example = "animals.Dog")
     String dtoType;
 
     @Option(displayName = "Data element",
-            description = "Replace the DTO as a method parameter when only this data element is used.")
+            description = "Replace the DTO as a method parameter when only this data element is used.",
+            example = "name")
     String dtoDataElement;
 
     @Override
@@ -93,7 +95,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex
                                                             .withVariables(ListUtils.map(v.getVariables(), nv -> {
                                                                 JavaType.Variable fieldType = nv.getName().getFieldType();
                                                                 return nv
-                                                                        .withName(nv.getName().withSimpleName(dtoDataElement))
+                                                                        .withName(nv.getName().withSimpleName(dtoDataElement).withType(memberType))
                                                                         .withType(memberType)
                                                                         .withVariableType(fieldType
                                                                                 .withName(dtoDataElement).withOwner(memberType));

diff --git a/src/main/java/org/openrewrite/java/migrate/jakarta/BeanValidationMessages.java b/src/main/java/org/openrewrite/java/migrate/jakarta/BeanValidationMessages.java
@@ -45,8 +45,8 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
         return Preconditions.check(new UsesType<>("javax.validation.constraints..*", true),
                 new JavaIsoVisitor<ExecutionContext>() {
                     @Override
-                    public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext executionContext) {
-                        J.Annotation a = super.visitAnnotation(annotation, executionContext);
+                    public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext ctx) {
+                        J.Annotation a = super.visitAnnotation(annotation, ctx);
                         if (!JAVAX_MATCHER.matches(a)) {
                             return a;
                         }

diff --git a/src/main/java/org/openrewrite/java/migrate/jakarta/UpdateBeanManagerMethods.java b/src/main/java/org/openrewrite/java/migrate/jakarta/UpdateBeanManagerMethods.java
@@ -42,19 +42,19 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
             private final MethodMatcher createInjectionTargetMatcher = new MethodMatcher("*.enterprise.inject.spi.BeanManager createInjectionTarget(..)", false);
 
             @Override
-            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ec) {
-                J.MethodInvocation mi = super.visitMethodInvocation(method, ec);
+            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
+                J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
                 if (fireEventMatcher.matches(method)) {
                     return JavaTemplate.builder("#{any(jakarta.enterprise.inject.spi.BeanManager)}.getEvent().fire(#{any(jakarta.enterprise.inject.spi.BeforeBeanDiscovery)})")
-                            .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ec, "jakarta.enterprise.cdi-api-3.0.0-M4"))
+                            .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "jakarta.enterprise.cdi-api-3.0.0-M4"))
                             .build()
                             .apply(updateCursor(mi),
                                     mi.getCoordinates().replace(),
                                     mi.getSelect(),
                                     mi.getArguments().get(0));
                 } else if (createInjectionTargetMatcher.matches(method)) {
                     return JavaTemplate.builder("#{any(jakarta.enterprise.inject.spi.BeanManager)}.getInjectionTargetFactory(#{any(jakarta.enterprise.inject.spi.AnnotatedType)}).createInjectionTarget(null)")
-                            .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ec, "jakarta.enterprise.cdi-api-3.0.0-M4"))
+                            .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "jakarta.enterprise.cdi-api-3.0.0-M4"))
                             .build()
                             .apply(updateCursor(mi),
                                     mi.getCoordinates().replace(),

diff --git a/src/main/java/org/openrewrite/java/migrate/jakarta/UpdateGetRealPath.java b/src/main/java/org/openrewrite/java/migrate/jakarta/UpdateGetRealPath.java
@@ -42,10 +42,10 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
             private final MethodMatcher METHOD_PATTERN = new MethodMatcher("jakarta.servlet.ServletRequest* getRealPath(String)", false);
 
             @Override
-            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ec) {
+            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
                 if (METHOD_PATTERN.matches(method)) {
                     return JavaTemplate.builder("#{any()}.getServletContext().getRealPath(#{any(String)})")
-                            .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ec, "jakarta.servlet-api-6.0.0"))
+                            .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "jakarta.servlet-api-6.0.0"))
                             .build()
                             .apply(updateCursor(method),
                                     method.getCoordinates().replace(),

diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindDataUsedOnDto.java b/src/main/java/org/openrewrite/java/migrate/search/FindDataUsedOnDto.java
@@ -18,7 +18,6 @@
 import lombok.EqualsAndHashCode;
 import lombok.Value;
 import org.openrewrite.*;
-import org.openrewrite.internal.StringUtils;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.MethodMatcher;
 import org.openrewrite.java.migrate.table.DtoDataUses;
@@ -33,7 +32,8 @@ public class FindDataUsedOnDto extends Recipe {
     transient DtoDataUses dtoDataUses = new DtoDataUses(this);
 
     @Option(displayName = "DTO type",
-            description = "The fully qualified name of the DTO.")
+            description = "The fully qualified name of the DTO.",
+            example = "com.example.dto.*")
     String dtoType;
 
     @Override

diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindDtoOverfetching.java b/src/main/java/org/openrewrite/java/migrate/search/FindDtoOverfetching.java
@@ -35,7 +35,8 @@
 @EqualsAndHashCode(callSuper = false)
 public class FindDtoOverfetching extends Recipe {
     @Option(displayName = "DTO type",
-            description = "The fully qualified name of the DTO.")
+            description = "The fully qualified name of the DTO.",
+            example = "com.example.dto.*")
     String dtoType;
 
     @Override

diff --git a/src/test/java/org/openrewrite/java/migrate/DontOverfetchDtoTest.java b/src/test/java/org/openrewrite/java/migrate/DontOverfetchDtoTest.java
@@ -16,15 +16,12 @@
 package org.openrewrite.java.migrate;
 
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
-import org.openrewrite.java.migrate.search.FindDtoOverfetching;
 import org.openrewrite.test.RewriteTest;
 import org.openrewrite.test.TypeValidation;
 
 import static org.openrewrite.java.Assertions.java;
 
-public class DontOverfetchDtoTest implements RewriteTest {
+class DontOverfetchDtoTest implements RewriteTest {
 
     @SuppressWarnings("LombokGetterMayBeUsed")
     @Test
@@ -39,11 +36,9 @@ void findDtoOverfetching() {
               public class Dog {
                   String name;
                   String breed;
-                  
                   public String getName() {
                       return name;
                   }
-                  
                   public String getBreed() {
                       return breed;
                   }
@@ -54,7 +49,7 @@ public String getBreed() {
           java(
             """
               import animals.Dog;
-                            
+              
               class Test {
                   boolean test(Dog dog, int age) {
                       if(dog.getName() != null) {
@@ -64,8 +59,6 @@ boolean test(Dog dog, int age) {
               }
               """,
             """
-              import animals.Dog;
-                            
               class Test {
                   boolean test(java.lang.String name, int age) {
                       if(name != null) {

diff --git a/src/test/java/org/openrewrite/java/migrate/RemoveIllegalSemicolonsTest.java b/src/test/java/org/openrewrite/java/migrate/RemoveIllegalSemicolonsTest.java
@@ -16,6 +16,7 @@
 package org.openrewrite.java.migrate;
 
 import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.ExpectedToFail;
 import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
@@ -32,6 +33,8 @@ public void defaults(RecipeSpec spec) {
 
     @Test
     @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/396")
+    @ExpectedToFail("Fails after adding whitespace validation in RewriteTest: " +
+                    "https://github.com/openrewrite/rewrite/commit/33d8f7d42f9bd6749ab93171fad31289b0e2bc97")
     void importSemicolon() {
         //language=java
         rewriteRun(