diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationTest.java
index b4c016da5f..b34fed62a7 100644
--- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationTest.java
+++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationTest.java
@@ -11,7 +11,7 @@ void identification() {
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 @@ void identification() {
" 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)",
diff --git a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/ConflictDetection.java b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/ConflictDetection.java
index d3af56d0d5..3869ed2c7b 100644
--- a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/ConflictDetection.java
+++ b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/ConflictDetection.java
@@ -2,11 +2,15 @@
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.ExpressionTree;
+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 +26,10 @@ private ConflictDetection() {}
*
* - Whether the rename would merely introduce a method overload, rather than clashing with an
* existing method declaration in its class or a supertype.
- *
- 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.)
+ *
- 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.)
*
*
* @param method The method considered for renaming.
@@ -41,8 +46,8 @@ public static Optional findMethodRenameBlocker(
"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 +63,38 @@ private static boolean isExistingMethodName(Type clazz, String name, VisitorStat
.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() {
+ @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;
+ }
+
+ return super.visitClass(tree, null);
+ }
+
+ @Override
+ public Boolean visitMethodInvocation(MethodInvocationTree tree, @Nullable Void unused) {
+ ExpressionTree methodSelect = tree.getMethodSelect();
+ if (methodSelect instanceof IdentifierTree identifier) {
+ if (name.contentEquals(identifier.getName())) {
+ return Boolean.TRUE;
+ }
+ }
+
+ return super.visitMethodInvocation(tree, null);
+ }
- private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) {
- String source = SourceCode.treeToString(tree, state);
- return source.subSequence(source.lastIndexOf('.') + 1, source.length());
+ @Override
+ public Boolean reduce(Boolean r1, Boolean r2) {
+ return Boolean.TRUE.equals(r1) || Boolean.TRUE.equals(r2);
+ }
+ }.scan(state.findEnclosing(ClassTree.class).getMembers(), null));
}
}
diff --git a/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/ConflictDetectionTest.java b/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/ConflictDetectionTest.java
index 525c422cb6..825ce567a4 100644
--- a/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/ConflictDetectionTest.java
+++ b/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/ConflictDetectionTest.java
@@ -21,13 +21,13 @@ void matcher() {
"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 @@ void matcher() {
"",
" 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 @@ void matcher() {
" // 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 {}",