Skip JUnit 4 lifecycle annotation when Jupiter equivalent already present (#1043)

When a method carries both a JUnit 4 lifecycle annotation (e.g. @before)
and its Jupiter equivalent (@beforeeach), changing the type of the JUnit 4
annotation produced a duplicate, non-repeatable annotation. Remove the
JUnit 4 annotation instead in that case.

Fixes #501

Author: timtebeek

src/main/java/org/openrewrite/java/testing/junit5/UpdateBeforeAfterAnnotations.java

@@ -20,18 +20,30 @@
 import org.openrewrite.Preconditions;
 import org.openrewrite.Recipe;
 import org.openrewrite.TreeVisitor;
+import org.openrewrite.java.AnnotationMatcher;
 import org.openrewrite.java.ChangeType;
 import org.openrewrite.java.JavaIsoVisitor;
+import org.openrewrite.java.RemoveAnnotationVisitor;
 import org.openrewrite.java.search.UsesType;
 import org.openrewrite.java.tree.J;
 
+import java.util.LinkedHashMap;
+import java.util.Map;
+
 public class UpdateBeforeAfterAnnotations extends Recipe {
     @Getter
     final String displayName = "Migrate JUnit 4 lifecycle annotations to JUnit Jupiter";
 
     @Getter
     final String description = "Replace JUnit 4's `@Before`, `@BeforeClass`, `@After`, and `@AfterClass` annotations with their JUnit Jupiter equivalents.";
 
+    private static final Map<String, String> JUNIT4_TO_JUPITER = new LinkedHashMap<String, String>() {{
+        put("org.junit.Before", "org.junit.jupiter.api.BeforeEach");
+        put("org.junit.After", "org.junit.jupiter.api.AfterEach");
+        put("org.junit.BeforeClass", "org.junit.jupiter.api.BeforeAll");
+        put("org.junit.AfterClass", "org.junit.jupiter.api.AfterAll");
+    }};
+
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
         return Preconditions.check(Preconditions.or(
@@ -48,11 +60,30 @@ public static class UpdateBeforeAfterAnnotationsVisitor extends JavaIsoVisitor<E
         @Override
         public J preVisit(J tree, ExecutionContext ctx) {
             stopAfterPreVisit();
-            doAfterVisit(new ChangeType("org.junit.Before", "org.junit.jupiter.api.BeforeEach", true).getVisitor());
-            doAfterVisit(new ChangeType("org.junit.After", "org.junit.jupiter.api.AfterEach", true).getVisitor());
-            doAfterVisit(new ChangeType("org.junit.BeforeClass", "org.junit.jupiter.api.BeforeAll", true).getVisitor());
-            doAfterVisit(new ChangeType("org.junit.AfterClass", "org.junit.jupiter.api.AfterAll", true).getVisitor());
+            // Remove the JUnit 4 annotation where the Jupiter equivalent is already present, to avoid creating a
+            // duplicate annotation when the type is subsequently changed below.
+            doAfterVisit(new RemoveRedundantJUnit4LifecycleAnnotations());
+            JUNIT4_TO_JUPITER.forEach((from, to) -> doAfterVisit(new ChangeType(from, to, true).getVisitor()));
             return tree;
         }
     }
+
+    private static class RemoveRedundantJUnit4LifecycleAnnotations extends JavaIsoVisitor<ExecutionContext> {
+
+        @Override
+        public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
+            J.MethodDeclaration md = super.visitMethodDeclaration(method, ctx);
+            for (Map.Entry<String, String> pair : JUNIT4_TO_JUPITER.entrySet()) {
+                AnnotationMatcher junit4 = new AnnotationMatcher("@" + pair.getKey());
+                AnnotationMatcher jupiter = new AnnotationMatcher("@" + pair.getValue());
+                boolean hasJunit4 = md.getLeadingAnnotations().stream().anyMatch(junit4::matches);
+                boolean hasJupiter = md.getLeadingAnnotations().stream().anyMatch(jupiter::matches);
+                if (hasJunit4 && hasJupiter) {
+                    md = (J.MethodDeclaration) new RemoveAnnotationVisitor(junit4)
+                            .visitNonNull(md, ctx, getCursor().getParentOrThrow());
+                }
+            }
+            return md;
+        }
+    }
 }

src/test/java/org/openrewrite/java/testing/junit5/UpdateBeforeAfterAnnotationsTest.java

@@ -209,6 +209,50 @@ void before() {
         );
     }
 
+    @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/501")
+    @Test
+    void removeJUnit4AnnotationWhenJupiterEquivalentAlreadyPresent() {
+        //language=java
+        rewriteRun(
+          spec -> spec.parser(JavaParser.fromJavaVersion()
+            .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "junit-jupiter-api")),
+          java(
+            """
+              import org.junit.Before;
+              import org.junit.After;
+              import org.junit.jupiter.api.BeforeEach;
+              import org.junit.jupiter.api.AfterEach;
+
+              class TestBase {
+                  @Before
+                  @BeforeEach
+                  void setup() {
+                  }
+
+                  @After
+                  @AfterEach
+                  void tearDown() {
+                  }
+              }
+              """,
+            """
+              import org.junit.jupiter.api.BeforeEach;
+              import org.junit.jupiter.api.AfterEach;
+
+              class TestBase {
+                  @BeforeEach
+                  void setup() {
+                  }
+
+                  @AfterEach
+                  void tearDown() {
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/59")
     @Test
     void retainPublicModifierOnOverriddenMethod() {