WIP!

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

@@ -32,6 +32,7 @@ import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.Matchers;
+import com.google.errorprone.refaster.Refaster;
 import com.google.errorprone.util.ASTHelpers;
 import com.google.errorprone.util.ASTHelpers.TargetType;
 import com.sun.source.tree.ExpressionTree;
@@ -84,6 +85,7 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
                   ImmutableTable.class.getCanonicalName())
               .named("copyOf"),
           staticMethod().onClass(Matchers.class.getCanonicalName()).namedAnyOf("allOf", "anyOf"),
+          staticMethod().onClass(Refaster.class.getCanonicalName()).namedAnyOf("anyOf"),
           staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"),
           staticMethod()
               .onClass("reactor.core.publisher.Flux")

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

@@ -0,0 +1,84 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
+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 static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
+
+import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableCollection;
+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.refaster.Refaster;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import tech.picnic.errorprone.utils.SourceCode;
+
+/**
+ * A {@link BugChecker} that flags method invocations without arguments, in cases where such
+ * invocation amounts to a no-op.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary = "Avoid no-op invocations of varargs methods without arguments",
+    link = BUG_PATTERNS_BASE_URL + "VacuousZeroArgMethodInvocation",
+    linkType = CUSTOM,
+    severity = SUGGESTION,
+    tags = SIMPLIFICATION)
+public final class VacuousZeroArgMethodInvocation extends BugChecker
+    implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> FLAGGED_INSTANCE_METHOD =
+      anyOf(
+          instanceMethod()
+              .onDescendantOf(ImmutableCollection.Builder.class.getCanonicalName())
+              .named("add"),
+          instanceMethod()
+              .onDescendantOfAny(
+                  "com.google.errorprone.BugCheckerRefactoringTestHelper",
+                  "com.google.errorprone.CompilationTestHelper")
+              .named("setArgs"));
+  private static final Matcher<ExpressionTree> FLAGGED_STATIC_METHOD =
+      staticMethod().onClass(Refaster.class.getCanonicalName()).named("anyOf");
+
+  /** Instantiates a new {@link VacuousZeroArgMethodInvocation} instance. */
+  public VacuousZeroArgMethodInvocation() {}
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (!tree.getArguments().isEmpty()) {
+      return Description.NO_MATCH;
+    }
+
+    if (FLAGGED_INSTANCE_METHOD.matches(tree, state)) {
+      ExpressionTree receiver = ASTHelpers.getReceiver(tree);
+      if (receiver == null) {
+        // XXX: Test this using an `ImmutableCollection.Builder` subtype in which we call `add()`.
+        // XXX: This call can be removed completely, unless the result is used in some way (by being
+        // dereferenced, or passed as an argument to another method).
+        return describeMatch(tree);
+      }
+
+      // XXX: This logic is also used in `NonEmptyMono`; worthy of a `SourceCode` utility method?
+      return describeMatch(
+          tree, SuggestedFix.replace(tree, SourceCode.treeToString(receiver, state)));
+    }
+
+    if (FLAGGED_STATIC_METHOD.matches(tree, state)) {
+      // XXX: Drop the method invocation if its result is not used in some way (by being
+      // dereferenced, or passed as an argument to another method).
+      return describeMatch(tree);
+    }
+
+    return Description.NO_MATCH;
+  }
+}

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

@@ -28,6 +28,7 @@ final class IdentityConversionTest {
             "import com.google.common.collect.ImmutableTable;",
             "import com.google.errorprone.matchers.Matcher;",
             "import com.google.errorprone.matchers.Matchers;",
+            "import com.google.errorprone.refaster.Refaster;",
             "import reactor.adapter.rxjava.RxJava2Adapter;",
             "import reactor.core.publisher.Flux;",
             "import reactor.core.publisher.Mono;",
@@ -157,6 +158,10 @@ final class IdentityConversionTest {
             "    Matcher anyOf2 = Matchers.anyOf(instanceMethod(), staticMethod());",
             "",
             "    // BUG: Diagnostic contains:",
+            "    String refasterBranch1 = Refaster.anyOf(\"foo\");",
+            "    String refasterBranch2 = Refaster.anyOf(\"foo\", \"bar\");",
+            "",
+            "    // BUG: Diagnostic contains:",
             "    Flux<Integer> flux1 = Flux.just(1).flatMap(e -> RxJava2Adapter.fluxToFlowable(Flux.just(2)));",
             "",
             "    // BUG: Diagnostic contains:",

error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/RefasterAnyOfUsage.java

@@ -25,6 +25,12 @@ import tech.picnic.errorprone.utils.SourceCode;
  * <p>Note that this logic can't be implemented as a Refaster rule, as the {@link Refaster} class is
  * treated specially.
  */
+// XXX: The single-argument case is also covered by the `IdentityConversion` check. Consider
+// replacing this check with an `VacuousZeroArgMethodInvocation` check that flags a wider range of
+// vargars method invocations without arguments. Examples of other methods to include
+// `ImmutableList.Builder#add` and `CompilationTestHelper#setArgs`. This check should suggest
+// dropping the method invocation if (a) it's contained in an `ExpressionStatementTree` or (b) it's
+// return type matches the receiver type.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "`Refaster#anyOf` should be passed at least two parameters",