Introduce FluxImplicitBlock check

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

@@ -0,0 +1,100 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.BugPattern.StandardTags.CONCURRENCY;
+import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
+
+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.fixes.SuggestedFixes;
+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.MethodInvocationTree;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.util.Position;
+import java.util.stream.Stream;
+import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
+
+/**
+ * A {@link BugChecker} that flags {@link reactor.core.publisher.Flux} operator usages that may
+ * implicitly cause the calling thread to be blocked.
+ *
+ * <p>Note that the methods flagged here are not themselves blocking, but iterating over the
+ * resulting {@link Iterable} or {@link Stream} may be.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary = "Avoid iterating over `Flux`es in an implicitly blocking manner",
+    link = BUG_PATTERNS_BASE_URL + "FluxImplicitBlock",
+    linkType = CUSTOM,
+    severity = WARNING,
+    tags = {CONCURRENCY, PERFORMANCE})
+public final class FluxImplicitBlock extends BugChecker implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> FLUX_WITH_IMPLICIT_BLOCK =
+      instanceMethod()
+          .onDescendantOf("reactor.core.publisher.Flux")
+          .namedAnyOf("toIterable", "toStream")
+          .withNoParameters();
+  private static final Supplier<Type> STREAM = Suppliers.typeFromString(Stream.class.getName());
+
+  /** Instantiates a new {@link FluxImplicitBlock} instance. */
+  public FluxImplicitBlock() {}
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (!FLUX_WITH_IMPLICIT_BLOCK.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    Description.Builder description =
+        buildDescription(tree).addFix(SuggestedFixes.addSuppressWarnings(state, canonicalName()));
+    if (ThirdPartyLibrary.GUAVA.isIntroductionAllowed(state)) {
+      description.addFix(
+          suggestBlockingElementCollection(
+              tree, "com.google.common.collect.ImmutableList.toImmutableList", state));
+    }
+    description.addFix(
+        suggestBlockingElementCollection(tree, "java.util.stream.Collectors.toList", state));
+
+    return description.build();
+  }
+
+  private static SuggestedFix suggestBlockingElementCollection(
+      MethodInvocationTree tree, String fullyQualifiedCollectorMethod, VisitorState state) {
+    SuggestedFix.Builder importSuggestion = SuggestedFix.builder();
+    String replacementMethodInvocation =
+        SuggestedFixes.qualifyStaticImport(fullyQualifiedCollectorMethod, importSuggestion, state);
+
+    boolean isStream =
+        ASTHelpers.isSubtype(ASTHelpers.getResultType(tree), STREAM.get(state), state);
+    String replacement =
+        String.format(
+            ".collect(%s()).block()%s", replacementMethodInvocation, isStream ? ".stream()" : "");
+    return importSuggestion.merge(replaceMethodInvocation(tree, replacement, state)).build();
+  }
+
+  private static SuggestedFix.Builder replaceMethodInvocation(
+      MethodInvocationTree tree, String replacement, VisitorState state) {
+    int startPosition = state.getEndPosition(ASTHelpers.getReceiver(tree));
+    int endPosition = state.getEndPosition(tree);
+
+    checkState(
+        startPosition != Position.NOPOS && endPosition != Position.NOPOS,
+        "Cannot locate method to be replaced in source code");
+
+    return SuggestedFix.builder().replace(startPosition, endPosition, replacement);
+  }
+}

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

