Skip to content

Commit

Permalink
Introduce IsIdentityOperation matcher for use by Refaster templates (…
Browse files Browse the repository at this point in the history 
…#749)
  • Loading branch information
@Stephan202
Stephan202 authored Oct 23, 2023
1 parent c3b950f commit 8130ddf
Show file tree
Hide file tree
Showing 18 changed files with 412 additions and 134 deletions.
2 changes: 2 additions & 0 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
// the target method such a modification may change the code's semantics or performance.
// XXX: Also flag `Stream#map`, `Mono#map` and `Flux#map` invocations where the given transformation
// is effectively the identity operation.
// XXX: Also flag nullary instance method invocations that represent an identity conversion, such as
// `Boolean#booleanValue()`, `Byte#byteValue()` and friends.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Avoid or clarify identity conversions",
Expand Down
27 changes: 15 additions & 12 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 @@ -183,14 +185,15 @@ Comparator<T> after(Comparator<T> cmp, ToLongFunction<? super T> function) {
}

/**
* Where applicable, prefer {@link Comparator#naturalOrder()} over {@link Function#identity()}, as
* it more clearly states intent.
* Where applicable, prefer {@link Comparator#naturalOrder()} over identity function-based
* comparisons, as the former 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
Loading

0 comments on commit 8130ddf

Please sign in to comment.