From 881cc12e9a8fded6fa34da8de6b7e615eb5ccdb5 Mon Sep 17 00:00:00 2001 From: mohamedsamehsalah Date: Tue, 19 Nov 2024 17:28:15 +0100 Subject: [PATCH] Improve publisher before template matching --- .../refasterrules/ReactorRules.java | 20 +++++-- .../refasterrules/ReactorRulesTestInput.java | 60 ++++++++++++------- .../refasterrules/ReactorRulesTestOutput.java | 54 +++++++++++------ .../matchers/IsFunctionReturningMono.java | 37 ++++++++++++ .../matchers/IsFunctionReturningMonoTest.java | 44 ++++++++++++++ 5 files changed, 171 insertions(+), 44 deletions(-) create mode 100644 refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMono.java create mode 100644 refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMonoTest.java 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 index 2517e5984d..09af490ab9 100644 --- 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 after(Mono mono) { static final class MonoUsing< D extends AutoCloseable, T, P extends Publisher, M extends Mono> { @BeforeTemplate - Mono before(Callable resourceSupplier, Function sourceSupplier) { + Mono before( + Callable resourceSupplier, + @Matches(IsFunctionReturningMono.class) Function sourceSupplier) { return Flux.using(resourceSupplier, sourceSupplier).single(); } @@ -565,7 +568,10 @@ Mono after(Callable resourceSupplier, Function sourceSupplier) { static final class MonoUsingEager< D extends AutoCloseable, T, P extends Publisher, M extends Mono> { @BeforeTemplate - Mono before(Callable resourceSupplier, Function sourceSupplier, boolean eager) { + Mono before( + Callable resourceSupplier, + @Matches(IsFunctionReturningMono.class) Function sourceSupplier, + boolean eager) { return Flux.using(resourceSupplier, sourceSupplier, eager).single(); } @@ -583,7 +589,9 @@ static final class MonoUsing2< D, T, P extends Publisher, M extends Mono> { @BeforeTemplate Mono before( - Callable resourceSupplier, Function sourceSupplier, Consumer resourceCleanup) { + Callable resourceSupplier, + @Matches(IsFunctionReturningMono.class) Function sourceSupplier, + Consumer resourceCleanup) { return Flux.using(resourceSupplier, sourceSupplier, resourceCleanup).single(); } @@ -603,7 +611,7 @@ static final class MonoUsing2Eager< @BeforeTemplate Mono before( Callable resourceSupplier, - Function sourceSupplier, + @Matches(IsFunctionReturningMono.class) Function sourceSupplier, Consumer resourceCleanup, boolean eager) { return Flux.using(resourceSupplier, sourceSupplier, resourceCleanup, eager).single(); @@ -632,7 +640,7 @@ static final class MonoUsingWhen< @BeforeTemplate Mono before( Publisher resourceSupplier, - Function resourceClosure, + @Matches(IsFunctionReturningMono.class) Function resourceClosure, Function asyncCleanup) { return Flux.usingWhen(resourceSupplier, resourceClosure, asyncCleanup).single(); } @@ -659,7 +667,7 @@ static final class MonoUsingWhen2< @BeforeTemplate Mono before( Publisher resourceSupplier, - Function resourceClosure, + @Matches(IsFunctionReturningMono.class) Function resourceClosure, Function asyncComplete, BiFunction asyncError, Function asyncCancel) { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index e885bc3eef..dc3dcb42a9 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -204,37 +204,55 @@ Mono testMonoSingle() { return Mono.just(1).flux().single(); } - Mono testMonoUsing() { - return Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo")) - .single(); + ImmutableSet> 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 testMonoUsingEager() { - return Flux.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"), false) - .single(); + ImmutableSet> 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 testMonoUsing2() { - return Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}).single(); + ImmutableSet> testMonoUsing2() { + return ImmutableSet.of( + Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}).single(), + Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}).single()); } - Mono testMonoUsing2Eager() { - return Flux.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false).single(); + ImmutableSet> 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 testMonoUsingWhen() { - return Flux.usingWhen(Mono.just("foo"), foo -> Mono.just("bar"), foo -> Mono.just("baz")) - .single(); + ImmutableSet> 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 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> 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> testFluxSwitchIfEmptyOfEmptyPublisher() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index f18fd50872..b9b8d22caa 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -208,33 +208,53 @@ Mono testMonoSingle() { return Mono.just(1).single(); } - Mono testMonoUsing() { - return Mono.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo")); + ImmutableSet> 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 testMonoUsingEager() { - return Mono.using(() -> new ByteArrayInputStream(new byte[] {}), s -> Mono.just("foo"), false); + ImmutableSet> 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 testMonoUsing2() { - return Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}); + ImmutableSet> testMonoUsing2() { + return ImmutableSet.of( + Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}), + Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}).single()); } - Mono testMonoUsing2Eager() { - return Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false); + ImmutableSet> testMonoUsing2Eager() { + return ImmutableSet.of( + Mono.using(() -> "foo", foo -> Mono.just("bar"), foo -> {}, false), + Flux.using(() -> "foo", foo -> Flux.just("bar"), foo -> {}, false).single()); } - Mono testMonoUsingWhen() { - return Mono.usingWhen(Mono.just("foo"), foo -> Mono.just("bar"), foo -> Mono.just("baz")); + ImmutableSet> 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 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> 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> testFluxSwitchIfEmptyOfEmptyPublisher() { diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMono.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMono.java new file mode 100644 index 0000000000..41a8b566fc --- /dev/null +++ b/refaster-support/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 { + private static final long serialVersionUID = 1L; + private static final Matcher MONO_TYPE = + isSameType(Suppliers.typeFromString("reactor.core.publisher.Mono")); + private static final Matcher 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/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMonoTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsFunctionReturningMonoTest.java new file mode 100644 index 0000000000..7b78040ded --- /dev/null +++ b/refaster-support/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> positive = s -> Mono.just(s);", + "", + " Function> negative = s -> Flux.just(s);", + "", + " Supplier> 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()); + } + } +}