From 57d559eacb7d591f188ca7dcc9ad7415510f814a Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Sun, 2 Jan 2022 16:00:09 +0100 Subject: [PATCH] Also flag `Flux#flatMapSequential` --- .../bugpatterns/FluxFlatMapUsageCheck.java | 14 +++++++------- .../bugpatterns/FluxFlatMapUsageCheckTest.java | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheck.java index 0a79d40cc92..d24828ae660 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheck.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheck.java @@ -24,12 +24,12 @@ import reactor.core.publisher.Flux; /** - * A {@link BugChecker} which flags usages of {@link Flux#flatMap(Function)}. + * A {@link BugChecker} which flags usages of {@link Flux#flatMap(Function) and {@link Flux#flatMapSequential(Function)}. * - *

{@link Flux#flatMap(Function)} eagerly performs up to {@link - * reactor.util.concurrent.Queues#SMALL_BUFFER_SIZE} subscriptions, interleaving the results as they - * are emitted. In most cases {@link Flux#concatMap(Function)} should be preferred, as it produces - * consistent results and avoids potentially saturating the thread pool on which subscription + *

{@link Flux#flatMap(Function)} and {@link Flux#flatMapSequential(Function)} eagerly perform up + * to {@link reactor.util.concurrent.Queues#SMALL_BUFFER_SIZE} subscriptions, interleaving the results + * as they are emitted. In most cases {@link Flux#concatMap(Function)} should be preferred, as it + * produces consistent results and avoids potentially saturating the thread pool on which subscription * happens. If {@code concatMap}'s single-subscription semantics are undesirable one should invoke a * {@code flatMap} or {@code flatMapSequential} overload with an explicit concurrency level. * @@ -40,7 +40,7 @@ @BugPattern( name = "FluxFlatMapUsage", summary = - "`Flux#flatMap` has subtle semantics; please use `Flux#concatMap` or explicitly specify the desired amount of concurrency", + "`Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; please use `Flux#concatMap` or explicitly specify the desired amount of concurrency", linkType = LinkType.NONE, severity = SeverityLevel.ERROR, tags = StandardTags.LIKELY_ERROR) @@ -51,7 +51,7 @@ public final class FluxFlatMapUsageCheck extends BugChecker private static final Matcher FLUX_FLATMAP = instanceMethod() .onDescendantOf("reactor.core.publisher.Flux") - .named("flatMap") + .namedAnyOf("flatMap", "flatMapSequential") .withParameters(Function.class.getName()); @Override diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java index 227863b3d93..e6dff10e8aa 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java @@ -29,17 +29,31 @@ void identification() { " Flux.just(1).flatMap(Flux::just);", " // BUG: Diagnostic contains:", " Flux.just(1).flatMap(i -> Flux.just(String.valueOf(i)));", + " // BUG: Diagnostic contains:", + " Flux.just(1).flatMapSequential(Flux::just);", + " // BUG: Diagnostic contains:", + " Flux.just(1).flatMapSequential(i -> Flux.just(String.valueOf(i)));", "", + " Mono.just(1).flatMap(Mono::just);", " Flux.just(1).concatMap(Flux::just);", + "", " Flux.just(1).flatMap(Flux::just, 1);", " Flux.just(1).flatMap(Flux::just, 1, 1);", " Flux.just(1).flatMap(Flux::just, throwable -> Flux.empty(), Flux::empty);", "", + " Flux.just(1).flatMapSequential(Flux::just, 1);", + " Flux.just(1).flatMapSequential(Flux::just, 1, 1);", + "", " // BUG: Diagnostic contains:", " this.>sink(Flux::flatMap);", " // BUG: Diagnostic contains:", " this.>sink(Flux::flatMap);", "", + " // BUG: Diagnostic contains:", + " this.>sink(Flux::flatMapSequential);", + " // BUG: Diagnostic contains:", + " this.>sink(Flux::flatMapSequential);", + "", " this.>sink(Mono::flatMap);", " }", "", @@ -59,6 +73,7 @@ void replacementFirstSuggestedFix() { "class A {", " void m() {", " Flux.just(1).flatMap(Flux::just);", + " Flux.just(1).flatMapSequential(Flux::just);", " }", "}") .addOutputLines( @@ -68,6 +83,7 @@ void replacementFirstSuggestedFix() { "class A {", " void m() {", " Flux.just(1).concatMap(Flux::just);", + " Flux.just(1).concatMap(Flux::just);", " }", "}") .doTest(); @@ -86,6 +102,7 @@ void replacementSecondSuggestedFix() { "", " void m() {", " Flux.just(1).flatMap(Flux::just);", + " Flux.just(1).flatMapSequential(Flux::just);", " }", "}") .addOutputLines( @@ -97,6 +114,7 @@ void replacementSecondSuggestedFix() { "", " void m() {", " Flux.just(1).flatMap(Flux::just, MAX_CONCURRENCY);", + " Flux.just(1).flatMapSequential(Flux::just, MAX_CONCURRENCY);", " }", "}") .doTest();