Do not reduce visibility of extended classes in TestsShouldNotBePublic (#524)

* Do not reduce visibility on extended classes

Fixes #309

* Update src/main/java/org/openrewrite/java/testing/cleanup/TestsShouldNotBePublic.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: timtebeek

src/main/java/org/openrewrite/java/testing/cleanup/TestsShouldNotBePublic.java

@@ -17,25 +17,27 @@
 
 import lombok.AllArgsConstructor;
 import lombok.EqualsAndHashCode;
+import lombok.RequiredArgsConstructor;
+import lombok.Value;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Option;
-import org.openrewrite.Recipe;
+import org.openrewrite.ScanningRecipe;
 import org.openrewrite.TreeVisitor;
 import org.openrewrite.internal.ListUtils;
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.ChangeMethodAccessLevelVisitor;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.MethodMatcher;
-import org.openrewrite.java.tree.*;
+import org.openrewrite.java.tree.Comment;
+import org.openrewrite.java.tree.Flag;
+import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.TypeUtils;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 
 @AllArgsConstructor
 @EqualsAndHashCode(callSuper = false)
-public class TestsShouldNotBePublic extends Recipe {
+public class TestsShouldNotBePublic extends ScanningRecipe<TestsShouldNotBePublic.Accumulator> {
 
     @Option(displayName = "Remove protected modifiers",
             description = "Also remove protected modifiers from test methods",
@@ -60,25 +62,46 @@ public Set<String> getTags() {
     }
 
     @Override
-    public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new TestsNotPublicVisitor(Boolean.TRUE.equals(removeProtectedModifiers));
+    public Accumulator getInitialValue(ExecutionContext ctx) {
+        return new Accumulator();
     }
 
+    @Override
+    public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
+        return new JavaIsoVisitor<ExecutionContext>() {
+            @Override
+            public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDeclaration, ExecutionContext ctx) {
+                J.ClassDeclaration cd = super.visitClassDeclaration(classDeclaration, ctx);
+                if (cd.getExtends() != null) {
+                    acc.extendedClasses.add(String.valueOf(cd.getExtends().getType()));
+                }
+                return cd;
+            }
+        };
+    }
+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
+        return new TestsNotPublicVisitor(Boolean.TRUE.equals(removeProtectedModifiers), acc);
+    }
+
+    public static class Accumulator {
+        Set<String> extendedClasses = new HashSet<>();
+    }
+
+    @RequiredArgsConstructor
     private static final class TestsNotPublicVisitor extends JavaIsoVisitor<ExecutionContext> {
         private final Boolean orProtected;
-
-        private TestsNotPublicVisitor(Boolean orProtected) {
-            this.orProtected = orProtected;
-        }
+        private final Accumulator acc;
 
         @Override
         public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) {
             J.ClassDeclaration c = super.visitClassDeclaration(classDecl, ctx);
 
             if (c.getKind() != J.ClassDeclaration.Kind.Type.Interface
                     && c.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Public)
-                    && c.getModifiers().stream().noneMatch(mod -> mod.getType() == J.Modifier.Type.Abstract)) {
-
+                    && c.getModifiers().stream().noneMatch(mod -> mod.getType() == J.Modifier.Type.Abstract)
+                    && !acc.extendedClasses.contains(String.valueOf(c.getType()))) {
                 boolean hasTestMethods = c.getBody().getStatements().stream()
                         .filter(org.openrewrite.java.tree.J.MethodDeclaration.class::isInstance)
                         .map(J.MethodDeclaration.class::cast)

src/test/java/org/openrewrite/java/testing/cleanup/TestsShouldNotBePublicTest.java

@@ -18,6 +18,7 @@
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
 import org.openrewrite.InMemoryExecutionContext;
+import org.openrewrite.Issue;
 import org.openrewrite.java.JavaParser;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
@@ -493,4 +494,42 @@ Collection<DynamicTest> testFactoryMethod() {
           )
         );
     }
+
+    @Test
+    @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/309")
+    void baseclassForTestsNeedsToStayPublic() {
+        //language=java
+        rewriteRun(
+          spec -> spec.recipe(new TestsShouldNotBePublic(true)),
+          java(
+            // base class for tests should stay public
+            """
+              package com.hello;
+
+              import org.junit.jupiter.api.BeforeEach;
+
+              public class MyTestBase {
+                @BeforeEach
+                void setUp() {
+                }
+              }
+              """
+          ),
+          java(
+            // test class extends base class from another package
+            """
+              package com.world;
+
+              import com.hello.MyTestBase;
+              import org.junit.jupiter.api.Test;
+
+              class MyTest extends MyTestBase {
+                @Test
+                void isWorking() {
+                }
+              }
+              """
+          )
+        );
+    }
 }