FAILED experiment

I sense that we can do better, but it won't be trivial.

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

@@ -4,26 +4,17 @@ 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;
@@ -32,21 +23,15 @@ 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;
 
 /**
@@ -64,8 +49,6 @@ 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",
@@ -74,140 +57,59 @@ 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.
      */
-    // XXX: Update the comment to reflect actual `tryCompileWithFix` usage.
-    return constructMethodRef(tree, state, tree.getBody())
+    return constructMethodRef(tree, tree.getBody())
         .map(SuggestedFix.Builder::build)
-        .filter(fix -> !verifySuggestions || tryCompileWithFix(state, fix))
+        .filter(
+            fix ->
+                SuggestedFixes.compilesWithFix(
+                    fix, state, ImmutableList.of(), /* onlyInSameCompilationUnit= */ true))
         .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, VisitorState state, Tree subTree) {
+      LambdaExpressionTree lambdaExpr, Tree subTree) {
     switch (subTree.getKind()) {
       case BLOCK:
-        return constructMethodRef(lambdaExpr, state, (BlockTree) subTree);
+        return constructMethodRef(lambdaExpr, (BlockTree) subTree);
       case EXPRESSION_STATEMENT:
-        return constructMethodRef(
-            lambdaExpr, state, ((ExpressionStatementTree) subTree).getExpression());
+        return constructMethodRef(lambdaExpr, ((ExpressionStatementTree) subTree).getExpression());
       case METHOD_INVOCATION:
-        return constructMethodRef(lambdaExpr, state, (MethodInvocationTree) subTree);
+        return constructMethodRef(lambdaExpr, (MethodInvocationTree) subTree);
       case PARENTHESIZED:
-        // XXX: Add test!
-        return constructMethodRef(lambdaExpr, state, ((ParenthesizedTree) subTree).getExpression());
+        return constructMethodRef(lambdaExpr, ((ParenthesizedTree) subTree).getExpression());
       case RETURN:
-        // XXX: This case isn't tested. Reachable?
-        // XXX: Should be possible with `{ return x; }`.
-        return constructMethodRef(lambdaExpr, state, ((ReturnTree) subTree).getExpression());
+        return constructMethodRef(lambdaExpr, ((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, VisitorState state, BlockTree subTree) {
-    // XXX: Add test with >1 statement.
+      LambdaExpressionTree lambdaExpr, BlockTree subTree) {
     return Optional.of(subTree.getStatements())
         .filter(statements -> statements.size() == 1)
-        .flatMap(statements -> constructMethodRef(lambdaExpr, state, statements.get(0)));
+        .flatMap(statements -> constructMethodRef(lambdaExpr, statements.get(0)));
   }
 
   // XXX: Replace nested `Optional` usage.
   @SuppressWarnings("NestedOptionals")
   private static Optional<SuggestedFix.Builder> constructMethodRef(
-      LambdaExpressionTree lambdaExpr, VisitorState state, MethodInvocationTree subTree) {
-    return matchParameters(lambdaExpr, subTree)
-        .flatMap(
-            expectedInstance -> constructMethodRef(lambdaExpr, state, subTree, expectedInstance));
+      LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree) {
+    return matchArguments(lambdaExpr, subTree)
+        .flatMap(expectedInstance -> constructMethodRef(lambdaExpr, subTree, expectedInstance));
   }
 
   private static Optional<SuggestedFix.Builder> constructMethodRef(
       LambdaExpressionTree lambdaExpr,
-      VisitorState state,
       MethodInvocationTree subTree,
       Optional<Name> expectedInstance) {
     ExpressionTree methodSelect = subTree.getMethodSelect();
@@ -217,54 +119,35 @@ 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, state, (MemberSelectTree) methodSelect, expectedInstance);
+        return constructMethodRef(lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance);
       default:
         throw new VerifyException("Unexpected type of expression: " + methodSelect.getKind());
     }
   }
 
   private static Optional<SuggestedFix.Builder> constructMethodRef(
-      LambdaExpressionTree lambdaExpr,
-      VisitorState state,
-      MemberSelectTree subTree,
-      Optional<Name> expectedInstance) {
+      LambdaExpressionTree lambdaExpr, MemberSelectTree subTree, Optional<Name> expectedInstance) {
     if (subTree.getExpression().getKind() != Kind.IDENTIFIER) {
-      // 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()`.
+      // XXX: Could be parenthesized. Handle. Also in other classes.
       /*
        * 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();
     }
 
-    // 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;
+    Name lhs = ((IdentifierTree) subTree.getExpression()).getName();
     if (expectedInstance.isEmpty()) {
       return constructFix(lambdaExpr, lhs, subTree.getIdentifier());
     }
 
-    Type lhsType = lhsSymbol.type;
+    Type lhsType = ASTHelpers.getType(subTree.getExpression());
     if (lhsType == null || !expectedInstance.orElseThrow().equals(lhs)) {
       return Optional.empty();
     }
@@ -275,66 +158,9 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
     return constructFix(lambdaExpr, lhsType.tsym, subTree.getIdentifier());
   }
 
-  /**
-   * 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(
+  private static Optional<Optional<Name>> matchArguments(
       LambdaExpressionTree lambdaExpr, MethodInvocationTree subTree) {
     ImmutableList<Name> expectedArguments = getVariables(lambdaExpr);
     List<? extends ExpressionTree> args = subTree.getArguments();
@@ -364,12 +190,7 @@ 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().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.
+    if (!"java.lang".equals(target.packge().toString())) {
       Name fqName = target.getQualifiedName();
       if (!sName.equals(fqName)) {
         return fix.map(b -> b.addImport(fqName.toString()));
@@ -379,15 +200,8 @@ 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/NonEmptyMono.java

@@ -0,0 +1,91 @@
+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.SIMPLIFICATION;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.instanceMethod;
+
+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.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import java.util.function.BiFunction;
+import reactor.core.publisher.Mono;
+import tech.picnic.errorprone.bugpatterns.util.SourceCode;
+
+/**
+ * A {@link BugChecker} which flags {@link Mono} operations that are known to be vacuous, given that
+ * they are invoked on a {@link Mono} that is known not to complete empty.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary = "Avoid vacuous operations on known non-empty `Mono`s",
+    linkType = NONE,
+    severity = WARNING,
+    tags = SIMPLIFICATION)
+// XXX: This check does not simplify `someFlux.defaultIfEmpty(T).{defaultIfEmpty(T),hasElements()}`,
+// as `someFlux.defaultIfEmpty(T)` yields a `Flux` rather than a `Mono`. Consider adding support for
+// these cases.
+// XXX: Given more advanced analysis many more expressions could be flagged. Consider
+// `Mono.just(someValue)`, `Flux.just(someNonEmptySequence)`,
+// `someMono.switchIfEmpty(someProvablyNonEmptyMono)` and many other variants.
+// XXX: Consider implementing a similar check for `Publisher`s that are known to complete without
+// emitting a value (e.g. `Mono.empty()`, `someFlux.then()`, ...), or known not to complete normally
+// (`Mono.never()`, `someFlux.repeat()`, `Mono.error(...)`, ...). The latter category could
+// potentially be split out further.
+public final class NonEmptyMono extends BugChecker implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> MONO_SIZE_CHECK =
+      instanceMethod()
+          .onDescendantOf("reactor.core.publisher.Mono")
+          .namedAnyOf("defaultIfEmpty", "single", "switchIfEmpty");
+  private static final Matcher<ExpressionTree> NON_EMPTY_MONO =
+      anyOf(
+          instanceMethod()
+              .onDescendantOf("reactor.core.publisher.Flux")
+              .namedAnyOf(
+                  "all",
+                  "any",
+                  "collect",
+                  "collectList",
+                  "collectMap",
+                  "collectMultimap",
+                  "collectSortedList",
+                  "count",
+                  "elementAt",
+                  "hasElement",
+                  "hasElements",
+                  "last",
+                  "reduceWith",
+                  "single"),
+          instanceMethod()
+              .onDescendantOf("reactor.core.publisher.Flux")
+              .named("reduce")
+              .withParameters(Object.class.getName(), BiFunction.class.getName()),
+          instanceMethod()
+              .onDescendantOf("reactor.core.publisher.Mono")
+              .namedAnyOf("defaultIfEmpty", "hasElement", "single"));
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (!MONO_SIZE_CHECK.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    ExpressionTree receiver = ASTHelpers.getReceiver(tree);
+    if (!NON_EMPTY_MONO.matches(receiver, state)) {
+      return Description.NO_MATCH;
+    }
+
+    return describeMatch(
+        tree, SuggestedFix.replace(tree, SourceCode.treeToString(receiver, state)));
+  }
+}

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

@@ -11,16 +11,12 @@ 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;",
@@ -265,18 +261,6 @@ 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/NonEmptyMonoTest.java

@@ -0,0 +1,155 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+final class NonEmptyMonoTest {
+  private final CompilationTestHelper compilationTestHelper =
+      CompilationTestHelper.newInstance(NonEmptyMono.class, getClass());
+  private final BugCheckerRefactoringTestHelper refactoringTestHelper =
+      BugCheckerRefactoringTestHelper.newInstance(NonEmptyMono.class, getClass());
+
+  @Test
+  void identification() {
+    compilationTestHelper
+        .addSourceLines(
+            "A.java",
+            "import static com.google.common.collect.ImmutableList.toImmutableList;",
+            "import static java.util.function.Function.identity;",
+            "",
+            "import com.google.common.collect.ImmutableList;",
+            "import com.google.common.collect.ImmutableMap;",
+            "import java.util.ArrayList;",
+            "import java.util.HashMap;",
+            "import java.util.List;",
+            "import reactor.core.publisher.Flux;",
+            "import reactor.core.publisher.Mono;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Mono.just(1).defaultIfEmpty(2);",
+            "    Mono.just(1).single();",
+            "    Mono.just(1).switchIfEmpty(Mono.just(2));",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).all(x -> true).defaultIfEmpty(true);",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).any(x -> true).single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collect(toImmutableList()).switchIfEmpty(Mono.just(ImmutableList.of()));",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collect(ArrayList::new, List::add).defaultIfEmpty(new ArrayList<>());",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectList().single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectMap(identity()).switchIfEmpty(Mono.just(ImmutableMap.of()));",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectMap(identity(), identity()).defaultIfEmpty(ImmutableMap.of());",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectMap(identity(), identity(), HashMap::new).single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectMultimap(identity()).switchIfEmpty(Mono.just(ImmutableMap.of()));",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectMultimap(identity(), identity()).defaultIfEmpty(ImmutableMap.of());",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectMultimap(identity(), identity(), HashMap::new).single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectSortedList().defaultIfEmpty(ImmutableList.of());",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).collectSortedList((o1, o2) -> 0).single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).count().switchIfEmpty(Mono.just(2L));",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).elementAt(0).defaultIfEmpty(1);",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).elementAt(0, 2).single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).hasElement(2).switchIfEmpty(Mono.just(true));",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).hasElements().defaultIfEmpty(true);",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).last().single();",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).last(2).switchIfEmpty(Mono.just(3));",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).reduceWith(() -> 0, Integer::sum).defaultIfEmpty(2);",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).single().single();",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).single(2).switchIfEmpty(Mono.just(3));",
+            "",
+            "    Flux.just(1).reduce(Integer::sum).defaultIfEmpty(2);",
+            "    // BUG: Diagnostic contains:",
+            "    Flux.just(1).reduce(2, Integer::sum).single();",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(1).defaultIfEmpty(1).switchIfEmpty(Mono.just(2));",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(1).hasElement().defaultIfEmpty(true);",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(1).single().single();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacement() {
+    refactoringTestHelper
+        .addInputLines(
+            "A.java",
+            "import static com.google.common.collect.ImmutableList.toImmutableList;",
+            "",
+            "import com.google.common.collect.ImmutableList;",
+            "import reactor.core.publisher.Flux;",
+            "import reactor.core.publisher.Mono;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).collect(toImmutableList()).single();",
+            "    Flux.just(1).collect(toImmutableList()).defaultIfEmpty(ImmutableList.of());",
+            "    Flux.just(1).collect(toImmutableList()).switchIfEmpty(Mono.just(ImmutableList.of()));",
+            "",
+            "    Mono.just(2).hasElement().single();",
+            "    Mono.just(2).hasElement().defaultIfEmpty(true);",
+            "    Mono.just(2).hasElement().switchIfEmpty(Mono.just(true));",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import static com.google.common.collect.ImmutableList.toImmutableList;",
+            "",
+            "import com.google.common.collect.ImmutableList;",
+            "import reactor.core.publisher.Flux;",
+            "import reactor.core.publisher.Mono;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).collect(toImmutableList());",
+            "    Flux.just(1).collect(toImmutableList());",
+            "    Flux.just(1).collect(toImmutableList());",
+            "",
+            "    Mono.just(2).hasElement();",
+            "    Mono.just(2).hasElement();",
+            "    Mono.just(2).hasElement();",
+            "  }",
+            "}")
+        .doTest(TEXT_MATCH);
+  }
+}

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

@@ -792,4 +792,39 @@ final class PrimitiveComparisonTest {
             "}")
         .doTest(TestMode.TEXT_MATCH);
   }
+
+  // XXX: This still fails: all replacements start at the same place...
+  @Test
+  void testReplacementOfMultipleSubexpressions() {
+    refactoringTestHelper
+        .addInputLines(
+            "in/A.java",
+            "import java.util.Comparator;",
+            "",
+            "interface A extends Comparable<A> {",
+            "  Comparator<A> cmp = Comparator.<A, A>comparing(o -> o)",
+            "      .thenComparingInt(o -> Byte.valueOf((byte) 0))",
+            "      .thenComparingInt(o -> Character.valueOf((char) 0))",
+            "      .thenComparingInt(o -> Short.valueOf((short) 0))",
+            "      .thenComparingInt(o -> Integer.valueOf(0))",
+            "      .thenComparingLong(o -> Long.valueOf(0))",
+            "      .thenComparingDouble(o -> Float.valueOf(0))",
+            "      .thenComparingDouble(o -> Double.valueOf(0));",
+            "}")
+        .addOutputLines(
+            "out/A.java",
+            "import java.util.Comparator;",
+            "",
+            "interface A extends Comparable<A> {",
+            "  Comparator<A> cmp = Comparator.<A, A>comparing(o -> o) ",
+            "      .thenComparing(o -> Byte.valueOf((byte) 0))",
+            "      .thenComparing(o -> Character.valueOf((char) 0))",
+            "      .thenComparing(o -> Short.valueOf((short) 0))",
+            "      .thenComparing(o -> Integer.valueOf(0))",
+            "      .thenComparing(o -> Long.valueOf(0))",
+            "      .thenComparing(o -> Float.valueOf(0))",
+            "      .thenComparing(o -> Double.valueOf(0));",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
 }

pom.xml

@@ -129,10 +129,9 @@
         default value. -->
         <build.number>LOCAL</build.number>
         <!-- Properties using which additional Error Prone flags can be
-        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 />
+        specified. Used by the `patch` and `self-check` profiles. -->
         <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
@@ -1351,14 +1350,16 @@
             <!-- 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>
-                <error-prone.self-check-args>-XepAllSuggestionsAsWarnings</error-prone.self-check-args>
+                <!-- 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>
             </properties>
             <build>
                 <plugins>
@@ -1551,7 +1552,7 @@
                                     -XepOpt:Nullness:Conservative=false
                                     <!-- Append additional custom arguments. -->
                                     ${error-prone.patch-args}
-                                    ${error-prone.extra-args}
+                                    ${error-prone.self-check-args}
                                 </arg>
                                 <!-- The Error Prone plugin makes certain
                                 assumptions about the state of the AST at the