Extract and improve method

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

@@ -0,0 +1,249 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
+import static com.google.errorprone.BugPattern.StandardTags.STYLE;
+import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.annotations;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.enclosingClass;
+import static com.google.errorprone.matchers.Matchers.hasModifier;
+import static com.google.errorprone.matchers.Matchers.isType;
+import static com.google.errorprone.matchers.Matchers.not;
+import static java.util.stream.Collectors.joining;
+import static tech.picnic.errorprone.bugpatterns.util.ConflictDetection.findMethodRenameBlocker;
+import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
+import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.HAS_METHOD_SOURCE;
+import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD;
+
+import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Streams;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.AnnotationTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.StatementTree;
+import com.sun.source.tree.Tree.Kind;
+import com.sun.source.tree.VariableTree;
+import com.sun.tools.javac.parser.Tokens.Comment;
+import com.sun.tools.javac.parser.Tokens.TokenKind;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+import javax.lang.model.element.Modifier;
+import javax.lang.model.element.Name;
+import tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers;
+import tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers;
+
+/**
+ * A {@link BugChecker} that flags non-canonical JUnit factory method declarations and usages.
+ *
+ * <p>At Picnic, we consider a JUnit factory method canonical if it:
+ *
+ * <ul>
+ *   <li>has the same name as the test method it provides test cases for, but with a `TestCases`
+ *       suffix, and
+ *   <li>has a comment which connects the return statement to the names of the parameters in the
+ *       corresponding test method.
+ * </ul>
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary = "JUnit factory method declaration can likely be improved",
+    link = BUG_PATTERNS_BASE_URL + "JUnitFactoryMethodDeclaration",
+    linkType = CUSTOM,
+    severity = SUGGESTION,
+    tags = STYLE)
+public final class JUnitFactoryMethodDeclaration extends BugChecker implements MethodTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<MethodTree> HAS_UNMODIFIABLE_SIGNATURE =
+      anyOf(
+          annotations(AT_LEAST_ONE, isType("java.lang.Override")),
+          allOf(
+              not(hasModifier(Modifier.FINAL)),
+              not(hasModifier(Modifier.PRIVATE)),
+              enclosingClass(hasModifier(Modifier.ABSTRACT))));
+
+  /** Instantiates a new {@link JUnitFactoryMethodDeclaration} instance. */
+  public JUnitFactoryMethodDeclaration() {}
+
+  @Override
+  public Description matchMethod(MethodTree tree, VisitorState state) {
+    if (!TEST_METHOD.matches(tree, state) || !HAS_METHOD_SOURCE.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    AnnotationTree methodSourceAnnotation =
+        ASTHelpers.getAnnotationWithSimpleName(
+            tree.getModifiers().getAnnotations(), "MethodSource");
+
+    Optional<ImmutableList<MethodTree>> factoryMethods =
+        MoreJUnitMatchers.extractSingleFactoryMethodName(methodSourceAnnotation)
+            .map(name -> MoreASTHelpers.findMethods(name, state));
+
+    if (factoryMethods.isEmpty() || factoryMethods.orElseThrow().size() != 1) {
+      return Description.NO_MATCH;
+    }
+
+    MethodTree factoryMethod = Iterables.getOnlyElement(factoryMethods.orElseThrow());
+    ImmutableList<Description> descriptions =
+        ImmutableList.<Description>builder()
+            .addAll(
+                getFactoryMethodNameFixes(
+                    tree.getName(), methodSourceAnnotation, factoryMethod, state))
+            .addAll(getReturnStatementCommentFixes(tree, factoryMethod, state))
+            .build();
+
+    descriptions.forEach(state::reportMatch);
+    return Description.NO_MATCH;
+  }
+
+  private ImmutableList<Description> getFactoryMethodNameFixes(
+      Name methodName,
+      AnnotationTree methodSourceAnnotation,
+      MethodTree factoryMethod,
+      VisitorState state) {
+    String expectedFactoryMethodName = methodName + "TestCases";
+
+    if (HAS_UNMODIFIABLE_SIGNATURE.matches(factoryMethod, state)
+        || factoryMethod.getName().toString().equals(expectedFactoryMethodName)) {
+      return ImmutableList.of();
+    }
+
+    Optional<String> blocker =
+        findMethodRenameBlocker(
+            ASTHelpers.getSymbol(factoryMethod), expectedFactoryMethodName, state);
+    if (blocker.isPresent()) {
+      reportMethodRenameBlocker(
+          factoryMethod, blocker.orElseThrow(), expectedFactoryMethodName, state);
+      return ImmutableList.of();
+    }
+
+    return ImmutableList.of(
+        buildDescription(methodSourceAnnotation)
+            .setMessage(
+                String.format(
+                    "The test cases should be supplied by a method named `%s`",
+                    expectedFactoryMethodName))
+            .addFix(
+                SuggestedFixes.updateAnnotationArgumentValues(
+                        methodSourceAnnotation,
+                        state,
+                        "value",
+                        ImmutableList.of("\"" + expectedFactoryMethodName + "\""))
+                    .build())
+            .build(),
+        buildDescription(factoryMethod)
+            .setMessage(
+                String.format(
+                    "The test cases should be supplied by a method named `%s`",
+                    expectedFactoryMethodName))
+            .addFix(SuggestedFixes.renameMethod(factoryMethod, expectedFactoryMethodName, state))
+            .build());
+  }
+
+  private void reportMethodRenameBlocker(
+      MethodTree tree, String reason, String suggestedName, VisitorState state) {
+    state.reportMatch(
+        buildDescription(tree)
+            .setMessage(
+                String.format(
+                    "The test cases should be supplied by a method named `%s` (but note that %s)",
+                    suggestedName, reason))
+            .build());
+  }
+
+  private ImmutableList<Description> getReturnStatementCommentFixes(
+      MethodTree testMethod, MethodTree factoryMethod, VisitorState state) {
+    String expectedComment = createCommentContainingParameters(testMethod);
+
+    List<? extends StatementTree> statements = factoryMethod.getBody().getStatements();
+
+    Stream<? extends StatementTree> returnStatementsNeedingComment =
+        Streams.mapWithIndex(statements.stream(), IndexedStatement::new)
+            .filter(indexedStatement -> indexedStatement.getStatement().getKind() == Kind.RETURN)
+            .filter(
+                indexedStatement ->
+                    !hasExpectedComment(
+                        factoryMethod,
+                        expectedComment,
+                        statements,
+                        indexedStatement.getIndex(),
+                        state))
+            .map(IndexedStatement::getStatement);
+
+    return returnStatementsNeedingComment
+        .map(
+            s ->
+                buildDescription(s)
+                    .setMessage(
+                        "The return statement should be prefixed by a comment giving the names of the test case parameters")
+                    .addFix(SuggestedFix.prefixWith(s, expectedComment + "\n"))
+                    .build())
+        .collect(toImmutableList());
+  }
+
+  private static String createCommentContainingParameters(MethodTree testMethod) {
+    return testMethod.getParameters().stream()
+        .map(VariableTree::getName)
+        .map(Object::toString)
+        .collect(joining(", ", "/* { ", " } */"));
+  }
+
+  private static boolean hasExpectedComment(
+      MethodTree factoryMethod,
+      String expectedComment,
+      List<? extends StatementTree> statements,
+      long statementIndex,
+      VisitorState state) {
+    int startPosition =
+        statementIndex > 0
+            ? state.getEndPosition(statements.get((int) statementIndex - 1))
+            : ASTHelpers.getStartPosition(factoryMethod);
+    int endPosition = state.getEndPosition(statements.get((int) statementIndex));
+
+    ImmutableList<Comment> comments =
+        extractReturnStatementComments(startPosition, endPosition, state);
+
+    return comments.stream()
+        .map(Comment::getText)
+        .anyMatch(comment -> comment.equals(expectedComment));
+  }
+
+  private static ImmutableList<Comment> extractReturnStatementComments(
+      int startPosition, int endPosition, VisitorState state) {
+    return state.getOffsetTokens(startPosition, endPosition).stream()
+        .filter(t -> t.kind() == TokenKind.RETURN)
+        .flatMap(errorProneToken -> errorProneToken.comments().stream())
+        .collect(toImmutableList());
+  }
+
+  private static final class IndexedStatement {
+    private final StatementTree statement;
+    private final long index;
+
+    private IndexedStatement(StatementTree statement, long index) {
+      this.statement = statement;
+      this.index = index;
+    }
+
+    public StatementTree getStatement() {
+      return statement;
+    }
+
+    public long getIndex() {
+      return index;
+    }
+  }
+}

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

