From 58b219419df4ffbdab7c2ee7372ba63d313e74b3 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Aug 2026 21:01:56 +0200 Subject: [PATCH 1/4] Remove now-invalid `@Override` when migrating `TestCase` subclasses `MigrateJUnitTestCase` only dropped `@Override` when it added a Jupiter lifecycle annotation, so `setUp()`/`tearDown()` that already carried `@Before`/`@After` kept an `@Override` that no longer compiles once `extends TestCase` is removed, as did `getName()` and `countTestCases()`. Now every method in a migrated `TestCase` subclass is checked against its remaining supertypes, skipping the `junit.framework` types that are going away, so `toString()`, interface implementations and overrides of an intermediate base that declares the method keep their annotation. Fixes #1103 --- .../testing/junit5/MigrateJUnitTestCase.java | 64 ++++- .../testing/junit5/JUnit5MigrationTest.java | 61 +++++ .../junit5/MigrateJUnitTestCaseTest.java | 249 ++++++++++++++++++ 3 files changed, 365 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java index 230129565..ea0253991 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java @@ -25,6 +25,7 @@ import org.openrewrite.java.*; import org.openrewrite.java.search.FindAnnotations; import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.Flag; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TextComment; @@ -148,7 +149,7 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex } else if ("tearDown".equals(md.getSimpleName()) && md.getLeadingAnnotations().stream().noneMatch(JUNIT_AFTER_ANNOTATION_MATCHER::matches)) { md = updateMethodDeclarationAnnotationAndModifier(md, "@AfterEach", "org.junit.jupiter.api.AfterEach", ctx); } - return md; + return maybeRemoveOverrideAnnotation(md, ctx); } @Override @@ -171,7 +172,6 @@ private J.MethodDeclaration updateMethodDeclarationAnnotationAndModifier(J.Metho .imports(fullyQualifiedAnnotation).build() .apply(getCursor(), methodDeclaration.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName))); md = maybeAddPublicModifier(md); - md = maybeRemoveOverrideAnnotation(md); maybeAddImport(fullyQualifiedAnnotation); } return md; @@ -187,14 +187,60 @@ private J.MethodDeclaration maybeAddPublicModifier(J.MethodDeclaration md) { return md.withModifiers(modifiers); } - private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md) { - return md.withLeadingAnnotations(ListUtils.map(md.getLeadingAnnotations(), annotation -> { - if (OVERRIDE_ANNOTATION_MATCHER.matches(annotation)) { - //noinspection DataFlowIssue - return null; + private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md, ExecutionContext ctx) { + if (md.getLeadingAnnotations().stream().noneMatch(OVERRIDE_ANNOTATION_MATCHER::matches) || + stillOverridesAfterMigration(md.getMethodType())) { + return md; + } + // Strip the body first, such that any `@Override` on methods of anonymous classes within is retained + J.MethodDeclaration withoutBody = (J.MethodDeclaration) new RemoveAnnotationVisitor(OVERRIDE_ANNOTATION_MATCHER) + .visitNonNull(md.withBody(null), ctx, getCursor().getParentOrThrow()); + return withoutBody.withBody(md.getBody()); + } + + /** + * Once the {@code junit.framework} supertypes are dropped, an {@code @Override} is only still valid if some + * other supertype declares a matching method; {@code toString()} keeps it, {@code setUp()} does not. + */ + private boolean stillOverridesAfterMigration(JavaType.@Nullable Method methodType) { + if (methodType == null) { + return true; + } + JavaType.FullyQualified declaringType = methodType.getDeclaringType(); + return declaresOverridableMethod(declaringType.getSupertype(), methodType) || + declaringType.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, methodType)); + } + + private boolean declaresOverridableMethod(JavaType.@Nullable FullyQualified type, JavaType.Method method) { + if (type == null) { + return false; + } + if (!"junit.framework".equals(type.getPackageName())) { + for (JavaType.Method candidate : type.getMethods()) { + if (!candidate.getFlags().contains(Flag.Private) && + !candidate.getFlags().contains(Flag.Static) && + candidate.getName().equals(method.getName()) && + parameterTypesMatch(candidate, method)) { + return true; + } + } + } + return declaresOverridableMethod(type.getSupertype(), method) || + type.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, method)); + } + + private boolean parameterTypesMatch(JavaType.Method a, JavaType.Method b) { + List aParameterTypes = a.getParameterTypes(); + List bParameterTypes = b.getParameterTypes(); + if (aParameterTypes.size() != bParameterTypes.size()) { + return false; + } + for (int i = 0; i < aParameterTypes.size(); i++) { + if (!TypeUtils.isOfType(aParameterTypes.get(i), bParameterTypes.get(i))) { + return false; } - return annotation; - })); + } + return true; } } } diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 46db301fd..2035a1cbb 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -1025,4 +1025,65 @@ public void shouldPass() { ) ); } + + @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/1103") + @Test + void removeOverrideOnMigratedTestCaseMethods() { + rewriteRun( + //language=java + java( + """ + import junit.framework.TestCase; + + import org.junit.Before; + import org.junit.Test; + + public class MathTest extends TestCase { + protected long value1; + + @Override + @Before + public void setUp() { + value1 = 2; + } + + @Override + public String toString() { + return "math"; + } + + @Test + public void testAdd() { + assertEquals(2, value1); + } + } + """, + """ + import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + public class MathTest { + protected long value1; + + @BeforeEach + public void setUp() { + value1 = 2; + } + + @Override + public String toString() { + return "math"; + } + + @Test + public void testAdd() { + assertEquals(2, value1); + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java b/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java index 6f903bc18..ab59e6d2e 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java @@ -176,6 +176,7 @@ public class MathTest extends CTest { protected long value2; @BeforeEach + @Override public void setUp() { value1 = 2; value2 = 3; @@ -188,6 +189,7 @@ public void testAdd() { } @AfterEach + @Override public void tearDown() { value1 = 0; value2 = 0; @@ -426,4 +428,251 @@ public void testApp() { ) ); } + + @Test + void removeOverrideWhenAlreadyAnnotatedWithBefore() { + rewriteRun( + spec -> spec.recipes( + new MigrateJUnitTestCase(), + new UpdateBeforeAfterAnnotations() + ), + //language=java + java( + """ + import junit.framework.TestCase; + + import org.junit.Before; + + public class MathTest extends TestCase { + protected long value1; + + @Override + @Before + public void setUp() { + value1 = 2; + } + + public void testAdd() { + assertEquals(2, value1); + } + } + """, + """ + import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + public class MathTest { + protected long value1; + + @BeforeEach + public void setUp() { + value1 = 2; + } + + @Test + public void testAdd() { + assertEquals(2, value1); + } + } + """ + ) + ); + } + + @Test + void removeOverrideFromOtherTestCaseMethods() { + //language=java + rewriteRun( + java( + """ + import junit.framework.TestCase; + + public class MathTest extends TestCase { + @Override + public String getName() { + return "math"; + } + + @Override + public int countTestCases() { + return 1; + } + + public void testAdd() { + assertEquals(2, 2); + } + } + """, + """ + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + public class MathTest { + public String getName() { + return "math"; + } + + public int countTestCases() { + return 1; + } + + @Test + public void testAdd() { + assertEquals(2, 2); + } + } + """ + ) + ); + } + + @Test + void retainOverrideOfObjectMethods() { + //language=java + rewriteRun( + java( + """ + import junit.framework.TestCase; + + public class MathTest extends TestCase { + @Override + public String toString() { + return "math"; + } + + @Override + public boolean equals(Object other) { + return other instanceof MathTest; + } + + @Override + public int hashCode() { + return 42; + } + + public void testAdd() { + assertEquals(2, 2); + } + } + """, + """ + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + public class MathTest { + @Override + public String toString() { + return "math"; + } + + @Override + public boolean equals(Object other) { + return other instanceof MathTest; + } + + @Override + public int hashCode() { + return 42; + } + + @Test + public void testAdd() { + assertEquals(2, 2); + } + } + """ + ) + ); + } + + @Test + void retainOverrideOfInterfaceMethod() { + //language=java + rewriteRun( + java( + """ + package com.abc; + public interface Named { + String describe(); + } + """ + ), + java( + """ + package com.abc; + import junit.framework.TestCase; + + public class MathTest extends TestCase implements Named { + @Override + public String describe() { + return "math"; + } + + public void testAdd() { + assertEquals(2, 2); + } + } + """, + """ + package com.abc; + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + public class MathTest implements Named { + @Override + public String describe() { + return "math"; + } + + @Test + public void testAdd() { + assertEquals(2, 2); + } + } + """ + ) + ); + } + + @Test + void retainOverrideInAnonymousClass() { + //language=java + rewriteRun( + java( + """ + import junit.framework.TestCase; + + public class MathTest extends TestCase { + @Override + public void setUp() { + Runnable runnable = new Runnable() { + @Override + public void run() { + } + }; + } + } + """, + """ + import org.junit.jupiter.api.BeforeEach; + + public class MathTest { + @BeforeEach + public void setUp() { + Runnable runnable = new Runnable() { + @Override + public void run() { + } + }; + } + } + """ + ) + ); + } } From d30bafd321e27c9a121de4eb3e48f89b5193adb9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Aug 2026 21:26:03 +0200 Subject: [PATCH 2/4] Simplify `@Override` removal after review --- .../testing/junit5/MigrateJUnitTestCase.java | 43 ++++++++++--------- .../junit5/MigrateJUnitTestCaseTest.java | 10 ++--- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java index ea0253991..f132bdcf8 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java @@ -33,7 +33,11 @@ import org.openrewrite.marker.Markers; import java.util.Comparator; +import java.util.HashSet; import java.util.List; +import java.util.Set; + +import static java.util.Arrays.asList; public class MigrateJUnitTestCase extends Recipe { @@ -102,6 +106,8 @@ public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionCon private static class TestCaseVisitor extends JavaIsoVisitor { private static final AnnotationMatcher OVERRIDE_ANNOTATION_MATCHER = new AnnotationMatcher("@java.lang.Override"); private static final MethodMatcher TEST_CASE_SUPER_MATCHER = new MethodMatcher("junit.framework.TestCase (..)"); + private static final Set SUPERTYPES_REMOVED_BY_MIGRATION = new HashSet<>(asList( + "junit.framework.TestCase", "junit.framework.Assert", "junit.framework.Test")); @Override public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { @@ -188,48 +194,43 @@ private J.MethodDeclaration maybeAddPublicModifier(J.MethodDeclaration md) { } private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md, ExecutionContext ctx) { - if (md.getLeadingAnnotations().stream().noneMatch(OVERRIDE_ANNOTATION_MATCHER::matches) || + if (md.getMethodType() == null || + md.getLeadingAnnotations().stream().noneMatch(OVERRIDE_ANNOTATION_MATCHER::matches) || stillOverridesAfterMigration(md.getMethodType())) { return md; } - // Strip the body first, such that any `@Override` on methods of anonymous classes within is retained J.MethodDeclaration withoutBody = (J.MethodDeclaration) new RemoveAnnotationVisitor(OVERRIDE_ANNOTATION_MATCHER) .visitNonNull(md.withBody(null), ctx, getCursor().getParentOrThrow()); return withoutBody.withBody(md.getBody()); } - /** - * Once the {@code junit.framework} supertypes are dropped, an {@code @Override} is only still valid if some - * other supertype declares a matching method; {@code toString()} keeps it, {@code setUp()} does not. - */ - private boolean stillOverridesAfterMigration(JavaType.@Nullable Method methodType) { - if (methodType == null) { - return true; - } - JavaType.FullyQualified declaringType = methodType.getDeclaringType(); - return declaresOverridableMethod(declaringType.getSupertype(), methodType) || - declaringType.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, methodType)); + private static boolean stillOverridesAfterMigration(JavaType.Method methodType) { + return anySupertypeDeclares(methodType.getDeclaringType(), methodType); + } + + private static boolean anySupertypeDeclares(JavaType.FullyQualified type, JavaType.Method method) { + return declaresOverridableMethod(type.getSupertype(), method) || + type.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, method)); } - private boolean declaresOverridableMethod(JavaType.@Nullable FullyQualified type, JavaType.Method method) { + private static boolean declaresOverridableMethod(JavaType.@Nullable FullyQualified type, JavaType.Method method) { if (type == null) { return false; } - if (!"junit.framework".equals(type.getPackageName())) { + if (!SUPERTYPES_REMOVED_BY_MIGRATION.contains(type.getFullyQualifiedName())) { for (JavaType.Method candidate : type.getMethods()) { - if (!candidate.getFlags().contains(Flag.Private) && - !candidate.getFlags().contains(Flag.Static) && - candidate.getName().equals(method.getName()) && + if (candidate.getName().equals(method.getName()) && + !candidate.hasFlags(Flag.Private) && + !candidate.hasFlags(Flag.Static) && parameterTypesMatch(candidate, method)) { return true; } } } - return declaresOverridableMethod(type.getSupertype(), method) || - type.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, method)); + return anySupertypeDeclares(type, method); } - private boolean parameterTypesMatch(JavaType.Method a, JavaType.Method b) { + private static boolean parameterTypesMatch(JavaType.Method a, JavaType.Method b) { List aParameterTypes = a.getParameterTypes(); List bParameterTypes = b.getParameterTypes(); if (aParameterTypes.size() != bParameterTypes.size()) { diff --git a/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java b/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java index ab59e6d2e..e5ec83b14 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java @@ -431,12 +431,8 @@ public void testApp() { @Test void removeOverrideWhenAlreadyAnnotatedWithBefore() { + //language=java rewriteRun( - spec -> spec.recipes( - new MigrateJUnitTestCase(), - new UpdateBeforeAfterAnnotations() - ), - //language=java java( """ import junit.framework.TestCase; @@ -458,7 +454,7 @@ public void testAdd() { } """, """ - import org.junit.jupiter.api.BeforeEach; + import org.junit.Before; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -466,7 +462,7 @@ public void testAdd() { public class MathTest { protected long value1; - @BeforeEach + @Before public void setUp() { value1 = 2; } From 522250b0bf944203c402bfcd453a86674b95fb80 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Aug 2026 22:00:19 +0200 Subject: [PATCH 3/4] Inline single-use `stillOverridesAfterMigration` --- .../java/testing/junit5/MigrateJUnitTestCase.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java index f132bdcf8..b47129856 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java @@ -194,9 +194,10 @@ private J.MethodDeclaration maybeAddPublicModifier(J.MethodDeclaration md) { } private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md, ExecutionContext ctx) { - if (md.getMethodType() == null || + JavaType.Method methodType = md.getMethodType(); + if (methodType == null || md.getLeadingAnnotations().stream().noneMatch(OVERRIDE_ANNOTATION_MATCHER::matches) || - stillOverridesAfterMigration(md.getMethodType())) { + anySupertypeDeclares(methodType.getDeclaringType(), methodType)) { return md; } J.MethodDeclaration withoutBody = (J.MethodDeclaration) new RemoveAnnotationVisitor(OVERRIDE_ANNOTATION_MATCHER) @@ -204,10 +205,6 @@ private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md return withoutBody.withBody(md.getBody()); } - private static boolean stillOverridesAfterMigration(JavaType.Method methodType) { - return anySupertypeDeclares(methodType.getDeclaringType(), methodType); - } - private static boolean anySupertypeDeclares(JavaType.FullyQualified type, JavaType.Method method) { return declaresOverridableMethod(type.getSupertype(), method) || type.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, method)); From 26c66e32611f74e09fddcefc623548feb73860f4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 24 Aug 2026 22:22:10 +0200 Subject: [PATCH 4/4] Delegate the override lookup to `TypeUtils.findOverriddenMethod` --- .../testing/junit5/MigrateJUnitTestCase.java | 43 ++++--------------- 1 file changed, 9 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java index b47129856..6d5890488 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java @@ -25,7 +25,6 @@ import org.openrewrite.java.*; import org.openrewrite.java.search.FindAnnotations; import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.Flag; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TextComment; @@ -35,6 +34,7 @@ import java.util.Comparator; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import static java.util.Arrays.asList; @@ -197,7 +197,7 @@ private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md JavaType.Method methodType = md.getMethodType(); if (methodType == null || md.getLeadingAnnotations().stream().noneMatch(OVERRIDE_ANNOTATION_MATCHER::matches) || - anySupertypeDeclares(methodType.getDeclaringType(), methodType)) { + stillOverridesAfterMigration(methodType)) { return md; } J.MethodDeclaration withoutBody = (J.MethodDeclaration) new RemoveAnnotationVisitor(OVERRIDE_ANNOTATION_MATCHER) @@ -205,40 +205,15 @@ private J.MethodDeclaration maybeRemoveOverrideAnnotation(J.MethodDeclaration md return withoutBody.withBody(md.getBody()); } - private static boolean anySupertypeDeclares(JavaType.FullyQualified type, JavaType.Method method) { - return declaresOverridableMethod(type.getSupertype(), method) || - type.getInterfaces().stream().anyMatch(i -> declaresOverridableMethod(i, method)); - } - - private static boolean declaresOverridableMethod(JavaType.@Nullable FullyQualified type, JavaType.Method method) { - if (type == null) { - return false; - } - if (!SUPERTYPES_REMOVED_BY_MIGRATION.contains(type.getFullyQualifiedName())) { - for (JavaType.Method candidate : type.getMethods()) { - if (candidate.getName().equals(method.getName()) && - !candidate.hasFlags(Flag.Private) && - !candidate.hasFlags(Flag.Static) && - parameterTypesMatch(candidate, method)) { - return true; - } + private static boolean stillOverridesAfterMigration(JavaType.Method method) { + Optional overridden = TypeUtils.findOverriddenMethod(method); + while (overridden.isPresent()) { + if (!SUPERTYPES_REMOVED_BY_MIGRATION.contains(overridden.get().getDeclaringType().getFullyQualifiedName())) { + return true; } + overridden = TypeUtils.findOverriddenMethod(overridden.get()); } - return anySupertypeDeclares(type, method); - } - - private static boolean parameterTypesMatch(JavaType.Method a, JavaType.Method b) { - List aParameterTypes = a.getParameterTypes(); - List bParameterTypes = b.getParameterTypes(); - if (aParameterTypes.size() != bParameterTypes.size()) { - return false; - } - for (int i = 0; i < aParameterTypes.size(); i++) { - if (!TypeUtils.isOfType(aParameterTypes.get(i), bParameterTypes.get(i))) { - return false; - } - } - return true; + return false; } } }