Fix failure to template annotations being applied to final method par…

…ameters. I couldn't find any coverage or purpose for the bit of AnnotationTemplateGenerator I removed. I can't see how it could ever have been correct so likely it was vestigial.
openrewrite · Jun 3, 2024 · 54f8209 · 54f8209
1 parent cfbb5b0
commit 54f8209
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 43 deletions.
diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java
@@ -29,7 +29,7 @@
 import static org.openrewrite.java.Assertions.java;
 import static org.openrewrite.test.RewriteTest.toRecipe;
 
-@SuppressWarnings({"ConstantConditions", "PatternVariableCanBeUsed", "UnnecessaryBoxing", "StatementWithEmptyBody", "UnusedAssignment"})
+@SuppressWarnings({"ConstantConditions", "PatternVariableCanBeUsed", "UnnecessaryBoxing", "StatementWithEmptyBody", "UnusedAssignment", "SizeReplaceableByIsEmpty", "ResultOfMethodCallIgnored", "RedundantOperationOnEmptyContainer"})
 class JavaTemplateTest implements RewriteTest {
 
     @Issue("https://github.com/openrewrite/rewrite/issues/2090")
@@ -297,7 +297,7 @@ class T {
                   void m() {
                       hashCode();
                   }
-                  
+              
                   void m2() {
                       hashCode();
                   }
@@ -720,20 +720,20 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext p) {
               class A {
                   public enum Type {
                       One;
-                            
+              
                       public Type(String t) {
                       }
-                            
+              
                       String t;
-                            
+              
                       public static Type fromType(String type) {
                           return null;
                       }
                   }
-                            
+              
                   public A(Type type) {}
                   public A() {}
-                            
+              
                   public void method(Type type) {
                       new A(type);
                   }
@@ -743,20 +743,20 @@ public void method(Type type) {
               class A {
                   public enum Type {
                       One;
-                            
+              
                       public Type(String t) {
                       }
-                            
+              
                       String t;
-                            
+              
                       public static Type fromType(String type) {
                           return null;
                       }
                   }
-                            
+              
                   public A(Type type) {}
                   public A() {}
-                            
+              
                   public void method(Type type) {
                       new A();
                   }
@@ -864,7 +864,7 @@ public J visitBinary(J.Binary binary, ExecutionContext p) {
           java(
             """
               import java.util.Collection;
-
+              
               class Test {
                   void doSomething(Collection<Object> c) {
                       assert c.size() > 0;
@@ -873,7 +873,7 @@ void doSomething(Collection<Object> c) {
               """,
             """
               import java.util.Collection;
-
+              
               class Test {
                   void doSomething(Collection<Object> c) {
                       assert !c.isEmpty();
@@ -1194,4 +1194,24 @@ interface Test {
           )
         );
     }
+
+    @Test
+    void finalMethodParameter() {
+        rewriteRun(
+          spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull", "@lombok.NonNull", null)),
+          java("""
+                    import org.jetbrains.annotations.NotNull;
+                    
+                    class A {
+                        String testMethod(@NotNull final String test) {}
+                    }
+                    """, """
+                    import lombok.NonNull;
+                    
+                    class A {
+                        String testMethod(@NonNull final String test) {}
+                    }
+                    """)
+        );
+    }
 }
diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ReplaceAnnotation.java b/rewrite-java/src/main/java/org/openrewrite/java/ReplaceAnnotation.java
@@ -21,7 +21,6 @@
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaCoordinates;
-import org.openrewrite.java.tree.JavaType;
 import org.openrewrite.java.tree.TypeUtils;
 
 @Value
@@ -75,35 +74,25 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
         };
     }
 
+    @Value
+    @EqualsAndHashCode(callSuper = false)
     public static class ReplaceAnnotationVisitor extends JavaIsoVisitor<ExecutionContext> {
-        private final AnnotationMatcher matcher;
-        private final JavaTemplate replacement;
-
-        public ReplaceAnnotationVisitor(AnnotationMatcher annotationMatcher, JavaTemplate replacement) {
-            super();
-            this.matcher = annotationMatcher;
-            this.replacement = replacement;
-        }
+        AnnotationMatcher matcher;
+        JavaTemplate replacement;
 
         @Override
-        public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) {
-            J.Annotation maybeReplacingAnnotation = super.visitAnnotation(a, ctx);
+        public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext ctx) {
+            J.Annotation a = super.visitAnnotation(annotation, ctx);
 
-            boolean keepAnnotation = !matcher.matches(maybeReplacingAnnotation);
-            if (keepAnnotation) {
-                return maybeReplacingAnnotation;
+            if (!matcher.matches(a)) {
+                return a;
             }
 
-
-            JavaType.FullyQualified replacedAnnotationType = TypeUtils.asFullyQualified(maybeReplacingAnnotation.getType());
-            maybeRemoveImport(replacedAnnotationType);
-
-            JavaCoordinates replaceCoordinate = maybeReplacingAnnotation.getCoordinates().replace();
-            J.Annotation replacement = this.replacement.apply(getCursor(), replaceCoordinate);
-
-            doAfterVisit(ShortenFullyQualifiedTypeReferences.modifyOnly(replacement));
-
-            return replacement;
+            maybeRemoveImport(TypeUtils.asFullyQualified(a.getType()));
+            JavaCoordinates replaceCoordinate = a.getCoordinates().replace();
+            a = replacement.apply(getCursor(), replaceCoordinate);
+            doAfterVisit(ShortenFullyQualifiedTypeReferences.modifyOnly(a));
+            return a;
         }
     }
 }

diff --git a/...ava/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java b/...ava/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java
@@ -188,11 +188,6 @@ private void template(Cursor cursor, J prior, StringBuilder before, StringBuilde
                 }
                 after.append('}');
             }
-        } else if (j instanceof J.VariableDeclarations) {
-            J.VariableDeclarations v = (J.VariableDeclarations) j;
-            if (v.hasModifier(J.Modifier.Type.Final)) {
-                before.insert(0, variable((J.VariableDeclarations) j, cursor) + '=');
-            }
         } else if (j instanceof J.NewClass) {
             J.NewClass n = (J.NewClass) j;
             n = n.withBody(null).withPrefix(Space.EMPTY);

diff --git a/...-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java b/...-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java
@@ -59,6 +59,11 @@ public J visitAnnotation(J.Annotation annotation, Integer integer) {
                 if (loc.equals(ANNOTATION_PREFIX) && mode.equals(JavaCoordinates.Mode.REPLACEMENT) &&
                     annotation.isScope(insertionPoint)) {
                     List<J.Annotation> gen = substitutions.unsubstitute(templateParser.parseAnnotations(getCursor(), substitutedTemplate));
+                    if (gen.isEmpty()) {
+                        throw new IllegalStateException("Unable to parse annotation from template: \n" +
+                                                        substitutedTemplate +
+                                                        "\nUse JavaTemplate.Builder.doBeforeParseTemplate() to see what stub is being generated and include it in any bug report.");
+                    }
                     return gen.get(0).withPrefix(annotation.getPrefix());
                 } else if (loc.equals(ANNOTATION_ARGUMENTS) && mode.equals(JavaCoordinates.Mode.REPLACEMENT) &&
                            annotation.isScope(insertionPoint)) {

diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenResolutionResult.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenResolutionResult.java
@@ -32,6 +32,7 @@
 import static java.util.Collections.emptyList;
 import static org.openrewrite.internal.StringUtils.matchesGlob;
 
+@SuppressWarnings("unused")
 @AllArgsConstructor
 @FieldDefaults(level = AccessLevel.PRIVATE)
 @Getter