Improve JUnitMethodDeclaration method rename heuristics (#1476)

These changes make it less likely that the check suggests a method
rename that would yield invalid code.

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationTest.java

@@ -11,7 +11,7 @@ final class JUnitMethodDeclarationTest {
     CompilationTestHelper.newInstance(JUnitMethodDeclaration.class, getClass())
         .addSourceLines(
             "A.java",
-            "import static org.junit.jupiter.params.provider.Arguments.arguments;",
+            "import static org.junit.jupiter.params.provider.Arguments.*;",
             "",
             "import org.junit.jupiter.api.AfterAll;",
             "import org.junit.jupiter.api.AfterEach;",
@@ -154,8 +154,10 @@ final class JUnitMethodDeclarationTest {
             "  void overload() {}",
             "",
             "  @Test",
-            "  // BUG: Diagnostic contains: (but note that `arguments` is already statically imported)",
-            "  void testArguments() {}",
+            "  // BUG: Diagnostic contains: (but note that another method named `arguments` is in scope)",
+            "  void testArguments() {",
+            "    arguments();",
+            "  }",
             "",
             "  @Test",
             "  // BUG: Diagnostic contains: (but note that `public` is not a valid identifier)",

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/ConflictDetection.java

@@ -1,12 +1,17 @@
 package tech.picnic.errorprone.utils;
 
+import static java.util.Objects.requireNonNull;
+
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.util.ASTHelpers;
-import com.sun.source.tree.ImportTree;
-import com.sun.source.tree.Tree;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.util.TreeScanner;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.code.Type;
 import java.util.Optional;
+import org.jspecify.annotations.Nullable;
 
 /** A set of helper methods for detecting conflicts that would be caused when applying fixes. */
 public final class ConflictDetection {
@@ -22,9 +27,10 @@ public final class ConflictDetection {
    * <ul>
    *   <li>Whether the rename would merely introduce a method overload, rather than clashing with an
    *       existing method declaration in its class or a supertype.
-   *   <li>Whether the rename would in fact clash with a static import. (It could be that a static
-   *       import of the same name is only referenced from lexical scopes in which the method under
-   *       consideration cannot be referenced directly.)
+   *   <li>Whether the rename would in fact change the target of an existing method invocation in
+   *       the scope of its containing class. (It could e.g. be that said invocation targets an
+   *       identically-named method with different parameter types in some non-static nested type
+   *       declaration.)
    * </ul>
    *
    * @param method The method considered for renaming.
@@ -41,8 +47,8 @@ public final class ConflictDetection {
               "a method named `%s` is already defined in this class or a supertype", newName));
     }
 
-    if (isSimpleNameStaticallyImported(newName, state)) {
-      return Optional.of(String.format("`%s` is already statically imported", newName));
+    if (isLocalMethodInvocation(newName, state)) {
+      return Optional.of(String.format("another method named `%s` is in scope", newName));
     }
 
     if (!SourceCode.isValidIdentifier(newName)) {
@@ -58,16 +64,35 @@ public final class ConflictDetection {
         .isPresent();
   }
 
-  private static boolean isSimpleNameStaticallyImported(String simpleName, VisitorState state) {
-    return state.getPath().getCompilationUnit().getImports().stream()
-        .filter(ImportTree::isStatic)
-        .map(ImportTree::getQualifiedIdentifier)
-        .map(tree -> getStaticImportSimpleName(tree, state))
-        .anyMatch(simpleName::contentEquals);
-  }
+  private static boolean isLocalMethodInvocation(String name, VisitorState state) {
+    return Boolean.TRUE.equals(
+        new TreeScanner<Boolean, @Nullable Void>() {
+          @Override
+          public Boolean visitClass(ClassTree tree, @Nullable Void unused) {
+            if (ASTHelpers.getSymbol(tree).isStatic()) {
+              /*
+               * Don't descend into static type definitions: in those context, any unqualified
+               * method invocation cannot refer to a method in the outer scope.
+               */
+              return Boolean.FALSE;
+            }
 
-  private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) {
-    String source = SourceCode.treeToString(tree, state);
-    return source.subSequence(source.lastIndexOf('.') + 1, source.length());
+            return super.visitClass(tree, null);
+          }
+
+          @Override
+          public Boolean visitMethodInvocation(MethodInvocationTree tree, @Nullable Void unused) {
+            return (tree.getMethodSelect() instanceof IdentifierTree identifier
+                    && name.contentEquals(identifier.getName()))
+                || super.visitMethodInvocation(tree, null);
+          }
+
+          @Override
+          public Boolean reduce(Boolean r1, Boolean r2) {
+            return Boolean.TRUE.equals(r1) || Boolean.TRUE.equals(r2);
+          }
+        }.scan(
+            requireNonNull(state.findEnclosing(ClassTree.class), "No enclosing class").getMembers(),
+            null));
   }
 }

error-prone-utils/src/test/java/tech/picnic/errorprone/utils/ConflictDetectionTest.java

@@ -21,13 +21,13 @@ final class ConflictDetectionTest {
             "pkg/A.java",
             "package pkg;",
             "",
-            "import static pkg.A.B.method3t;",
+            "import static pkg.A.StaticType.method3t;",
+            "import static pkg.A.StaticType.method4t;",
             "",
             "import pkg.A.method4t;",
             "",
             "class A {",
             "  void method1() {",
-            "    method3t();",
             "    method4(method4t.class);",
             "  }",
             "",
@@ -37,7 +37,7 @@ final class ConflictDetectionTest {
             "",
             "  void method2t() {}",
             "",
-            "  // BUG: Diagnostic contains: `method3t` is already statically imported",
+            "  // BUG: Diagnostic contains: another method named `method3t` is in scope",
             "  void method3() {}",
             "",
             "  void method4(Object o) {}",
@@ -45,8 +45,34 @@ final class ConflictDetectionTest {
             "  // BUG: Diagnostic contains: `int` is not a valid identifier",
             "  void in() {}",
             "",
-            "  static class B {",
-            "    static void method3t() {}",
+            "  class InstanceType {",
+            "    void m() {",
+            "      System.out.println(method3t());",
+            "    }",
+            "  }",
+            "",
+            "  static class StaticType {",
+            "    static int method3t() {",
+            "      return 0;",
+            "    }",
+            "",
+            "    static void method4t() {",
+            "      method4t();",
+            "    }",
+            "  }",
+            "",
+            "  record RecordType() {",
+            "    void m() {",
+            "      method4t();",
+            "    }",
+            "  }",
+            "",
+            "  enum EnumType {",
+            "    ELEM;",
+            "",
+            "    void m() {",
+            "      method4t();",
+            "    }",
             "  }",
             "",
             "  class method4t {}",