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 b82cd948c..ac741396c 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCase.java @@ -17,9 +17,11 @@ import lombok.Getter; import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; +import org.openrewrite.SourceFile; import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.*; @@ -27,6 +29,7 @@ import org.openrewrite.java.search.FindAnnotations; import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.Comment; +import org.openrewrite.java.tree.Flag; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.Space; @@ -34,6 +37,7 @@ import org.openrewrite.java.tree.TextComment; import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.Markers; +import org.openrewrite.marker.SearchResult; import java.util.*; @@ -41,6 +45,7 @@ public class MigrateJUnitTestCase extends Recipe { + private static final AnnotationMatcher OVERRIDE_ANNOTATION_MATCHER = new AnnotationMatcher("@java.lang.Override"); private static final AnnotationMatcher JUNIT_TEST_ANNOTATION_MATCHER = new AnnotationMatcher("@org.junit.Test"); private static final AnnotationMatcher JUNIT_AFTER_ANNOTATION_MATCHER = new AnnotationMatcher("@org.junit.*After*"); private static final AnnotationMatcher JUNIT_BEFORE_ANNOTATION_MATCHER = new AnnotationMatcher("@org.junit.*Before*"); @@ -80,8 +85,29 @@ public TreeVisitor getVisitor() { Preconditions.not(new DeclaresMethod<>(TEST_CASE_RUN_MATCHER)) ), new JavaIsoVisitor() { + @Override + public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { + return sourceFile instanceof J.CompilationUnit && super.isAcceptable(sourceFile, ctx); + } + @Override public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + boolean[] unsupported = {false}; + J.CompilationUnit checked = (J.CompilationUnit) new JavaIsoVisitor() { + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext executionContext) { + J.ClassDeclaration owner = getCursor().firstEnclosing(J.ClassDeclaration.class); + if (owner == null || !isSupertypeTestCase(owner.getType()) || !isSuite(method) || + !containsTestSetup(method, getCursor().getParentTreeCursor()) || testSetupLifecycleMethods(method, owner, getCursor()) != null) { + return super.visitMethodDeclaration(method, executionContext); + } + unsupported[0] = true; + return SearchResult.found(method, "Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods"); + } + }.visitNonNull(cu, ctx, getCursor().getParentTreeCursor()); + if (unsupported[0]) { + return checked; + } J.CompilationUnit c = super.visitCompilationUnit(cu, ctx); doAfterVisit(new TestCaseVisitor()); // ChangeType for org.junit.Assert method invocations because TestCase extends org.junit.Assert @@ -133,8 +159,100 @@ private Space commentOut(Statement statement, Space next) { }); } + private static boolean isSuite(J.MethodDeclaration method) { + return method.hasModifier(J.Modifier.Type.Static) && + SUITE_MATCHER.matches(method.getMethodType()) && + (TypeUtils.isOfClassType(method.getMethodType().getReturnType(), "junit.framework.Test") || + TypeUtils.isOfClassType(method.getMethodType().getReturnType(), "junit.framework.TestSuite")); + } + + private static boolean containsTestSetup(J.MethodDeclaration method, Cursor parent) { + boolean[] found = {false}; + new JavaIsoVisitor() { + @Override + public J.NewClass visitNewClass(J.NewClass newClass, boolean[] result) { + if (TypeUtils.isAssignableTo("junit.extensions.TestSetup", newClass.getType())) { + result[0] = true; + } + return super.visitNewClass(newClass, result); + } + }.visitNonNull(method, found, parent); + return found[0]; + } + + private static @Nullable List testSetupLifecycleMethods(J.MethodDeclaration suite, J.ClassDeclaration owner, Cursor suiteCursor) { + if (suite.getMethodType() == null || !suite.getMethodType().getParameterTypes().isEmpty() || + suite.getTypeParameters() != null && !suite.getTypeParameters().isEmpty() || + suite.getBody() == null || suite.getBody().getStatements().size() != 1 || + !(suite.getBody().getStatements().get(0) instanceof J.Return)) { + return null; + } + J.Return returned = (J.Return) suite.getBody().getStatements().get(0); + if (!(returned.getExpression() instanceof J.NewClass)) { + return null; + } + J.NewClass decorator = (J.NewClass) returned.getExpression(); + if (decorator.getClazz() == null || !TypeUtils.isOfClassType(decorator.getClazz().getType(), "junit.extensions.TestSetup") || + decorator.getBody() == null || decorator.getArguments().size() != 1 || + !(decorator.getArguments().get(0) instanceof J.NewClass)) { + return null; + } + J.NewClass wrapped = (J.NewClass) decorator.getArguments().get(0); + if (wrapped.getBody() != null || !TypeUtils.isOfClassType(wrapped.getType(), "junit.framework.TestSuite") || + wrapped.getArguments().size() != 1 || !(wrapped.getArguments().get(0) instanceof J.FieldAccess)) { + return null; + } + J.FieldAccess testClass = (J.FieldAccess) wrapped.getArguments().get(0); + if (!"class".equals(testClass.getSimpleName()) || !TypeUtils.isOfType(testClass.getTarget().getType(), owner.getType())) { + return null; + } + List methods = new ArrayList<>(); + Cursor returnCursor = new Cursor(new Cursor(suiteCursor, suite.getBody()), returned); + Cursor decoratorBodyCursor = new Cursor(new Cursor(returnCursor, decorator), decorator.getBody()); + for (Statement statement : decorator.getBody().getStatements()) { + if (!(statement instanceof J.MethodDeclaration)) { + return null; + } + J.MethodDeclaration method = (J.MethodDeclaration) statement; + if (!("setUp".equals(method.getSimpleName()) || "tearDown".equals(method.getSimpleName())) || + method.getBody() == null || method.getMethodType() == null || + method.hasModifier(J.Modifier.Type.Synchronized) || + method.getLeadingAnnotations().stream().anyMatch(annotation -> !OVERRIDE_ANNOTATION_MATCHER.matches(annotation)) || + !method.getMethodType().getParameterTypes().isEmpty()) { + return null; + } + boolean[] instanceReference = {false}; + new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, boolean[] result) { + JavaType.Variable field = identifier.getFieldType(); + if ("this".equals(identifier.getSimpleName()) || "super".equals(identifier.getSimpleName()) || + field != null && field.getOwner() instanceof JavaType.FullyQualified && !field.hasFlags(Flag.Static) && + TypeUtils.isAssignableTo(((JavaType.FullyQualified) field.getOwner()).getFullyQualifiedName(), decorator.getType())) { + result[0] = true; + } + return super.visitIdentifier(identifier, result); + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation invocation, boolean[] result) { + JavaType.Method type = invocation.getMethodType(); + if (invocation.getSelect() == null && (type == null || + !type.hasFlags(Flag.Static))) { + result[0] = true; + } + return super.visitMethodInvocation(invocation, result); + } + }.visitNonNull(method.getBody(), instanceReference, new Cursor(decoratorBodyCursor, method)); + if (instanceReference[0]) { + return null; + } + methods.add(method); + } + return methods; + } + 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")); @@ -144,7 +262,55 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex if (!isSupertypeTestCase(classDecl.getType())) { return classDecl; } - J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); + Set methodNames = new HashSet<>(); + if (classDecl.getType() != null) { + classDecl.getType().getVisibleMethods().forEachRemaining(method -> methodNames.add(method.getName())); + } + for (Statement statement : classDecl.getBody().getStatements()) { + if (statement instanceof J.MethodDeclaration) { + methodNames.add(((J.MethodDeclaration) statement).getSimpleName()); + } + } + J.ClassDeclaration withFixtures = classDecl.withBody(classDecl.getBody().withStatements( + ListUtils.flatMap(classDecl.getBody().getStatements(), statement -> { + if (!(statement instanceof J.MethodDeclaration) || !isSuite((J.MethodDeclaration) statement)) { + return statement; + } + List lifecycleMethods = testSetupLifecycleMethods((J.MethodDeclaration) statement, classDecl, + new Cursor(new Cursor(getCursor(), classDecl.getBody()), statement)); + if (lifecycleMethods == null) { + return statement; + } + maybeRemoveImport("junit.extensions.TestSetup"); + maybeRemoveImport("junit.framework.Test"); + maybeRemoveImport("junit.framework.TestSuite"); + return ListUtils.mapFirst(ListUtils.map(lifecycleMethods, method -> { + boolean setup = "setUp".equals(method.getSimpleName()); + String baseName = setup ? "beforeAll" : "afterAll"; + String name = baseName; + for (int suffix = 1; !methodNames.add(name); suffix++) { + name = baseName + suffix; + } + JavaType.Method methodType = method.getMethodType(); + if (methodType != null) { + methodType = methodType.withName(name).withDeclaringType(classDecl.getType()) + .withFlags(EnumSet.of(Flag.Public, Flag.Static)); + } + String annotation = setup ? "BeforeAll" : "AfterAll"; + Cursor bodyCursor = new Cursor(getCursor(), classDecl.getBody()); + J.MethodDeclaration lifecycle = JavaTemplate.builder("@" + annotation + " public static void " + name + "() {}") + .contextSensitive() + .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "junit-jupiter-api-5")) + .imports("org.junit.jupiter.api." + annotation).build() + .apply(new Cursor(bodyCursor, method), method.getCoordinates().replace()); + lifecycle = lifecycle.withBody(method.getBody()).withThrows(method.getThrows()) + .withPrefix(method.getPrefix()).withMethodType(methodType) + .withName(lifecycle.getName().withType(methodType)); + maybeAddImport("org.junit.jupiter.api." + annotation); + return maybeAutoFormat(method, lifecycle, ctx, bodyCursor); + }), first -> first.withPrefix(statement.getPrefix())); + }))); + J.ClassDeclaration cd = super.visitClassDeclaration(withFixtures, ctx); if (cd.getExtends() != null && cd.getExtends().getType() != null) { JavaType.FullyQualified fullQualifiedExtension = TypeUtils.asFullyQualified(cd.getExtends().getType()); if (fullQualifiedExtension != null && "junit.framework.TestCase".equals(fullQualifiedExtension.getFullyQualifiedName())) { @@ -168,10 +334,7 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex } // Remove suite() methods that return junit.framework.Test or TestSuite - if (md.hasModifier(J.Modifier.Type.Static) && - SUITE_MATCHER.matches(md.getMethodType()) && - (TypeUtils.isOfClassType(md.getMethodType().getReturnType(), "junit.framework.Test") || - TypeUtils.isOfClassType(md.getMethodType().getReturnType(), "junit.framework.TestSuite"))) { + if (isSuite(md)) { maybeRemoveImport("junit.framework.Test"); maybeRemoveImport("junit.framework.TestSuite"); return null; 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 57d8b699b..0c43c5b25 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/MigrateJUnitTestCaseTest.java @@ -16,14 +16,18 @@ package org.openrewrite.java.testing.junit5; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.Issue; import org.openrewrite.java.JavaParser; +import org.openrewrite.kotlin.KotlinParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.kotlin.Assertions.kotlin; class MigrateJUnitTestCaseTest implements RewriteTest { @@ -431,6 +435,464 @@ public void testApp() { ); } + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void migrateTestSetupFixtures(boolean compositeMigration) { + rewriteRun( + spec -> { + if (compositeMigration) { + spec.recipeFromResources("org.openrewrite.java.testing.junit5.JUnit4to5Migration"); + } + }, + //language=java + java( + """ + package fixture0; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class MathTest extends TestCase { + private static String resource; + + public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class)) { + @Override + protected void setUp() throws Exception { + setUpOnce(); + } + + @Override + protected void tearDown() throws Exception { + tearDownOnce(); + } + }; + } + + static void setUpOnce() throws Exception { + resource = "open"; + } + + static void tearDownOnce() throws Exception { + resource = null; + } + + public void testResource() { + assertEquals("open", resource); + } + } + """, + """ + package fixture0; + + import org.junit.jupiter.api.AfterAll; + import org.junit.jupiter.api.BeforeAll; + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertEquals; + + class MathTest { + private static String resource; + + @BeforeAll + public static void beforeAll() throws Exception { + setUpOnce(); + } + + @AfterAll + public static void afterAll() throws Exception { + tearDownOnce(); + } + + static void setUpOnce() throws Exception { + resource = "open"; + } + + static void tearDownOnce() throws Exception { + resource = null; + } + + @Test + public void testResource() { + assertEquals("open", resource); + } + } + """ + ), + //language=java + java( + """ + package fixture1; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class MathTest extends TestCase { + private static String resource; + private static int counter; + + public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class)) { + @Override + protected void setUp() throws Exception { + resource = "open"; + counter = 1; + System.setProperty("example.mode", "test"); + } + + @Override + protected void tearDown() throws Exception { + resource = null; + System.clearProperty("example.mode"); + } + }; + } + + static void beforeAll() {} + + static void afterAll() {} + + @Override + protected void setUp() throws Exception { + super.setUp(); + counter++; + } + + @Override + protected void tearDown() throws Exception { + counter--; + super.tearDown(); + } + } + """, + """ + package fixture1; + + import org.junit.jupiter.api.AfterAll; + import org.junit.jupiter.api.AfterEach; + import org.junit.jupiter.api.BeforeAll; + import org.junit.jupiter.api.BeforeEach; + + class MathTest { + private static String resource; + private static int counter; + + @BeforeAll + public static void beforeAll1() throws Exception { + resource = "open"; + counter = 1; + System.setProperty("example.mode", "test"); + } + + @AfterAll + public static void afterAll1() throws Exception { + resource = null; + System.clearProperty("example.mode"); + } + + static void beforeAll() {} + + static void afterAll() {} + + @BeforeEach + public void setUp() throws Exception { + counter++; + } + + @AfterEach + public void tearDown() throws Exception { + counter--; + } + } + """ + ), + //language=java + java( + """ + package fixture2; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class BaseTest extends TestCase { + protected void beforeAll() {} + + protected static void afterAll() {} + } + + class MathTest extends BaseTest { + public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class)) { + @Override + protected void setUp() { + System.setProperty("example.mode", "test"); + } + + @Override + protected void tearDown() { + System.clearProperty("example.mode"); + } + }; + } + } + """, + """ + package fixture2; + + import org.junit.jupiter.api.AfterAll; + import org.junit.jupiter.api.BeforeAll; + + class BaseTest { + protected void beforeAll() {} + + protected static void afterAll() {} + } + + class MathTest extends BaseTest { + @BeforeAll + public static void beforeAll1() { + System.setProperty("example.mode", "test"); + } + + @AfterAll + public static void afterAll1() { + System.clearProperty("example.mode"); + } + } + """ + ), + //language=java + java( + """ + package fixture3; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class MathTest extends TestCase { + public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class)) { + private String resource; + + @Override + protected void setUp() { + resource = "open"; + } + }; + } + } + """, + """ + package fixture3; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class MathTest extends TestCase { + /*~~(Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods)~~>*/public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class)) { + private String resource; + + @Override + protected void setUp() { + resource = "open"; + } + }; + } + } + """ + ), + //language=java + java( + """ + package fixture4; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class MathTest extends TestCase { + public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class) { + @Override + public int countTestCases() { + return 1; + } + }) { + @Override + protected void setUp() { + System.setProperty("example.mode", "test"); + } + }; + } + } + """, + """ + package fixture4; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class MathTest extends TestCase { + /*~~(Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods)~~>*/public static Test suite() { + return new TestSetup(new TestSuite(MathTest.class) { + @Override + public int countTestCases() { + return 1; + } + }) { + @Override + protected void setUp() { + System.setProperty("example.mode", "test"); + } + }; + } + } + """ + ), + //language=java + java( + """ + package references; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class InstanceReferenceTest extends TestCase { + public static Test suite() { + return new TestSetup(new TestSuite(InstanceReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + this.toString(); + } + }; + } + } + + class InheritedMethodReferenceTest extends TestCase { + public static Test suite() { + return new TestSetup(new TestSuite(InheritedMethodReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + getTest(); + } + }; + } + } + + class InheritedFieldReferenceTest extends TestCase { + public static Test suite() { + return new TestSetup(new TestSuite(InheritedFieldReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + fTest.countTestCases(); + } + }; + } + } + + class SuperReferenceTest extends TestCase { + public static Test suite() { + return new TestSetup(new TestSuite(SuperReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + super.setUp(); + } + }; + } + } + """, + """ + package references; + + import junit.extensions.TestSetup; + import junit.framework.Test; + import junit.framework.TestCase; + import junit.framework.TestSuite; + + class InstanceReferenceTest extends TestCase { + /*~~(Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods)~~>*/public static Test suite() { + return new TestSetup(new TestSuite(InstanceReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + this.toString(); + } + }; + } + } + + class InheritedMethodReferenceTest extends TestCase { + /*~~(Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods)~~>*/public static Test suite() { + return new TestSetup(new TestSuite(InheritedMethodReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + getTest(); + } + }; + } + } + + class InheritedFieldReferenceTest extends TestCase { + /*~~(Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods)~~>*/public static Test suite() { + return new TestSetup(new TestSuite(InheritedFieldReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + fTest.countTestCases(); + } + }; + } + } + + class SuperReferenceTest extends TestCase { + /*~~(Migrate this TestSetup fixture manually to JUnit Jupiter lifecycle methods)~~>*/public static Test suite() { + return new TestSetup(new TestSuite(SuperReferenceTest.class)) { + @Override + protected void setUp() throws Exception { + super.setUp(); + } + }; + } + } + """ + ) + ); + } + + @Test + void leaveKotlinTestCasesUnchanged() { + rewriteRun( + spec -> spec.parser(KotlinParser.builder() + .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "hamcrest-3")), + //language=kotlin + kotlin( + """ + import junit.framework.TestCase + + class MathTest : TestCase() { + override fun setUp() { + super.setUp() + } + } + """ + ) + ); + } + @Test void removeOverrideWhenAlreadyAnnotatedWithBefore() { //language=java