Update after rebase with latest improvements

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitFactoryMethodDeclaration.java

@@ -122,7 +122,9 @@ public final class JUnitFactoryMethodDeclaration extends BugChecker implements M
       return ImmutableList.of();
     }
 
-    Optional<String> blocker = findMethodRenameBlocker(expectedFactoryMethodName, state);
+    Optional<String> blocker =
+        findMethodRenameBlocker(
+            ASTHelpers.getSymbol(factoryMethod), expectedFactoryMethodName, state);
     if (blocker.isPresent()) {
       reportMethodRenameBlocker(
           factoryMethod, blocker.orElseThrow(), expectedFactoryMethodName, state);

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclaration.java

@@ -85,7 +85,7 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
     tryCanonicalizeMethodName(symbol)
         .ifPresent(
             newName ->
-                findMethodRenameBlocker(newName, state)
+                findMethodRenameBlocker(symbol, newName, state)
                     .ifPresentOrElse(
                         blocker -> reportMethodRenameBlocker(tree, blocker, state),
                         () -> fixBuilder.merge(SuggestedFixes.renameMethod(tree, newName, state))));

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetection.java

@@ -1,11 +1,13 @@
 package tech.picnic.errorprone.bugpatterns.util;
 
-import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isReservedKeyword;
-import static tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers.methodExistsInEnclosingClass;
+import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isValidIdentifier;
 
 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.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Type;
 import java.util.Optional;
 
 /**
@@ -30,28 +32,37 @@ public final class ConflictDetection {
    *       consideration cannot be referenced directly.)
    * </ul>
    *
-   * @param methodName The proposed name to assign.
+   * @param method XXX The proposed name to assign.
+   * @param newName The proposed name to assign.
    * @param state The {@link VisitorState} to use for searching for blockers.
    * @return A human-readable argument against assigning the proposed name to an existing method, or
    *     {@link Optional#empty()} if no blocker was found.
    */
-  public static Optional<String> findMethodRenameBlocker(String methodName, VisitorState state) {
-    if (methodExistsInEnclosingClass(methodName, state)) {
+  public static Optional<String> findMethodRenameBlocker(
+      MethodSymbol method, String newName, VisitorState state) {
+    if (isExistingMethodName(method.owner.type, newName, state)) {
       return Optional.of(
-          String.format("a method named `%s` already exists in this class", methodName));
+          String.format(
+              "a method named `%s` is already defined in this class or a supertype", newName));
     }
 
-    if (isSimpleNameStaticallyImported(methodName, state)) {
-      return Optional.of(String.format("`%s` is already statically imported", methodName));
+    if (isSimpleNameStaticallyImported(newName, state)) {
+      return Optional.of(String.format("`%s` is already statically imported", newName));
     }
 
-    if (isReservedKeyword(methodName)) {
-      return Optional.of(String.format("`%s` is a reserved keyword", methodName));
+    if (!isValidIdentifier(newName)) {
+      return Optional.of(String.format("`%s` is not a valid identifier", newName));
     }
 
     return Optional.empty();
   }
 
+  private static boolean isExistingMethodName(Type clazz, String name, VisitorState state) {
+    return ASTHelpers.matchingMethods(state.getName(name), method -> true, clazz, state.getTypes())
+        .findAny()
+        .isPresent();
+  }
+
   private static boolean isSimpleNameStaticallyImported(String simpleName, VisitorState state) {
     return state.getPath().getCompilationUnit().getImports().stream()
         .filter(ImportTree::isStatic)

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java

@@ -29,7 +29,12 @@ final class StringRules {
   static final class StringIsEmpty {
     @BeforeTemplate
     boolean before(String str) {
-      return Refaster.anyOf(str.length() == 0, str.length() <= 0, str.length() < 1);
+      return str.equals("");
+    }
+
+    @BeforeTemplate
+    boolean before2(String str) {
+      return str.length() == 0;
     }
 
     @AfterTemplate

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

@@ -74,8 +74,9 @@ final class JUnitFactoryMethodDeclarationTest {
             "",
             "  void method5TestCases() {}",
             "",
-            "  // BUG: Diagnostic contains: (but note that a method named `method5TestCases` already exists in",
-            "  // this class)",
+            "  // BUG: Diagnostic contains:  The test cases should be supplied by a method named",
+            "  // `method5TestCases` (but note that a method named `method5TestCases`  already defined in this",
+            "  // class or a supertype)",
             "  private static Stream<Arguments> testCasesForMethod5() {",
             "    /* { foo, bar, baz } */",
             "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",

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

@@ -6,7 +6,9 @@ import com.google.errorprone.BugPattern;
 import com.google.errorprone.CompilationTestHelper;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.MethodTree;
 import org.junit.jupiter.api.Test;
 
@@ -23,8 +25,8 @@ final class ConflictDetectionTest {
             "    foo3t();",
             "  }",
             "",
-            "  // BUG: Diagnostic contains: [RenameBlockerFlagger] a method named `foo2t` already exists in this",
-            "  // class",
+            "  // BUG: Diagnostic contains: [RenameBlockerFlagger] a method named `foo2t` is already defined in",
+            "  // this class or a supertype",
             "  private void foo2() {}",
             "",
             "  private void foo2t() {}",
@@ -32,7 +34,7 @@ final class ConflictDetectionTest {
             "  // BUG: Diagnostic contains: [RenameBlockerFlagger] `foo3t` is already statically imported",
             "  private void foo3() {}",
             "",
-            "  // BUG: Diagnostic contains: [RenameBlockerFlagger] `int` is a reserved keyword",
+            "  // BUG: Diagnostic contains: [RenameBlockerFlagger] `int` is not a valid identifier",
             "  private void in() {}",
             "}")
         .addSourceLines(
@@ -46,13 +48,13 @@ final class ConflictDetectionTest {
   }
 
   @BugPattern(summary = "Flags blockers for renaming methods", severity = ERROR)
-  public static final class RenameBlockerFlagger extends BugChecker
-      implements BugChecker.MethodTreeMatcher {
+  public static final class RenameBlockerFlagger extends BugChecker implements MethodTreeMatcher {
     private static final long serialVersionUID = 1L;
 
     @Override
     public Description matchMethod(MethodTree tree, VisitorState state) {
-      return ConflictDetection.findMethodRenameBlocker(tree.getName() + "t", state)
+      return ConflictDetection.findMethodRenameBlocker(
+              ASTHelpers.getSymbol(tree), tree.getName() + "t", state)
           .map(blocker -> buildDescription(tree).setMessage(blocker).build())
           .orElse(Description.NO_MATCH);
     }