From 54f8209d24b4f2405e2c575501aef9aa97120443 Mon Sep 17 00:00:00 2001 From: Sam Snyder Date: Mon, 3 Jun 2024 12:39:10 -0700 Subject: [PATCH] Fix failure to template annotations being applied to final method parameters. 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/java/JavaTemplateTest.java | 48 +++++++++++++------ .../openrewrite/java/ReplaceAnnotation.java | 37 +++++--------- .../template/AnnotationTemplateGenerator.java | 5 -- .../template/JavaTemplateJavaExtension.java | 5 ++ .../maven/tree/MavenResolutionResult.java | 1 + 5 files changed, 53 insertions(+), 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 index f7ed8542fbe..9ceb08c90f1 100755 --- 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 c) { assert c.size() > 0; @@ -873,7 +873,7 @@ void doSomething(Collection c) { """, """ import java.util.Collection; - + class Test { void doSomething(Collection 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 index 0534d699c50..a011406b816 100644 --- 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 getVisitor() { }; } + @Value + @EqualsAndHashCode(callSuper = false) public static class ReplaceAnnotationVisitor extends JavaIsoVisitor { - 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/rewrite-java/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java index 0f44d7bcb69..85be53d01dd 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/AnnotationTemplateGenerator.java +++ b/rewrite-java/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/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java index e91cf3279b5..fb19668af89 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java +++ b/rewrite-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 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 index 6a710b90e93..ddf23071d86 100644 --- 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