From 92dc483acdb6c27587a1c9926ccad3514e6b241f Mon Sep 17 00:00:00 2001 From: Filipe Roque Date: Fri, 19 Jul 2024 15:51:14 +0100 Subject: [PATCH 1/4] Fix AddOrUpdateAnnotationAttribute for values of type Class --- .../AddOrUpdateAnnotationAttributeTest.java | 133 ++++++++++++++++++ .../java/AddOrUpdateAnnotationAttribute.java | 28 ++++ 2 files changed, 161 insertions(+) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java index 0314c26a7a6..24b1b644f70 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java @@ -53,6 +53,37 @@ public class A { ); } + @Test + void addValueAttributeClass() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "Integer.class", null)), + java( + """ + package org.example; + public @interface Foo { + Class value(); + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(Integer.class) + public class A { + } + """ + ) + ); + } + @Test void updateValueAttribute() { rewriteRun( @@ -85,6 +116,38 @@ public class A { ); } + @Test + void updateValueAttributeClass() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "Integer.class", null)), + java( + """ + package org.example; + public @interface Foo { + Class value(); + } + """ + ), + + java( + """ + import org.example.Foo; + + @Foo(Long.class) + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(Integer.class) + public class A { + } + """ + ) + ); + } + @Test void removeValueAttribute() { rewriteRun( @@ -117,6 +180,38 @@ public class A { ); } + @Test + void removeValueAttributeClass() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, null, null)), + java( + """ + package org.example; + public @interface Foo { + Class value(); + } + """ + ), + + java( + """ + import org.example.Foo; + + @Foo(Long.class) + public class A { + } + """, + """ + import org.example.Foo; + + @Foo + public class A { + } + """ + ) + ); + } + @Test void addNamedAttribute() { rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "timeout", "500", null)), @@ -304,6 +399,44 @@ void foo() { ); } + @Test + void implicitValueToExplicitValueClass() { + rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)) + .expectedCyclesThatMakeChanges(2), + java( + """ + package org.junit; + public @interface Test { + long other() default 0L; + Class value(); + } + """ + ), + java( + """ + import org.junit.Test; + + class SomeTest { + + @Test(Integer.class) + void foo() { + } + } + """, + """ + import org.junit.Test; + + class SomeTest { + + @Test(other = 1, value = Integer.class) + void foo() { + } + } + """ + ) + ); + } + @Test void dontChangeWhenSetToAddOnly() { rewriteRun( diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java index fa55101ede3..6b228f2a12b 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java @@ -133,6 +133,34 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { .apply(getCursor(), finalA.getCoordinates().replaceArguments(), it) ).getArguments().get(0); } + } else if (it instanceof J.FieldAccess) { + // The only way anything except an assignment can appear is if there's an implicit assignment to "value" + if (attributeName == null || "value".equals(attributeName)) { + if (newAttributeValue == null) { + return null; + } + J.FieldAccess value = (J.FieldAccess) it; + if (newAttributeValue.equals(value.toString()) || Boolean.TRUE.equals(addOnly)) { + foundAttributeWithDesiredValue.set(true); + return it; + } + + JavaType.ShallowClass newClass = JavaType.ShallowClass.build(newAttributeValue); + Expression currentTarget = value.getTarget(); + Expression newTarget = ((J.Identifier) currentTarget) + .withType(newClass) + .withSimpleName(newClass.getOwningClass().getClassName()); + return value + .withType(newClass) + .withTarget(newTarget); + } else { + //noinspection ConstantConditions + return ((J.Annotation) JavaTemplate.builder("value = #{any(java.lang.Class)}") + .contextSensitive() + .build() + .apply(getCursor(), finalA.getCoordinates().replaceArguments(), it) + ).getArguments().get(0); + } } return it; }); From b7f06e8337703605c5dee0a1a94464995a3d5ce6 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jul 2024 12:59:21 +0200 Subject: [PATCH 2/4] Correct type information on new attribute & add FQN test --- .../AddOrUpdateAnnotationAttributeTest.java | 97 ++++++++++++------- .../java/AddOrUpdateAnnotationAttribute.java | 22 ++--- 2 files changed, 72 insertions(+), 47 deletions(-) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java index 24b1b644f70..8831d52237f 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java @@ -37,14 +37,14 @@ void addValueAttribute() { java( """ import org.example.Foo; - + @Foo public class A { } """, """ import org.example.Foo; - + @Foo("hello") public class A { } @@ -68,14 +68,14 @@ void addValueAttributeClass() { java( """ import org.example.Foo; - + @Foo public class A { } """, """ import org.example.Foo; - + @Foo(Integer.class) public class A { } @@ -84,6 +84,37 @@ public class A { ); } + @Test + void addValueAttributeFullyQualifiedClass() { + rewriteRun( + spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.example.Foo", null, "java.math.BigDecimal.class", null)), + java( + """ + package org.example; + public @interface Foo { + Class value(); + } + """ + ), + java( + """ + import org.example.Foo; + + @Foo + public class A { + } + """, + """ + import org.example.Foo; + + @Foo(java.math.BigDecimal.class) + public class A { + } + """ + ) + ); + } + @Test void updateValueAttribute() { rewriteRun( @@ -100,14 +131,14 @@ void updateValueAttribute() { java( """ import org.example.Foo; - + @Foo("goodbye") public class A { } """, """ import org.example.Foo; - + @Foo("hello") public class A { } @@ -132,14 +163,14 @@ void updateValueAttributeClass() { java( """ import org.example.Foo; - + @Foo(Long.class) public class A { } """, """ import org.example.Foo; - + @Foo(Integer.class) public class A { } @@ -226,9 +257,9 @@ void addNamedAttribute() { java( """ import org.junit.Test; - + class SomeTest { - + @Test void foo() { } @@ -236,9 +267,9 @@ void foo() { """, """ import org.junit.Test; - + class SomeTest { - + @Test(timeout = 500) void foo() { } @@ -263,9 +294,9 @@ void replaceAttribute() { java( """ import org.junit.Test; - + class SomeTest { - + @Test(timeout = 1) void foo() { } @@ -273,9 +304,9 @@ void foo() { """, """ import org.junit.Test; - + class SomeTest { - + @Test(timeout = 500) void foo() { } @@ -300,9 +331,9 @@ void removeAttribute() { java( """ import org.junit.Test; - + class SomeTest { - + @Test(timeout = 1) void foo() { } @@ -310,9 +341,9 @@ void foo() { """, """ import org.junit.Test; - + class SomeTest { - + @Test void foo() { } @@ -339,9 +370,9 @@ void preserveExistingAttributes() { java( """ import org.junit.Test; - + class SomeTest { - + @Test(foo = "") void foo() { } @@ -349,9 +380,9 @@ void foo() { """, """ import org.junit.Test; - + class SomeTest { - + @Test(timeout = 500, foo = "") void foo() { } @@ -377,9 +408,9 @@ void implicitValueToExplicitValue() { java( """ import org.junit.Test; - + class SomeTest { - + @Test("foo") void foo() { } @@ -387,9 +418,9 @@ void foo() { """, """ import org.junit.Test; - + class SomeTest { - + @Test(other = 1, value = "foo") void foo() { } @@ -415,9 +446,9 @@ void implicitValueToExplicitValueClass() { java( """ import org.junit.Test; - + class SomeTest { - + @Test(Integer.class) void foo() { } @@ -425,9 +456,9 @@ void foo() { """, """ import org.junit.Test; - + class SomeTest { - + @Test(other = 1, value = Integer.class) void foo() { } @@ -453,7 +484,7 @@ void dontChangeWhenSetToAddOnly() { java( """ import org.junit.Test; - + class SomeTest { @Test(other = 0) void foo() { diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java index 6b228f2a12b..df00100241a 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java @@ -43,7 +43,7 @@ public String getDescription() { "or adds the argument if it is not already set."; } - @Option(displayName = "Annotation Type", + @Option(displayName = "Annotation type", description = "The fully qualified name of the annotation.", example = "org.junit.Test") String annotationType; @@ -61,7 +61,7 @@ public String getDescription() { @Nullable String attributeValue; - @Option(displayName = "Add Only", + @Option(displayName = "Add only", description = "When set to `true` will not change existing annotation attribute values.") @Nullable Boolean addOnly; @@ -144,22 +144,16 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { foundAttributeWithDesiredValue.set(true); return it; } - - JavaType.ShallowClass newClass = JavaType.ShallowClass.build(newAttributeValue); - Expression currentTarget = value.getTarget(); - Expression newTarget = ((J.Identifier) currentTarget) - .withType(newClass) - .withSimpleName(newClass.getOwningClass().getClassName()); - return value - .withType(newClass) - .withTarget(newTarget); + //noinspection ConstantConditions + return ((J.Annotation) JavaTemplate.apply(newAttributeValue, getCursor(), finalA.getCoordinates().replaceArguments())) + .getArguments().get(0); } else { //noinspection ConstantConditions - return ((J.Annotation) JavaTemplate.builder("value = #{any(java.lang.Class)}") + return ((J.Annotation) JavaTemplate.builder("value = #{any()}") .contextSensitive() .build() - .apply(getCursor(), finalA.getCoordinates().replaceArguments(), it) - ).getArguments().get(0); + .apply(getCursor(), finalA.getCoordinates().replaceArguments(), it)) + .getArguments().get(0); } } return it; From a1c3170b946661c245df34b0b5472b3fe1e9afd6 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jul 2024 13:13:45 +0200 Subject: [PATCH 3/4] Remove need for second cycle --- .../AddOrUpdateAnnotationAttributeTest.java | 6 ++--- .../java/AddOrUpdateAnnotationAttribute.java | 22 ++++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java index 8831d52237f..a04d9fc4594 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/AddOrUpdateAnnotationAttributeTest.java @@ -394,8 +394,7 @@ void foo() { @Test void implicitValueToExplicitValue() { - rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)) - .expectedCyclesThatMakeChanges(2), + rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)), java( """ package org.junit; @@ -432,8 +431,7 @@ void foo() { @Test void implicitValueToExplicitValueClass() { - rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)) - .expectedCyclesThatMakeChanges(2), + rewriteRun(spec -> spec.recipe(new AddOrUpdateAnnotationAttribute("org.junit.Test", "other", "1", null)), java( """ package org.junit; diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java index df00100241a..9727de44963 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java @@ -95,7 +95,7 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { } } else { // First assume the value exists amongst the arguments and attempt to update it - AtomicBoolean foundAttributeWithDesiredValue = new AtomicBoolean(false); + AtomicBoolean foundOrSetAttributeWithDesiredValue = new AtomicBoolean(false); final J.Annotation finalA = a; List newArgs = ListUtils.map(currentArgs, it -> { if (it instanceof J.Assignment) { @@ -104,24 +104,24 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { if (attributeName == null || !attributeName.equals(var.getSimpleName())) { return it; } + foundOrSetAttributeWithDesiredValue.set(true); J.Literal value = (J.Literal) as.getAssignment(); if (newAttributeValue == null) { return null; } if (newAttributeValue.equals(value.getValueSource()) || Boolean.TRUE.equals(addOnly)) { - foundAttributeWithDesiredValue.set(true); return it; } return as.withAssignment(value.withValue(newAttributeValue).withValueSource(newAttributeValue)); } else if (it instanceof J.Literal) { // The only way anything except an assignment can appear is if there's an implicit assignment to "value" if (attributeName == null || "value".equals(attributeName)) { + foundOrSetAttributeWithDesiredValue.set(true); if (newAttributeValue == null) { return null; } J.Literal value = (J.Literal) it; if (newAttributeValue.equals(value.getValueSource()) || Boolean.TRUE.equals(addOnly)) { - foundAttributeWithDesiredValue.set(true); return it; } return ((J.Literal) it).withValue(newAttributeValue).withValueSource(newAttributeValue); @@ -136,12 +136,12 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { } else if (it instanceof J.FieldAccess) { // The only way anything except an assignment can appear is if there's an implicit assignment to "value" if (attributeName == null || "value".equals(attributeName)) { + foundOrSetAttributeWithDesiredValue.set(true); if (newAttributeValue == null) { return null; } J.FieldAccess value = (J.FieldAccess) it; if (newAttributeValue.equals(value.toString()) || Boolean.TRUE.equals(addOnly)) { - foundAttributeWithDesiredValue.set(true); return it; } //noinspection ConstantConditions @@ -158,8 +158,11 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { } return it; }); - if (foundAttributeWithDesiredValue.get() || newArgs != currentArgs) { - return a.withArguments(newArgs); + if (newArgs != currentArgs) { + a = a.withArguments(newArgs); + } + if (foundOrSetAttributeWithDesiredValue.get()) { + return a; } // There was no existing value to update, so add a new value into the argument list String effectiveName = (attributeName == null) ? "value" : attributeName; @@ -167,10 +170,9 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { J.Assignment as = (J.Assignment) ((J.Annotation) JavaTemplate.builder("#{} = #{}") .contextSensitive() .build() - .apply(getCursor(), a.getCoordinates().replaceArguments(), effectiveName, newAttributeValue) - ).getArguments().get(0); - List newArguments = ListUtils.concat(as, a.getArguments()); - a = a.withArguments(newArguments); + .apply(getCursor(), a.getCoordinates().replaceArguments(), effectiveName, newAttributeValue)) + .getArguments().get(0); + a = a.withArguments(ListUtils.concat(as, a.getArguments())); a = autoFormat(a, ctx); } From bc10a53dd910b492de9de4b4aeafec7471f6aedb Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 25 Jul 2024 13:17:48 +0200 Subject: [PATCH 4/4] Document two branches of the nested conditionals --- .../org/openrewrite/java/AddOrUpdateAnnotationAttribute.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java index 9727de44963..aba2d59d588 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/AddOrUpdateAnnotationAttribute.java @@ -126,6 +126,7 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { } return ((J.Literal) it).withValue(newAttributeValue).withValueSource(newAttributeValue); } else { + // Make the attribute name explicit, before we add the new value below //noinspection ConstantConditions return ((J.Annotation) JavaTemplate.builder("value = #{}") .contextSensitive() @@ -148,6 +149,7 @@ public J.Annotation visitAnnotation(J.Annotation a, ExecutionContext ctx) { return ((J.Annotation) JavaTemplate.apply(newAttributeValue, getCursor(), finalA.getCoordinates().replaceArguments())) .getArguments().get(0); } else { + // Make the attribute name explicit, before we add the new value below //noinspection ConstantConditions return ((J.Annotation) JavaTemplate.builder("value = #{any()}") .contextSensitive()