Improve MethodReferenceUsageCheck, document further improvements

See:
- https://github.com/spring-projects/spring-boot/releases/tag/v2.7.3
- https://github.com/spring-projects/spring-boot/compare/v2.7.2...v2.7.3

error-prone-contrib/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <groupId>tech.picnic.error-prone-support</groupId>
         <artifactId>error-prone-support</artifactId>
-        <version>0.1.0</version>
+        <version>0.1.1-SNAPSHOT</version>
     </parent>
 
     <artifactId>error-prone-contrib</artifactId>
@@ -143,6 +143,11 @@
             <artifactId>assertj-core</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.immutables</groupId>
+            <artifactId>value-annotations</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.junit.jupiter</groupId>
             <artifactId>junit-jupiter-api</artifactId>

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

@@ -20,7 +20,6 @@ import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.Tree;
-import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import java.util.Optional;
 
 /** A {@link BugChecker} which flags empty methods that seemingly can simply be deleted. */
@@ -47,8 +46,7 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
       return Description.NO_MATCH;
     }
 
-    MethodSymbol sym = ASTHelpers.getSymbol(tree);
-    if (sym == null || ASTHelpers.methodCanBeOverridden(sym)) {
+    if (ASTHelpers.methodCanBeOverridden(ASTHelpers.getSymbol(tree))) {
       return Description.NO_MATCH;
     }
 

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

@@ -0,0 +1,81 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.NONE;
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.enclosingClass;
+import static com.google.errorprone.matchers.Matchers.hasAnnotation;
+import static com.google.errorprone.matchers.Matchers.hasModifier;
+import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
+import static com.google.errorprone.matchers.Matchers.methodReturns;
+import static com.google.errorprone.matchers.Matchers.not;
+
+import com.google.auto.service.AutoService;
+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.sun.source.tree.MethodTree;
+import java.util.SortedSet;
+import javax.lang.model.element.Modifier;
+
+/**
+ * A {@link BugChecker} which flags {@link SortedSet} property declarations inside
+ * {@code @Value.Immutable}- and {@code @Value.Modifiable}-annotated types that lack a
+ * {@code @Value.NaturalOrder} or {@code @Value.ReverseOrder} annotation.
+ *
+ * <p>Without such an annotation:
+ *
+ * <ul>
+ *   <li>deserialization of the enclosing type requires that the associated JSON property is
+ *       present, contrary to the way in which Immutables handles other collection properties; and
+ *   <li>different instances may use different comparator implementations (e.g. deserialization
+ *       would default to natural order sorting), potentially leading to subtle bugs.
+ * </ul>
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary =
+        "`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be "
+            + "annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`",
+    linkType = NONE,
+    severity = ERROR,
+    tags = LIKELY_ERROR)
+public final class ImmutablesSortedSetComparator extends BugChecker implements MethodTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<MethodTree> METHOD_LACKS_ANNOTATION =
+      allOf(
+          methodReturns(isSubtypeOf(SortedSet.class)),
+          anyOf(
+              allOf(
+                  hasModifier(Modifier.ABSTRACT),
+                  enclosingClass(
+                      anyOf(
+                          hasAnnotation("org.immutables.value.Value.Immutable"),
+                          hasAnnotation("org.immutables.value.Value.Modifiable")))),
+              hasAnnotation("org.immutables.value.Value.Default")),
+          not(
+              anyOf(
+                  hasAnnotation("org.immutables.value.Value.NaturalOrder"),
+                  hasAnnotation("org.immutables.value.Value.ReverseOrder"))));
+
+  @Override
+  public Description matchMethod(MethodTree tree, VisitorState state) {
+    if (!METHOD_LACKS_ANNOTATION.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    SuggestedFix.Builder builder = SuggestedFix.builder();
+    String valueTypeIdentifier =
+        SuggestedFixes.qualifyType(state, builder, "org.immutables.value.Value");
+    return describeMatch(
+        tree,
+        builder.prefixWith(tree, String.format("@%s.NaturalOrder ", valueTypeIdentifier)).build());
+  }
+}

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

@@ -176,8 +176,7 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
   }
 
   private static Optional<String> tryCanonicalizeMethodName(MethodTree tree) {
-    return Optional.ofNullable(ASTHelpers.getSymbol(tree))
-        .map(sym -> sym.getQualifiedName().toString())
+    return Optional.of(ASTHelpers.getSymbol(tree).getQualifiedName().toString())
         .filter(name -> name.startsWith(TEST_PREFIX))
         .map(name -> name.substring(TEST_PREFIX.length()))
         .filter(not(String::isEmpty))

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

@@ -62,7 +62,10 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
           "com.fasterxml.jackson.annotation.JsonPropertyOrder#value",
           "io.swagger.annotations.ApiImplicitParams#value",
           "io.swagger.v3.oas.annotations.Parameters#value",
-          "javax.xml.bind.annotation.XmlType#propOrder");
+          "javax.xml.bind.annotation.XmlType#propOrder",
+          "org.springframework.context.annotation.PropertySource#value",
+          "org.springframework.test.context.TestPropertySource#locations",
+          "org.springframework.test.context.TestPropertySource#value");
   private static final String FLAG_PREFIX = "LexicographicalAnnotationAttributeListing:";
   private static final String INCLUDED_ANNOTATIONS_FLAG = FLAG_PREFIX + "Includes";
   private static final String EXCLUDED_ANNOTATIONS_FLAG = FLAG_PREFIX + "Excludes";
@@ -175,21 +178,21 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
     new TreeScanner<Void, Void>() {
       @Nullable
       @Override
-      public Void visitIdentifier(IdentifierTree node, Void ctx) {
+      public Void visitIdentifier(IdentifierTree node, @Nullable Void ctx) {
         nodes.add(tokenize(node));
         return super.visitIdentifier(node, ctx);
       }
 
       @Nullable
       @Override
-      public Void visitLiteral(LiteralTree node, Void ctx) {
+      public Void visitLiteral(LiteralTree node, @Nullable Void ctx) {
         nodes.add(tokenize(node));
         return super.visitLiteral(node, ctx);
       }
 
       @Nullable
       @Override
-      public Void visitPrimitiveType(PrimitiveTypeTree node, Void ctx) {
+      public Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void ctx) {
         nodes.add(tokenize(node));
         return super.visitPrimitiveType(node, ctx);
       }

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

@@ -4,17 +4,26 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.errorprone.BugPattern.LinkType.NONE;
 import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
 import static com.google.errorprone.BugPattern.StandardTags.STYLE;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.hasAnnotation;
+import static java.util.function.Predicate.not;
 
 import com.google.auto.service.AutoService;
 import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
 import com.google.errorprone.BugPattern;
+import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.LambdaExpressionTreeMatcher;
+import com.google.errorprone.fixes.Fix;
 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.refaster.annotation.AfterTemplate;
+import com.google.errorprone.refaster.annotation.BeforeTemplate;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.ExpressionStatementTree;
@@ -23,15 +32,21 @@ import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.LambdaExpressionTree;
 import com.sun.source.tree.MemberSelectTree;
 import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ParenthesizedTree;
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.Tree.Kind;
 import com.sun.source.tree.VariableTree;
+import com.sun.source.util.TreePath;
 import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Types;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Stream;
 import javax.lang.model.element.Name;
 
 /**
@@ -49,6 +64,8 @@ import javax.lang.model.element.Name;
 // `not(some::reference)`.
 // XXX: This check is extremely inefficient due to its reliance on `SuggestedFixes.compilesWithFix`.
 // Palantir's `LambdaMethodReference` check seems to suffer a similar issue at this time.
+// XXX: Does this check also do `() -> something.someConst` -> `something::someConst`?
+// XXX: Don't rewrite `() -> field.m()` to `field::m` for non-final fields.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "Prefer method references over lambda expressions",
@@ -57,57 +74,140 @@ import javax.lang.model.element.Name;
     tags = STYLE)
 public final class MethodReferenceUsage extends BugChecker implements LambdaExpressionTreeMatcher {
   private static final long serialVersionUID = 1L;
+  private static final Matcher<Tree> REFASTER_TEMPLATE_METHOD =
+      anyOf(
+          hasAnnotation(AfterTemplate.class.getName()),
+          hasAnnotation(BeforeTemplate.class.getName()));
+  private static final String VERIFY_SUGGESTIONS_FLAG = "MethodReferenceUsage:VerifySuggestions";
+  /**
+   * Tells whether this class is loaded in inside a {@link SuggestedFixes#compilesWithFix(Fix,
+   * VisitorState, ImmutableList, boolean)} invocation. This allows {@link
+   * #tryCompileWithFix(VisitorState, SuggestedFix)} to prevent a very expensive chain of nested
+   * compilations.
+   */
+  // XXX: Review whether, given the most recent improvements, we need the `compilesWithFix` fallback
+  // at all. (Likely at least one open point remains: handling of generic types.)
+  private static final boolean IS_NESTED_INVOCATION =
+      StackWalker.getInstance()
+          .walk(
+              frames -> {
+                String className = MethodReferenceUsage.class.getCanonicalName();
+                return frames.filter(f -> f.getClassName().equals(className)).limit(2).count() > 1;
+              });
+
+  private final boolean verifySuggestions;
+
+  /** Instantiates the default {@link MethodReferenceUsage}. */
+  public MethodReferenceUsage() {
+    this(ErrorProneFlags.empty());
+  }
+
+  /**
+   * Instantiates a customized {@link MethodReferenceUsage}.
+   *
+   * @param flags Any provided command line flags.
+   */
+  public MethodReferenceUsage(ErrorProneFlags flags) {
+    verifySuggestions = flags.getBoolean(VERIFY_SUGGESTIONS_FLAG).orElse(Boolean.FALSE);
+  }
 
   @Override
   public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
+    MethodTree enclosingMethod = state.findEnclosing(MethodTree.class);
+    if (enclosingMethod != null && REFASTER_TEMPLATE_METHOD.matches(enclosingMethod, state)) {
+      /*
+       * Within Refaster template methods variable references may stand in for complex expressions.
+       * Additionally, `@Placeholder` methods cannot be used as method handles. For both these
+       * reasons it is not safe to replace lambda expressions with method references inside Refaster
+       * templates.
+       */
+      // XXX: This is too strict; we should more explicitly handle `@Placeholder` and method
+      // parameters. For example, `() -> Optional.empty()` _should_ be flagged.
+      return Description.NO_MATCH;
+    }
+
+    if (isPassedToSameArgCountMethodOverload(state)) {
+      // XXX: When a lambda expression is passed to an overloaded method, replacing it with a method
+      // reference may introduce an ambiguity about the method that is intended to be invoked. An
+      // example is a pair of overloads accepting a `Runnable` and `Supplier<T>` respectively, where
+      // the lambda expression in question returns a value: in this case the first overload is
+      // selected, but when converted to a method reference the intended target is no longer clear.
+      // Right now any lambda expression passed to an method with an overload accepting the same
+      // number of arguments is ignored. Improve this detection logic.
+      return Description.NO_MATCH;
+    }
+
     /*
      * Lambda expressions can be used in several places where method references cannot, either
      * because the latter are not syntactically valid or ambiguous. Rather than encoding all these
      * edge cases we try to compile the code with the suggested fix, to see whether this works.
      */
-    return constructMethodRef(tree, tree.getBody())
+    // XXX: Update the comment to reflect actual `tryCompileWithFix` usage.
+    return constructMethodRef(tree, state, tree.getBody())
         .map(SuggestedFix.Builder::build)
-        .filter(
-            fix ->
-                SuggestedFixes.compilesWithFix(
-                    fix, state, ImmutableList.of(), /* onlyInSameCompilationUnit= */ true))
+        .filter(fix -> !verifySuggestions || tryCompileWithFix(state, fix))
         .map(fix -> describeMatch(tree, fix))
         .orElse(Description.NO_MATCH);
   }
 
