From b0a9d77bceeda0c26cfb5dec09b12951a0153f1c Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Tue, 11 Aug 2026 20:05:45 +0200 Subject: [PATCH 1/2] Preserve meaningful method signatures in RemoveMethodsOnlyCallSuper --- .../RemoveMethodsOnlyCallSuper.java | 33 +++++-- .../RemoveMethodsOnlyCallSuperTest.java | 92 +++++++++++++++++++ 2 files changed, 119 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java b/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java index 29bc46da4..46fee9261 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java @@ -24,13 +24,13 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.AnnotationMatcher; import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.service.AnnotationService; import org.openrewrite.java.tree.*; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; import java.time.Duration; import java.util.List; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Collections.singleton; @@ -85,11 +85,8 @@ public TreeVisitor getVisitor() { return md; } - // Skip if method has annotations other than @Override - for (J.Annotation annotation : service(AnnotationService.class).getAllAnnotations(getCursor())) { - if (!OVERRIDE.matches(annotation)) { - return md; - } + if (hasSemanticAnnotation(md)) { + return md; } // Skip if method has Javadoc comments @@ -110,6 +107,11 @@ public TreeVisitor getVisitor() { return md; } + if (md.hasModifier(J.Modifier.Type.Strictfp) && + (superCall.getMethodType() == null || !superCall.getMethodType().hasFlags(Flag.Strictfp))) { + return md; + } + // Skip if method widens visibility compared to the overridden method if (widensVisibility(methodType)) { return md; @@ -132,6 +134,25 @@ public TreeVisitor getVisitor() { return null; } + private boolean hasSemanticAnnotation(J.MethodDeclaration method) { + AtomicBoolean found = new AtomicBoolean(); + new JavaVisitor() { + @Override + public J.Annotation visitAnnotation(J.Annotation annotation, AtomicBoolean ignored) { + if (!OVERRIDE.matches(annotation)) { + found.set(true); + } + return annotation; + } + + @Override + public J.Block visitBlock(J.Block block, AtomicBoolean ignored) { + return block; + } + }.visit(method, found); + return found.get(); + } + private boolean argumentsMatchParameters(List parameters, List arguments) { int argIndex = 0; int paramCount = 0; diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java index 5f10fe5aa..39fbaa864 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java @@ -381,6 +381,98 @@ void save() { ); } + @Test + void doNotChangeMethodsWithSignatureAnnotations() { + rewriteRun( + //language=java + java( + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target({ElementType.PARAMETER, ElementType.TYPE_USE}) + @interface Nullable {} + + class Parent { + void foo(String s) { + } + + String[] bar() { + return new String[0]; + } + } + + class Child extends Parent { + @Override + void foo(@Nullable String s) { + super.foo(s); + } + + @Override + String @Nullable [] bar() { + return super.bar(); + } + } + """ + ) + ); + } + + @Test + void handlesStrictfpAccordingToTheSuperMethod() { + rewriteRun( + //language=java + java( + """ + class OrdinaryParent { + void foo() { + } + } + + class OrdinaryChild extends OrdinaryParent { + @Override + strictfp void foo() { + super.foo(); + } + } + + class StrictParent { + strictfp void foo() { + } + } + + class StrictChild extends StrictParent { + @Override + strictfp void foo() { + super.foo(); + } + } + """, + """ + class OrdinaryParent { + void foo() { + } + } + + class OrdinaryChild extends OrdinaryParent { + @Override + strictfp void foo() { + super.foo(); + } + } + + class StrictParent { + strictfp void foo() { + } + } + + class StrictChild extends StrictParent { + } + """ + ) + ); + } + @Test void doNotChangeMethodThatWidensVisibility() { rewriteRun( From eaf1137fa40718ac49bc01c7910661567eca5bce Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 17 Aug 2026 10:59:50 +0200 Subject: [PATCH 2/2] Use .reduce and split strictfp tests --- .../RemoveMethodsOnlyCallSuper.java | 11 ++-- .../RemoveMethodsOnlyCallSuperTest.java | 51 ++++++++++--------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java b/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java index 46fee9261..d900e03a3 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuper.java @@ -23,6 +23,7 @@ import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; import org.openrewrite.java.AnnotationMatcher; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.tree.*; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; @@ -135,10 +136,9 @@ public TreeVisitor getVisitor() { } private boolean hasSemanticAnnotation(J.MethodDeclaration method) { - AtomicBoolean found = new AtomicBoolean(); - new JavaVisitor() { + return new JavaIsoVisitor() { @Override - public J.Annotation visitAnnotation(J.Annotation annotation, AtomicBoolean ignored) { + public J.Annotation visitAnnotation(J.Annotation annotation, AtomicBoolean found) { if (!OVERRIDE.matches(annotation)) { found.set(true); } @@ -146,11 +146,10 @@ public J.Annotation visitAnnotation(J.Annotation annotation, AtomicBoolean ignor } @Override - public J.Block visitBlock(J.Block block, AtomicBoolean ignored) { + public J.Block visitBlock(J.Block block, AtomicBoolean found) { return block; } - }.visit(method, found); - return found.get(); + }.reduce(method, new AtomicBoolean()).get(); } private boolean argumentsMatchParameters(List parameters, List arguments) { diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java index 39fbaa864..b42902b2b 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveMethodsOnlyCallSuperTest.java @@ -419,29 +419,47 @@ void foo(@Nullable String s) { } @Test - void handlesStrictfpAccordingToTheSuperMethod() { + void doNotChangeStrictfpMethod() { rewriteRun( //language=java java( """ - class OrdinaryParent { + class Parent { void foo() { } } - - class OrdinaryChild extends OrdinaryParent { + """ + ), + //language=java + java( + """ + class Child extends Parent { @Override strictfp void foo() { super.foo(); } } + """ + ) + ); + } - class StrictParent { + @Test + void removeStrictfpMethodWhenSuperIsStrictfpToo() { + rewriteRun( + //language=java + java( + """ + class Parent { strictfp void foo() { } } - - class StrictChild extends StrictParent { + """ + ), + //language=java + java( + """ + class Child extends Parent { @Override strictfp void foo() { super.foo(); @@ -449,24 +467,7 @@ strictfp void foo() { } """, """ - class OrdinaryParent { - void foo() { - } - } - - class OrdinaryChild extends OrdinaryParent { - @Override - strictfp void foo() { - super.foo(); - } - } - - class StrictParent { - strictfp void foo() { - } - } - - class StrictChild extends StrictParent { + class Child extends Parent { } """ )