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 c92668319f..ac9808f2c5 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 @@ -247,11 +247,43 @@ Flux after(Flux flux, Publisher other, BiFunction combinator) } } + /** Prefer {@link Flux#zipWithIterable(Iterable)} over more contrived alternatives. */ + static final class FluxZipWithIterable { + @BeforeTemplate + Flux> before(Flux flux, Iterable iterable) { + return Flux.zip(flux, Flux.fromIterable(iterable)); + } + + @AfterTemplate + Flux> after(Flux flux, Iterable iterable) { + return flux.zipWithIterable(iterable); + } + } + + /** Prefer {@link Flux#zipWithIterable(Iterable, BiFunction)} over more contrived alternatives. */ + static final class FluxZipWithIterableBiFunction { + @BeforeTemplate + Flux before( + Flux flux, + Iterable iterable, + BiFunction function) { + return flux.zipWith(Flux.fromIterable(iterable), function); + } + + @AfterTemplate + Flux after( + Flux flux, + Iterable iterable, + BiFunction function) { + return flux.zipWithIterable(iterable, function); + } + } + /** * Prefer {@link Flux#zipWithIterable(Iterable)} with a chained combinator over {@link * Flux#zipWithIterable(Iterable, BiFunction)}, as the former generally yields more readable code. */ - static final class FluxZipWithIterable { + static final class FluxZipWithIterableMapFunction { @BeforeTemplate Flux before(Flux flux, Iterable iterable, BiFunction combinator) { return flux.zipWithIterable(iterable, combinator); @@ -328,7 +360,10 @@ Flux after(Supplier supplier) { static final class MonoThenReturn { @BeforeTemplate Mono before(Mono mono, S object) { - return mono.then(Mono.just(object)); + return Refaster.anyOf( + mono.ignoreElement().thenReturn(object), + mono.then().thenReturn(object), + mono.then(Mono.just(object))); } @AfterTemplate @@ -401,7 +436,7 @@ Mono before(Mono mono) { @BeforeTemplate Mono<@Nullable Void> before2(Mono<@Nullable Void> mono) { - return mono.then(); + return Refaster.anyOf(mono.ignoreElement(), mono.then()); } // XXX: Replace this rule with an extension of the `IdentityConversion` rule, supporting @@ -485,14 +520,46 @@ Flux after( } } + /** Avoid contrived alternatives to {@link Mono#flatMapIterable(Function)}. */ + static final class MonoFlatMapIterable { + @BeforeTemplate + Flux before(Mono mono, Function> function) { + return Refaster.anyOf( + mono.map(function).flatMapIterable(identity()), mono.flux().concatMapIterable(function)); + } + + @AfterTemplate + Flux after(Mono mono, Function> function) { + return mono.flatMapIterable(function); + } + } + + /** + * Prefer {@link Mono#flatMapIterable(Function)} to flatten a {@link Mono} of some {@link + * Iterable} over less efficient alternatives. + */ + static final class MonoFlatMapIterableIdentity> { + @BeforeTemplate + Flux before(Mono mono) { + return mono.flatMapMany(Flux::fromIterable); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + Flux after(Mono mono) { + return mono.flatMapIterable(identity()); + } + } + /** - * Prefer {@link Flux#concatMapIterable(Function)} over {@link Flux#flatMapIterable(Function)}, as - * the former has equivalent semantics but a clearer name. + * Prefer {@link Flux#concatMapIterable(Function)} over alternatives with less clear syntax or + * semantics. */ static final class FluxConcatMapIterable { @BeforeTemplate Flux before(Flux flux, Function> function) { - return flux.flatMapIterable(function); + return Refaster.anyOf( + flux.flatMapIterable(function), flux.map(function).concatMapIterable(identity())); } @AfterTemplate @@ -502,14 +569,16 @@ 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. + * Prefer {@link Flux#concatMapIterable(Function, int)} over alternatives with less clear syntax + * or semantics. */ static final class FluxConcatMapIterableWithPrefetch { @BeforeTemplate Flux before( Flux flux, Function> function, int prefetch) { - return flux.flatMapIterable(function, prefetch); + return Refaster.anyOf( + flux.flatMapIterable(function, prefetch), + flux.map(function).concatMapIterable(identity(), prefetch)); } @AfterTemplate @@ -741,7 +810,7 @@ Flux after(Mono mono) { static final class MonoThen { @BeforeTemplate Mono<@Nullable Void> before(Mono mono) { - return mono.flux().then(); + return Refaster.anyOf(mono.ignoreElement().then(), mono.flux().then()); } @AfterTemplate @@ -750,6 +819,128 @@ static final class MonoThen { } } + /** Avoid vacuous invocations of {@link Flux#ignoreElements()}. */ + static final class FluxThen { + @BeforeTemplate + Mono<@Nullable Void> before(Flux flux) { + return flux.ignoreElements().then(); + } + + @BeforeTemplate + Mono<@Nullable Void> before2(Flux<@Nullable Void> flux) { + return flux.ignoreElements(); + } + + @AfterTemplate + Mono<@Nullable Void> after(Flux flux) { + return flux.then(); + } + } + + /** Avoid vacuous invocations of {@link Mono#ignoreElement()}. */ + static final class MonoThenEmpty { + @BeforeTemplate + Mono<@Nullable Void> before(Mono mono, Publisher<@Nullable Void> publisher) { + return mono.ignoreElement().thenEmpty(publisher); + } + + @AfterTemplate + Mono<@Nullable Void> after(Mono mono, Publisher<@Nullable Void> publisher) { + return mono.thenEmpty(publisher); + } + } + + /** Avoid vacuous invocations of {@link Flux#ignoreElements()}. */ + static final class FluxThenEmpty { + @BeforeTemplate + Mono<@Nullable Void> before(Flux flux, Publisher<@Nullable Void> publisher) { + return flux.ignoreElements().thenEmpty(publisher); + } + + @AfterTemplate + Mono<@Nullable Void> after(Flux flux, Publisher<@Nullable Void> publisher) { + return flux.thenEmpty(publisher); + } + } + + /** Avoid vacuous invocations of {@link Mono#ignoreElement()}. */ + static final class MonoThenMany { + @BeforeTemplate + Flux before(Mono mono, Publisher publisher) { + return mono.ignoreElement().thenMany(publisher); + } + + @AfterTemplate + Flux after(Mono mono, Publisher publisher) { + return mono.thenMany(publisher); + } + } + + /** + * Prefer explicit invocation of {@link Mono#flux()} over implicit conversions from {@link Mono} + * to {@link Flux}. + */ + static final class MonoThenMonoFlux { + @BeforeTemplate + Flux before(Mono mono1, Mono mono2) { + return mono1.thenMany(mono2); + } + + @AfterTemplate + Flux after(Mono mono1, Mono mono2) { + return mono1.then(mono2).flux(); + } + } + + /** Avoid vacuous invocations of {@link Flux#ignoreElements()}. */ + static final class FluxThenMany { + @BeforeTemplate + Flux before(Flux flux, Publisher publisher) { + return flux.ignoreElements().thenMany(publisher); + } + + @AfterTemplate + Flux after(Flux flux, Publisher publisher) { + return flux.thenMany(publisher); + } + } + + /** Avoid vacuous invocations of {@link Mono#ignoreElement()}. */ + static final class MonoThenMono { + @BeforeTemplate + Mono before(Mono mono1, Mono mono2) { + return mono1.ignoreElement().then(mono2); + } + + @BeforeTemplate + Mono<@Nullable Void> before2(Mono mono1, Mono<@Nullable Void> mono2) { + return mono1.thenEmpty(mono2); + } + + @AfterTemplate + Mono after(Mono mono1, Mono mono2) { + return mono1.then(mono2); + } + } + + /** Avoid vacuous invocations of {@link Flux#ignoreElements()}. */ + static final class FluxThenMono { + @BeforeTemplate + Mono before(Flux flux, Mono mono) { + return flux.ignoreElements().then(mono); + } + + @BeforeTemplate + Mono<@Nullable Void> before2(Flux flux, Mono<@Nullable Void> mono) { + return flux.thenEmpty(mono); + } + + @AfterTemplate + Mono after(Flux flux, Mono mono) { + return flux.then(mono); + } + } + /** Prefer {@link Mono#singleOptional()} over more contrived alternatives. */ // XXX: Consider creating a plugin that flags/discourages `Mono>` method return // types, just as we discourage nullable `Boolean`s and `Optional`s. @@ -812,8 +1003,30 @@ Mono after(Mono mono, Function> fun static final class MonoFlatMapMany { @BeforeTemplate @SuppressWarnings("NestedPublishers") - Flux before(Mono mono, Function> function) { - return mono.map(function).flatMapMany(identity()); + Flux before( + Mono mono, + Function> function, + boolean delayUntilEnd, + int maxConcurrency, + int prefetch) { + return Refaster.anyOf( + mono.map(function).flatMapMany(identity()), + mono.flux().concatMap(function), + mono.flux().concatMap(function, prefetch), + mono.flux().concatMapDelayError(function), + mono.flux().concatMapDelayError(function, prefetch), + mono.flux().concatMapDelayError(function, delayUntilEnd, prefetch), + mono.flux().flatMap(function, maxConcurrency), + mono.flux().flatMap(function, maxConcurrency, prefetch), + mono.flux().flatMapDelayError(function, maxConcurrency, prefetch), + mono.flux().flatMapSequential(function, maxConcurrency), + mono.flux().flatMapSequential(function, maxConcurrency, prefetch), + mono.flux().flatMapSequentialDelayError(function, maxConcurrency, prefetch)); + } + + @BeforeTemplate + Flux before(Mono mono, Function> function) { + return mono.flux().switchMap(function); } @AfterTemplate 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 7f8021f70c..002df87ca7 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 @@ -98,7 +98,16 @@ Flux testFluxZipWithCombinator() { return Flux.just("foo", "bar").zipWith(Flux.just(1, 2), String::repeat); } - Flux testFluxZipWithIterable() { + Flux> testFluxZipWithIterable() { + return Flux.zip(Flux.just("foo", "bar"), Flux.fromIterable(ImmutableSet.of(1, 2))); + } + + Flux testFluxZipWithIterableBiFunction() { + return Flux.just("foo", "bar") + .zipWith(Flux.fromIterable(ImmutableSet.of(1, 2)), String::repeat); + } + + Flux testFluxZipWithIterableMapFunction() { return Flux.just("foo", "bar").zipWithIterable(ImmutableSet.of(1, 2), String::repeat); } @@ -118,8 +127,11 @@ Flux testFluxErrorSupplier() { return Flux.error(() -> ((Supplier) null).get()); } - Mono testMonoThenReturn() { - return Mono.empty().then(Mono.just("foo")); + ImmutableSet> testMonoThenReturn() { + return ImmutableSet.of( + Mono.just(1).ignoreElement().thenReturn("foo"), + Mono.just(2).then().thenReturn("bar"), + Mono.just(3).then(Mono.just("baz"))); } Flux testFluxTake() { @@ -141,6 +153,7 @@ ImmutableSet> testMonoIdentity() { Mono.just(1).switchIfEmpty(Mono.empty()), Mono.just(2).flux().next(), Mono.just(3).flux().singleOrEmpty(), + Mono.empty().ignoreElement(), Mono.empty().then(), Mono.>empty().map(ImmutableList::copyOf)); } @@ -168,12 +181,26 @@ ImmutableSet> testFluxConcatMapWithPrefetch() { Flux.just(3).map(Mono::just).concatMap(identity(), 5)); } - Flux testFluxConcatMapIterable() { - return Flux.just(1, 2).flatMapIterable(ImmutableList::of); + ImmutableSet> testMonoFlatMapIterable() { + return ImmutableSet.of( + Mono.just(1).map(ImmutableSet::of).flatMapIterable(identity()), + Mono.just(2).flux().concatMapIterable(ImmutableSet::of)); + } + + Flux testMonoFlatMapIterableIdentity() { + return Mono.just(ImmutableSet.of(1)).flatMapMany(Flux::fromIterable); } - Flux testFluxConcatMapIterableWithPrefetch() { - return Flux.just(1, 2).flatMapIterable(ImmutableList::of, 3); + ImmutableSet> testFluxConcatMapIterable() { + return ImmutableSet.of( + Flux.just(1).flatMapIterable(ImmutableList::of), + Flux.just(2).map(ImmutableList::of).concatMapIterable(identity())); + } + + ImmutableSet> testFluxConcatMapIterableWithPrefetch() { + return ImmutableSet.of( + Flux.just(1).flatMapIterable(ImmutableList::of, 3), + Flux.just(2).map(ImmutableList::of).concatMapIterable(identity(), 3)); } Flux testMonoFlatMapToFlux() { @@ -251,8 +278,45 @@ ImmutableSet> testMonoFlux() { Flux.concat(Mono.just("baz"))); } - Mono testMonoThen() { - return Mono.just("foo").flux().then(); + ImmutableSet> testMonoThen() { + return ImmutableSet.of(Mono.just("foo").ignoreElement().then(), Mono.just("bar").flux().then()); + } + + ImmutableSet> testFluxThen() { + return ImmutableSet.of( + Flux.just("foo").ignoreElements().then(), Flux.empty().ignoreElements()); + } + + Mono testMonoThenEmpty() { + return Mono.just("foo").ignoreElement().thenEmpty(Mono.empty()); + } + + Mono testFluxThenEmpty() { + return Flux.just("foo").ignoreElements().thenEmpty(Mono.empty()); + } + + Flux testMonoThenMany() { + return Mono.just("foo").ignoreElement().thenMany(Flux.just("bar")); + } + + Flux testMonoThenMonoFlux() { + return Mono.just("foo").thenMany(Mono.just("bar")); + } + + Flux testFluxThenMany() { + return Flux.just("foo").ignoreElements().thenMany(Flux.just("bar")); + } + + ImmutableSet> testMonoThenMono() { + return ImmutableSet.of( + Mono.just("foo").ignoreElement().then(Mono.just("bar")), + Mono.just("baz").thenEmpty(Mono.empty())); + } + + ImmutableSet> testFluxThenMono() { + return ImmutableSet.of( + Flux.just("foo").ignoreElements().then(Mono.just("bar")), + Flux.just("baz").thenEmpty(Mono.empty())); } ImmutableSet>> testMonoSingleOptional() { @@ -273,8 +337,21 @@ Mono testMonoFlatMap() { return Mono.just("foo").map(Mono::just).flatMap(identity()); } - Flux testMonoFlatMapMany() { - return Mono.just("foo").map(Mono::just).flatMapMany(identity()); + ImmutableSet> testMonoFlatMapMany() { + return ImmutableSet.of( + Mono.just(1).map(Mono::just).flatMapMany(identity()), + Mono.just(2).flux().concatMap(Mono::just), + Mono.just(3).flux().concatMap(Mono::just, 2), + Mono.just(4).flux().concatMapDelayError(Mono::just), + Mono.just(5).flux().concatMapDelayError(Mono::just, 2), + Mono.just(6).flux().concatMapDelayError(Mono::just, false, 2), + Mono.just(7).flux().flatMap(Mono::just, 2), + Mono.just(8).flux().flatMap(Mono::just, 2, 3), + Mono.just(9).flux().flatMapDelayError(Mono::just, 2, 3), + Mono.just(10).flux().flatMapSequential(Mono::just, 2), + Mono.just(11).flux().flatMapSequential(Mono::just, 2, 3), + Mono.just(12).flux().flatMapSequentialDelayError(Mono::just, 2, 3), + Mono.just(13).flux().switchMap(Mono::just)); } ImmutableSet> testConcatMapIterableIdentity() { 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 b12af508c4..9b24aa3b24 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 @@ -102,7 +102,15 @@ Flux testFluxZipWithCombinator() { .map(TupleUtils.function(String::repeat)); } - Flux testFluxZipWithIterable() { + Flux> testFluxZipWithIterable() { + return Flux.just("foo", "bar").zipWithIterable(ImmutableSet.of(1, 2)); + } + + Flux testFluxZipWithIterableBiFunction() { + return Flux.just("foo", "bar").zipWithIterable(ImmutableSet.of(1, 2), String::repeat); + } + + Flux testFluxZipWithIterableMapFunction() { return Flux.just("foo", "bar") .zipWithIterable(ImmutableSet.of(1, 2)) .map(function(String::repeat)); @@ -124,8 +132,11 @@ Flux testFluxErrorSupplier() { return Flux.error(((Supplier) null)); } - Mono testMonoThenReturn() { - return Mono.empty().thenReturn("foo"); + ImmutableSet> testMonoThenReturn() { + return ImmutableSet.of( + Mono.just(1).thenReturn("foo"), + Mono.just(2).thenReturn("bar"), + Mono.just(3).thenReturn("baz")); } Flux testFluxTake() { @@ -147,6 +158,7 @@ ImmutableSet> testMonoIdentity() { Mono.just(2), Mono.just(3), Mono.empty(), + Mono.empty(), Mono.>empty()); } @@ -172,12 +184,26 @@ ImmutableSet> testFluxConcatMapWithPrefetch() { Flux.just(3).concatMap(Mono::just, 5)); } - Flux testFluxConcatMapIterable() { - return Flux.just(1, 2).concatMapIterable(ImmutableList::of); + ImmutableSet> testMonoFlatMapIterable() { + return ImmutableSet.of( + Mono.just(1).flatMapIterable(ImmutableSet::of), + Mono.just(2).flatMapIterable(ImmutableSet::of)); + } + + Flux testMonoFlatMapIterableIdentity() { + return Mono.just(ImmutableSet.of(1)).flatMapIterable(identity()); + } + + ImmutableSet> testFluxConcatMapIterable() { + return ImmutableSet.of( + Flux.just(1).concatMapIterable(ImmutableList::of), + Flux.just(2).concatMapIterable(ImmutableList::of)); } - Flux testFluxConcatMapIterableWithPrefetch() { - return Flux.just(1, 2).concatMapIterable(ImmutableList::of, 3); + ImmutableSet> testFluxConcatMapIterableWithPrefetch() { + return ImmutableSet.of( + Flux.just(1).concatMapIterable(ImmutableList::of, 3), + Flux.just(2).concatMapIterable(ImmutableList::of, 3)); } Flux testMonoFlatMapToFlux() { @@ -250,8 +276,42 @@ ImmutableSet> testMonoFlux() { Mono.just("foo").flux(), Mono.just("bar").flux(), Mono.just("baz").flux()); } - Mono testMonoThen() { - return Mono.just("foo").then(); + ImmutableSet> testMonoThen() { + return ImmutableSet.of(Mono.just("foo").then(), Mono.just("bar").then()); + } + + ImmutableSet> testFluxThen() { + return ImmutableSet.of(Flux.just("foo").then(), Flux.empty().then()); + } + + Mono testMonoThenEmpty() { + return Mono.just("foo").thenEmpty(Mono.empty()); + } + + Mono testFluxThenEmpty() { + return Flux.just("foo").thenEmpty(Mono.empty()); + } + + Flux testMonoThenMany() { + return Mono.just("foo").thenMany(Flux.just("bar")); + } + + Flux testMonoThenMonoFlux() { + return Mono.just("foo").then(Mono.just("bar")).flux(); + } + + Flux testFluxThenMany() { + return Flux.just("foo").thenMany(Flux.just("bar")); + } + + ImmutableSet> testMonoThenMono() { + return ImmutableSet.of( + Mono.just("foo").then(Mono.just("bar")), Mono.just("baz").then(Mono.empty())); + } + + ImmutableSet> testFluxThenMono() { + return ImmutableSet.of( + Flux.just("foo").then(Mono.just("bar")), Flux.just("baz").then(Mono.empty())); } ImmutableSet>> testMonoSingleOptional() { @@ -270,8 +330,21 @@ Mono testMonoFlatMap() { return Mono.just("foo").flatMap(Mono::just); } - Flux testMonoFlatMapMany() { - return Mono.just("foo").flatMapMany(Mono::just); + ImmutableSet> testMonoFlatMapMany() { + return ImmutableSet.of( + Mono.just(1).flatMapMany(Mono::just), + Mono.just(2).flatMapMany(Mono::just), + Mono.just(3).flatMapMany(Mono::just), + Mono.just(4).flatMapMany(Mono::just), + Mono.just(5).flatMapMany(Mono::just), + Mono.just(6).flatMapMany(Mono::just), + Mono.just(7).flatMapMany(Mono::just), + Mono.just(8).flatMapMany(Mono::just), + Mono.just(9).flatMapMany(Mono::just), + Mono.just(10).flatMapMany(Mono::just), + Mono.just(11).flatMapMany(Mono::just), + Mono.just(12).flatMapMany(Mono::just), + Mono.just(13).flatMapMany(Mono::just)); } ImmutableSet> testConcatMapIterableIdentity() {