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..6d5890488 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java @@ -32,7 +32,12 @@ import org.openrewrite.marker.Markers; 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; public class MigrateJUnitTestCase extends Recipe { @@ -101,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) { @@ -148,7 +155,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 +178,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 +193,27 @@ 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) { + JavaType.Method methodType = md.getMethodType(); + if (methodType == null || + md.getLeadingAnnotations().stream().noneMatch(OVERRIDE_ANNOTATION_MATCHER::matches) || + stillOverridesAfterMigration(methodType)) { + return md; + } + J.MethodDeclaration withoutBody = (J.MethodDeclaration) new RemoveAnnotationVisitor(OVERRIDE_ANNOTATION_MATCHER) + .visitNonNull(md.withBody(null), ctx, getCursor().getParentOrThrow()); + return withoutBody.withBody(md.getBody()); + } + + 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; } - return annotation; - })); + overridden = TypeUtils.findOverriddenMethod(overridden.get()); + } + return false; } } } 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..e5ec83b14 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,247 @@ public void testApp() { ) ); } + + @Test + void removeOverrideWhenAlreadyAnnotatedWithBefore() { + //language=java + rewriteRun( + 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.Before; + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + public class MathTest { + protected long value1; + + @Before + 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() { + } + }; + } + } + """ + ) + ); + } }