+  private static boolean isPassedToSameArgCountMethodOverload(VisitorState state) {
+    TreePath parent = state.getPath().getParentPath();
+    if (parent == null) {
+      return false;
+    }
+
+    Symbol symbol = ASTHelpers.getSymbol(parent.getLeaf());
+    if (!(symbol instanceof MethodSymbol)) {
+      return false;
+    }
+
+    MethodSymbol method = (MethodSymbol) symbol;
+    return getOverloads(method).anyMatch(m -> m.params().size() == method.params.size());
+  }
+
   private static Optional<SuggestedFix.Builder> constructMethodRef(
-      LambdaExpressionTree lambdaExpr, Tree subTree) {
+      LambdaExpressionTree lambdaExpr, VisitorState state, Tree subTree) {
     switch (subTree.getKind()) {
       case BLOCK:
-        return constructMethodRef(lambdaExpr, (BlockTree) subTree);
+        return constructMethodRef(lambdaExpr, state, (BlockTree) subTree);
       case EXPRESSION_STATEMENT:
-        return constructMethodRef(lambdaExpr, ((ExpressionStatementTree) subTree).getExpression());
+        return constructMethodRef(
+            lambdaExpr, state, ((ExpressionStatementTree) subTree).getExpression());
       case METHOD_INVOCATION:
-        return constructMethodRef(lambdaExpr, (MethodInvocationTree) subTree);
+        return constructMethodRef(lambdaExpr, state, (MethodInvocationTree) subTree);
       case PARENTHESIZED:
-        return constructMethodRef(lambdaExpr, ((ParenthesizedTree) subTree).getExpression());
+        // XXX: Add test!
+        return constructMethodRef(lambdaExpr, state, ((ParenthesizedTree) subTree).getExpression());
       case RETURN:
-        return constructMethodRef(lambdaExpr, ((ReturnTree) subTree).getExpression());
+        // XXX: This case isn't tested. Reachable?
+        // XXX: Should be possible with `{ return x; }`.
+        return constructMethodRef(lambdaExpr, state, ((ReturnTree) subTree).getExpression());
       default:
+        // XXX: Explicitly handle known cases and throw an exception otherwise?
         return Optional.empty();
     }
   }
 
   private static Optional<SuggestedFix.Builder> constructMethodRef(
-      LambdaExpressionTree lambdaExpr, BlockTree subTree) {
+      LambdaExpressionTree lambdaExpr, VisitorState state, BlockTree subTree) {
+    // XXX: Add test with >1 statement.
     return Optional.of(subTree.getStatements())
         .filter(statements -> statements.size() == 1)
-        .flatMap(statements -> constructMethodRef(lambdaExpr, statements.get(0)));
+        .flatMap(statements -> constructMethodRef(lambdaExpr, state, statements.get(0)));
   }
 
