Improve publisher before template matching

PicnicSupermarket · Nov 19, 2024 · 881cc12 · 881cc12
1 parent 49c3848
commit 881cc12
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 44 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
@@ -55,6 +55,7 @@
 import tech.picnic.errorprone.refaster.annotation.Description;
 import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 import tech.picnic.errorprone.refaster.matchers.IsEmpty;
+import tech.picnic.errorprone.refaster.matchers.IsFunctionReturningMono;
 import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation;
 import tech.picnic.errorprone.refaster.matchers.ThrowsCheckedException;
 
@@ -548,7 +549,9 @@ Mono<T> after(Mono<T> mono) {
   static final class MonoUsing<
       D extends AutoCloseable, T, P extends Publisher<? extends T>, M extends Mono<? extends T>> {
     @BeforeTemplate
-    Mono<T> before(Callable<D> resourceSupplier, Function<D, P> sourceSupplier) {
+    Mono<T> before(
+        Callable<D> resourceSupplier,
+        @Matches(IsFunctionReturningMono.class) Function<D, P> sourceSupplier) {
       return Flux.using(resourceSupplier, sourceSupplier).single();
     }
 
@@ -565,7 +568,10 @@ Mono<T> after(Callable<D> resourceSupplier, Function<D, M> sourceSupplier) {
   static final class MonoUsingEager<
       D extends AutoCloseable, T, P extends Publisher<? extends T>, M extends Mono<? extends T>> {
     @BeforeTemplate
-    Mono<T> before(Callable<D> resourceSupplier, Function<D, P> sourceSupplier, boolean eager) {
+    Mono<T> before(
+        Callable<D> resourceSupplier,
+        @Matches(IsFunctionReturningMono.class) Function<D, P> sourceSupplier,
+        boolean eager) {
       return Flux.using(resourceSupplier, sourceSupplier, eager).single();
     }
 
@@ -583,7 +589,9 @@ static final class MonoUsing2<
       D, T, P extends Publisher<? extends T>, M extends Mono<? extends T>> {
     @BeforeTemplate
     Mono<T> before(
-        Callable<D> resourceSupplier, Function<D, P> sourceSupplier, Consumer<D> resourceCleanup) {
+        Callable<D> resourceSupplier,
+        @Matches(IsFunctionReturningMono.class) Function<D, P> sourceSupplier,
+        Consumer<D> resourceCleanup) {
       return Flux.using(resourceSupplier, sourceSupplier, resourceCleanup).single();
     }
 
@@ -603,7 +611,7 @@ static final class MonoUsing2Eager<
     @BeforeTemplate
     Mono<T> before(
         Callable<D> resourceSupplier,
-        Function<D, P> sourceSupplier,
+        @Matches(IsFunctionReturningMono.class) Function<D, P> sourceSupplier,
         Consumer<D> resourceCleanup,
         boolean eager) {
       return Flux.using(resourceSupplier, sourceSupplier, resourceCleanup, eager).single();
@@ -632,7 +640,7 @@ static final class MonoUsingWhen<
     @BeforeTemplate
     Mono<T> before(
         Publisher<D> resourceSupplier,
-        Function<D, P> resourceClosure,
+        @Matches(IsFunctionReturningMono.class) Function<D, P> resourceClosure,
         Function<D, P2> asyncCleanup) {
       return Flux.usingWhen(resourceSupplier, resourceClosure, asyncCleanup).single();
     }
@@ -659,7 +667,7 @@ static final class MonoUsingWhen2<
     @BeforeTemplate
     Mono<T> before(
         Publisher<D> resourceSupplier,
-        Function<D, P> resourceClosure,
+        @Matches(IsFunctionReturningMono.class) Function<D, P> resourceClosure,
         Function<D, P2> asyncComplete,
         BiFunction<D, ? super Throwable, P2> asyncError,
         Function<D, P2> asyncCancel) {

diff --git a/...ontrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/...ontrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java
@@ -204,37 +204,55 @@ Mono<Integer> testMonoSingle() {
     return Mono.just(1).flux().single();
   }
 
-  Mono<String> testMonoUsing() {
-    return Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"))
-        .single();
+  ImmutableSet<Mono<String>> testMonoUsing() {
+    return ImmutableSet.of(
+        Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo")).single(),
+        Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Flux.just("bar")).single());
   }
 
-  Mono<String> testMonoUsingEager() {
-    return Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"), false)
-        .single();
+  ImmutableSet<Mono<String>> testMonoUsingEager() {
+    return ImmutableSet.of(
+        Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"), false)
+            .single(),
+        Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Flux.just("bar"), false)
+            .single());
   }
 
-  Mono<String> testMonoUsing2() {
-    return Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}).single();
+  ImmutableSet<Mono<String>> testMonoUsing2() {
+    return ImmutableSet.of(
+        Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}).single(),
+        Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}).single());
   }
 
-  Mono<String> testMonoUsing2Eager() {
-    return Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false).single();
+  ImmutableSet<Mono<String>> testMonoUsing2Eager() {
+    return ImmutableSet.of(
+        Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false).single(),
+        Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}, false).single());
   }
 
-  Mono<String> testMonoUsingWhen() {
-    return Flux.usingWhen(Mono.just("foo"), foo -> Mono.just("bar"), foo -> Mono.just("baz"))
-        .single();
+  ImmutableSet<Mono<String>> testMonoUsingWhen() {
+    return ImmutableSet.of(
+        Flux.usingWhen(Mono.just("foo"), foo -> Mono.just("bar"), foo -> Mono.just("baz")).single(),
+        Flux.usingWhen(Mono.just("foo"), foo -> Flux.just("bar"), foo -> Mono.just("baz"))
+            .single());
   }
 
-  Mono<String> testMonoUsingWhen2() {
-    return Flux.usingWhen(
-            Mono.just("foo"),
-            foo -> Mono.just("bar"),
-            foo -> Mono.just("baz"),
-            (foo, e) -> Mono.just("qux"),
-            foo -> Mono.just("thud"))
-        .single();
+  ImmutableSet<Mono<String>> testMonoUsingWhen2() {
+    return ImmutableSet.of(
+        Flux.usingWhen(
+                Mono.just("foo"),
+                foo -> Mono.just("bar"),
+                foo -> Mono.just("baz"),
+                (foo, e) -> Mono.just("qux"),
+                foo -> Mono.just("thud"))
+            .single(),
+        Flux.usingWhen(
+                Mono.just("foo"),
+                foo -> Flux.just("bar"),
+                foo -> Mono.just("baz"),
+                (foo, e) -> Mono.just("qux"),
+                foo -> Mono.just("thud"))
+            .single());
   }
 
   ImmutableSet<Flux<Integer>> testFluxSwitchIfEmptyOfEmptyPublisher() {

diff --git a/...ntrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/...ntrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java
@@ -208,33 +208,53 @@ Mono<Integer> testMonoSingle() {
     return Mono.just(1).single();
   }
 
-  Mono<String> testMonoUsing() {
-    return Mono.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"));
+  ImmutableSet<Mono<String>> testMonoUsing() {
+    return ImmutableSet.of(
+        Mono.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo")),
+        Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Flux.just("bar")).single());
   }
 
-  Mono<String> testMonoUsingEager() {
-    return Mono.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"), false);
+  ImmutableSet<Mono<String>> testMonoUsingEager() {
+    return ImmutableSet.of(
+        Mono.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"), false),
+        Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Flux.just("bar"), false)
+            .single());
   }
 
-  Mono<String> testMonoUsing2() {
-    return Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {});
+  ImmutableSet<Mono<String>> testMonoUsing2() {
+    return ImmutableSet.of(
+        Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}),
+        Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}).single());
   }
 