@@ -0,0 +1,182 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.common.base.Predicates.and;
+import static com.google.common.base.Predicates.containsPattern;
+import static com.google.common.base.Predicates.not;
+import static com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers.SECOND;
+import static com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers.THIRD;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+import org.reactivestreams.Publisher;
+import reactor.core.CorePublisher;
+import reactor.core.publisher.Flux;
+
+final class FluxImplicitBlockTest {
+  @Test
+  void identification() {
+    CompilationTestHelper.newInstance(FluxImplicitBlock.class, getClass())
+        .expectErrorMessage(
+            "X",
+            and(
+                containsPattern("SuppressWarnings"),
+                containsPattern("toImmutableList"),
+                containsPattern("toList")))
+        .addSourceLines(
+            "A.java",
+            "import com.google.common.collect.ImmutableList;",
+            "import java.util.stream.Stream;",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    // BUG: Diagnostic matches: X",
+            "    Flux.just(1).toIterable();",
+            "    // BUG: Diagnostic matches: X",
+            "    Flux.just(2).toStream();",
+            "    // BUG: Diagnostic matches: X",
+            "    long count = Flux.just(3).toStream().count();",
+            "",
+            "    Flux.just(4).toIterable(1);",
+            "    Flux.just(5).toIterable(2, null);",
+            "    Flux.just(6).toStream(3);",
+            "    new Foo().toIterable();",
+            "    new Foo().toStream();",
+            "  }",
+            "",
+            "  class Foo<T> {",
+            "    Iterable<T> toIterable() {",
+            "      return ImmutableList.of();",
+            "    }",
+            "",
+            "    Stream<T> toStream() {",
+            "      return Stream.empty();",
+            "    }",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void identificationWithoutGuavaOnClasspath() {
+    CompilationTestHelper.newInstance(FluxImplicitBlock.class, getClass())
+        .withClasspath(CorePublisher.class, Flux.class, Publisher.class)
+        .expectErrorMessage("X", not(containsPattern("toImmutableList")))
+        .addSourceLines(
+            "A.java",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    // BUG: Diagnostic matches: X",
+            "    Flux.just(1).toIterable();",
+            "    // BUG: Diagnostic matches: X",
+            "    Flux.just(2).toStream();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacementFirstSuggestedFix() {
+    BugCheckerRefactoringTestHelper.newInstance(FluxImplicitBlock.class, getClass())
+        .addInputLines(
+            "A.java",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).toIterable();",
+            "    Flux.just(2).toStream();",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  @SuppressWarnings(\"FluxImplicitBlock\")",
+            "  void m() {",
+            "    Flux.just(1).toIterable();",
+            "    Flux.just(2).toStream();",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+
+  @Test
+  void replacementSecondSuggestedFix() {
+    BugCheckerRefactoringTestHelper.newInstance(FluxImplicitBlock.class, getClass())
+        .setFixChooser(SECOND)
+        .addInputLines(
+            "A.java",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).toIterable();",
+            "    Flux.just(2).toStream();",
+            "    Flux.just(3).toIterable().iterator();",
+            "    Flux.just(4).toStream().count();",
+            "    Flux.just(5) /* a */./* b */ toIterable /* c */(/* d */ ) /* e */;",
+            "    Flux.just(6) /* a */./* b */ toStream /* c */(/* d */ ) /* e */;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import static com.google.common.collect.ImmutableList.toImmutableList;",
+            "",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).collect(toImmutableList()).block();",
+            "    Flux.just(2).collect(toImmutableList()).block().stream();",
+            "    Flux.just(3).collect(toImmutableList()).block().iterator();",
+            "    Flux.just(4).collect(toImmutableList()).block().stream().count();",
+            "    Flux.just(5).collect(toImmutableList()).block() /* e */;",
+            "    Flux.just(6).collect(toImmutableList()).block().stream() /* e */;",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+
+  @Test
+  void replacementThirdSuggestedFix() {
+    BugCheckerRefactoringTestHelper.newInstance(FluxImplicitBlock.class, getClass())
+        .setFixChooser(THIRD)
+        .addInputLines(
+            "A.java",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).toIterable();",
+            "    Flux.just(2).toStream();",
+            "    Flux.just(3).toIterable().iterator();",
+            "    Flux.just(4).toStream().count();",
+            "    Flux.just(5) /* a */./* b */ toIterable /* c */(/* d */ ) /* e */;",
+            "    Flux.just(6) /* a */./* b */ toStream /* c */(/* d */ ) /* e */;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import static java.util.stream.Collectors.toList;",
+            "",
+            "import reactor.core.publisher.Flux;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Flux.just(1).collect(toList()).block();",
+            "    Flux.just(2).collect(toList()).block().stream();",
+            "    Flux.just(3).collect(toList()).block().iterator();",
+            "    Flux.just(4).collect(toList()).block().stream().count();",
+            "    Flux.just(5).collect(toList()).block() /* e */;",
+            "    Flux.just(6).collect(toList()).block().stream() /* e */;",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+}