+  // XXX: Replace nested `Optional` usage.
+  @SuppressWarnings("NestedOptionals")
   private static Optional<SuggestedFix.Builder> constructMethodRef(
-      LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree) {
-    return matchArguments(lambdaExpr, subTree)
-        .flatMap(expectedInstance -> constructMethodRef(lambdaExpr, subTree, expectedInstance));
+      LambdaExpressionTree lambdaExpr, VisitorState state, MethodInvocationTree subTree) {
+    return matchParameters(lambdaExpr, subTree)
+        .flatMap(
+            expectedInstance -> constructMethodRef(lambdaExpr, state, subTree, expectedInstance));
   }
 
   private static Optional<SuggestedFix.Builder> constructMethodRef(
       LambdaExpressionTree lambdaExpr,
+      VisitorState state,
       MethodInvocationTree subTree,
       Optional<Name> expectedInstance) {
     ExpressionTree methodSelect = subTree.getMethodSelect();
@@ -117,35 +217,54 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
           /* Direct method call; there is no matching "implicit parameter". */
           return Optional.empty();
         }
+        // XXX: Here too test for ambiguous method references.
         Symbol sym = ASTHelpers.getSymbol(methodSelect);
         if (!sym.isStatic()) {
           return constructFix(lambdaExpr, "this", methodSelect);
         }
         return constructFix(lambdaExpr, sym.owner, methodSelect);
       case MEMBER_SELECT:
-        return constructMethodRef(lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance);
+        return constructMethodRef(
+            lambdaExpr, state, (MemberSelectTree) methodSelect, expectedInstance);
       default:
         throw new VerifyException("Unexpected type of expression: " + methodSelect.getKind());
     }
   }
 
   private static Optional<SuggestedFix.Builder> constructMethodRef(
-      LambdaExpressionTree lambdaExpr, MemberSelectTree subTree, Optional<Name> expectedInstance) {
+      LambdaExpressionTree lambdaExpr,
+      VisitorState state,
+      MemberSelectTree subTree,
+      Optional<Name> expectedInstance) {
     if (subTree.getExpression().getKind() != Kind.IDENTIFIER) {
-      // XXX: Could be parenthesized. Handle. Also in other classes.
+      // XXX: Could be parenthesized. Handle. Also in other classes. Maybe consult
+      // `SideEffectAnalysis`?
+      // XXX: This branch isn't tested. Fix. Maybe something like `foo.bar().baz()`.
       /*
        * Only suggest a replacement if the method select's expression provably doesn't have
        * side-effects. Otherwise the replacement may not be behavior preserving.
        */
+      // XXX: So do this ^.
       return Optional.empty();
     }
 
-    Name lhs = ((IdentifierTree) subTree.getExpression()).getName();
+    // XXX: Check whether this cast is safe in all cases.
+    MethodSymbol method = (MethodSymbol) ASTHelpers.getSymbol(subTree);
+    Symbol lhsSymbol = ASTHelpers.getSymbol(subTree.getExpression());
+    if (method.isStatic() && lhsSymbol instanceof VarSymbol) {
+      return Optional.empty();
+    }
+
+    if (hasAmbiguousMethodReference(method, state)) {
+      return Optional.empty();
+    }
+
+    Name lhs = lhsSymbol.name;
     if (expectedInstance.isEmpty()) {
       return constructFix(lambdaExpr, lhs, subTree.getIdentifier());
     }
 
-    Type lhsType = ASTHelpers.getType(subTree.getExpression());
+    Type lhsType = lhsSymbol.type;
     if (lhsType == null || !expectedInstance.orElseThrow().equals(lhs)) {
       return Optional.empty();
     }
@@ -156,7 +275,66 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
     return constructFix(lambdaExpr, lhsType.tsym, subTree.getIdentifier());
   }
 