-  Mono<String> testMonoUsing2Eager() {
-    return Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false);
+  ImmutableSet<Mono<String>> testMonoUsing2Eager() {
+    return ImmutableSet.of(
+        Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false),
+        Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}, false).single());
   }
 
-  Mono<String> testMonoUsingWhen() {
-    return Mono.usingWhen(Mono.just("foo"), foo -> Mono.just("bar"), foo -> Mono.just("baz"));
+  ImmutableSet<Mono<String>> testMonoUsingWhen() {
+    return ImmutableSet.of(
+        Mono.usingWhen(Mono.just("foo"), foo -> Mono.just("bar"), foo -> Mono.just("baz")),
+        Flux.usingWhen(Mono.just("foo"), foo -> Flux.just("bar"), foo -> Mono.just("baz"))
+            .single());
   }
 
-  Mono<String> testMonoUsingWhen2() {
-    return Mono.usingWhen(
-        Mono.just("foo"),
-        foo -> Mono.just("bar"),
-        foo -> Mono.just("baz"),
-        (foo, e) -> Mono.just("qux"),
-        foo -> Mono.just("thud"));
+  ImmutableSet<Mono<String>> testMonoUsingWhen2() {
+    return ImmutableSet.of(
+        Mono.usingWhen(
+            Mono.just("foo"),
+            foo -> Mono.just("bar"),
+            foo -> Mono.just("baz"),
+            (foo, e) -> Mono.just("qux"),
+            foo -> Mono.just("thud")),
+        Flux.usingWhen(
+                Mono.just("foo"),
+                foo -> Flux.just("bar"),
+                foo -> Mono.just("baz"),
+                (foo, e) -> Mono.just("qux"),
+                foo -> Mono.just("thud"))
+            .single());
   }
 
   ImmutableSet<Flux<Integer>> testFluxSwitchIfEmptyOfEmptyPublisher() {

diff --git a/...pport/src/main/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMono.java b/...pport/src/main/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMono.java
@@ -0,0 +1,37 @@
+package tech.picnic.errorprone.refaster.matchers;
+
+import static com.google.errorprone.matchers.Matchers.isSameType;
+
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.suppliers.Suppliers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.LambdaExpressionTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
+
+/**
+ * A matcher of lambda expressions that are of type {@code java.util.function.Function} that returns
+ * a {@code reactor.core.publisher.Mono}.
+ */
+public final class IsFunctionReturningMono implements Matcher<ExpressionTree> {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<Tree> MONO_TYPE =
+      isSameType(Suppliers.typeFromString("reactor.core.publisher.Mono"));
+  private static final Matcher<Tree> FUNCTION_TYPE =
+      isSameType(Suppliers.typeFromString("java.util.function.Function"));
+
+  /** Instantiates a new {@link IsFunctionReturningMono} instance. */
+  public IsFunctionReturningMono() {}
+
+  @Override
+  public boolean matches(ExpressionTree tree, VisitorState state) {
+    if (tree.getKind() != Kind.LAMBDA_EXPRESSION) {
+      return false;
+    }
+
+    LambdaExpressionTree lambdaExpressionTree = (LambdaExpressionTree) tree;
+    return MONO_TYPE.matches(lambdaExpressionTree.getBody(), state)
+        && FUNCTION_TYPE.matches(lambdaExpressionTree, state);
+  }
+}
diff --git a/...t/src/test/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMonoTest.java b/...t/src/test/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMonoTest.java
@@ -0,0 +1,44 @@
+package tech.picnic.errorprone.refaster.matchers;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.CompilationTestHelper;
+import com.google.errorprone.bugpatterns.BugChecker;
+import org.junit.jupiter.api.Test;
+
+final class IsFunctionReturningMonoTest {
+  @Test
+  void matches() {
+    CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
+        .addSourceLines(
+            "A.java",
+            "import java.util.function.Function;",
+            "import java.util.function.Supplier;",
+            "import reactor.core.publisher.Flux;",
+            "import reactor.core.publisher.Mono;",
+            "",
+            "class A {",
+            "// BUG: Diagnostic contains:",
+            " Function<String, Mono<String>> positive = s -> Mono.just(s);",
+            "",
+            " Function<String, Flux<String>> negative = s -> Flux.just(s);",
+            "",
+            " Supplier<Mono<String>> negative2 = () -> Mono.just(\"s\");",
+            "}")
+        .doTest();
+  }
+
+  /** A {@link BugChecker} that simply delegates to {@link IsFunctionReturningMono}. */
+  @BugPattern(summary = "Flags expressions matched by `IsFunctionReturningMono`", severity = ERROR)
+  public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
+    private static final long serialVersionUID = 1L;
+
+    // XXX: This is a false positive reported by Checkstyle. See
+    // https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120.
+    @SuppressWarnings("RedundantModifier")
+    public MatcherTestChecker() {
+      super(new IsFunctionReturningMono());
+    }
+  }
+}