From 8650148d5352afacfda561b3efb94c1896f1a151 Mon Sep 17 00:00:00 2001 From: Bastien Diederichs Date: Wed, 5 Oct 2022 16:13:42 +0200 Subject: [PATCH 1/4] ANS-1289 Prefer `flatMapIterable(identity())` over `flatMap(i -> Flux.fromIterable(i))` --- .../refastertemplates/ReactorTemplates.java | 18 ++++++++++++++++++ .../ReactorTemplatesTestInput.java | 4 ++++ .../ReactorTemplatesTestOutput.java | 5 +++++ 3 files changed, 27 insertions(+) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java index ef56d3a32b..c655e4426e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java @@ -2,6 +2,7 @@ import static com.google.common.collect.MoreCollectors.toOptional; import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; +import static java.util.function.Function.identity; import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.MoreCollectors; @@ -271,6 +272,23 @@ Flux after(Flux flux) { } } + /** + * Prefer {@link Flux#flatMapIterable(Function)} (Class)} over {@link Flux#flatMap(Function)} with + * {@link Flux#fromIterable(Iterable)}. + */ + static final class FluxFlatMapIterable { + @BeforeTemplate + Flux before(Flux> flux) { + return flux.flatMap(list -> Flux.fromIterable(list)); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + Flux after(Flux> flux) { + return flux.flatMapIterable(identity()); + } + } + /** Prefer {@link Mono#onErrorComplete()} over more contrived alternatives. */ static final class MonoOnErrorComplete { @BeforeTemplate diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java index 4844a8dcab..eb0e22b283 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java @@ -95,6 +95,10 @@ Flux testFluxCast() { return Flux.just(1).map(Number.class::cast); } + Flux testFluxFlatMapIterable() { + return Flux.just(ImmutableList.of("1")).flatMap(list -> Flux.fromIterable(list)); + } + Mono testMonoOnErrorComplete() { return Mono.just(1).onErrorResume(e -> Mono.empty()); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java index 34a39c6259..71a068d3d7 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java @@ -1,6 +1,7 @@ package tech.picnic.errorprone.refastertemplates; import static com.google.common.collect.MoreCollectors.toOptional; +import static java.util.function.Function.identity; import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableList; @@ -94,6 +95,10 @@ Flux testFluxCast() { return Flux.just(1).cast(Number.class); } + Flux testFluxFlatMapIterable() { + return Flux.just(ImmutableList.of("1")).flatMapIterable(identity()); + } + Mono testMonoOnErrorComplete() { return Mono.just(1).onErrorComplete(); } From 0c414ea4fffead333d9bfa42cb7d461a6ec154a0 Mon Sep 17 00:00:00 2001 From: Bastien Diederichs Date: Wed, 5 Oct 2022 17:59:17 +0200 Subject: [PATCH 2/4] ANS-1289 Extend to concatMap --- .../refastertemplates/ReactorTemplates.java | 28 ++++++++++++++++--- .../ReactorTemplatesTestInput.java | 12 ++++++-- .../ReactorTemplatesTestOutput.java | 12 ++++++-- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java index c655e4426e..db12fc7864 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java @@ -273,19 +273,39 @@ Flux after(Flux flux) { } /** - * Prefer {@link Flux#flatMapIterable(Function)} (Class)} over {@link Flux#flatMap(Function)} with + * Prefer {@link Flux#concatMapIterable(Function)} over {@link Flux#concatMap(Function)} with * {@link Flux#fromIterable(Iterable)}. */ - static final class FluxFlatMapIterable { + static final class FluxConcatMapFromIterable { @BeforeTemplate Flux before(Flux> flux) { - return flux.flatMap(list -> Flux.fromIterable(list)); + return Refaster.anyOf( + flux.concatMap(list -> Flux.fromIterable(list)), flux.concatMap(Flux::fromIterable)); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) Flux after(Flux> flux) { - return flux.flatMapIterable(identity()); + return flux.concatMapIterable(identity()); + } + } + + /** + * Prefer {@link Flux#flatMapIterable(Function, int)} over {@link Flux#flatMap(Function, int)} + * with {@link Flux#fromIterable(Iterable)}. + */ + static final class FluxFlatMapFromIterable { + @BeforeTemplate + Flux before(Flux> flux, int concurrency) { + return Refaster.anyOf( + flux.flatMap(list -> Flux.fromIterable(list), concurrency), + flux.flatMap(Flux::fromIterable, concurrency)); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + Flux after(Flux> flux, int concurrency) { + return flux.flatMapIterable(identity(), concurrency); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java index eb0e22b283..f8826dff95 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java @@ -95,8 +95,16 @@ Flux testFluxCast() { return Flux.just(1).map(Number.class::cast); } - Flux testFluxFlatMapIterable() { - return Flux.just(ImmutableList.of("1")).flatMap(list -> Flux.fromIterable(list)); + ImmutableSet> testFluxConcatMapFromIterable() { + return ImmutableSet.of( + Flux.just(ImmutableList.of("1")).concatMap(list -> Flux.fromIterable(list)), + Flux.just(ImmutableList.of("1")).concatMap(Flux::fromIterable)); + } + + ImmutableSet> testFluxFlatMapFromIterable() { + return ImmutableSet.of( + Flux.just(ImmutableList.of("1")).flatMap(list -> Flux.fromIterable(list), 1), + Flux.just(ImmutableList.of("1")).flatMap(Flux::fromIterable, 2)); } Mono testMonoOnErrorComplete() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java index 71a068d3d7..00852f5520 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java @@ -95,8 +95,16 @@ Flux testFluxCast() { return Flux.just(1).cast(Number.class); } - Flux testFluxFlatMapIterable() { - return Flux.just(ImmutableList.of("1")).flatMapIterable(identity()); + ImmutableSet> testFluxConcatMapFromIterable() { + return ImmutableSet.of( + Flux.just(ImmutableList.of("1")).concatMapIterable(identity()), + Flux.just(ImmutableList.of("1")).concatMapIterable(identity())); + } + + ImmutableSet> testFluxFlatMapFromIterable() { + return ImmutableSet.of( + Flux.just(ImmutableList.of("1")).flatMapIterable(identity(), 1), + Flux.just(ImmutableList.of("1")).flatMapIterable(identity(), 2)); } Mono testMonoOnErrorComplete() { From 651905bfba80d7cb94d59654e23ea92fde04503f Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 6 Oct 2022 07:42:16 +0200 Subject: [PATCH 3/4] Suggestions --- .../refastertemplates/ReactorTemplates.java | 60 +++++++++++++++---- .../ReactorTemplatesTestInput.java | 21 +++++-- .../ReactorTemplatesTestOutput.java | 21 +++++-- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java index db12fc7864..5555c6ee3f 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java @@ -178,6 +178,26 @@ Flux after(Flux flux, Function } } + /** Prefer {@link Flux#concatMap(Function, int)} over more contrived alternatives. */ + static final class FluxConcatMapWithPrefetch { + @BeforeTemplate + Flux before( + Flux flux, + Function> function, + int prefetch) { + return Refaster.anyOf( + flux.flatMap(function, 1, prefetch), flux.flatMapSequential(function, 1, prefetch)); + } + + @AfterTemplate + Flux after( + Flux flux, + Function> function, + int prefetch) { + return flux.concatMap(function, prefetch); + } + } + /** * Prefer {@link Flux#concatMapIterable(Function)} over {@link Flux#flatMapIterable(Function)}, as * the former has equivalent semantics but a clearer name. @@ -194,6 +214,24 @@ Flux after(Flux flux, Function> } } + /** + * Prefer {@link Flux#concatMapIterable(Function, int)} over {@link Flux#flatMapIterable(Function, + * int)}, as the former has equivalent semantics but a clearer name. + */ + static final class FluxConcatMapIterableWithPrefetch { + @BeforeTemplate + Flux before( + Flux flux, Function> function, int prefetch) { + return flux.flatMapIterable(function, prefetch); + } + + @AfterTemplate + Flux after( + Flux flux, Function> function, int prefetch) { + return flux.concatMapIterable(function, prefetch); + } + } + /** * Don't use {@link Mono#flatMapMany(Function)} to implicitly convert a {@link Mono} to a {@link * Flux}. @@ -273,10 +311,10 @@ Flux after(Flux flux) { } /** - * Prefer {@link Flux#concatMapIterable(Function)} over {@link Flux#concatMap(Function)} with - * {@link Flux#fromIterable(Iterable)}. + * Prefer {@link Flux#concatMapIterable(Function)} over alternatives that require an additional + * subscription. */ - static final class FluxConcatMapFromIterable { + static final class ConcatMapIterableIdentity { @BeforeTemplate Flux before(Flux> flux) { return Refaster.anyOf( @@ -291,21 +329,21 @@ Flux after(Flux> flux) { } /** - * Prefer {@link Flux#flatMapIterable(Function, int)} over {@link Flux#flatMap(Function, int)} - * with {@link Flux#fromIterable(Iterable)}. + * Prefer {@link Flux#concatMapIterable(Function, int)} over alternatives that require an + * additional subscription. */ - static final class FluxFlatMapFromIterable { + static final class ConcatMapIterableIdentityWithPrefetch { @BeforeTemplate - Flux before(Flux> flux, int concurrency) { + Flux before(Flux> flux, int prefetch) { return Refaster.anyOf( - flux.flatMap(list -> Flux.fromIterable(list), concurrency), - flux.flatMap(Flux::fromIterable, concurrency)); + flux.concatMap(list -> Flux.fromIterable(list), prefetch), + flux.concatMap(Flux::fromIterable, prefetch)); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - Flux after(Flux> flux, int concurrency) { - return flux.flatMapIterable(identity(), concurrency); + Flux after(Flux> flux, int prefetch) { + return flux.concatMapIterable(identity(), prefetch); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java index f8826dff95..aef41daaca 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestInput.java @@ -69,10 +69,19 @@ ImmutableSet> testFluxConcatMap() { Flux.just(1).flatMap(Mono::just, 1), Flux.just(2).flatMapSequential(Mono::just, 1)); } + ImmutableSet> testFluxConcatMapWithPrefetch() { + return ImmutableSet.of( + Flux.just(1).flatMap(Mono::just, 1, 3), Flux.just(2).flatMapSequential(Mono::just, 1, 4)); + } + Flux testFluxConcatMapIterable() { return Flux.just(1, 2).flatMapIterable(ImmutableList::of); } + Flux testFluxConcatMapIterableWithPrefetch() { + return Flux.just(1, 2).flatMapIterable(ImmutableList::of, 3); + } + Flux testMonoFlatMapToFlux() { return Mono.just("foo").flatMapMany(s -> Mono.just(s + s)); } @@ -95,16 +104,16 @@ Flux testFluxCast() { return Flux.just(1).map(Number.class::cast); } - ImmutableSet> testFluxConcatMapFromIterable() { + ImmutableSet> testConcatMapIterableIdentity() { return ImmutableSet.of( - Flux.just(ImmutableList.of("1")).concatMap(list -> Flux.fromIterable(list)), - Flux.just(ImmutableList.of("1")).concatMap(Flux::fromIterable)); + Flux.just(ImmutableList.of("foo")).concatMap(list -> Flux.fromIterable(list)), + Flux.just(ImmutableList.of("bar")).concatMap(Flux::fromIterable)); } - ImmutableSet> testFluxFlatMapFromIterable() { + ImmutableSet> testConcatMapIterableIdentityWithPrefetch() { return ImmutableSet.of( - Flux.just(ImmutableList.of("1")).flatMap(list -> Flux.fromIterable(list), 1), - Flux.just(ImmutableList.of("1")).flatMap(Flux::fromIterable, 2)); + Flux.just(ImmutableList.of("foo")).concatMap(list -> Flux.fromIterable(list), 1), + Flux.just(ImmutableList.of("bar")).concatMap(Flux::fromIterable, 2)); } Mono testMonoOnErrorComplete() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java index 00852f5520..a57a3c561a 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refastertemplates/ReactorTemplatesTestOutput.java @@ -69,10 +69,19 @@ ImmutableSet> testFluxConcatMap() { return ImmutableSet.of(Flux.just(1).concatMap(Mono::just), Flux.just(2).concatMap(Mono::just)); } + ImmutableSet> testFluxConcatMapWithPrefetch() { + return ImmutableSet.of( + Flux.just(1).concatMap(Mono::just, 3), Flux.just(2).concatMap(Mono::just, 4)); + } + Flux testFluxConcatMapIterable() { return Flux.just(1, 2).concatMapIterable(ImmutableList::of); } + Flux testFluxConcatMapIterableWithPrefetch() { + return Flux.just(1, 2).concatMapIterable(ImmutableList::of, 3); + } + Flux testMonoFlatMapToFlux() { return Mono.just("foo").flatMap(s -> Mono.just(s + s)).flux(); } @@ -95,16 +104,16 @@ Flux testFluxCast() { return Flux.just(1).cast(Number.class); } - ImmutableSet> testFluxConcatMapFromIterable() { + ImmutableSet> testConcatMapIterableIdentity() { return ImmutableSet.of( - Flux.just(ImmutableList.of("1")).concatMapIterable(identity()), - Flux.just(ImmutableList.of("1")).concatMapIterable(identity())); + Flux.just(ImmutableList.of("foo")).concatMapIterable(identity()), + Flux.just(ImmutableList.of("bar")).concatMapIterable(identity())); } - ImmutableSet> testFluxFlatMapFromIterable() { + ImmutableSet> testConcatMapIterableIdentityWithPrefetch() { return ImmutableSet.of( - Flux.just(ImmutableList.of("1")).flatMapIterable(identity(), 1), - Flux.just(ImmutableList.of("1")).flatMapIterable(identity(), 2)); + Flux.just(ImmutableList.of("foo")).concatMapIterable(identity(), 1), + Flux.just(ImmutableList.of("bar")).concatMapIterable(identity(), 2)); } Mono testMonoOnErrorComplete() { From 0f7df62fe4dbefa75b9cfb531bc562c2d3398251 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 7 Oct 2022 21:43:40 +0200 Subject: [PATCH 4/4] Change generic from `S` to `T` --- .../refastertemplates/ReactorTemplates.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java index 5555c6ee3f..68391d463e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java @@ -314,16 +314,16 @@ Flux after(Flux flux) { * Prefer {@link Flux#concatMapIterable(Function)} over alternatives that require an additional * subscription. */ - static final class ConcatMapIterableIdentity { + static final class ConcatMapIterableIdentity { @BeforeTemplate - Flux before(Flux> flux) { + Flux before(Flux> flux) { return Refaster.anyOf( flux.concatMap(list -> Flux.fromIterable(list)), flux.concatMap(Flux::fromIterable)); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - Flux after(Flux> flux) { + Flux after(Flux> flux) { return flux.concatMapIterable(identity()); } } @@ -332,9 +332,9 @@ Flux after(Flux> flux) { * Prefer {@link Flux#concatMapIterable(Function, int)} over alternatives that require an * additional subscription. */ - static final class ConcatMapIterableIdentityWithPrefetch { + static final class ConcatMapIterableIdentityWithPrefetch { @BeforeTemplate - Flux before(Flux> flux, int prefetch) { + Flux before(Flux> flux, int prefetch) { return Refaster.anyOf( flux.concatMap(list -> Flux.fromIterable(list), prefetch), flux.concatMap(Flux::fromIterable, prefetch)); @@ -342,7 +342,7 @@ Flux before(Flux> flux, int prefetch) { @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - Flux after(Flux> flux, int prefetch) { + Flux after(Flux> flux, int prefetch) { return flux.concatMapIterable(identity(), prefetch); } }