-  private static Optional<Optional<Name>> matchArguments(
+  /**
+   * Tells whether the given method has an overload that would lead to an ambiguous method
+   * reference.
+   */
+  private static boolean hasAmbiguousMethodReference(MethodSymbol method, VisitorState state) {
+    return getOverloads(method)
+        .anyMatch(m -> haveAmbiguousMethodReferences(m, method, state.getTypes()));
+  }
+
+  /** Returns any overloads of the given method defined on the same class. */
+  // XXX: This probably doesn't return overloads defined by supertypes. Review and extend if
+  // necessary.
+  private static Stream<MethodSymbol> getOverloads(MethodSymbol method) {
+    return Streams.stream(ASTHelpers.enclosingClass(method).members().getSymbolsByName(method.name))
+        .filter(MethodSymbol.class::isInstance)
+        .map(MethodSymbol.class::cast)
+        .filter(not(method::equals));
+  }
+
+  /** Tells whether method references to the given methods would be mutually ambiguous. */
+  // XXX: This doesn't necessarily identify all ambiguous cases; carefully read the JLS and update
+  // this logic if necessary.
+  private static boolean haveAmbiguousMethodReferences(
+      MethodSymbol method1, MethodSymbol method2, Types types) {
+    if (method1.isStatic() == method2.isStatic()) {
+      return false;
+    }
+
+    if (method1.isStatic()) {
+      return haveAmbiguousMethodReferences(method2, method1, types);
+    }
+
+    com.sun.tools.javac.util.List<VarSymbol> params1 = method1.params();
+    com.sun.tools.javac.util.List<VarSymbol> params2 = method2.params();
+    if (params1.size() != params2.size() - 1) {
+      return false;
+    }
+
+    // XXX: Here and below: perhaps `isAssignable` is more appropriate than `isConvertible`.
+    if (!types.isConvertible(method1.owner.asType(), params2.get(0).asType())) {
+      return false;
+    }
+
+    for (int i = 0; i < params1.size(); i++) {
+      if (!types.isConvertible(params1.get(0).asType(), params2.get(i + 1).asType())) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  /**
+   * Attempts to match the given method invocation's arguments against the rightmost parameters of
+   * the provided lambda expression, in order; if successful with zero or one lambda parameter
+   * unaccounted for, then said parameter is returned.
+   */
+  // XXX: Refactor or replace inner `Optional` with a custom type.
+  @SuppressWarnings("NestedOptionals")
+  private static Optional<Optional<Name>> matchParameters(
       LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree) {
     ImmutableList<Name> expectedArguments = getVariables(lambdaExpr);
     List<? extends ExpressionTree> args = subTree.getArguments();
@@ -186,7 +364,12 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
     Name sName = target.getSimpleName();
     Optional<SuggestedFix.Builder> fix = constructFix(lambdaExpr, sName, methodName);
 
-    if (!"java.lang".equals(target.packge().toString())) {
+    if (!"java.lang".equals(target.packge().getQualifiedName().toString())) {
+      // XXX: Check whether the type can be imported. If not, skip the suggestion. (In other words:
+      // if another type with simple name `sName` is already imported, then this suggested fix would
+      // introduce a compilation failure.)
+      // XXX: Make sure `SuggestedFixes.qualifyType` handles `java.lang` and the double-import case,
+      // then use that method.
       Name fqName = target.getQualifiedName();
       if (!sName.equals(fqName)) {
         return fix.map(b -> b.addImport(fqName.toString()));
@@ -196,8 +379,15 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
     return fix;
   }
 
+  // XXX: As-is this method shouldn't return an `Optional`.
   private static Optional<SuggestedFix.Builder> constructFix(
       LambdaExpressionTree lambdaExpr, Object target, Object methodName) {
     return Optional.of(SuggestedFix.builder().replace(lambdaExpr, target + "::" + methodName));
   }
+
+  private static boolean tryCompileWithFix(VisitorState state, SuggestedFix fix) {
+    return !IS_NESTED_INVOCATION
+        && SuggestedFixes.compilesWithFix(
+            fix, state, ImmutableList.of(), /* onlyInSameCompilationUnit= */ true);
+  }
 }

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

@@ -0,0 +1,51 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.NONE;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
+
+import com.google.auto.service.AutoService;
+import com.google.common.collect.Iterables;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.suppliers.Supplier;
+import com.google.errorprone.suppliers.Suppliers;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.util.List;
+import java.util.Optional;
+
+/** A {@link BugChecker} which flags nesting of {@link Optional Optionals}. */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary =
+        "Avoid nesting `Optional`s inside `Optional`s; the resultant code is hard to reason about",
+    linkType = NONE,
+    severity = WARNING,
+    tags = FRAGILE_CODE)
+public final class NestedOptionals extends BugChecker implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Supplier<Type> OPTIONAL = Suppliers.typeFromClass(Optional.class);
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    return isOptionalOfOptional(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
+  }
+
+  private static boolean isOptionalOfOptional(Tree tree, VisitorState state) {
+    Type optionalType = OPTIONAL.get(state);
+    Type type = ASTHelpers.getType(tree);
+    if (!ASTHelpers.isSubtype(type, optionalType, state)) {
+      return false;
+    }
+
+    List<Type> typeArguments = type.getTypeArguments();
+    return !typeArguments.isEmpty()
+        && ASTHelpers.isSubtype(Iterables.getOnlyElement(typeArguments), optionalType, state);
+  }
+}

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

@@ -6,7 +6,6 @@ import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
 import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
-import static java.util.function.Predicate.not;
 import static java.util.stream.Collectors.joining;
 
 import com.google.auto.service.AutoService;
@@ -84,16 +83,15 @@ public final class PrimitiveComparison extends BugChecker implements MethodInvoc
 
   private static Optional<Fix> attemptMethodInvocationReplacement(
       MethodInvocationTree tree, Type cmpType, boolean isStatic, VisitorState state) {
-    return Optional.ofNullable(ASTHelpers.getSymbol(tree))
-        .map(methodSymbol -> methodSymbol.getSimpleName().toString())
-        .flatMap(
-            actualMethodName ->
-                Optional.of(getPreferredMethod(cmpType, isStatic, state))
-                    .filter(not(actualMethodName::equals)))
-        .map(
-            preferredMethodName ->
-                prefixTypeArgumentsIfRelevant(preferredMethodName, tree, cmpType, state))
-        .map(preferredMethodName -> suggestFix(tree, preferredMethodName, state));
+    String actualMethodName = ASTHelpers.getSymbol(tree).getSimpleName().toString();
+    String preferredMethodName = getPreferredMethod(cmpType, isStatic, state);
+    if (actualMethodName.equals(preferredMethodName)) {
+      return Optional.empty();
+    }
+
+    return Optional.of(
+        suggestFix(
+            tree, prefixTypeArgumentsIfRelevant(preferredMethodName, tree, cmpType, state), state));
   }
 
   /**

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

@@ -63,6 +63,7 @@ public final class RequestMappingAnnotation extends BugChecker implements Method
                       AT_LEAST_ONE,
                       anyOf(
                           isType(ANN_PACKAGE_PREFIX + "PathVariable"),
+                          isType(ANN_PACKAGE_PREFIX + "RequestAttribute"),
                           isType(ANN_PACKAGE_PREFIX + "RequestBody"),
                           isType(ANN_PACKAGE_PREFIX + "RequestHeader"),
                           isType(ANN_PACKAGE_PREFIX + "RequestParam"))),

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

@@ -66,6 +66,7 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
           "com.google.errorprone.BugPattern.LinkType",
           "com.google.errorprone.BugPattern.SeverityLevel",
           "com.google.errorprone.BugPattern.StandardTags",
+          "com.google.errorprone.matchers.Matchers",
           "com.google.errorprone.refaster.ImportPolicy",
           "com.mongodb.client.model.Accumulators",
           "com.mongodb.client.model.Aggregates",

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/AssertJThrowingCallableTemplates.java

@@ -403,6 +403,32 @@ final class AssertJThrowingCallableTemplates {
     }
   }
 
+  static final class AssertThatThrownByHasMessageParameters {
+    @BeforeTemplate
+    @SuppressWarnings("AssertThatThrownBy" /* Matches strictly more specific expressions. */)
+    AbstractObjectAssert<?, ?> before(
+        Class<? extends Throwable> exceptionType,
+        ThrowingCallable throwingCallable,
+        String message,
+        @Repeated Object parameters) {
+      return assertThatExceptionOfType(exceptionType)
+          .isThrownBy(throwingCallable)
+          .withMessage(message, parameters);
+    }
+
+    @AfterTemplate
+    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
+    AbstractObjectAssert<?, ?> after(
+        Class<? extends Throwable> exceptionType,
+        ThrowingCallable throwingCallable,
+        String message,
+        @Repeated Object parameters) {
+      return assertThatThrownBy(throwingCallable)
+          .isInstanceOf(exceptionType)
+          .hasMessage(message, parameters);
+    }
+  }
+
   // XXX: Drop this template in favour of a generic Error Prone check which flags
   // `String.format(...)` arguments to a wide range of format methods.
   static final class AbstractThrowableAssertHasMessage {

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/OptionalTemplates.java

@@ -239,9 +239,16 @@ final class OptionalTemplates {
     }
   }
 
-  /** Within a stream's map operation unconditional {@link Optional#get()} calls can be avoided. */
-  // XXX: An alternative approach is to `.flatMap(Optional::stream)`. That may be a bit longer, but
-  // yield nicer code. Think about it.
+  /**
+   * Within a stream's map operation unconditional {@link Optional#orElseThrow()} calls can be
+   * avoided.
+   *
+   * <p><strong>Warning:</strong> this rewrite rule is not completely behavior preserving. The
+   * original code throws an exception if the mapping operation does not produce a value, while the
+   * replacement does not.
+   */
+  // XXX: An alternative approach is to use `.flatMap(Optional::stream)`. That may be a bit longer,
+  // but yields nicer code. Think about it.
   abstract static class StreamMapToOptionalGet<T, S> {
     @Placeholder
     abstract Optional<S> toOptionalFunction(@MayOptionallyUse T element);
@@ -311,6 +318,7 @@ final class OptionalTemplates {
   /** Prefer {@link Optional#or(Supplier)} over more verbose alternatives. */
   abstract static class OptionalOrOtherOptional<T> {
     @BeforeTemplate
+    @SuppressWarnings("NestedOptionals" /* Auto-fix for the `NestedOptionals` check. */)
     Optional<T> before(Optional<T> optional1, Optional<T> optional2) {
       // XXX: Note that rewriting the first and third variant will change the code's behavior if
       // `optional2` has side-effects.

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

@@ -0,0 +1,182 @@
+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 ImmutablesSortedSetComparatorTest {
+  private final CompilationTestHelper compilationTestHelper =
+      CompilationTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass());
+  private final BugCheckerRefactoringTestHelper refactoringTestHelper =
+      BugCheckerRefactoringTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass());
+
+  @Test
+  void identification() {
+    compilationTestHelper
+        .addSourceLines(
+            "A.java",
+            "import com.google.common.collect.ContiguousSet;",
+            "import com.google.common.collect.ImmutableSet;",
+            "import com.google.common.collect.ImmutableSortedSet;",
+            "import java.util.NavigableSet;",
+            "import java.util.Set;",
+            "import java.util.SortedSet;",
+            "import java.util.TreeSet;",
+            "import org.immutables.value.Value;",
+            "",
+            "interface A {",
+            "  @Value.Immutable",
+            "  interface ImmutableInterface {",
+            "    Set<String> set();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    SortedSet<String> sortedSet();",
+            "",
+            "    @Value.NaturalOrder",
+            "    SortedSet<String> sortedSet2();",
+            "  }",
+            "",
+            "  @Value.Modifiable",
+            "  interface ModifiableInterfaceWithDefaults {",
+            "    @Value.Default",
+            "    default Set<Integer> set() {",
+            "      return new TreeSet<>();",
+            "    }",
+            "",
+            "    @Value.Default",
+            "    // BUG: Diagnostic contains:",
+            "    default NavigableSet<Integer> navigableSet() {",
+            "      return new TreeSet<>();",
+            "    }",
+            "",
+            "    @Value.Default",
+            "    @Value.ReverseOrder",
+            "    default NavigableSet<Integer> navigableSet2() {",
+            "      return new TreeSet<>();",
+            "    }",
+            "",
+            "    default NavigableSet<Integer> nonPropertyNavigableSet() {",
+            "      return new TreeSet<>();",
+            "    }",
+            "  }",
+            "",
+            "  interface NonImmutablesInterface {",
+            "    SortedSet<String> sortedSet();",
+            "  }",
+            "",
+            "  @Value.Immutable",
+            "  abstract class AbstractImmutableWithDefaults {",
+            "    @Value.Default",
+            "    ImmutableSet<Integer> immutableSet() {",
+            "      return ImmutableSet.of();",
+            "    }",
+            "",
+            "    @Value.Default",
+            "    // BUG: Diagnostic contains:",
+            "    ImmutableSortedSet<String> immutableSortedSet() {",
+            "      return ImmutableSortedSet.of();",
+            "    }",
+            "",
+            "    @Value.Default",
+            "    @Value.NaturalOrder",
+            "    ImmutableSortedSet<String> immutableSortedSet2() {",
+            "      return ImmutableSortedSet.of();",
+            "    }",
+            "",
+            "    ImmutableSortedSet<String> nonPropertyImmutableSortedSet() {",
+            "      return ImmutableSortedSet.of();",
+            "    }",
+            "  }",
+            "",
+            "  @Value.Modifiable",
+            "  abstract class AbstractModifiable {",
+            "    abstract ImmutableSet<Integer> immutableSet();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    abstract ContiguousSet<Integer> contiguousSet();",
+            "",
+            "    @Value.ReverseOrder",
+            "    abstract ContiguousSet<Integer> contiguousSet2();",
+            "  }",
+            "",
+            "  abstract class AbstractNonImmutables {",
+            "    abstract SortedSet<Integer> sortedSet();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacement() {
+    refactoringTestHelper
+        .addInputLines(
+            "A.java",
+            "import com.google.common.collect.ImmutableSortedSet;",
+            "import java.util.SortedSet;",
+            "import org.immutables.value.Value;",
+            "",
+            "@Value.Immutable",
+            "abstract class A {",
+            "  abstract ImmutableSortedSet<String> sortedSet();",
+            "",
+            "  @Value.Modifiable",
+            "  interface B {",
+            "    SortedSet<String> sortedSet();",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import com.google.common.collect.ImmutableSortedSet;",
+            "import java.util.SortedSet;",
+            "import org.immutables.value.Value;",
+            "",
+            "@Value.Immutable",
+            "abstract class A {",
+            "  @Value.NaturalOrder",
+            "  abstract ImmutableSortedSet<String> sortedSet();",
+            "",
+            "  @Value.Modifiable",
+            "  interface B {",
+            "    @Value.NaturalOrder",
+            "    SortedSet<String> sortedSet();",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+
+  @Test
+  void replacementWithImportClash() {
+    refactoringTestHelper
+        .addInputLines(
+            "MySpringService.java",
+            "import com.google.common.collect.ImmutableSortedSet;",
+            "import org.springframework.beans.factory.annotation.Value;",
+            "",
+            "class MySpringService {",
+            "  MySpringService(@Value(\"${someProperty}\") String prop) {}",
+            "  ;",
+            "",
+            "  @org.immutables.value.Value.Immutable",
+            "  interface A {",
+            "    ImmutableSortedSet<String> sortedSet();",
+            "  }",
+            "}")
+        .addOutputLines(
+            "MySpringService.java",
+            "import com.google.common.collect.ImmutableSortedSet;",
+            "import org.springframework.beans.factory.annotation.Value;",
+            "",
+            "class MySpringService {",
+            "  MySpringService(@Value(\"${someProperty}\") String prop) {}",
+            "  ;",
+            "",
+            "  @org.immutables.value.Value.Immutable",
+            "  interface A {",
+            "    @org.immutables.value.Value.NaturalOrder",
+            "    ImmutableSortedSet<String> sortedSet();",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+}

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

@@ -35,6 +35,8 @@ final class LexicographicalAnnotationAttributeListingTest {
             "import io.swagger.v3.oas.annotations.Parameters;",
             "import java.math.RoundingMode;",
             "import javax.xml.bind.annotation.XmlType;",
+            "import org.springframework.context.annotation.PropertySource;",
+            "import org.springframework.test.context.TestPropertySource;",
             "",
             "interface A {",
             "  @interface Foo {",
@@ -144,7 +146,16 @@ final class LexicographicalAnnotationAttributeListingTest {
             "  A secondEndpoint();",
             "",
             "  @XmlType(propOrder = {\"field2\", \"field1\"})",
-            "  class Dummy {}",
+            "  class XmlTypeDummy {}",
+            "",
+            "  @PropertySource({\"field2\", \"field1\"})",
+            "  class PropertySourceDummy {}",
+            "",
+            "  @TestPropertySource(locations = {\"field2\", \"field1\"})",
+            "  class FirstTestPropertySourceDummy {}",
+            "",
+            "  @TestPropertySource({\"field2\", \"field1\"})",
+            "  class SecondTestPropertySourceDummy {}",
             "}")
         .doTest();
   }

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

@@ -11,12 +11,16 @@ final class MethodReferenceUsageTest {
   private final BugCheckerRefactoringTestHelper refactoringTestHelper =
       BugCheckerRefactoringTestHelper.newInstance(MethodReferenceUsage.class, getClass());
 
+  // XXX: Disable the `replacement` test and verify using PIT that this test covers all
+  // identification cases.
   @Test
   void identification() {
     compilationTestHelper
         .addSourceLines(
             "A.java",
             "import com.google.common.collect.Streams;",
+            "import com.google.errorprone.refaster.annotation.AfterTemplate;",
+            "import com.google.errorprone.refaster.annotation.BeforeTemplate;",
             "import java.util.HashMap;",
             "import java.util.Map;",
             "import java.util.function.IntConsumer;",
@@ -261,6 +265,18 @@ final class MethodReferenceUsageTest {
             "        });",
             "  }",
             "",
+            "  @AfterTemplate",
+            "  void refasterBeforeTemplateFunctionCalls() {",
+            "    s.forEach(v -> String.valueOf(v));",
+            "    s.forEach((v) -> { String.valueOf(v); });",
+            "  }",
+            "",
+            "  @BeforeTemplate",
+            "  void refasterAfterTemplateFunctionCalls() {",
+            "    m.forEach((k, v) -> m.put(k, v));",
+            "    m.forEach((Integer k, Integer v) -> { m.put(k, v); });",
+            "  }",
+            "",
             "  void assortedOtherEdgeCases() {",
             "    s.forEach(v -> String.valueOf(v.toString()));",
             "    TernaryOp o1 = (a, b, c) -> String.valueOf(a);",

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

@@ -0,0 +1,42 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+final class NestedOptionalsTest {
+  private final CompilationTestHelper compilationTestHelper =
+      CompilationTestHelper.newInstance(NestedOptionals.class, getClass());
+
+  @Test
+  void identification() {
+    compilationTestHelper
+        .addSourceLines(
+            "A.java",
+            "import java.util.Optional;",
+            "import java.util.stream.Stream;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Optional.empty();",
+            "    Optional.of(1);",
+            "    // BUG: Diagnostic contains:",
+            "    Optional.of(Optional.empty());",
+            "    // BUG: Diagnostic contains:",
+            "    Optional.of(Optional.of(1));",
+            "",
+            "    Optional.ofNullable(null);",
+            "    // BUG: Diagnostic contains:",
+            "    Optional.ofNullable((Optional) null);",
+            "",
+            "    Optional.of(\"foo\").map(String::length);",
+            "    // BUG: Diagnostic contains:",
+            "    Optional.of(\"foo\").map(Optional::of);",
+            "",
+            "    Stream.of(\"foo\").findFirst();",
+            "    // BUG: Diagnostic contains:",
+            "    Stream.of(\"foo\").map(Optional::of).findFirst();",
+            "  }",
+            "}")
+        .doTest();
+  }
+}

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

@@ -25,6 +25,7 @@ final class RequestMappingAnnotationTest {
             "import org.springframework.web.bind.annotation.PathVariable;",
             "import org.springframework.web.bind.annotation.PostMapping;",
             "import org.springframework.web.bind.annotation.PutMapping;",
+            "import org.springframework.web.bind.annotation.RequestAttribute;",
             "import org.springframework.web.bind.annotation.RequestBody;",
             "import org.springframework.web.bind.annotation.RequestHeader;",
             "import org.springframework.web.bind.annotation.RequestMapping;",
@@ -47,12 +48,15 @@ final class RequestMappingAnnotationTest {
             "  A properPathVariable(@PathVariable String param);",
             "",
             "  @PatchMapping",
-            "  A properRequestBody(@RequestBody String body);",
+            "  A properRequestAttribute(@RequestAttribute String attribute);",
             "",
             "  @PostMapping",
-            "  A properRequestHeader(@RequestHeader String header);",
+            "  A properRequestBody(@RequestBody String body);",
             "",
             "  @PutMapping",
+            "  A properRequestHeader(@RequestHeader String header);",
+            "",
+            "  @RequestMapping",
             "  A properRequestParam(@RequestParam String param);",
             "",
             "  @RequestMapping",

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/AssertJThrowingCallableTemplatesTestInput.java

@@ -99,7 +99,6 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
     return assertThatIOException().isThrownBy(() -> {}).withMessage("foo");
   }
 
-  @SuppressWarnings("AssertThatThrownByIOException")
   AbstractObjectAssert<?, ?> testAssertThatThrownByIOExceptionHasMessageParameters() {
     return assertThatIOException().isThrownBy(() -> {}).withMessage("foo %s", "bar");
   }
@@ -114,6 +113,12 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
         .withMessage("foo");
   }
 
+  AbstractObjectAssert<?, ?> testAssertThatThrownByHasMessageParameters() {
+    return assertThatExceptionOfType(IllegalArgumentException.class)
+        .isThrownBy(() -> {})
+        .withMessage("foo %s", "bar");
+  }
+
   ImmutableSet<AbstractThrowableAssert<?, ? extends Throwable>>
       testAbstractThrowableAssertHasMessage() {
     return ImmutableSet.of(

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/AssertJThrowingCallableTemplatesTestOutput.java

@@ -120,7 +120,6 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
     return assertThatThrownBy(() -> {}).isInstanceOf(IOException.class).hasMessage("foo");
   }
 
-  @SuppressWarnings("AssertThatThrownByIOException")
   AbstractObjectAssert<?, ?> testAssertThatThrownByIOExceptionHasMessageParameters() {
     return assertThatThrownBy(() -> {}).isInstanceOf(IOException.class).hasMessage("foo %s", "bar");
   }
@@ -135,6 +134,12 @@ final class AssertJThrowingCallableTemplatesTest implements RefasterTemplateTest
         .hasMessage("foo");
   }
 
+  AbstractObjectAssert<?, ?> testAssertThatThrownByHasMessageParameters() {
+    return assertThatThrownBy(() -> {})
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("foo %s", "bar");
+  }
+
   ImmutableSet<AbstractThrowableAssert<?, ? extends Throwable>>
       testAbstractThrowableAssertHasMessage() {
     return ImmutableSet.of(

pom.xml

@@ -4,7 +4,7 @@
 
     <groupId>tech.picnic.error-prone-support</groupId>
     <artifactId>error-prone-support</artifactId>
-    <version>0.1.0</version>
+    <version>0.1.1-SNAPSHOT</version>
     <packaging>pom</packaging>
 
     <name>Picnic :: Error Prone Support</name>
@@ -48,7 +48,7 @@
 
     <scm>
         <developerConnection>scm:git:git@github.com:PicnicSupermarket/error-prone-support.git</developerConnection>
-        <tag>v0.1.0</tag>
+        <tag>HEAD</tag>
         <url>https://github.com/PicnicSupermarket/error-prone-support</url>
     </scm>
     <issueManagement>
@@ -129,9 +129,10 @@
         default value. -->
         <build.number>LOCAL</build.number>
         <!-- Properties using which additional Error Prone flags can be
-        specified. Used by the `patch` and `self-check` profiles. -->
+        specified. The first may be used to specify flags on the command line;
+        the second is used by the `patch` profile. -->
+        <error-prone.extra-args />
         <error-prone.patch-args />
-        <error-prone.self-check-args />
         <!-- The Maven `groupId` under which Error Prone dependencies are
         published. By default we use an official Error Prone release. This
         property allows the `error-prone-fork` profile below to build the
@@ -150,8 +151,8 @@
         <version.findbugs-format-string>3.0.0</version.findbugs-format-string>
         <version.guava-beta-checker>1.0</version.guava-beta-checker>
         <version.jdk>11</version.jdk>
-        <version.maven>3.6.3</version.maven>
-        <version.mockito>4.6.1</version.mockito>
+        <version.maven>3.8.6</version.maven>
+        <version.mockito>4.7.0</version.mockito>
         <version.nopen-checker>1.0.1</version.nopen-checker>
         <version.nullaway>0.9.9</version.nullaway>
         <!-- XXX: Two other dependencies are potentially of interest:
@@ -317,7 +318,7 @@
             <dependency>
                 <groupId>io.projectreactor</groupId>
                 <artifactId>reactor-bom</artifactId>
-                <version>2020.0.21</version>
+                <version>2020.0.22</version>
                 <type>pom</type>
                 <scope>import</scope>
             </dependency>
@@ -386,6 +387,11 @@
                 <artifactId>hamcrest-core</artifactId>
                 <version>2.2</version>
             </dependency>
+            <dependency>
+                <groupId>org.immutables</groupId>
+                <artifactId>value-annotations</artifactId>
+                <version>2.9.0</version>
+            </dependency>
             <dependency>
                 <groupId>org.junit</groupId>
                 <artifactId>junit-bom</artifactId>
@@ -420,7 +426,7 @@
             <dependency>
                 <groupId>org.springframework.boot</groupId>
                 <artifactId>spring-boot-test</artifactId>
-                <version>2.7.2</version>
+                <version>2.7.3</version>
             </dependency>
             <dependency>
                 <groupId>org.testng</groupId>
@@ -982,7 +988,7 @@
                 <plugin>
                     <groupId>org.apache.maven.plugins</groupId>
                     <artifactId>maven-javadoc-plugin</artifactId>
-                    <version>3.4.0</version>
+                    <version>3.4.1</version>
                     <configuration>
                         <!-- All relevant doclint checks are performed during
                         the compilation phase; no need to recheck during
@@ -1345,16 +1351,14 @@
             <!-- Applies the Error Prone checks defined by this project to the
             code base itself. Assumes that a prior build has already installed
             the project in the local Maven repository. -->
+            <!-- XXX: Find a way to assert that test code (both inline
+            `BugChecker` test code and the Refaster test files) does not
+            exhibit anti-patterns other than those associated with the
+            check/template under test. Ideally all test cases are
+            realistic. -->
             <id>self-check</id>
             <properties>
-                <!-- XXX: `MethodReferenceUsage` is an extremely expensive
-                check due to its use of `SuggestedFixes.compilesWithFix`. Maybe
-                we should drop it altogether? -->
-                <!-- XXX: Find a way to assert that test code (both inline
-                `BugChecker` test code and the Refaster test files) does not
-                exhibit anti-patterns other than those associated with the
-                check/template under test. Ideally all test cases are realistic. -->
-                <error-prone.self-check-args>-XepAllSuggestionsAsWarnings -Xep:MethodReferenceUsage:OFF</error-prone.self-check-args>
+                <error-prone.self-check-args>-XepAllSuggestionsAsWarnings</error-prone.self-check-args>
             </properties>
             <build>
                 <plugins>
@@ -1536,9 +1540,6 @@
                                     -Xep:StaticOrDefaultInterfaceMethod:OFF
                                     <!-- We generally discourage `var` use. -->
                                     -Xep:Varifier:OFF
-                                    <!-- XXX: This check flags false positives.
-                                    See https://github.com/google/error-prone/issues/2679. -->
-                                    -Xep:VoidMissingNullable:OFF
                                     -XepOpt:CheckReturnValue:CheckAllConstructors=true
                                     <!-- XXX: Enable once there are fewer
                                     false-positives.
@@ -1550,7 +1551,7 @@
                                     -XepOpt:Nullness:Conservative=false
                                     <!-- Append additional custom arguments. -->
                                     ${error-prone.patch-args}
-                                    ${error-prone.self-check-args}
+                                    ${error-prone.extra-args}
                                 </arg>
                                 <!-- The Error Prone plugin makes certain
                                 assumptions about the state of the AST at the

refaster-compiler/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <groupId>tech.picnic.error-prone-support</groupId>
         <artifactId>error-prone-support</artifactId>
-        <version>0.1.0</version>
+        <version>0.1.1-SNAPSHOT</version>
     </parent>
 
     <artifactId>refaster-compiler</artifactId>

refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java

@@ -28,7 +28,6 @@ import java.io.ObjectOutputStream;
 import java.io.UncheckedIOException;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import javax.annotation.Nullable;
 import javax.tools.FileObject;
 import javax.tools.JavaFileManager;
@@ -73,7 +72,7 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
     return Boolean.TRUE.equals(
         new TreeScanner<Boolean, Void>() {
           @Override
-          public Boolean visitAnnotation(AnnotationTree node, Void v) {
+          public Boolean visitAnnotation(AnnotationTree node, @Nullable Void v) {
             Symbol sym = ASTHelpers.getSymbol(node);
             return (sym != null
                     && sym.getQualifiedName()
@@ -94,7 +93,7 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
     new TreeScanner<Void, Void>() {
       @Nullable
       @Override
-      public Void visitClass(ClassTree node, Void v) {
+      public Void visitClass(ClassTree node, @Nullable Void v) {
         rules.putAll(node, RefasterRuleBuilderScanner.extractRules(node, context));
         return super.visitClass(node, null);
       }
@@ -103,16 +102,10 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
   }
 
   private FileObject getOutputFile(TaskEvent taskEvent, ClassTree tree) throws IOException {
-    String packageName =
-        Optional.ofNullable(ASTHelpers.getSymbol(tree))
-            .map(ASTHelpers::enclosingPackage)
-            .map(PackageSymbol::toString)
-            .orElse("");
-    CharSequence className =
-        Optional.ofNullable(ASTHelpers.getSymbol(tree))
-            .map(RefasterRuleCompilerTaskListener::toSimpleFlatName)
-            .orElseGet(tree::getSimpleName);
-    String relativeName = className + ".refaster";
+    ClassSymbol symbol = ASTHelpers.getSymbol(tree);
+    PackageSymbol enclosingPackage = ASTHelpers.enclosingPackage(symbol);
+    String packageName = enclosingPackage == null ? "" : enclosingPackage.toString();
+    String relativeName = toSimpleFlatName(symbol) + ".refaster";
 
     JavaFileManager fileManager = context.get(JavaFileManager.class);
     return fileManager.getFileForOutput(

refaster-runner/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <groupId>tech.picnic.error-prone-support</groupId>
         <artifactId>error-prone-support</artifactId>
-        <version>0.1.0</version>
+        <version>0.1.1-SNAPSHOT</version>
     </parent>
 
     <artifactId>refaster-runner</artifactId>

refaster-support/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <groupId>tech.picnic.error-prone-support</groupId>
         <artifactId>error-prone-support</artifactId>
-        <version>0.1.0</version>
+        <version>0.1.1-SNAPSHOT</version>
     </parent>
 
     <artifactId>refaster-support</artifactId>

refaster-support/src/main/java/tech/picnic/errorprone/refaster/util/IsArray.java

@@ -1,14 +1,15 @@
 package tech.picnic.errorprone.refaster.util;
 
+import static com.google.errorprone.matchers.Matchers.isArrayType;
+
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.matchers.Matcher;
-import com.google.errorprone.matchers.Matchers;
 import com.sun.source.tree.ExpressionTree;
 
 /** A matcher of array-typed expressions, for use with Refaster's {@code @Matches} annotation. */
 public final class IsArray implements Matcher<ExpressionTree> {
   private static final long serialVersionUID = 1L;
-  private static final Matcher<ExpressionTree> DELEGATE = Matchers.isArrayType();
+  private static final Matcher<ExpressionTree> DELEGATE = isArrayType();
 
   @Override
   public boolean matches(ExpressionTree tree, VisitorState state) {

refaster-test-support/pom.xml

@@ -5,7 +5,7 @@
     <parent>
         <groupId>tech.picnic.error-prone-support</groupId>
         <artifactId>error-prone-support</artifactId>
-        <version>0.1.0</version>
+        <version>0.1.1-SNAPSHOT</version>
     </parent>
 
     <artifactId>refaster-test-support</artifactId>