Introduce EmptyReactivePublisher bug checker

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

@@ -0,0 +1,135 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+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 static com.google.errorprone.matchers.Matchers.isSubtypeOf;
+import static com.google.errorprone.matchers.Matchers.staticMethod;
+import static com.google.errorprone.matchers.Matchers.typePredicateMatcher;
+import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
+import static tech.picnic.errorprone.utils.MoreTypePredicates.isSubTypeOf;
+import static tech.picnic.errorprone.utils.MoreTypes.generic;
+import static tech.picnic.errorprone.utils.MoreTypes.type;
+
+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.matchers.Matcher;
+import com.google.errorprone.suppliers.Supplier;
+import com.google.errorprone.suppliers.Suppliers;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.LambdaExpressionTree;
+import com.sun.source.tree.MemberReferenceTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Type;
+import java.util.function.Consumer;
+import org.reactivestreams.Publisher;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+/**
+ * A {@link BugChecker} that flags {@link Publisher} operations that are known to be vacuous, given
+ * that they are invoked on a {@link Mono} or {@link Flux} that do not emit next signals.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary = "Avoid vacuous operations on empty `Publisher`s",
+    link = BUG_PATTERNS_BASE_URL + "EmptyReactivePublisher",
+    linkType = CUSTOM,
+    severity = WARNING,
+    tags = SIMPLIFICATION)
+public final class EmptyReactivePublisher extends BugChecker
+    implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+
+  private static final Supplier<Type> MONO =
+      Suppliers.typeFromString("reactor.core.publisher.Mono");
+  private static final Supplier<Type> FLUX =
+      Suppliers.typeFromString("reactor.core.publisher.Flux");
+  private static final Matcher<Tree> CONSUMER =
+      isSubtypeOf(type(Consumer.class.getCanonicalName()));
+  private static final Supplier<Type> VOID = type(Void.class.getCanonicalName());
+
+  private static final Matcher<ExpressionTree> EMPTY_FLUX =
+      anyOf(
+          staticMethod().onDescendantOf(FLUX).named("empty"),
+          typePredicateMatcher(isSubTypeOf(generic(FLUX, VOID))));
+  private static final Matcher<ExpressionTree> EMPTY_MONO =
+      anyOf(
+          staticMethod().onDescendantOf(MONO).named("empty"),
+          typePredicateMatcher(isSubTypeOf(generic(MONO, VOID))));
+
+  private static final Matcher<ExpressionTree> VACUOUS_EMPTY_FLUX_OPERATORS =
+      instanceMethod()
+          .onDescendantOf(FLUX)
+          .namedAnyOf(
+              "concatMap",
+              "doOnNext",
+              "filter",
+              "flatMap",
+              "flatMapIterable",
+              "flatMapSequential",
+              "handle",
+              "map");
+  private static final Matcher<ExpressionTree> VACUOUS_EMPTY_MONO_OPERATORS =
+      instanceMethod()
+          .onDescendantOf(MONO)
+          .namedAnyOf(
+              "doOnNext", "filter", "flatMap", "flatMapMany", "flatMapIterable", "handle", "map");
+
+  private static final Matcher<ExpressionTree> SUBSCRIBE =
+      instanceMethod()
+          .onDescendantOf(Suppliers.typeFromString("org.reactivestreams.Publisher"))
+          .named("subscribe");
+
+  /** Instantiates a new {@link EmptyReactivePublisher} instance. */
+  public EmptyReactivePublisher() {}
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    ExpressionTree receiver = ASTHelpers.getReceiver(tree);
+    if (receiver == null
+        || (!EMPTY_FLUX.matches(receiver, state) && !EMPTY_MONO.matches(receiver, state))) {
+      return Description.NO_MATCH;
+    }
+
+    if (SUBSCRIBE.matches(tree, state)) {
+      // First argument passed to `#subscribe` is always an on next signal `Consumer`.
+      ExpressionTree firstArgument = Iterables.getFirst(tree.getArguments(), null);
+      if ((firstArgument instanceof LambdaExpressionTree
+              || firstArgument instanceof MemberReferenceTree)
+          && CONSUMER.matches(firstArgument, state)) {
+        return buildDescription(firstArgument)
+            .setMessage("Passing an on next signal `Consumer` on empty `Publisher`s is a no-op")
+            .build();
+      }
+    }
+
+    if (EMPTY_FLUX.matches(receiver, state) && VACUOUS_EMPTY_FLUX_OPERATORS.matches(tree, state)) {
+      return buildDescription(tree)
+          .setMessage(
+              String.format(
+                  "Operator `%s` on an empty `Flux`s is a no-op",
+                  ASTHelpers.getSymbol(tree).getSimpleName()))
+          .build();
+    }
+
+    if (EMPTY_MONO.matches(receiver, state) && VACUOUS_EMPTY_MONO_OPERATORS.matches(tree, state)) {
+      return buildDescription(tree)
+          .setMessage(
+              String.format(
+                  "Operator `%s` on an empty `Mono`s is a no-op",
+                  ASTHelpers.getSymbol(tree).getSimpleName()))
+          .build();
+    }
+    return Description.NO_MATCH;
+  }
+}

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

