From d275168c9b2ca482d2d3b704a249dd036dc42369 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 13 Aug 2026 20:11:16 +0200 Subject: [PATCH 1/2] Add `RemoveDuplicateAnnotations` When several distinct annotations are migrated to a single new annotation -- for instance `javax.annotation.Nullable` and `javax.annotation.CheckForNull` both becoming `org.jspecify.annotations.Nullable` -- each `ChangeType` rewrites its own annotation, leaving the element annotated twice. Remove annotations repeated on the same element, keeping the first occurrence. Only annotations semantically equal to an earlier one are removed, and `@Repeatable` annotations are left alone. --- .../RemoveDuplicateAnnotations.java | 137 ++++++++ .../resources/META-INF/rewrite/recipes.csv | 7 +- .../RemoveDuplicateAnnotationsTest.java | 295 ++++++++++++++++++ 3 files changed, 437 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotations.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotationsTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotations.java b/src/main/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotations.java new file mode 100644 index 000000000..0e0ceb32f --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotations.java @@ -0,0 +1,137 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Option; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.TypeMatcher; +import org.openrewrite.java.search.SemanticallyEqual; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.TypeUtils; + +import java.util.ArrayList; +import java.util.List; + +@EqualsAndHashCode(callSuper = false) +@Value +public class RemoveDuplicateAnnotations extends Recipe { + + @Option(displayName = "Annotation type", + description = "The type of annotation to deduplicate, as a type pattern. " + + "Defaults to any annotation.", + example = "org.jspecify.annotations.*", + required = false) + @Nullable + String annotationType; + + String displayName = "Remove duplicate annotations"; + + String description = "Remove annotations that are repeated on the same element, keeping only the first occurrence. " + + "Duplicates typically arise when several distinct annotations are migrated to a single new annotation, " + + "such as when both `javax.annotation.Nullable` and `javax.annotation.CheckForNull` become " + + "`org.jspecify.annotations.Nullable`. " + + "`@Repeatable` annotations are left alone, as repeating those is meaningful."; + + @Override + public TreeVisitor getVisitor() { + JavaIsoVisitor visitor = new JavaIsoVisitor() { + final @Nullable TypeMatcher typeMatcher = annotationType == null ? null : new TypeMatcher(annotationType); + + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); + return cd.withLeadingAnnotations(removeDuplicates(cd.getLeadingAnnotations())); + } + + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { + J.MethodDeclaration md = super.visitMethodDeclaration(method, ctx); + return md.withLeadingAnnotations(removeDuplicates(md.getLeadingAnnotations())); + } + + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx); + return mv.withLeadingAnnotations(removeDuplicates(mv.getLeadingAnnotations())); + } + + @Override + public J.Modifier visitModifier(J.Modifier modifier, ExecutionContext ctx) { + J.Modifier m = super.visitModifier(modifier, ctx); + return m.withAnnotations(removeDuplicates(m.getAnnotations())); + } + + @Override + public J.AnnotatedType visitAnnotatedType(J.AnnotatedType annotatedType, ExecutionContext ctx) { + J.AnnotatedType at = super.visitAnnotatedType(annotatedType, ctx); + return at.withAnnotations(removeDuplicates(at.getAnnotations())); + } + + @Override + public J.ArrayType visitArrayType(J.ArrayType arrayType, ExecutionContext ctx) { + J.ArrayType at = super.visitArrayType(arrayType, ctx); + return at.withAnnotations(removeDuplicates(at.getAnnotations())); + } + + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, ExecutionContext ctx) { + J.Identifier id = super.visitIdentifier(identifier, ctx); + return id.withAnnotations(removeDuplicates(id.getAnnotations())); + } + + private @Nullable List removeDuplicates(@Nullable List annotations) { + if (annotations == null || annotations.size() < 2) { + return annotations; + } + List kept = new ArrayList<>(annotations.size()); + return ListUtils.filter(annotations, annotation -> { + if (isDeduplicable(annotation) && + kept.stream().anyMatch(earlier -> SemanticallyEqual.areEqual(earlier, annotation))) { + return false; + } + kept.add(annotation); + return true; + }); + } + + private boolean isDeduplicable(J.Annotation annotation) { + JavaType.FullyQualified fq = TypeUtils.asFullyQualified(annotation.getType()); + return fq != null && !isRepeatable(fq) && (typeMatcher == null || typeMatcher.matches(fq)); + } + + private boolean isRepeatable(JavaType.FullyQualified annotationType) { + for (JavaType.FullyQualified metaAnnotation : annotationType.getAnnotations()) { + //noinspection ConstantValue + if (metaAnnotation != null && TypeUtils.isOfClassType(metaAnnotation, "java.lang.annotation.Repeatable")) { + return true; + } + } + return false; + } + }; + return annotationType == null ? visitor : Preconditions.check(new UsesType<>(annotationType, null), visitor); + } +} diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index a7951e3a3..417d4db7a 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -49,7 +49,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FinalizeLocalVariables,Finalize local variables,Adds the `final` modifier keyword to local variables which are not reassigned.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FinalizeMethodArguments,Finalize method arguments,Adds the `final` modifier keyword to method parameters.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FinalizePrivateFields,Finalize private fields,Adds the `final` modifier keyword to private instance variables which are not reassigned.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, -maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FindMissingJavadocOnPublicMethods,Find public methods missing Javadoc,"Locates `public` method declarations that are not documented with a Javadoc comment, marks them with a search result, and records them in a data table.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.MissingJavadocOnPublicMethods"",""displayName"":""Public methods missing Javadoc"",""instanceName"":""Public methods missing Javadoc"",""description"":""Public method declarations that are not documented with a Javadoc comment."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the undocumented method.""},{""name"":""className"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class declaring the method.""},{""name"":""methodName"",""type"":""String"",""displayName"":""Method name"",""description"":""The name of the undocumented method.""},{""name"":""parameterTypes"",""type"":""String"",""displayName"":""Parameter types"",""description"":""A comma-separated list of the method's parameter types, empty for no-arg methods.""}]}]" +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FindMissingJavadocOnPublicMethods,Find public methods missing Javadoc,"Locates `public` method declarations that are not documented with a Javadoc comment, marks them with a search result, and records them in a data table.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.MissingJavadocOnPublicMethods"",""displayName"":""Public methods missing Javadoc"",""instanceName"":""Public methods missing Javadoc"",""description"":""Public method declarations that are not documented with a Javadoc comment."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the undocumented method.""},{""name"":""className"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class declaring the method.""},{""name"":""methodName"",""type"":""String"",""displayName"":""Method name"",""description"":""The name of the undocumented method.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FindNewExceptionWithoutCause,Find new exceptions thrown without the caught exception,"Finds `catch` blocks that throw a newly created exception without referencing the caught exception, which discards the original exception's stack trace and message. Data flow (taint) tracking is used to establish whether the caught exception—or any value derived from it—reaches the thrown exception, so indirect references through local variables and string concatenation are not falsely reported. This mirrors PMD's `PreserveStackTrace` rule.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.ExceptionsWithoutCause"",""displayName"":""Exceptions thrown without the caught cause"",""instanceName"":""Exceptions thrown without the caught cause"",""description"":""New exceptions thrown from a `catch` block that do not reference the caught exception."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the offending `throw`.""},{""name"":""caughtType"",""type"":""String"",""displayName"":""Caught exception type"",""description"":""The declared type of the exception caught by the enclosing `catch` clause.""},{""name"":""thrownType"",""type"":""String"",""displayName"":""Thrown exception type"",""description"":""The type of the new exception thrown without referencing the caught exception.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.FixStringFormatExpressions,Fix `String#format` and `String#formatted` expressions,"Fix `String#format` and `String#formatted` expressions by replacing `\n` newline characters with `%n` and removing any unused arguments. Note this recipe is scoped to only transform format expressions which do not specify the argument index. Using `%n` ensures the correct platform-specific line separator, and removing unused arguments eliminates dead code that may mask a mismatch between the format string and its parameters.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ForLoopControlVariablePostfixOperators,`for` loop counters should use postfix operators,Replace `for` loop control variables using pre-increment (`++i`) or pre-decrement (`--i`) operators with their post-increment (`i++`) or post-decrement (`i++`) notation equivalents.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, @@ -100,6 +100,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReferentialEqualityToObjectEquals,Replace referential equality operators with Object equals method invocations when the operands both override `Object.equals(Object obj)`,"Using `==` or `!=` compares object references, not the equality of two objects. This modifies code where both sides of a binary operation (`==` or `!=`) override `Object.equals(Object obj)` except when the comparison is within an overridden `Object.equals(Object obj)` method declaration itself. The resulting transformation must be carefully reviewed since any modifications change the program's semantics. When a class defines its own notion of equality through `equals`, using reference comparison is almost always a bug that causes logically identical objects to be treated as different.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveCallsToObjectFinalize,Remove `Object.finalize()` invocations,"Remove calls to `Object.finalize()`. This method is called during garbage collection and calling it manually is misleading. Explicit finalize invocations can trigger resource cleanup prematurely while the object is still in use, leading to unpredictable behavior.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveCallsToSystemGc,Remove garbage collection invocations,Removes calls to `System.gc()` and `Runtime.gc()`. When to invoke garbage collection is best left to the JVM. Manual GC calls produce unpredictable results across different JVM implementations and can cause unnecessary application pauses.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveDuplicateAnnotations,Remove duplicate annotations,"Remove annotations that are repeated on the same element, keeping only the first occurrence. Duplicates typically arise when several distinct annotations are migrated to a single new annotation, such as when both `javax.annotation.Nullable` and `javax.annotation.CheckForNull` become `org.jspecify.annotations.Nullable`. `@Repeatable` annotations are left alone, as repeating those is meaningful.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,"[{""name"":""annotationType"",""type"":""String"",""displayName"":""Annotation type"",""description"":""The type of annotation to deduplicate, as a type pattern. Defaults to any annotation."",""example"":""org.jspecify.annotations.*""}]", maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveDuplicateConditions,Related "if/else if" conditions should not be the same,"When an `if`/`else if` chain contains the same condition more than once, the second branch can never execute because the first matching branch always wins. The duplicate branch is dead code and should be removed.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveEmptyJavaDocParameters,"Remove JavaDoc `@param`, `@return`, and `@throws` with no description","Removes `@param`, `@return`, and `@throws` with no description from JavaDocs.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveExtraSemicolons,Remove extra semicolons,"Removes not needed semicolons. Semicolons are considered not needed: @@ -202,6 +203,8 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly The `throws` declaration is retained on overridable methods (package-private and `protected` methods on non-`final` classes), and on `public` methods overridden within the same source file, so that a subclass override which does throw the exception keeps compiling. Overrides in other source files cannot be detected without a scanning recipe and are therefore not accounted for. +When a `throws` declaration is removed, any `@throws` or `@exception` JavaDoc tag documenting that exception is removed along with it, so that the documentation does not describe an exception the method no longer declares. + Declaring exceptions that are never thrown misleads callers into writing unnecessary error-handling code and obscures the method's true behavior.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UnwrapElseAfterReturn,Unwrap else block after return or throw statement,"Unwraps the else block when the if block ends with a return or throw statement, reducing nesting and improving code readability.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UnwrapRepeatableAnnotations,Unwrap `@Repeatable` annotations,"Java 8 introduced the concept of `@Repeatable` annotations, making the wrapper annotation unnecessary. Using the repeatable form directly reduces nesting and makes the individual annotations easier to scan.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, @@ -211,7 +214,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseDiamondOperator,Use the diamond operator,"The diamond operator (`<>`) should be used. Java 7 introduced the diamond operator to reduce the verbosity of generics code. For instance, instead of having to declare a `List`'s type in both its declaration and its constructor, you can now simplify the constructor declaration with `<>`, and the compiler will infer the type. Repeating type arguments that the compiler can already deduce is unnecessary boilerplate that clutters the code.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseForEachRemoveInsteadOfSetRemoveAll,Replace `java.util.Set#removeAll(java.util.Collection)` with `java.util.Collection#forEach(Set::remove)`,Using `java.util.Collection#forEach(Set::remove)` rather than `java.util.Set#removeAll(java.util.Collection)` may improve performance due to a possible O(n^2) complexity.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseJavaStyleArrayDeclarations,No C-style array declarations,"Change C-Style array declarations `int i[];` to `int[] i;`. Keeping the brackets with the type groups all type information in one place, so readers do not have to inspect both the type and the variable name to determine whether something is an array.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, -maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface,Use lambda expressions instead of anonymous classes,"Instead of anonymous class declarations, use a lambda where possible. Using lambdas to replace anonymous classes can lead to more expressive and maintainable code, improve code readability, reduce code duplication, and achieve better performance in some cases.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface,Use lambda expressions instead of anonymous classes,"Instead of anonymous class declarations, use a lambda where possible. Using lambdas to replace anonymous classes can lead to more expressive and maintainable code, improve code readability, reduce code duplication, and achieve better performance in some cases.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.AnonymousFunctionalInterfaceImplementations"",""displayName"":""Anonymous functional interface implementations"",""instanceName"":""Anonymous functional interface implementations"",""description"":""Every anonymous class that implements a functional interface, whether or not it could be rewritten to a lambda, plus the sites that could not be decided either way because the supertype carries incomplete type attribution. Sites that were not rewritten carry the reason why."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the anonymous class.""},{""name"":""enclosingClass"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class containing the anonymous class.""},{""name"":""functionalInterface"",""type"":""String"",""displayName"":""Functional interface"",""description"":""The fully qualified name of the functional interface being implemented, or the supertype as written at the site when it did not resolve.""},{""name"":""method"",""type"":""String"",""displayName"":""Method"",""description"":""The name of the interface's single abstract method, or empty when type attribution was too incomplete to identify one.""},{""name"":""convertible"",""type"":""boolean"",""displayName"":""Convertible to lambda"",""description"":""Whether the anonymous class could be rewritten to a lambda automatically.""},{""name"":""reason"",""type"":""String"",""displayName"":""Reason"",""description"":""Why the anonymous class was not rewritten, or empty when it was. Reasons naming missing type information mark sites the recipe is blind to rather than sites that are genuinely unconvertible, which usually means the LST was built without the dependencies those types come from.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseListSort,"Replace invocations of `Collections#sort(List, Comparator)` with `List#sort(Comparator)`","The `java.util.Collections#sort(..)` implementation defers to the `java.util.List#sort(Comparator)`, replaced it with the `java.util.List#sort(Comparator)` implementation for better readability.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseMapContainsKey,Use `Map#containsKey`,`map.keySet().contains(a)` can be simplified to `map.containsKey(a)`.,2,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseObjectNotifyAll,Replaces `Object.notify()` with `Object.notifyAll()`,"`Object.notifyAll()` and `Object.notify()` both wake up sleeping threads, but `Object.notify()` only rouses one while `Object.notifyAll()` rouses all of them. Since `Object.notify()` might not wake up the right thread, `Object.notifyAll()` should be used instead. See [this](https://wiki.sei.cmu.edu/confluence/display/java/THI02-J.+Notify+all+waiting+threads+rather+than+a+single+thread) for more information. Using `notify()` in a multi-waiter scenario risks leaving threads permanently stalled when the wrong one is awakened.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotationsTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotationsTest.java new file mode 100644 index 000000000..d33416742 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateAnnotationsTest.java @@ -0,0 +1,295 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/1199") +class RemoveDuplicateAnnotationsTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec + .recipe(new RemoveDuplicateAnnotations("org.jspecify.annotations.*")) + .parser(JavaParser.fromJavaVersion().classpath("jspecify")); + } + + @DocumentExample + @Test + void removeDuplicateAnnotationOnMethodParameter() { + rewriteRun( + //language=java + java( + """ + import org.jspecify.annotations.Nullable; + + class Foo { + public void add(@Nullable @org.jspecify.annotations.Nullable final String bar) { + } + } + """, + """ + import org.jspecify.annotations.Nullable; + + class Foo { + public void add(@Nullable final String bar) { + } + } + """ + ) + ); + } + + @Test + void removeDuplicateAnnotationOnField() { + rewriteRun( + //language=java + java( + """ + import org.jspecify.annotations.Nullable; + + class Foo { + @Nullable + @Nullable + private String bar; + } + """, + """ + import org.jspecify.annotations.Nullable; + + class Foo { + @Nullable + private String bar; + } + """ + ) + ); + } + + @Test + void removeDuplicateAnnotationOnMethodReturnType() { + rewriteRun( + //language=java + java( + """ + import org.jspecify.annotations.Nullable; + + class Foo { + @Nullable + @Nullable + public String bar() { + return null; + } + } + """, + """ + import org.jspecify.annotations.Nullable; + + class Foo { + @Nullable + public String bar() { + return null; + } + } + """ + ) + ); + } + + @Test + void removeDuplicateAnnotationOnClass() { + rewriteRun( + //language=java + java( + """ + import org.jspecify.annotations.NullMarked; + + @NullMarked + @NullMarked + class Foo { + } + """, + """ + import org.jspecify.annotations.NullMarked; + + @NullMarked + class Foo { + } + """ + ) + ); + } + + @Test + void removeDuplicateTypeUseAnnotation() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + import org.jspecify.annotations.Nullable; + + class Foo { + private List<@Nullable @Nullable String> bar; + } + """, + """ + import java.util.List; + import org.jspecify.annotations.Nullable; + + class Foo { + private List<@Nullable String> bar; + } + """ + ) + ); + } + + @Test + void retainSingleAnnotation() { + rewriteRun( + //language=java + java( + """ + import org.jspecify.annotations.NonNull; + import org.jspecify.annotations.Nullable; + + class Foo { + public @Nullable String bar(@NonNull String baz) { + return null; + } + } + """ + ) + ); + } + + @Test + void retainDistinctAnnotations() { + rewriteRun( + //language=java + java( + """ + import org.jspecify.annotations.NonNull; + import org.jspecify.annotations.Nullable; + + class Foo { + public void bar(@Nullable @NonNull String baz) { + } + } + """ + ) + ); + } + + @Test + void retainAnnotationsOfOtherTypes() { + rewriteRun( + //language=java + java( + """ + class Foo { + @SuppressWarnings("all") + @SuppressWarnings("all") + public void bar() { + } + } + """ + ) + ); + } + + @Test + void removeDuplicateAnnotationOfAnyTypeByDefault() { + rewriteRun( + spec -> spec.recipe(new RemoveDuplicateAnnotations(null)), + //language=java + java( + """ + class Foo { + @SuppressWarnings("all") + @SuppressWarnings("all") + public void bar() { + } + } + """, + """ + class Foo { + @SuppressWarnings("all") + public void bar() { + } + } + """ + ) + ); + } + + @Test + void retainAnnotationsWithDifferentArguments() { + rewriteRun( + spec -> spec.recipe(new RemoveDuplicateAnnotations(null)), + //language=java + java( + """ + @interface Tag { + String value(); + } + + class Foo { + @Tag("a") + @Tag("b") + private String bar; + } + """ + ) + ); + } + + @Test + void retainRepeatableAnnotations() { + rewriteRun( + spec -> spec.recipe(new RemoveDuplicateAnnotations(null)), + //language=java + java( + """ + import java.lang.annotation.Repeatable; + + @Repeatable(Tags.class) + @interface Tag { + String value(); + } + + @interface Tags { + Tag[] value(); + } + + class Foo { + @Tag("a") + @Tag("a") + private String bar; + } + """ + ) + ); + } +} From e2e55fef56ec6373bd678472463691e0f5ad5a12 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 13 Aug 2026 20:27:59 +0200 Subject: [PATCH 2/2] Skip the JavaScript RPC tests when their npm package is unavailable A published rewrite-javascript snapshot pins an exact @openrewrite/rewrite version, and the npm release of that version can lag the Maven one. The RPC process then has nothing to run and every typescript() test fails. warmJavaScriptRpcCache already writes its marker only for an install that succeeded, so publish that as javaScriptRpcAvailable and let the tests that need the RPC server skip rather than fail the build over a gap upstream. This also covers a machine without Node, which the warm task already declines to require. --- build.gradle.kts | 15 ++++++- .../AllBranchesIdenticalTest.java | 1 + .../AnnotateNullableMethodsTest.java | 1 + .../CollapsibleIfStatementsTest.java | 1 + .../staticanalysis/DefaultComesLastTest.java | 1 + .../MergeIdenticalBranchesTest.java | 1 + .../RemoveDuplicateConditionsTest.java | 1 + .../RemoveSelfAssignmentTest.java | 1 + ...RemoveUnconditionalValueOverwriteTest.java | 1 + .../RemoveUnusedLocalVariablesTest.java | 1 + .../staticanalysis/RequiresJavaScriptRpc.java | 40 +++++++++++++++++++ ...implifyRedundantLogicalExpressionTest.java | 1 + 12 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/openrewrite/staticanalysis/RequiresJavaScriptRpc.java diff --git a/build.gradle.kts b/build.gradle.kts index 75c319e6a..68e9117f3 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,6 +1,7 @@ @file:Suppress("UnstableApiUsage") import java.io.InputStream +import org.gradle.process.CommandLineArgumentProvider import org.gradle.process.ExecOperations import org.gradle.kotlin.dsl.support.serviceOf @@ -57,11 +58,16 @@ dependencies { // into the same `~/.npm/_npx` directory, and the resulting overlap leaves the package half-written, so // the tests fail with "RPC process shut down early". Installing once up front keeps every spawn a // cache hit. +// +// The marker is present once that install succeeded and the tests can spawn the package, and absent +// when it did not happen, whether for a missing npx or a version npm does not have. +val javaScriptRpcMarker = layout.buildDirectory.file("tmp/warmJavaScriptRpcCache/version.txt") + val warmJavaScriptRpcCache by tasks.registering { description = "Installs the npm package that the JavaScript RPC tests spawn, so they never race on a cold npx cache." val rewriteJavaScriptJars = configurations.named("testRuntimeClasspath") .map { classpath -> classpath.filter { it.name.startsWith("rewrite-javascript-") } } - val marker = layout.buildDirectory.file("tmp/warmJavaScriptRpcCache/version.txt") + val marker = javaScriptRpcMarker val npx = if (System.getProperty("os.name").lowercase().contains("windows")) "npx.cmd" else "npx" val execOperations = serviceOf() @@ -109,6 +115,13 @@ val warmJavaScriptRpcCache by tasks.registering { tasks.withType { jvmArgs("-Xmx1g", "-Xms512m") dependsOn(warmJavaScriptRpcCache) + // A published rewrite-javascript snapshot pins an exact @openrewrite/rewrite version, and the npm + // release of that version can lag the Maven one, leaving nothing for the RPC process to run. Tests + // annotated with @RequiresJavaScriptRpc skip rather than fail the build over that gap upstream. + val marker = javaScriptRpcMarker + jvmArgumentProviders.add(CommandLineArgumentProvider { + listOf("-DjavaScriptRpcAvailable=${marker.get().asFile.isFile}") + }) } tasks.withType { diff --git a/src/test/java/org/openrewrite/staticanalysis/AllBranchesIdenticalTest.java b/src/test/java/org/openrewrite/staticanalysis/AllBranchesIdenticalTest.java index add79971c..9499ab97a 100644 --- a/src/test/java/org/openrewrite/staticanalysis/AllBranchesIdenticalTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/AllBranchesIdenticalTest.java @@ -248,6 +248,7 @@ void test(boolean a) { } @Test + @RequiresJavaScriptRpc void collapseIdenticalBranchesTypeScript() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/AnnotateNullableMethodsTest.java b/src/test/java/org/openrewrite/staticanalysis/AnnotateNullableMethodsTest.java index c3e436c7a..865a348a4 100644 --- a/src/test/java/org/openrewrite/staticanalysis/AnnotateNullableMethodsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/AnnotateNullableMethodsTest.java @@ -666,6 +666,7 @@ public class Test { } @Test + @RequiresJavaScriptRpc void typescriptCode() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/CollapsibleIfStatementsTest.java b/src/test/java/org/openrewrite/staticanalysis/CollapsibleIfStatementsTest.java index 772b87329..a795c0d8a 100644 --- a/src/test/java/org/openrewrite/staticanalysis/CollapsibleIfStatementsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CollapsibleIfStatementsTest.java @@ -297,6 +297,7 @@ void test(boolean a, boolean b) { } @Test + @RequiresJavaScriptRpc void mergeNestedIfsTypeScript() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/DefaultComesLastTest.java b/src/test/java/org/openrewrite/staticanalysis/DefaultComesLastTest.java index e546d4422..3df9f8213 100644 --- a/src/test/java/org/openrewrite/staticanalysis/DefaultComesLastTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/DefaultComesLastTest.java @@ -780,6 +780,7 @@ void test (int state) { } @Test + @RequiresJavaScriptRpc void doNotChangeNonJavaLanguages() { rewriteRun( typescript( diff --git a/src/test/java/org/openrewrite/staticanalysis/MergeIdenticalBranchesTest.java b/src/test/java/org/openrewrite/staticanalysis/MergeIdenticalBranchesTest.java index 94d99fac0..bcbe14917 100644 --- a/src/test/java/org/openrewrite/staticanalysis/MergeIdenticalBranchesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/MergeIdenticalBranchesTest.java @@ -269,6 +269,7 @@ void test(boolean a, boolean b) { } @Test + @RequiresJavaScriptRpc void mergeIdenticalBranchesTypeScript() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateConditionsTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateConditionsTest.java index a15afd044..135b65ca8 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateConditionsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveDuplicateConditionsTest.java @@ -235,6 +235,7 @@ void test(int x) { } @Test + @RequiresJavaScriptRpc void removeDuplicateElseIfTypeScript() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveSelfAssignmentTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveSelfAssignmentTest.java index 392d2be55..b9c0ff127 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveSelfAssignmentTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveSelfAssignmentTest.java @@ -246,6 +246,7 @@ public J.Identifier visitIdentifier(J.Identifier identifier, Integer p) { } @Test + @RequiresJavaScriptRpc void removeSelfAssignmentTypeScript() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnconditionalValueOverwriteTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnconditionalValueOverwriteTest.java index 53031a145..22767228e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnconditionalValueOverwriteTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnconditionalValueOverwriteTest.java @@ -240,6 +240,7 @@ void test() { } @Test + @RequiresJavaScriptRpc void removeOverwrittenMapSetTypeScript() { rewriteRun( //language=typescript diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java index fccbbd330..78833550d 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java @@ -1373,6 +1373,7 @@ fun initializerRemoved() : String { } @Nested + @RequiresJavaScriptRpc class Typescript { @Test void noChange() { diff --git a/src/test/java/org/openrewrite/staticanalysis/RequiresJavaScriptRpc.java b/src/test/java/org/openrewrite/staticanalysis/RequiresJavaScriptRpc.java new file mode 100644 index 000000000..c83c52621 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/RequiresJavaScriptRpc.java @@ -0,0 +1,40 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.condition.DisabledIfSystemProperty; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a test that parses JavaScript or TypeScript, which spawns an out-of-process RPC server from the + * {@code @openrewrite/rewrite} npm package, pinned to the exact version of the resolved + * {@code rewrite-javascript} jar. + *

+ * The {@code warmJavaScriptRpcCache} build task installs that package up front and sets + * {@code javaScriptRpcAvailable} to whether it succeeded. It does not when npx is absent, or while a + * published {@code rewrite-javascript} snapshot is still waiting on its matching npm release; the RPC + * process then has nothing to run, so these tests skip instead of failing over a gap upstream. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +@DisabledIfSystemProperty(named = "javaScriptRpcAvailable", matches = "false", + disabledReason = "The @openrewrite/rewrite npm package matching the resolved rewrite-javascript version is not installed") +public @interface RequiresJavaScriptRpc { +} diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyRedundantLogicalExpressionTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyRedundantLogicalExpressionTest.java index 121b75bb6..3922fb1d0 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyRedundantLogicalExpressionTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyRedundantLogicalExpressionTest.java @@ -300,6 +300,7 @@ boolean test(boolean a) { } @Test + @RequiresJavaScriptRpc void simplifyLogicalAndTypeScript() { rewriteRun( //language=typescript