From a27838d952e7fe86e64c864ae58545b9ad484b5e Mon Sep 17 00:00:00 2001 From: mohamedsamehsalah Date: Tue, 30 May 2023 21:37:58 +0200 Subject: [PATCH] Proposal --- ...fPublishers.java => NestedPublishers.java} | 38 +++++++++++++------ .../refasterrules/ReactorRules.java | 6 ++- ...ersTest.java => NestedPublishersTest.java} | 21 ++++++++-- 3 files changed, 48 insertions(+), 17 deletions(-) rename error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/{MonoOfPublishers.java => NestedPublishers.java} (55%) rename error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/{MonoOfPublishersTest.java => NestedPublishersTest.java} (66%) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MonoOfPublishers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NestedPublishers.java similarity index 55% rename from error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MonoOfPublishers.java rename to error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NestedPublishers.java index 768206cc5dd..d359f3e1d75 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MonoOfPublishers.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NestedPublishers.java @@ -30,29 +30,45 @@ @AutoService(BugChecker.class) @BugPattern( summary = - "Avoid nesting `Publisher`s inside `Mono`s; the resultant code is hard to reason about", - link = BUG_PATTERNS_BASE_URL + "MonoOfPublishers", + "Avoid nesting `Publisher`s inside `Publishers`s; the resultant code is hard to reason about", + link = BUG_PATTERNS_BASE_URL + "NestedPublishers", linkType = CUSTOM, severity = WARNING, tags = FRAGILE_CODE) -public final class MonoOfPublishers extends BugChecker implements MethodInvocationTreeMatcher { +public final class NestedPublishers extends BugChecker implements MethodInvocationTreeMatcher { private static final long serialVersionUID = 1L; - private static final Supplier MONO = - Suppliers.typeFromString("reactor.core.publisher.Mono"); - private static final Supplier MONO_OF_PUBLISHERS = + private static final Supplier PUBLISHER = type("org.reactivestreams.Publisher"); + private static final Supplier PUBLISHER_OF_PUBLISHERS = VisitorState.memoize( - generic(MONO, subOf(generic(type("org.reactivestreams.Publisher"), unbound())))); + generic(PUBLISHER, subOf(generic(type("org.reactivestreams.Publisher"), unbound())))); + private static final Supplier GROUPED_FLUX = + VisitorState.memoize( + generic( + Suppliers.typeFromString("reactor.core.publisher.GroupedFlux"), + unbound(), + unbound())); - /** Instantiates a new {@link MonoOfPublishers} instance. */ - public MonoOfPublishers() {} + /** Instantiates a new {@link NestedPublishers} instance. */ + public NestedPublishers() {} @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - Type type = MONO_OF_PUBLISHERS.get(state); - if (type == null || !state.getTypes().isSubtype(ASTHelpers.getType(tree), type)) { + Type publisherOfPublisherType = PUBLISHER_OF_PUBLISHERS.get(state); + Type groupedFluxType = GROUPED_FLUX.get(state); + Type treeType = ASTHelpers.getType(tree); + if ((publisherOfPublisherType == null + || !state.getTypes().isSubtype(treeType, publisherOfPublisherType)) + || (groupedFluxType != null + && isTypeArgumentGroupedFlux(state, groupedFluxType, treeType))) { return Description.NO_MATCH; } return describeMatch(tree); } + + private static boolean isTypeArgumentGroupedFlux( + VisitorState state, Type groupedFluxType, Type treeType) { + return treeType.getTypeArguments().stream() + .anyMatch(typez -> ASTHelpers.isSameType(typez, groupedFluxType, state)); + } } 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 ee5aa21dcf9..bc8d7bb46eb 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 @@ -435,6 +435,7 @@ Flux after(Flux flux) { /** Prefer {@link Flux#concatMap(Function)} over more contrived alternatives. */ static final class FluxConcatMap { @BeforeTemplate + @SuppressWarnings("NestedPublishers") Flux before(Flux flux, Function> function) { return Refaster.anyOf( flux.flatMap(function, 1), @@ -451,6 +452,7 @@ Flux after(Flux flux, Function /** Prefer {@link Flux#concatMap(Function, int)} over more contrived alternatives. */ static final class FluxConcatMapWithPrefetch { @BeforeTemplate + @SuppressWarnings("NestedPublishers") Flux before( Flux flux, Function> function, @@ -782,7 +784,7 @@ Flux after(Flux flux) { /** Prefer {@link Mono#flatMap(Function)} over more contrived alternatives. */ static final class MonoFlatMap { @BeforeTemplate - @SuppressWarnings("MonoOfPublishers") + @SuppressWarnings("NestedPublishers") Mono before(Mono mono, Function> function) { return mono.map(function).flatMap(identity()); } @@ -796,7 +798,7 @@ Mono after(Mono mono, Function> fun /** Prefer {@link Mono#flatMapMany(Function)} over more contrived alternatives. */ static final class MonoFlatMapMany { @BeforeTemplate - @SuppressWarnings("MonoOfPublishers") + @SuppressWarnings("NestedPublishers") Flux before(Mono mono, Function> function) { return mono.map(function).flatMapMany(identity()); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MonoOfPublishersTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NestedPublishersTest.java similarity index 66% rename from error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MonoOfPublishersTest.java rename to error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NestedPublishersTest.java index 1963c4aab60..1bedc403275 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MonoOfPublishersTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NestedPublishersTest.java @@ -3,10 +3,10 @@ import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; -final class MonoOfPublishersTest { +final class NestedPublishersTest { @Test void identification() { - CompilationTestHelper.newInstance(MonoOfPublishers.class, getClass()) + CompilationTestHelper.newInstance(NestedPublishers.class, getClass()) .addSourceLines( "A.java", "import reactor.core.publisher.Flux;", @@ -14,8 +14,6 @@ void identification() { "", "class A {", " void m() {", - " Mono.empty();", - " Mono.just(1);", " // BUG: Diagnostic contains:", " Mono.just(Mono.empty());", " // BUG: Diagnostic contains:", @@ -38,6 +36,21 @@ void identification() { " Mono.justOrEmpty(1).map(Mono::just);", " // BUG: Diagnostic contains:", " Mono.justOrEmpty(1).map(Flux::just);", + "", + " // BUG: Diagnostic contains:", + " Flux.just(Flux.empty());", + " // BUG: Diagnostic contains:", + " Flux.just(Flux.just(1));", + " // BUG: Diagnostic contains:", + " Flux.just(Mono.empty());", + " // BUG: Diagnostic contains:", + " Flux.just(Mono.just(1));", + " // BUG: Diagnostic contains:", + " Flux.just(1).map(Flux::just);", + " // BUG: Diagnostic contains:", + " Flux.just(1).map(Mono::just);", + "", + " Flux.just(1, 2).groupBy(i -> i);", " }", "}") .doTest();