@@ -0,0 +1,91 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+final class EmptyReactivePublisherTest {
+  @Test
+  void identification() {
+    CompilationTestHelper.newInstance(EmptyReactivePublisher.class, getClass())
+        .addSourceLines(
+            "A.java",
+            "import com.google.common.collect.ImmutableSet;",
+            "import reactor.core.publisher.Flux;",
+            "import reactor.core.publisher.Mono;",
+            "import reactor.util.context.Context;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Mono.just(1).doOnNext(i -> {});",
+            "    Mono.just(2).filter(i -> true);",
+            "    Mono.just(3).flatMap(Mono::just);",
+            "    Mono.just(4).flatMapMany(Flux::just);",
+            "    Mono.just(5).flatMapIterable(ImmutableSet::of);",
+            "    Mono.just(6).handle((i, j) -> {});",
+            "    Mono.just(7).map(i -> i);",
+            "",
+            "    // BUG: Diagnostic contains: doOnNext",
+            "    Mono.empty().doOnNext(i -> {});",
+            "    // BUG: Diagnostic contains: filter",
+            "    Mono.empty().filter(i -> true);",
+            "    // BUG: Diagnostic contains: flatMap",
+            "    Mono.empty().flatMap(Mono::just);",
+            "    // BUG: Diagnostic contains: flatMapMany",
+            "    Mono.empty().flatMapMany(Flux::just);",
+            "    // BUG: Diagnostic contains: flatMapIterable",
+            "    Mono.empty().flatMapIterable(ImmutableSet::of);",
+            "    // BUG: Diagnostic contains: handle",
+            "    Mono.empty().handle((i, j) -> {});",
+            "    // BUG: Diagnostic contains: map",
+            "    Mono.empty().map(i -> i);",
+            "",
+            "    // BUG: Diagnostic contains: doOnNext",
+            "    Flux.empty().doOnNext(i -> {});",
+            "    // BUG: Diagnostic contains: filter",
+            "    Flux.empty().filter(i -> true);",
+            "    // BUG: Diagnostic contains: concatMap",
+            "    Flux.empty().concatMap(Mono::just);",
+            "    // BUG: Diagnostic contains: flatMap",
+            "    Flux.empty().flatMap(Mono::just);",
+            "    // BUG: Diagnostic contains: flatMapSequential",
+            "    Flux.empty().flatMapSequential(Flux::just);",
+            "    // BUG: Diagnostic contains: flatMapIterable",
+            "    Flux.empty().flatMapIterable(ImmutableSet::of);",
+            "    // BUG: Diagnostic contains: handle",
+            "    Flux.empty().handle((i, j) -> {});",
+            "    // BUG: Diagnostic contains: map",
+            "    Flux.empty().map(i -> i);",
+            "",
+            "    // BUG: Diagnostic contains: doOnNext",
+            "    Mono.just(8).then().doOnNext(i -> {});",
+            "    // BUG: Diagnostic contains: filter",
+            "    Mono.just(9).then().filter(i -> true);",
+            "    // BUG: Diagnostic contains: flatMap",
+            "    Mono.just(10).then().flatMap(Mono::just);",
+            "    // BUG: Diagnostic contains: flatMapMany",
+            "    Mono.just(11).then().flatMapMany(Flux::just);",
+            "    // BUG: Diagnostic contains: flatMapIterable",
+            "    Mono.just(12).then().flatMapIterable(ImmutableSet::of);",
+            "    // BUG: Diagnostic contains: handle",
+            "    Mono.just(13).then().handle((i, j) -> {});",
+            "    // BUG: Diagnostic contains: map",
+            "    Mono.just(14).then().map(i -> i);",
+            "",
+            "    Mono.just(15).subscribe();",
+            "    Mono.just(16).subscribe(null, t -> {});",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(17).then().subscribe(i -> {});",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(18).then().subscribe(i -> {}, t -> {});",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(19).then().subscribe(i -> {}, t -> {}, () -> {});",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(20).then().subscribe(i -> {}, t -> {}, () -> {}, j -> {});",
+            "    // BUG: Diagnostic contains:",
+            "    Mono.just(21).then().subscribe(i -> {}, t -> {}, () -> {}, Context.empty());",
+            "  }",
+            "}")
+        .doTest();
+  }
+}