Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce IsIdentityOperation matcher for use by Refaster templates #749

Merged
merged 4 commits into from
Oct 23, 2023
Merged

Introduce IsIdentityOperation matcher for use by Refaster templates #749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b560535
Introduce `IsIdentityOperation` matcher for use by Refaster templates
Stephan202 Aug 12, 2023
1928545
Simplify, improve coverge
Stephan202 Aug 12, 2023
3c6a1d8
Apply in more cases
Stephan202 Oct 7, 2023
7880617
Track a thought
Stephan202 Oct 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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