Skip to content

Commit

Permalink
Introduce IsIdentityOperation matcher for use by Refaster templates
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202
Stephan202 committed Aug 14, 2023
1 parent 4af7b21 commit 70461d4
Show file tree
Hide file tree
Showing 17 changed files with 353 additions and 105 deletions.
23 changes: 13 additions & 10 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import static java.util.Comparator.comparingLong;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;

import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
Expand All @@ -16,6 +15,7 @@
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Matches;
import com.google.errorprone.refaster.annotation.Repeated;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Arrays;
Expand All @@ -28,6 +28,7 @@
import java.util.function.ToLongFunction;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation;

/** Refaster rules related to expressions dealing with {@link Comparator}s. */
@OnlineDocumentation
Expand All @@ -36,12 +37,12 @@ private ComparatorRules() {}

/** Prefer {@link Comparator#naturalOrder()} over more complicated constructs. */
static final class NaturalOrder<T extends Comparable<? super T>> {
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
@BeforeTemplate
Comparator<T> before() {
Comparator<T> before(
@Matches(IsIdentityOperation.class) Function<? super T, ? extends T> keyExtractor) {
return Refaster.anyOf(
T::compareTo,
comparing(Refaster.anyOf(identity(), v -> v)),
comparing(keyExtractor),
Collections.<T>reverseOrder(reverseOrder()),
Comparator.<T>reverseOrder().reversed());
}
Expand Down Expand Up @@ -72,10 +73,11 @@ Comparator<T> after() {
}

static final class CustomComparator<T> {
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
@BeforeTemplate
Comparator<T> before(Comparator<T> cmp) {
return comparing(Refaster.anyOf(identity(), v -> v), cmp);
Comparator<T> before(
Comparator<T> cmp,
@Matches(IsIdentityOperation.class) Function<? super T, ? extends T> keyExtractor) {
return comparing(keyExtractor, cmp);
}

@AfterTemplate
Expand Down Expand Up @@ -187,10 +189,11 @@ Comparator<T> after(Comparator<T> cmp, ToLongFunction<? super T> function) {
* it more clearly states intent.
*/
static final class ThenComparingNaturalOrder<T extends Comparable<? super T>> {
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
@BeforeTemplate
Comparator<T> before(Comparator<T> cmp) {
return cmp.thenComparing(Refaster.anyOf(identity(), v -> v));
Comparator<T> before(
Comparator<T> cmp,
@Matches(IsIdentityOperation.class) Function<? super T, ? extends T> keyExtractor) {
return cmp.thenComparing(keyExtractor);
}

@AfterTemplate
Expand Down
25 changes: 14 additions & 11 deletions ...ontrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap;
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.function.Function.identity;

import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMultimap;
Expand All @@ -16,6 +15,7 @@
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Matches;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.Placeholder;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
Expand All @@ -25,6 +25,7 @@
import java.util.function.Function;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation;

/** Refaster rules related to expressions dealing with {@link ImmutableListMultimap}s. */
@OnlineDocumentation
Expand Down Expand Up @@ -173,27 +174,29 @@ ImmutableListMultimap<K, V> after(Stream<E> stream) {
* Prefer {@link Multimaps#index(Iterable, com.google.common.base.Function)} over the stream-based
* alternative.
*/
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
static final class IndexIterableToImmutableListMultimap<K, V> {
@BeforeTemplate
ImmutableListMultimap<K, V> before(
Iterator<V> iterable, Function<? super V, ? extends K> keyFunction) {
return Streams.stream(iterable)
.collect(toImmutableListMultimap(keyFunction, Refaster.anyOf(identity(), v -> v)));
Iterator<V> iterable,
Function<? super V, ? extends K> keyFunction,
@Matches(IsIdentityOperation.class) Function<? super V, ? extends V> valueFunction) {
return Streams.stream(iterable).collect(toImmutableListMultimap(keyFunction, valueFunction));
}

@BeforeTemplate
ImmutableListMultimap<K, V> before(
Iterable<V> iterable, Function<? super V, ? extends K> keyFunction) {
return Streams.stream(iterable)
.collect(toImmutableListMultimap(keyFunction, Refaster.anyOf(identity(), v -> v)));
Iterable<V> iterable,
Function<? super V, ? extends K> keyFunction,
@Matches(IsIdentityOperation.class) Function<? super V, ? extends V> valueFunction) {
return Streams.stream(iterable).collect(toImmutableListMultimap(keyFunction, valueFunction));
}

@BeforeTemplate
ImmutableListMultimap<K, V> before(
Collection<V> iterable, Function<? super V, ? extends K> keyFunction) {
return iterable.stream()
.collect(toImmutableListMultimap(keyFunction, Refaster.anyOf(identity(), v -> v)));
Collection<V> iterable,
Function<? super V, ? extends K> keyFunction,
@Matches(IsIdentityOperation.class) Function<? super V, ? extends V> valueFunction) {
return iterable.stream().collect(toImmutableListMultimap(keyFunction, valueFunction));
}

@AfterTemplate
Expand Down
49 changes: 28 additions & 21 deletions ...r-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static java.util.function.Function.identity;

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableMap;
Expand All @@ -13,6 +12,7 @@
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Matches;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.Placeholder;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
Expand All @@ -23,6 +23,7 @@
import java.util.function.Function;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation;

/** Refaster rules related to expressions dealing with {@link ImmutableMap}s. */
@OnlineDocumentation
Expand Down Expand Up @@ -63,27 +64,29 @@ ImmutableMap<K, V> after(Map.Entry<? extends K, ? extends V> entry) {
* Prefer {@link Maps#toMap(Iterable, com.google.common.base.Function)} over more contrived
* alternatives.
*/
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
static final class IterableToImmutableMap<K, V> {
@BeforeTemplate
ImmutableMap<K, V> before(
Iterator<K> iterable, Function<? super K, ? extends V> valueFunction) {
return Streams.stream(iterable)
.collect(toImmutableMap(Refaster.anyOf(identity(), k -> k), valueFunction));
Iterator<K> iterable,
Function<? super K, ? extends V> valueFunction,
@Matches(IsIdentityOperation.class) Function<? super K, ? extends K> keyFunction) {
return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction));
}

@BeforeTemplate
ImmutableMap<K, V> before(
Iterable<K> iterable, Function<? super K, ? extends V> valueFunction) {
return Streams.stream(iterable)
.collect(toImmutableMap(Refaster.anyOf(identity(), k -> k), valueFunction));
Iterable<K> iterable,
Function<? super K, ? extends V> valueFunction,
@Matches(IsIdentityOperation.class) Function<? super K, ? extends K> keyFunction) {
return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction));
}

@BeforeTemplate
ImmutableMap<K, V> before(
Collection<K> iterable, Function<? super K, ? extends V> valueFunction) {
return iterable.stream()
.collect(toImmutableMap(Refaster.anyOf(identity(), k -> k), valueFunction));
Collection<K> iterable,
Function<? super K, ? extends V> valueFunction,
@Matches(IsIdentityOperation.class) Function<? super K, ? extends K> keyFunction) {
return iterable.stream().collect(toImmutableMap(keyFunction, valueFunction));
}

@BeforeTemplate
Expand Down Expand Up @@ -157,25 +160,29 @@ ImmutableMap<K, V> after(Stream<E> stream) {
* Prefer {@link Maps#uniqueIndex(Iterable, com.google.common.base.Function)} over the
* stream-based alternative.
*/
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
static final class IndexIterableToImmutableMap<K, V> {
@BeforeTemplate
ImmutableMap<K, V> before(Iterator<V> iterable, Function<? super V, ? extends K> keyFunction) {
return Streams.stream(iterable)
.collect(toImmutableMap(keyFunction, Refaster.anyOf(identity(), v -> v)));
ImmutableMap<K, V> before(
Iterator<V> iterable,
Function<? super V, ? extends K> keyFunction,
@Matches(IsIdentityOperation.class) Function<? super V, ? extends V> valueFunction) {
return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction));
}

@BeforeTemplate
ImmutableMap<K, V> before(Iterable<V> iterable, Function<? super V, ? extends K> keyFunction) {
return Streams.stream(iterable)
.collect(toImmutableMap(keyFunction, Refaster.anyOf(identity(), v -> v)));
ImmutableMap<K, V> before(
Iterable<V> iterable,
Function<? super V, ? extends K> keyFunction,
@Matches(IsIdentityOperation.class) Function<? super V, ? extends V> valueFunction) {
return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction));
}

@BeforeTemplate
ImmutableMap<K, V> before(
Collection<V> iterable, Function<? super V, ? extends K> keyFunction) {
return iterable.stream()
.collect(toImmutableMap(keyFunction, Refaster.anyOf(identity(), v -> v)));
Collection<V> iterable,
Function<? super V, ? extends K> keyFunction,
@Matches(IsIdentityOperation.class) Function<? super V, ? extends V> valueFunction) {
return iterable.stream().collect(toImmutableMap(keyFunction, valueFunction));
}

@AfterTemplate
Expand Down
56 changes: 35 additions & 21 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 @@ -18,6 +18,7 @@
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Matches;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.NotMatches;
import com.google.errorprone.refaster.annotation.Placeholder;
Expand Down Expand Up @@ -48,6 +49,7 @@
import tech.picnic.errorprone.refaster.annotation.Description;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.annotation.Severity;
import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation;
import tech.picnic.errorprone.refaster.matchers.ThrowsCheckedException;

/** Refaster rules related to Reactor expressions and statements. */
Expand Down Expand Up @@ -405,7 +407,8 @@ Mono<T> before(Mono<T> mono) {
}

// XXX: Replace this rule with an extension of the `IdentityConversion` rule, supporting
// `Stream#map`, `Mono#map` and `Flux#map`.
// `Stream#map`, `Mono#map` and `Flux#map`. Alternatively, extend the `IsIdentityOperation`
// matcher and use it to constrain the matched `map` argument.
@BeforeTemplate
Mono<ImmutableList<T>> before3(Mono<ImmutableList<T>> mono) {
return mono.map(ImmutableList::copyOf);
Expand Down Expand Up @@ -446,41 +449,44 @@ Flux<T> after(Flux<T> flux) {
}

/** Prefer {@link Flux#concatMap(Function)} over more contrived alternatives. */
static final class FluxConcatMap<T, S> {
static final class FluxConcatMap<T, S, P extends Publisher<? extends S>> {
@BeforeTemplate
@SuppressWarnings("NestedPublishers")
Flux<S> before(Flux<T> flux, Function<? super T, ? extends Publisher<? extends S>> function) {
Flux<S> before(
Flux<T> flux,
Function<? super T, ? extends P> function,
@Matches(IsIdentityOperation.class)
Function<? super P, ? extends Publisher<? extends S>> identityOperation) {
return Refaster.anyOf(
flux.flatMap(function, 1),
flux.flatMapSequential(function, 1),
flux.map(function).concatMap(identity()));
flux.map(function).concatMap(identityOperation));
}

@AfterTemplate
Flux<S> after(Flux<T> flux, Function<? super T, ? extends Publisher<? extends S>> function) {
Flux<S> after(Flux<T> flux, Function<? super T, ? extends P> function) {
return flux.concatMap(function);
}
}

/** Prefer {@link Flux#concatMap(Function, int)} over more contrived alternatives. */
static final class FluxConcatMapWithPrefetch<T, S> {
static final class FluxConcatMapWithPrefetch<T, S, P extends Publisher<? extends S>> {
@BeforeTemplate
@SuppressWarnings("NestedPublishers")
Flux<S> before(
Flux<T> flux,
Function<? super T, ? extends Publisher<? extends S>> function,
int prefetch) {
Function<? super T, ? extends P> function,
int prefetch,
@Matches(IsIdentityOperation.class)
Function<? super P, ? extends Publisher<? extends S>> identityOperation) {
return Refaster.anyOf(
flux.flatMap(function, 1, prefetch),
flux.flatMapSequential(function, 1, prefetch),
flux.map(function).concatMap(identity(), prefetch));
flux.map(function).concatMap(identityOperation, prefetch));
}

@AfterTemplate
Flux<S> after(
Flux<T> flux,
Function<? super T, ? extends Publisher<? extends S>> function,
int prefetch) {
Flux<S> after(Flux<T> flux, Function<? super T, ? extends P> function, int prefetch) {
return flux.concatMap(function, prefetch);
}
}
Expand Down Expand Up @@ -795,29 +801,37 @@ Flux<S> after(Flux<T> flux) {
}

/** Prefer {@link Mono#flatMap(Function)} over more contrived alternatives. */
static final class MonoFlatMap<S, T> {
static final class MonoFlatMap<S, T, P extends Mono<? extends T>> {
@BeforeTemplate
@SuppressWarnings("NestedPublishers")
Mono<T> before(Mono<S> mono, Function<? super S, ? extends Mono<? extends T>> function) {
return mono.map(function).flatMap(identity());
Mono<T> before(
Mono<S> mono,
Function<? super S, ? extends P> function,
@Matches(IsIdentityOperation.class)
Function<? super P, ? extends Mono<? extends T>> identityOperation) {
return mono.map(function).flatMap(identityOperation);
}

@AfterTemplate
Mono<T> after(Mono<S> mono, Function<? super S, ? extends Mono<? extends T>> function) {
Mono<T> after(Mono<S> mono, Function<? super S, ? extends P> function) {
return mono.flatMap(function);
}
}

/** Prefer {@link Mono#flatMapMany(Function)} over more contrived alternatives. */
static final class MonoFlatMapMany<S, T> {
static final class MonoFlatMapMany<S, T, P extends Publisher<? extends T>> {
@BeforeTemplate
@SuppressWarnings("NestedPublishers")
Flux<T> before(Mono<S> mono, Function<? super S, ? extends Publisher<? extends T>> function) {
return mono.map(function).flatMapMany(identity());
Flux<T> before(
Mono<S> mono,
Function<? super S, ? extends P> function,
@Matches(IsIdentityOperation.class)
Function<? super P, ? extends Publisher<? extends T>> identityOperation) {
return mono.map(function).flatMapMany(identityOperation);
}

@AfterTemplate
Flux<T> after(Mono<S> mono, Function<? super S, ? extends Publisher<? extends T>> function) {
Flux<T> after(Mono<S> mono, Function<? super S, ? extends P> function) {
return mono.flatMapMany(function);
}
}
Expand Down
9 changes: 6 additions & 3 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
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.function.Predicate.not;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.filtering;
Expand Down Expand Up @@ -49,6 +48,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation;
import tech.picnic.errorprone.refaster.matchers.IsLambdaExpressionOrMethodReference;
import tech.picnic.errorprone.refaster.matchers.IsRefasterAsVarargs;

Expand Down Expand Up @@ -630,8 +630,11 @@ R after(

static final class StreamsConcat<T> {
@BeforeTemplate
Stream<T> before(@Repeated Stream<T> stream) {
return Stream.of(Refaster.asVarargs(stream)).flatMap(Refaster.anyOf(identity(), s -> s));
Stream<T> before(
@Repeated Stream<T> stream,
@Matches(IsIdentityOperation.class)
Function<? super Stream<T>, ? extends Stream<? extends T>> mapper) {
return Stream.of(Refaster.asVarargs(stream)).flatMap(mapper);
}

@AfterTemplate
Expand Down
Loading

0 comments on commit 70461d4

Please sign in to comment.