jlengrand/error-prone-support
1
0
Fork 0
mirror of https://github.com/jlengrand/error-prone-support.git synced 2026-03-10 08:11:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Introduce additional Reactor Refaster rules (#969)

Browse Source 
In various contexts, suggest more efficient and/or less verbose
constructs.
This commit is contained in:
Stephan Schroevers
2024-01-21 13:36:06 +01:00
committed by GitHub
parent 9e6d35569f
commit 0aa612073f
3 changed files with 234 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
View File

@@ -5,7 +5,11 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
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;
@@ -41,6 +45,7 @@ import org.jspecify.annotations.Nullable;
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;
@@ -596,6 +601,11 @@ final class ReactorRules {
/** 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,
@@ -608,7 +618,7 @@ final class ReactorRules {
}
@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);
}
}
@@ -950,11 +960,12 @@ final class ReactorRules {
}
}
/** 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
@@ -992,11 +1003,11 @@ final class ReactorRules {
}
}
/** 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
@@ -1031,12 +1042,15 @@ final class ReactorRules {
/** Prefer {@link Mono#singleOptional()} over more contrived alternatives. */
// XXX: Consider creating a plugin that flags/discourages `Mono<Optional<T>>` method return
// types, just as we discourage nullable `Boolean`s and `Optional`s.
// XXX: The `mono.transform(Mono::singleOptional)` replacement is a special case of a more general
// rule. Consider introducing an Error Prone check for this.
static final class MonoSingleOptional<T> {
@BeforeTemplate
Mono<Optional<T>> before(Mono<T> mono) {
return Refaster.anyOf(
mono.flux().collect(toOptional()),
mono.map(Optional::of).defaultIfEmpty(Optional.empty()));
mono.map(Optional::of).defaultIfEmpty(Optional.empty()),
mono.transform(Mono::singleOptional));
}
@AfterTemplate
@@ -1579,6 +1593,107 @@ final class ReactorRules {
}
}
/** 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).singleOrEmpty();
}
}
/**
* 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)).singleOrEmpty();
}
}
/** 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).singleOrEmpty();
}
}
/**
* 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)).singleOrEmpty();
}
}
/** 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.
@@ -1611,7 +1726,7 @@ final class ReactorRules {
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

67
error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java
View File

@@ -2,8 +2,11 @@ package tech.picnic.errorprone.refasterrules;
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;
@@ -21,6 +24,7 @@ import java.util.concurrent.Callable;
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;
@@ -38,6 +42,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
ImmutableCollection.class,
ImmutableMap.class,
assertThat(0),
maxBy(null),
minBy(null),
naturalOrder(),
toCollection(null),
toImmutableList(),
toOptional());
@@ -218,10 +225,11 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
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() {
@@ -336,8 +344,10 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
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() {
@@ -351,7 +361,8 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
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() {
@@ -363,7 +374,8 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
ImmutableSet<Mono<Optional<String>>> testMonoSingleOptional() {
return ImmutableSet.of(
Mono.just("foo").flux().collect(toOptional()),
Mono.just("bar").map(Optional::of).defaultIfEmpty(Optional.empty()));
Mono.just("bar").map(Optional::of).defaultIfEmpty(Optional.empty()),
Mono.just("baz").transform(Mono::singleOptional));
}
Mono<Number> testMonoCast() {
@@ -525,6 +537,40 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
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)));
}
@@ -533,8 +579,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
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() {

63
error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java
View File

@@ -3,8 +3,11 @@ package tech.picnic.errorprone.refasterrules;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
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;
import static reactor.function.TupleUtils.function;
@@ -24,6 +27,7 @@ import java.util.function.Supplier;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.function.TupleUtils;
import reactor.math.MathFlux;
import reactor.test.StepVerifier;
import reactor.test.publisher.PublisherProbe;
import reactor.util.context.Context;
@@ -41,6 +45,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
ImmutableCollection.class,
ImmutableMap.class,
assertThat(0),
maxBy(null),
minBy(null),
naturalOrder(),
toCollection(null),
toImmutableList(),
toOptional());
@@ -223,8 +230,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
return ImmutableSet.of(
Mono.just(1).flatMapIterable(ImmutableSet::of),
Mono.just(2).flatMapIterable(ImmutableSet::of),
Mono.just(3).map(ImmutableSet::of).flatMapIterable(v -> ImmutableSet.of()),
Mono.just(4).flatMapIterable(ImmutableSet::of));
Mono.just(3).flatMapIterable(ImmutableSet::of),
Mono.just(4).map(ImmutableSet::of).flatMapIterable(v -> ImmutableSet.of()),
Mono.just(5).flatMapIterable(ImmutableSet::of));
}
Flux<Integer> testMonoFlatMapIterableIdentity() {
@@ -333,8 +341,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
return Flux.just("foo").thenEmpty(Mono.empty());
}
Flux<String> testMonoThenMany() {
return Mono.just("foo").thenMany(Flux.just("bar"));
ImmutableSet<Flux<String>> testMonoThenMany() {
return ImmutableSet.of(
Mono.just("foo").thenMany(Flux.just("bar")), Mono.just("baz").thenMany(Flux.just("qux")));
}
Flux<String> testMonoThenMonoFlux() {
@@ -347,7 +356,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
ImmutableSet<Mono<?>> testMonoThenMono() {
return ImmutableSet.of(
Mono.just("foo").then(Mono.just("bar")), Mono.just("baz").then(Mono.<Void>empty()));
Mono.just("foo").then(Mono.just("bar")),
Mono.just("baz").then(Mono.just("qux")),
Mono.just("quux").then(Mono.<Void>empty()));
}
ImmutableSet<Mono<?>> testFluxThenMono() {
@@ -356,7 +367,10 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
}
ImmutableSet<Mono<Optional<String>>> testMonoSingleOptional() {
return ImmutableSet.of(Mono.just("foo").singleOptional(), Mono.just("bar").singleOptional());
return ImmutableSet.of(
Mono.just("foo").singleOptional(),
Mono.just("bar").singleOptional(),
Mono.just("baz").singleOptional());
}
Mono<Number> testMonoCast() {
@@ -514,6 +528,38 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
return Flux.just(1).collect(toImmutableSet());
}
Flux<Integer> testFluxSort() {
return Flux.just(1).sort();
}
Mono<Integer> testFluxTransformMin() {
return Flux.just(1).transform(MathFlux::min).singleOrEmpty();
}
ImmutableSet<Mono<Integer>> testFluxTransformMinWithComparator() {
return ImmutableSet.of(
Flux.just(1).transform(f -> MathFlux.min(f, reverseOrder())).singleOrEmpty(),
Flux.just(2).transform(f -> MathFlux.min(f, reverseOrder())).singleOrEmpty());
}
Mono<Integer> testFluxTransformMax() {
return Flux.just(1).transform(MathFlux::max).singleOrEmpty();
}
ImmutableSet<Mono<Integer>> testFluxTransformMaxWithComparator() {
return ImmutableSet.of(
Flux.just(1).transform(f -> MathFlux.max(f, reverseOrder())).singleOrEmpty(),
Flux.just(2).transform(f -> MathFlux.max(f, reverseOrder())).singleOrEmpty());
}
ImmutableSet<Mono<Integer>> testMathFluxMin() {
return ImmutableSet.of(MathFlux.min(Flux.just(1)), MathFlux.min(Flux.just(2)));
}
ImmutableSet<Mono<Integer>> testMathFluxMax() {
return ImmutableSet.of(MathFlux.max(Flux.just(1)), MathFlux.max(Flux.just(2)));
}
ImmutableSet<Context> testContextEmpty() {
return ImmutableSet.of(Context.empty(), Context.of(ImmutableMap.of(1, 2)));
}
@@ -522,8 +568,9 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
return ImmutableSet.of(PublisherProbe.empty(), PublisherProbe.empty());
}
StepVerifier.FirstStep<Integer> testStepVerifierFromMono() {
return Mono.just(1).as(StepVerifier::create);
ImmutableSet<StepVerifier.FirstStep<Integer>> testStepVerifierFromMono() {
return ImmutableSet.of(
Mono.just(1).as(StepVerifier::create), Mono.just(2).as(StepVerifier::create));
}
StepVerifier.FirstStep<Integer> testStepVerifierFromFlux() {