@@ -8,8 +8,8 @@ import static com.google.errorprone.matchers.Matchers.enclosingClass;
 import static com.google.errorprone.matchers.Matchers.hasModifier;
 import static com.google.errorprone.matchers.Matchers.not;
 import static java.util.function.Predicate.not;
+import static tech.picnic.errorprone.bugpatterns.util.ConflictDetection.findMethodRenameBlocker;
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
-import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isValidIdentifier;
 import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.SETUP_OR_TEARDOWN_METHOD;
 import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD;
 
@@ -25,15 +25,10 @@ import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.util.ASTHelpers;
-import com.sun.source.tree.ImportTree;
 import com.sun.source.tree.MethodTree;
-import com.sun.source.tree.Tree;
-import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
-import com.sun.tools.javac.code.Type;
 import java.util.Optional;
 import javax.lang.model.element.Modifier;
-import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 
 /** A {@link BugChecker} that flags non-canonical JUnit method declarations. */
 // XXX: Consider introducing a class-level check that enforces that test classes:
@@ -106,61 +101,7 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
             .build());
   }
 
-  /**
-   * If applicable, returns a human-readable argument against assigning the given name to an
-   * existing method.
-   *
-   * <p>This method implements imperfect heuristics. Things it currently does not consider include
-   * the following:
-   *
-   * <ul>
-   *   <li>Whether the rename would merely introduce a method overload, rather than clashing with an
-   *       existing method declaration.
-   *   <li>Whether the rename would cause a method in a superclass to be overridden.
-   *   <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.)
-   * </ul>
-   */
-  private 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` 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 (!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)
-        .map(ImportTree::getQualifiedIdentifier)
-        .map(tree -> getStaticImportSimpleName(tree, state))
-        .anyMatch(simpleName::contentEquals);
-  }
-
-  private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) {
-    String source = SourceCode.treeToString(tree, state);
-    return source.subSequence(source.lastIndexOf('.') + 1, source.length());
-  }
-
-  private static Optional<String> tryCanonicalizeMethodName(Symbol symbol) {
+  private static Optional<String> tryCanonicalizeMethodName(MethodSymbol symbol) {
     return Optional.of(symbol.getQualifiedName().toString())
         .filter(name -> name.startsWith(TEST_PREFIX))
         .map(name -> name.substring(TEST_PREFIX.length()))

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

@@ -0,0 +1,78 @@
+package tech.picnic.errorprone.bugpatterns.util;
+
+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;
+
+/**
+ * A set of helper methods for finding conflicts which would be caused by applying certain fixes.
+ */
+public final class ConflictDetection {
+  private ConflictDetection() {}
+
+  /**
+   * If applicable, returns a human-readable argument against assigning the given name to an
+   * existing method.
+   *
+   * <p>This method implements imperfect heuristics. Things it currently does not consider include
+   * the following:
+   *
+   * <ul>
+   *   <li>Whether the rename would merely introduce a method overload, rather than clashing with an
+   *       existing method declaration.
+   *   <li>Whether the rename would cause a method in a superclass to be overridden.
+   *   <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.)
+   * </ul>
+   *
+   * @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(
+      MethodSymbol method, String newName, VisitorState state) {
+    if (isExistingMethodName(method.owner.type, newName, state)) {
+      return Optional.of(
+          String.format(
+              "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 (!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)
+        .map(ImportTree::getQualifiedIdentifier)
+        .map(tree -> getStaticImportSimpleName(tree, state))
+        .anyMatch(simpleName::contentEquals);
+  }
+
+  private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) {
+    String source = SourceCode.treeToString(tree, state);
+    return source.subSequence(source.lastIndexOf('.') + 1, source.length());
+  }
+}

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

@@ -10,14 +10,19 @@ import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.hasMetaAnnota
 
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.errorprone.matchers.AnnotationMatcherUtils;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.MultiMatcher;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
+import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.NewArrayTree;
+import com.sun.tools.javac.code.Type;
+import java.util.Optional;
+import javax.lang.model.type.TypeKind;
 import org.jspecify.annotations.Nullable;
 
 /**
@@ -78,4 +83,24 @@ public final class MoreJUnitMatchers {
     return requireNonNullElse(
         Strings.emptyToNull(ASTHelpers.constValue(tree, String.class)), annotatedMethodName);
   }
+
+  /**
+   * Extracts the name of the JUnit factory method from a {@link
+   * org.junit.jupiter.params.provider.MethodSource} annotation.
+   *
+   * @param methodSourceAnnotation The {@link org.junit.jupiter.params.provider.MethodSource}
+   *     annotation to extract a method name from.
+   * @return The name of the factory method referred to in the annotation. Alternatively, {@link
+   *     Optional#empty()} if there is more than one.
+   */
+  public static Optional<String> extractSingleFactoryMethodName(
+      AnnotationTree methodSourceAnnotation) {
+    ExpressionTree attributeExpression =
+        ((AssignmentTree) Iterables.getOnlyElement(methodSourceAnnotation.getArguments()))
+            .getExpression();
+    Type attributeType = ASTHelpers.getType(attributeExpression);
+    return attributeType.getKind() == TypeKind.ARRAY
+        ? Optional.empty()
+        : Optional.of(attributeType.stringValue());
+  }
 }

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

@@ -0,0 +1,185 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+final class JUnitFactoryMethodDeclarationTest {
+  @Test
+  void identification() {
+    CompilationTestHelper.newInstance(JUnitFactoryMethodDeclaration.class, getClass())
+        .addSourceLines(
+            "A.java",
+            "import static org.junit.jupiter.params.provider.Arguments.arguments;",
+            "",
+            "import java.util.List;",
+            "import java.util.stream.Stream;",
+            "import org.junit.jupiter.params.ParameterizedTest;",
+            "import org.junit.jupiter.params.provider.Arguments;",
+            "import org.junit.jupiter.params.provider.MethodSource;",
+            "",
+            "class A extends SuperA {",
+            "  @ParameterizedTest",
+            "  // BUG: Diagnostic contains: The test cases should be supplied by a method named",
+            "  // `method1TestCases`",
+            "  @MethodSource(\"testCasesForMethod1\")",
+            "  void method1(int foo, boolean bar, String baz) {}",
+            "",
+            "  // BUG: Diagnostic contains: The test cases should be supplied by a method named",
+            "  // `method1TestCases`",
+            "  private static Stream<Arguments> testCasesForMethod1() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method2TestCases\")",
+            "  void method2(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> method2TestCases() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "",
+            "  private static void method2TestCases(int i) {}",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method3TestCases\")",
+            "  void method3(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> method3TestCases() {",
+            "    // BUG: Diagnostic contains: The return statement should be prefixed by a comment giving the",
+            "    // names of the test case parameters",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method4TestCases\")",
+            "  void method4(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> method4TestCases() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"testCasesForMethod5\")",
+            "  void method5(int foo, boolean bar, String baz) {}",
+            "",
+            "  void method5TestCases() {}",
+            "",
+            "  // BUG: Diagnostic contains: The test cases should be supplied by a method named",
+            "  // `method5TestCases` (but note that a method named `method5TestCases` is 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\"));",
+            "  }",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method6TestCases\")",
+            "  void method6(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> method6TestCases() {",
+            "    List<Arguments> arguments = List.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "    /* { foo, bar, baz } */",
+            "    return arguments.stream();",
+            "  }",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method7TestCases\")",
+            "  void method7(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> method7TestCases() {",
+            "    List<Arguments> arguments = List.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "    if (true == true) {",
+            "      /* { foo, bar, baz } */",
+            "      return arguments.stream();",
+            "    }",
+            "    // BUG: Diagnostic contains: The return statement should be prefixed by a comment giving the",
+            "    // names of the test case parameters",
+            "    return arguments.stream();",
+            "  }",
+            "",
+            "  private static Stream<Arguments> method8TestCases() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method8TestCases\")",
+            "  void method8(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> testCasesForMethod9() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "",
+            "  @MethodSource(\"testCasesForMethod9\")",
+            "  void method9(int foo, boolean bar, String baz) {}",
+            "",
+            "  @ParameterizedTest",
+            "  void method10(int foo, boolean bar, String baz) {}",
+            "",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"testCasesForMethod11\")",
+            "  void method11(int foo, boolean bar, String baz) {}",
+            "",
+            "  @Override",
+            "  Stream<Arguments> testCasesForMethod11() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "}")
+        .addSourceLines(
+            "SuperA.java",
+            "abstract class SuperA {",
+            "  abstract Object testCasesForMethod11();",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacement() {
+    BugCheckerRefactoringTestHelper.newInstance(JUnitFactoryMethodDeclaration.class, getClass())
+        .addInputLines(
+            "A.java",
+            "import static org.junit.jupiter.params.provider.Arguments.arguments;",
+            "",
+            "import java.util.stream.Stream;",
+            "import org.junit.jupiter.params.ParameterizedTest;",
+            "import org.junit.jupiter.params.provider.Arguments;",
+            "import org.junit.jupiter.params.provider.MethodSource;",
+            "",
+            "class A {",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"testCasesForMethod1\")",
+            "  void method1(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> testCasesForMethod1() {",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import static org.junit.jupiter.params.provider.Arguments.arguments;",
+            "",
+            "import java.util.stream.Stream;",
+            "import org.junit.jupiter.params.ParameterizedTest;",
+            "import org.junit.jupiter.params.provider.Arguments;",
+            "import org.junit.jupiter.params.provider.MethodSource;",
+            "",
+            "class A {",
+            "  @ParameterizedTest",
+            "  @MethodSource(\"method1TestCases\")",
+            "  void method1(int foo, boolean bar, String baz) {}",
+            "",
+            "  private static Stream<Arguments> method1TestCases() {",
+            "    /* { foo, bar, baz } */",
+            "    return Stream.of(arguments(1, true, \"A\"), arguments(2, false, \"B\"));",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+}

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

@@ -0,0 +1,67 @@
+package tech.picnic.errorprone.bugpatterns.util;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+
+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 com.sun.tools.javac.code.Symbol;
+import org.junit.jupiter.api.Test;
+
+final class ConflictDetectionTest {
+  @Test
+  void matcher() {
+    CompilationTestHelper.newInstance(RenameBlockerFlagger.class, getClass())
+        .addSourceLines(
+            "/A.java",
+            "import static pkg.B.foo3t;",
+            "",
+            "class A {",
+            "  private void foo1() {",
+            "    foo3t();",
+            "  }",
+            "",
+            "  // BUG: Diagnostic contains: a method named `foo2t` is already defined in this class or a",
+            "  // supertype",
+            "  private void foo2() {}",
+            "",
+            "  private void foo2t() {}",
+            "",
+            "  // BUG: Diagnostic contains: `foo3t` is already statically imported",
+            "  private void foo3() {}",
+            "",
+            "  // BUG: Diagnostic contains: `int` is not a valid identifier",
+            "  private void in() {}",
+            "}")
+        .addSourceLines(
+            "/pkg/B.java",
+            "package pkg;",
+            "",
+            "public class B {",
+            "  public static void foo3t() {}",
+            "}")
+        .doTest();
+  }
+
+  /**
+   * A {@link BugChecker} that flags method rename blockers found by {@link
+   * ConflictDetection#findMethodRenameBlocker(Symbol.MethodSymbol, String, VisitorState)}.
+   */
+  @BugPattern(summary = "Flags blockers for renaming methods", severity = ERROR)
+  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(
+              ASTHelpers.getSymbol(tree), tree.getName() + "t", state)
+          .map(blocker -> buildDescription(tree).setMessage(blocker).build())
+          .orElse(Description.NO_MATCH);
+    }
+  }
+}

error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java

@@ -73,6 +73,7 @@ final class RefasterRulesTest {
   private static Stream<Arguments> validateRuleCollectionTestCases() {
     // XXX: Drop the filter once we have added tests for AssertJ! We can then also replace this
     // method with `@ValueSource(classes = {...})`.
+    /* { clazz } */
     return RULE_COLLECTIONS.stream()
         .filter(not(AssertJRules.class::equals))
         .map(Arguments::arguments);