Skip to content

Commit

Permalink
Introduce additional Reactor Refaster rules
Browse files Browse the repository at this point in the history 
In various contexts, suggest more efficient and/or less verbose
constructs.
  • Loading branch information
@Stephan202
Stephan202 committed Jan 15, 2024
1 parent b2552e6 commit 9607fbd
Show file tree
Hide file tree
Showing 3 changed files with 224 additions and 22 deletions.
124 changes: 118 additions & 6 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
import static com.google.common.collect.MoreCollectors.toOptional;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.maxBy;
import static java.util.stream.Collectors.minBy;
import static java.util.stream.Collectors.toCollection;
import static org.assertj.core.api.Assertions.assertThat;
import static reactor.function.TupleUtils.function;
Expand Down Expand Up @@ -41,6 +45,7 @@
import org.reactivestreams.Publisher;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.math.MathFlux;
import reactor.test.StepVerifier;
import reactor.test.publisher.PublisherProbe;
import reactor.util.context.Context;
Expand Down Expand Up @@ -596,6 +601,11 @@ Flux<S> after(Flux<T> flux, Function<? super T, ? extends P> function, int prefe

/** Avoid contrived alternatives to {@link Mono#flatMapIterable(Function)}. */
static final class MonoFlatMapIterable<T, S, I extends Iterable<? extends S>> {
@BeforeTemplate
Flux<S> before(Mono<T> mono, Function<? super T, I> function) {
return mono.map(function).flatMapMany(Flux::fromIterable);
}

@BeforeTemplate
Flux<S> before(
Mono<T> mono,
Expand All @@ -608,7 +618,7 @@ Flux<S> before(
}

@AfterTemplate
Flux<S> after(Mono<T> mono, Function<? super T, ? extends Iterable<? extends S>> function) {
Flux<S> after(Mono<T> mono, Function<? super T, I> function) {
return mono.flatMapIterable(function);
}
}
Expand Down Expand Up @@ -950,11 +960,12 @@ static final class FluxThenEmpty<T> {
}
}

/** Avoid vacuous invocations of {@link Mono#ignoreElement()}. */
/** Avoid vacuous operations prior to invocation of {@link Mono#thenMany(Publisher)}. */
static final class MonoThenMany<T, S> {
@BeforeTemplate
Flux<S> before(Mono<T> mono, Publisher<S> publisher) {
return mono.ignoreElement().thenMany(publisher);
return Refaster.anyOf(
mono.ignoreElement().thenMany(publisher), mono.flux().thenMany(publisher));
}

@AfterTemplate
Expand Down Expand Up @@ -992,11 +1003,11 @@ Flux<S> after(Flux<T> flux, Publisher<S> publisher) {
}
}

/** Avoid vacuous invocations of {@link Mono#ignoreElement()}. */
/** Avoid vacuous operations prior to invocation of {@link Mono#then(Mono)}. */
static final class MonoThenMono<T, S> {
@BeforeTemplate
Mono<S> before(Mono<T> mono1, Mono<S> mono2) {
return mono1.ignoreElement().then(mono2);
return Refaster.anyOf(mono1.ignoreElement().then(mono2), mono1.flux().then(mono2));
}

@BeforeTemplate
Expand Down Expand Up @@ -1579,6 +1590,107 @@ Mono<ImmutableSet<T>> after(Flux<T> flux) {
}
}

/** Prefer {@link Flux#sort()} over more verbose alternatives. */
static final class FluxSort<T extends Comparable<? super T>> {
@BeforeTemplate
Flux<T> before(Flux<T> flux) {
return flux.sort(naturalOrder());
}

@AfterTemplate
Flux<T> after(Flux<T> flux) {
return flux.sort();
}
}

/** Prefer {@link MathFlux#min(Publisher)} over less efficient alternatives. */
static final class FluxTransformMin<T extends Comparable<? super T>> {
@BeforeTemplate
Mono<T> before(Flux<T> flux) {
return flux.sort().next();
}

@AfterTemplate
Mono<T> after(Flux<T> flux) {
return flux.transform(MathFlux::min).next();
}
}

/**
* Prefer {@link MathFlux#min(Publisher, Comparator)} over less efficient or more verbose
* alternatives.
*/
static final class FluxTransformMinWithComparator<T extends Comparable<? super T>> {
@BeforeTemplate
Mono<T> before(Flux<T> flux, Comparator<? super T> cmp) {
return Refaster.anyOf(
flux.sort(cmp).next(), flux.collect(minBy(cmp)).flatMap(Mono::justOrEmpty));
}

@AfterTemplate
Mono<T> after(Flux<T> flux, Comparator<? super T> cmp) {
return flux.transform(f -> MathFlux.min(f, cmp)).next();
}
}

/** Prefer {@link MathFlux#max(Publisher)} over less efficient alternatives. */
static final class FluxTransformMax<T extends Comparable<? super T>> {
@BeforeTemplate
Mono<T> before(Flux<T> flux) {
return flux.sort().last();
}

@AfterTemplate
Mono<T> after(Flux<T> flux) {
return flux.transform(MathFlux::max).next();
}
}

/**
* Prefer {@link MathFlux#max(Publisher, Comparator)} over less efficient or more verbose
* alternatives.
*/
static final class FluxTransformMaxWithComparator<T extends Comparable<? super T>> {
@BeforeTemplate
Mono<T> before(Flux<T> flux, Comparator<? super T> cmp) {
return Refaster.anyOf(
flux.sort(cmp).last(), flux.collect(maxBy(cmp)).flatMap(Mono::justOrEmpty));
}

@AfterTemplate
Mono<T> after(Flux<T> flux, Comparator<? super T> cmp) {
return flux.transform(f -> MathFlux.max(f, cmp)).next();
}
}

/** Prefer {@link MathFlux#min(Publisher)} over more contrived alternatives. */
static final class MathFluxMin<T extends Comparable<? super T>> {
@BeforeTemplate
Mono<T> before(Publisher<T> publisher) {
return Refaster.anyOf(
MathFlux.min(publisher, naturalOrder()), MathFlux.max(publisher, reverseOrder()));
}

@AfterTemplate
Mono<T> after(Publisher<T> publisher) {
return MathFlux.min(publisher);
}
}

/** Prefer {@link MathFlux#max(Publisher)} over more contrived alternatives. */
static final class MathFluxMax<T extends Comparable<? super T>> {
@BeforeTemplate
Mono<T> before(Publisher<T> publisher) {
return Refaster.anyOf(
MathFlux.min(publisher, reverseOrder()), MathFlux.max(publisher, naturalOrder()));
}

@AfterTemplate
Mono<T> after(Publisher<T> publisher) {
return MathFlux.max(publisher);
}
}

/** Prefer {@link reactor.util.context.Context#empty()}} over more verbose alternatives. */
// XXX: Introduce Refaster rules or a `BugChecker` that maps `(Immutable)Map.of(k, v)` to
// `Context.of(k, v)` and likewise for multi-pair overloads.
Expand Down Expand Up @@ -1611,7 +1723,7 @@ PublisherProbe<T> after() {
static final class StepVerifierFromMono<T> {
@BeforeTemplate
StepVerifier.FirstStep<? extends T> before(Mono<T> mono) {
return StepVerifier.create(mono);
return Refaster.anyOf(StepVerifier.create(mono), mono.flux().as(StepVerifier::create));
}

@AfterTemplate
Expand Down
64 changes: 55 additions & 9 deletions ...ontrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.MoreCollectors.toOptional;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.maxBy;
import static java.util.stream.Collectors.minBy;
import static java.util.stream.Collectors.toCollection;
import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -21,6 +24,7 @@
import java.util.function.Supplier;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.math.MathFlux;
import reactor.test.StepVerifier;
import reactor.test.publisher.PublisherProbe;
import reactor.util.context.Context;
Expand All @@ -38,6 +42,9 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
ImmutableCollection.class,
ImmutableMap.class,
assertThat(0),
maxBy(null),
minBy(null),
naturalOrder(),
toCollection(null),
toImmutableList(),
toOptional());
Expand Down Expand Up @@ -218,10 +225,11 @@ ImmutableSet<Flux<Integer>> testFluxConcatMapWithPrefetch() {

ImmutableSet<Flux<Integer>> testMonoFlatMapIterable() {
return ImmutableSet.of(
Mono.just(1).map(ImmutableSet::of).flatMapIterable(identity()),
Mono.just(2).map(ImmutableSet::of).flatMapIterable(v -> v),
Mono.just(3).map(ImmutableSet::of).flatMapIterable(v -> ImmutableSet.of()),
Mono.just(4).flux().concatMapIterable(ImmutableSet::of));
Mono.just(1).map(ImmutableSet::of).flatMapMany(Flux::fromIterable),
Mono.just(2).map(ImmutableSet::of).flatMapIterable(identity()),
Mono.just(3).map(ImmutableSet::of).flatMapIterable(v -> v),
Mono.just(4).map(ImmutableSet::of).flatMapIterable(v -> ImmutableSet.of()),
Mono.just(5).flux().concatMapIterable(ImmutableSet::of));
}

Flux<Integer> testMonoFlatMapIterableIdentity() {
Expand Down Expand Up @@ -336,8 +344,10 @@ Mono<Void> testFluxThenEmpty() {
return Flux.just("foo").ignoreElements().thenEmpty(Mono.empty());
}

Flux<String> testMonoThenMany() {
return Mono.just("foo").ignoreElement().thenMany(Flux.just("bar"));
ImmutableSet<Flux<String>> testMonoThenMany() {
return ImmutableSet.of(
Mono.just("foo").ignoreElement().thenMany(Flux.just("bar")),
Mono.just("baz").ignoreElement().thenMany(Flux.just("qux")));
}

Flux<String> testMonoThenMonoFlux() {
Expand All @@ -351,7 +361,8 @@ Flux<String> testFluxThenMany() {
ImmutableSet<Mono<?>> testMonoThenMono() {
return ImmutableSet.of(
Mono.just("foo").ignoreElement().then(Mono.just("bar")),
Mono.just("baz").thenEmpty(Mono.<Void>empty()));
Mono.just("baz").flux().then(Mono.just("qux")),
Mono.just("quux").thenEmpty(Mono.<Void>empty()));
}

ImmutableSet<Mono<?>> testFluxThenMono() {
Expand Down Expand Up @@ -525,6 +536,40 @@ Mono<ImmutableSet<Integer>> testFluxCollectToImmutableSet() {
return Flux.just(1).collect(toImmutableList()).map(ImmutableSet::copyOf);
}

Flux<Integer> testFluxSort() {
return Flux.just(1).sort(naturalOrder());
}

Mono<Integer> testFluxTransformMin() {
return Flux.just(1).sort().next();
}

ImmutableSet<Mono<Integer>> testFluxTransformMinWithComparator() {
return ImmutableSet.of(
Flux.just(1).sort(reverseOrder()).next(),
Flux.just(2).collect(minBy(reverseOrder())).flatMap(Mono::justOrEmpty));
}

Mono<Integer> testFluxTransformMax() {
return Flux.just(1).sort().last();
}

ImmutableSet<Mono<Integer>> testFluxTransformMaxWithComparator() {
return ImmutableSet.of(
Flux.just(1).sort(reverseOrder()).last(),
Flux.just(2).collect(maxBy(reverseOrder())).flatMap(Mono::justOrEmpty));
}

ImmutableSet<Mono<Integer>> testMathFluxMin() {
return ImmutableSet.of(
MathFlux.min(Flux.just(1), naturalOrder()), MathFlux.max(Flux.just(2), reverseOrder()));
}

ImmutableSet<Mono<Integer>> testMathFluxMax() {
return ImmutableSet.of(
MathFlux.min(Flux.just(1), reverseOrder()), MathFlux.max(Flux.just(2), naturalOrder()));
}

ImmutableSet<Context> testContextEmpty() {
return ImmutableSet.of(Context.of(ImmutableMap.of()), Context.of(ImmutableMap.of(1, 2)));
}
Expand All @@ -533,8 +578,9 @@ ImmutableSet<PublisherProbe<Void>> testPublisherProbeEmpty() {
return ImmutableSet.of(PublisherProbe.of(Mono.empty()), PublisherProbe.of(Flux.empty()));
}

StepVerifier.FirstStep<Integer> testStepVerifierFromMono() {
return StepVerifier.create(Mono.just(1));
ImmutableSet<StepVerifier.FirstStep<Integer>> testStepVerifierFromMono() {
return ImmutableSet.of(
StepVerifier.create(Mono.just(1)), Mono.just(2).flux().as(StepVerifier::create));
}

StepVerifier.FirstStep<Integer> testStepVerifierFromFlux() {
Expand Down
Loading

0 comments on commit 9607fbd

Please sign in to comment.