diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java index 0d4d3f2769..fb11dbc86e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java @@ -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", diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java index 965044902d..a37ef407c2 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ComparatorRules.java @@ -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; @@ -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; @@ -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 @@ -36,12 +37,12 @@ private ComparatorRules() {} /** Prefer {@link Comparator#naturalOrder()} over more complicated constructs. */ static final class NaturalOrder> { - // XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other. @BeforeTemplate - Comparator before() { + Comparator before( + @Matches(IsIdentityOperation.class) Function keyExtractor) { return Refaster.anyOf( T::compareTo, - comparing(Refaster.anyOf(identity(), v -> v)), + comparing(keyExtractor), Collections.reverseOrder(reverseOrder()), Comparator.reverseOrder().reversed()); } @@ -72,10 +73,11 @@ Comparator after() { } static final class CustomComparator { - // XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other. @BeforeTemplate - Comparator before(Comparator cmp) { - return comparing(Refaster.anyOf(identity(), v -> v), cmp); + Comparator before( + Comparator cmp, + @Matches(IsIdentityOperation.class) Function keyExtractor) { + return comparing(keyExtractor, cmp); } @AfterTemplate @@ -183,14 +185,15 @@ Comparator after(Comparator cmp, ToLongFunction 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> { - // XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other. @BeforeTemplate - Comparator before(Comparator cmp) { - return cmp.thenComparing(Refaster.anyOf(identity(), v -> v)); + Comparator before( + Comparator cmp, + @Matches(IsIdentityOperation.class) Function keyExtractor) { + return cmp.thenComparing(keyExtractor); } @AfterTemplate diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java index b68a7a1601..3a943ab86c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRules.java @@ -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; @@ -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; @@ -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 @@ -173,27 +174,29 @@ ImmutableListMultimap after(Stream 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 { @BeforeTemplate ImmutableListMultimap before( - Iterator iterable, Function keyFunction) { - return Streams.stream(iterable) - .collect(toImmutableListMultimap(keyFunction, Refaster.anyOf(identity(), v -> v))); + Iterator iterable, + Function keyFunction, + @Matches(IsIdentityOperation.class) Function valueFunction) { + return Streams.stream(iterable).collect(toImmutableListMultimap(keyFunction, valueFunction)); } @BeforeTemplate ImmutableListMultimap before( - Iterable iterable, Function keyFunction) { - return Streams.stream(iterable) - .collect(toImmutableListMultimap(keyFunction, Refaster.anyOf(identity(), v -> v))); + Iterable iterable, + Function keyFunction, + @Matches(IsIdentityOperation.class) Function valueFunction) { + return Streams.stream(iterable).collect(toImmutableListMultimap(keyFunction, valueFunction)); } @BeforeTemplate ImmutableListMultimap before( - Collection iterable, Function keyFunction) { - return iterable.stream() - .collect(toImmutableListMultimap(keyFunction, Refaster.anyOf(identity(), v -> v))); + Collection iterable, + Function keyFunction, + @Matches(IsIdentityOperation.class) Function valueFunction) { + return iterable.stream().collect(toImmutableListMultimap(keyFunction, valueFunction)); } @AfterTemplate diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java index 49021f1a5a..8c0f9c6267 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java @@ -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; @@ -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; @@ -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 @@ -63,27 +64,29 @@ ImmutableMap after(Map.Entry 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 { @BeforeTemplate ImmutableMap before( - Iterator iterable, Function valueFunction) { - return Streams.stream(iterable) - .collect(toImmutableMap(Refaster.anyOf(identity(), k -> k), valueFunction)); + Iterator iterable, + Function valueFunction, + @Matches(IsIdentityOperation.class) Function keyFunction) { + return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction)); } @BeforeTemplate ImmutableMap before( - Iterable iterable, Function valueFunction) { - return Streams.stream(iterable) - .collect(toImmutableMap(Refaster.anyOf(identity(), k -> k), valueFunction)); + Iterable iterable, + Function valueFunction, + @Matches(IsIdentityOperation.class) Function keyFunction) { + return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction)); } @BeforeTemplate ImmutableMap before( - Collection iterable, Function valueFunction) { - return iterable.stream() - .collect(toImmutableMap(Refaster.anyOf(identity(), k -> k), valueFunction)); + Collection iterable, + Function valueFunction, + @Matches(IsIdentityOperation.class) Function keyFunction) { + return iterable.stream().collect(toImmutableMap(keyFunction, valueFunction)); } @BeforeTemplate @@ -157,25 +160,29 @@ ImmutableMap after(Stream 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 { @BeforeTemplate - ImmutableMap before(Iterator iterable, Function keyFunction) { - return Streams.stream(iterable) - .collect(toImmutableMap(keyFunction, Refaster.anyOf(identity(), v -> v))); + ImmutableMap before( + Iterator iterable, + Function keyFunction, + @Matches(IsIdentityOperation.class) Function valueFunction) { + return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction)); } @BeforeTemplate - ImmutableMap before(Iterable iterable, Function keyFunction) { - return Streams.stream(iterable) - .collect(toImmutableMap(keyFunction, Refaster.anyOf(identity(), v -> v))); + ImmutableMap before( + Iterable iterable, + Function keyFunction, + @Matches(IsIdentityOperation.class) Function valueFunction) { + return Streams.stream(iterable).collect(toImmutableMap(keyFunction, valueFunction)); } @BeforeTemplate ImmutableMap before( - Collection iterable, Function keyFunction) { - return iterable.stream() - .collect(toImmutableMap(keyFunction, Refaster.anyOf(identity(), v -> v))); + Collection iterable, + Function keyFunction, + @Matches(IsIdentityOperation.class) Function valueFunction) { + return iterable.stream().collect(toImmutableMap(keyFunction, valueFunction)); } @AfterTemplate diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java index e11c719e5a..0a323cc36e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java @@ -49,6 +49,7 @@ import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; import tech.picnic.errorprone.refaster.annotation.Severity; import tech.picnic.errorprone.refaster.matchers.IsEmpty; +import tech.picnic.errorprone.refaster.matchers.IsIdentityOperation; import tech.picnic.errorprone.refaster.matchers.ThrowsCheckedException; /** Refaster rules related to Reactor expressions and statements. */ @@ -509,7 +510,8 @@ Mono before(Mono 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> before3(Mono> mono) { return mono.map(ImmutableList::copyOf); @@ -550,51 +552,59 @@ Flux after(Flux flux) { } /** Prefer {@link Flux#concatMap(Function)} over more contrived alternatives. */ - static final class FluxConcatMap { + static final class FluxConcatMap> { @BeforeTemplate @SuppressWarnings("NestedPublishers") - Flux before(Flux flux, Function> function) { + Flux before( + Flux flux, + Function function, + @Matches(IsIdentityOperation.class) + Function> identityOperation) { return Refaster.anyOf( flux.flatMap(function, 1), flux.flatMapSequential(function, 1), - flux.map(function).concatMap(identity())); + flux.map(function).concatMap(identityOperation)); } @AfterTemplate - Flux after(Flux flux, Function> function) { + Flux after(Flux flux, Function function) { return flux.concatMap(function); } } /** Prefer {@link Flux#concatMap(Function, int)} over more contrived alternatives. */ - static final class FluxConcatMapWithPrefetch { + static final class FluxConcatMapWithPrefetch> { @BeforeTemplate @SuppressWarnings("NestedPublishers") Flux before( Flux flux, - Function> function, - int prefetch) { + Function function, + int prefetch, + @Matches(IsIdentityOperation.class) + Function> 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 after( - Flux flux, - Function> function, - int prefetch) { + Flux after(Flux flux, Function function, int prefetch) { return flux.concatMap(function, prefetch); } } /** Avoid contrived alternatives to {@link Mono#flatMapIterable(Function)}. */ - static final class MonoFlatMapIterable { + static final class MonoFlatMapIterable> { @BeforeTemplate - Flux before(Mono mono, Function> function) { + Flux before( + Mono mono, + Function function, + @Matches(IsIdentityOperation.class) + Function> identityOperation) { return Refaster.anyOf( - mono.map(function).flatMapIterable(identity()), mono.flux().concatMapIterable(function)); + mono.map(function).flatMapIterable(identityOperation), + mono.flux().concatMapIterable(function)); } @AfterTemplate @@ -624,11 +634,15 @@ Flux after(Mono mono) { * Prefer {@link Flux#concatMapIterable(Function)} over alternatives with less clear syntax or * semantics. */ - static final class FluxConcatMapIterable { + static final class FluxConcatMapIterable> { @BeforeTemplate - Flux before(Flux flux, Function> function) { + Flux before( + Flux flux, + Function function, + @Matches(IsIdentityOperation.class) + Function> identityOperation) { return Refaster.anyOf( - flux.flatMapIterable(function), flux.map(function).concatMapIterable(identity())); + flux.flatMapIterable(function), flux.map(function).concatMapIterable(identityOperation)); } @AfterTemplate @@ -641,13 +655,17 @@ Flux after(Flux flux, Function> * Prefer {@link Flux#concatMapIterable(Function, int)} over alternatives with less clear syntax * or semantics. */ - static final class FluxConcatMapIterableWithPrefetch { + static final class FluxConcatMapIterableWithPrefetch> { @BeforeTemplate Flux before( - Flux flux, Function> function, int prefetch) { + Flux flux, + Function function, + int prefetch, + @Matches(IsIdentityOperation.class) + Function> identityOperation) { return Refaster.anyOf( flux.flatMapIterable(function, prefetch), - flux.map(function).concatMapIterable(identity(), prefetch)); + flux.map(function).concatMapIterable(identityOperation, prefetch)); } @AfterTemplate @@ -1081,31 +1099,37 @@ Flux after(Flux flux, Class clazz) { } /** Prefer {@link Mono#flatMap(Function)} over more contrived alternatives. */ - static final class MonoFlatMap { + static final class MonoFlatMap> { @BeforeTemplate @SuppressWarnings("NestedPublishers") - Mono before(Mono mono, Function> function) { - return mono.map(function).flatMap(identity()); + Mono before( + Mono mono, + Function function, + @Matches(IsIdentityOperation.class) + Function> identityOperation) { + return mono.map(function).flatMap(identityOperation); } @AfterTemplate - Mono after(Mono mono, Function> function) { + Mono after(Mono mono, Function function) { return mono.flatMap(function); } } /** Prefer {@link Mono#flatMapMany(Function)} over more contrived alternatives. */ - static final class MonoFlatMapMany { + static final class MonoFlatMapMany> { @BeforeTemplate @SuppressWarnings("NestedPublishers") Flux before( Mono mono, - Function> function, + Function function, + @Matches(IsIdentityOperation.class) + Function> identityOperation, boolean delayUntilEnd, int maxConcurrency, int prefetch) { return Refaster.anyOf( - mono.map(function).flatMapMany(identity()), + mono.map(function).flatMapMany(identityOperation), mono.flux().concatMap(function), mono.flux().concatMap(function, prefetch), mono.flux().concatMapDelayError(function), @@ -1125,7 +1149,7 @@ Flux before(Mono mono, Function> functio } @AfterTemplate - Flux after(Mono mono, Function> function) { + Flux after(Mono mono, Function function) { return mono.flatMapMany(function); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java index 2d302236d1..be5a2b4709 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java @@ -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; @@ -51,6 +50,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; @@ -357,6 +357,8 @@ boolean before(Stream stream, Predicate predicate) { stream.filter(predicate).findAny().isEmpty()); } + // XXX: Consider extending `@Matches(IsIdentityOperation.class)` such that it can replace this + // template's `Refaster.anyOf` usage. @BeforeTemplate boolean before2( Stream stream, @@ -395,6 +397,8 @@ boolean before(Stream stream, Predicate predicate) { !stream.noneMatch(predicate), stream.filter(predicate).findAny().isPresent()); } + // XXX: Consider extending `@Matches(IsIdentityOperation.class)` such that it can replace this + // template's `Refaster.anyOf` usage. @BeforeTemplate boolean before2( Stream stream, @@ -415,6 +419,8 @@ boolean before(Stream stream, Predicate predicate) { return stream.noneMatch(Refaster.anyOf(not(predicate), predicate.negate())); } + // XXX: Consider extending `@Matches(IsIdentityOperation.class)` such that it can replace this + // template's `Refaster.anyOf` usage. @BeforeTemplate boolean before2( Stream stream, @@ -632,8 +638,11 @@ R after( static final class StreamsConcat { @BeforeTemplate - Stream before(@Repeated Stream stream) { - return Stream.of(Refaster.asVarargs(stream)).flatMap(Refaster.anyOf(identity(), s -> s)); + Stream before( + @Repeated Stream stream, + @Matches(IsIdentityOperation.class) + Function, ? extends Stream> mapper) { + return Stream.of(Refaster.asVarargs(stream)).flatMap(mapper); } @AfterTemplate diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestInput.java index 52ff1c3b6f..b20174934f 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestInput.java @@ -31,6 +31,7 @@ ImmutableSet> testNaturalOrder() { String::compareTo, Comparator.comparing(identity()), Comparator.comparing(s -> s), + Comparator.comparing(s -> 0), Collections.reverseOrder(reverseOrder()), Comparator.reverseOrder().reversed()); } @@ -45,7 +46,8 @@ ImmutableSet> testReverseOrder() { ImmutableSet> testCustomComparator() { return ImmutableSet.of( Comparator.comparing(identity(), Comparator.comparingInt(String::length)), - Comparator.comparing(s -> s, Comparator.comparingInt(String::length))); + Comparator.comparing(s -> s, Comparator.comparingInt(String::length)), + Comparator.comparing(s -> "foo", Comparator.comparingInt(String::length))); } Comparator testThenComparing() { @@ -86,7 +88,8 @@ Comparator testThenComparingLong() { ImmutableSet> testThenComparingNaturalOrder() { return ImmutableSet.of( Comparator.naturalOrder().thenComparing(identity()), - Comparator.naturalOrder().thenComparing(s -> s)); + Comparator.naturalOrder().thenComparing(s -> s), + Comparator.naturalOrder().thenComparing(s -> 0)); } ImmutableSet testCompareTo() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestOutput.java index 7c3662bb13..088b2f01a9 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ComparatorRulesTestOutput.java @@ -28,7 +28,12 @@ public ImmutableSet elidedTypesAndStaticImports() { ImmutableSet> testNaturalOrder() { return ImmutableSet.of( - naturalOrder(), naturalOrder(), naturalOrder(), naturalOrder(), naturalOrder()); + naturalOrder(), + naturalOrder(), + naturalOrder(), + Comparator.comparing(s -> 0), + naturalOrder(), + naturalOrder()); } ImmutableSet> testReverseOrder() { @@ -38,7 +43,9 @@ ImmutableSet> testReverseOrder() { ImmutableSet> testCustomComparator() { return ImmutableSet.of( - Comparator.comparingInt(String::length), Comparator.comparingInt(String::length)); + Comparator.comparingInt(String::length), + Comparator.comparingInt(String::length), + Comparator.comparing(s -> "foo", Comparator.comparingInt(String::length))); } Comparator testThenComparing() { @@ -73,7 +80,8 @@ Comparator testThenComparingLong() { ImmutableSet> testThenComparingNaturalOrder() { return ImmutableSet.of( Comparator.naturalOrder().thenComparing(naturalOrder()), - Comparator.naturalOrder().thenComparing(naturalOrder())); + Comparator.naturalOrder().thenComparing(naturalOrder()), + Comparator.naturalOrder().thenComparing(s -> 0)); } ImmutableSet testCompareTo() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestInput.java index 6ad5befdeb..27b231042c 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestInput.java @@ -76,11 +76,22 @@ ImmutableListMultimap testStreamOfMapEntriesToImmutableListMult ImmutableSet> testIndexIterableToImmutableListMultimap() { return ImmutableSet.of( - ImmutableList.of(1).stream().collect(toImmutableListMultimap(n -> n * 2, identity())), - Streams.stream(ImmutableList.of(2)::iterator) - .collect(toImmutableListMultimap(Integer::valueOf, n -> n)), + Streams.stream(ImmutableList.of(1).iterator()) + .collect(toImmutableListMultimap(n -> n * 2, identity())), + Streams.stream(ImmutableList.of(2).iterator()) + .collect(toImmutableListMultimap(n -> n * 2, v -> v)), Streams.stream(ImmutableList.of(3).iterator()) - .collect(toImmutableListMultimap(n -> n.intValue(), identity()))); + .collect(toImmutableListMultimap(n -> n * 2, v -> 0)), + Streams.stream(ImmutableList.of(4)::iterator) + .collect(toImmutableListMultimap(Integer::valueOf, identity())), + Streams.stream(ImmutableList.of(5)::iterator) + .collect(toImmutableListMultimap(Integer::valueOf, v -> v)), + Streams.stream(ImmutableList.of(6)::iterator) + .collect(toImmutableListMultimap(Integer::valueOf, v -> 0)), + ImmutableList.of(7).stream() + .collect(toImmutableListMultimap(n -> n.intValue(), identity())), + ImmutableList.of(8).stream().collect(toImmutableListMultimap(n -> n.intValue(), v -> v)), + ImmutableList.of(9).stream().collect(toImmutableListMultimap(n -> n.intValue(), v -> 0))); } ImmutableListMultimap testTransformMultimapValuesToImmutableListMultimap() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestOutput.java index c4f5274e96..ca4dbdfd3b 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableListMultimapRulesTestOutput.java @@ -64,9 +64,17 @@ ImmutableListMultimap testStreamOfMapEntriesToImmutableListMult ImmutableSet> testIndexIterableToImmutableListMultimap() { return ImmutableSet.of( - Multimaps.index(ImmutableList.of(1), n -> n * 2), - Multimaps.index(ImmutableList.of(2)::iterator, Integer::valueOf), - Multimaps.index(ImmutableList.of(3).iterator(), n -> n.intValue())); + Multimaps.index(ImmutableList.of(1).iterator(), n -> n * 2), + Multimaps.index(ImmutableList.of(2).iterator(), n -> n * 2), + Streams.stream(ImmutableList.of(3).iterator()) + .collect(toImmutableListMultimap(n -> n * 2, v -> 0)), + Multimaps.index(ImmutableList.of(4)::iterator, Integer::valueOf), + Multimaps.index(ImmutableList.of(5)::iterator, Integer::valueOf), + Streams.stream(ImmutableList.of(6)::iterator) + .collect(toImmutableListMultimap(Integer::valueOf, v -> 0)), + Multimaps.index(ImmutableList.of(7), n -> n.intValue()), + Multimaps.index(ImmutableList.of(8), n -> n.intValue()), + ImmutableList.of(9).stream().collect(toImmutableListMultimap(n -> n.intValue(), v -> 0))); } ImmutableListMultimap testTransformMultimapValuesToImmutableListMultimap() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java index aab420c62c..2302bb4c9e 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestInput.java @@ -34,11 +34,21 @@ ImmutableSet> testEntryToImmutableMap() { ImmutableSet> testIterableToImmutableMap() { return ImmutableSet.of( ImmutableList.of(1).stream().collect(toImmutableMap(identity(), n -> n * 2)), - Streams.stream(ImmutableList.of(2)::iterator) - .collect(toImmutableMap(n -> n, Integer::valueOf)), - Streams.stream(ImmutableList.of(3).iterator()) + ImmutableList.of(2).stream().collect(toImmutableMap(k -> k, n -> n * 2)), + ImmutableList.of(3).stream().collect(toImmutableMap(k -> 0, n -> n * 2)), + Streams.stream(ImmutableList.of(4)::iterator) + .collect(toImmutableMap(identity(), Integer::valueOf)), + Streams.stream(ImmutableList.of(5)::iterator) + .collect(toImmutableMap(k -> k, Integer::valueOf)), + Streams.stream(ImmutableList.of(6)::iterator) + .collect(toImmutableMap(k -> 0, Integer::valueOf)), + Streams.stream(ImmutableList.of(7).iterator()) .collect(toImmutableMap(identity(), n -> n.intValue())), - ImmutableMap.copyOf(Maps.asMap(ImmutableSet.of(4), Integer::valueOf))); + Streams.stream(ImmutableList.of(8).iterator()) + .collect(toImmutableMap(k -> k, n -> n.intValue())), + Streams.stream(ImmutableList.of(9).iterator()) + .collect(toImmutableMap(k -> 0, n -> n.intValue())), + ImmutableMap.copyOf(Maps.asMap(ImmutableSet.of(10), Integer::valueOf))); } ImmutableSet> testEntryIterableToImmutableMap() { @@ -63,10 +73,20 @@ ImmutableMap testStreamOfMapEntriesToImmutableMap() { ImmutableSet> testIndexIterableToImmutableMap() { return ImmutableSet.of( ImmutableList.of(1).stream().collect(toImmutableMap(n -> n * 2, identity())), - Streams.stream(ImmutableList.of(2)::iterator) - .collect(toImmutableMap(Integer::valueOf, n -> n)), - Streams.stream(ImmutableList.of(3).iterator()) - .collect(toImmutableMap(n -> n.intValue(), identity()))); + ImmutableList.of(2).stream().collect(toImmutableMap(n -> n * 2, v -> v)), + ImmutableList.of(3).stream().collect(toImmutableMap(n -> n * 2, v -> 0)), + Streams.stream(ImmutableList.of(4)::iterator) + .collect(toImmutableMap(Integer::valueOf, identity())), + Streams.stream(ImmutableList.of(5)::iterator) + .collect(toImmutableMap(Integer::valueOf, v -> v)), + Streams.stream(ImmutableList.of(6)::iterator) + .collect(toImmutableMap(Integer::valueOf, v -> 0)), + Streams.stream(ImmutableList.of(7).iterator()) + .collect(toImmutableMap(n -> n.intValue(), identity())), + Streams.stream(ImmutableList.of(8).iterator()) + .collect(toImmutableMap(n -> n.intValue(), v -> v)), + Streams.stream(ImmutableList.of(9).iterator()) + .collect(toImmutableMap(n -> n.intValue(), v -> 0))); } ImmutableSet> testTransformMapValuesToImmutableMap() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java index e4bfabe76c..3365f3fd59 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ImmutableMapRulesTestOutput.java @@ -33,9 +33,17 @@ ImmutableSet> testEntryToImmutableMap() { ImmutableSet> testIterableToImmutableMap() { return ImmutableSet.of( Maps.toMap(ImmutableList.of(1), n -> n * 2), - Maps.toMap(ImmutableList.of(2)::iterator, Integer::valueOf), - Maps.toMap(ImmutableList.of(3).iterator(), n -> n.intValue()), - Maps.toMap(ImmutableSet.of(4), Integer::valueOf)); + Maps.toMap(ImmutableList.of(2), n -> n * 2), + ImmutableList.of(3).stream().collect(toImmutableMap(k -> 0, n -> n * 2)), + Maps.toMap(ImmutableList.of(4)::iterator, Integer::valueOf), + Maps.toMap(ImmutableList.of(5)::iterator, Integer::valueOf), + Streams.stream(ImmutableList.of(6)::iterator) + .collect(toImmutableMap(k -> 0, Integer::valueOf)), + Maps.toMap(ImmutableList.of(7).iterator(), n -> n.intValue()), + Maps.toMap(ImmutableList.of(8).iterator(), n -> n.intValue()), + Streams.stream(ImmutableList.of(9).iterator()) + .collect(toImmutableMap(k -> 0, n -> n.intValue())), + Maps.toMap(ImmutableSet.of(10), Integer::valueOf)); } ImmutableSet> testEntryIterableToImmutableMap() { @@ -54,8 +62,16 @@ ImmutableMap testStreamOfMapEntriesToImmutableMap() { ImmutableSet> testIndexIterableToImmutableMap() { return ImmutableSet.of( Maps.uniqueIndex(ImmutableList.of(1), n -> n * 2), - Maps.uniqueIndex(ImmutableList.of(2)::iterator, Integer::valueOf), - Maps.uniqueIndex(ImmutableList.of(3).iterator(), n -> n.intValue())); + Maps.uniqueIndex(ImmutableList.of(2), n -> n * 2), + ImmutableList.of(3).stream().collect(toImmutableMap(n -> n * 2, v -> 0)), + Maps.uniqueIndex(ImmutableList.of(4)::iterator, Integer::valueOf), + Maps.uniqueIndex(ImmutableList.of(5)::iterator, Integer::valueOf), + Streams.stream(ImmutableList.of(6)::iterator) + .collect(toImmutableMap(Integer::valueOf, v -> 0)), + Maps.uniqueIndex(ImmutableList.of(7).iterator(), n -> n.intValue()), + Maps.uniqueIndex(ImmutableList.of(8).iterator(), n -> n.intValue()), + Streams.stream(ImmutableList.of(9).iterator()) + .collect(toImmutableMap(n -> n.intValue(), v -> 0))); } ImmutableSet> testTransformMapValuesToImmutableMap() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java index c119cffe14..adcdd1243c 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java @@ -202,20 +202,26 @@ ImmutableSet> testFluxConcatMap() { return ImmutableSet.of( Flux.just(1).flatMap(Mono::just, 1), Flux.just(2).flatMapSequential(Mono::just, 1), - Flux.just(3).map(Mono::just).concatMap(identity())); + Flux.just(3).map(Mono::just).concatMap(identity()), + Flux.just(4).map(Mono::just).concatMap(v -> v), + Flux.just(5).map(Mono::just).concatMap(v -> Mono.empty())); } ImmutableSet> testFluxConcatMapWithPrefetch() { return ImmutableSet.of( Flux.just(1).flatMap(Mono::just, 1, 3), Flux.just(2).flatMapSequential(Mono::just, 1, 4), - Flux.just(3).map(Mono::just).concatMap(identity(), 5)); + Flux.just(3).map(Mono::just).concatMap(identity(), 5), + Flux.just(4).map(Mono::just).concatMap(v -> v, 6), + Flux.just(5).map(Mono::just).concatMap(v -> Mono.empty(), 7)); } ImmutableSet> testMonoFlatMapIterable() { return ImmutableSet.of( Mono.just(1).map(ImmutableSet::of).flatMapIterable(identity()), - Mono.just(2).flux().concatMapIterable(ImmutableSet::of)); + 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)); } Flux testMonoFlatMapIterableIdentity() { @@ -225,13 +231,17 @@ Flux testMonoFlatMapIterableIdentity() { ImmutableSet> testFluxConcatMapIterable() { return ImmutableSet.of( Flux.just(1).flatMapIterable(ImmutableList::of), - Flux.just(2).map(ImmutableList::of).concatMapIterable(identity())); + Flux.just(2).map(ImmutableList::of).concatMapIterable(identity()), + Flux.just(3).map(ImmutableList::of).concatMapIterable(v -> v), + Flux.just(4).map(ImmutableList::of).concatMapIterable(v -> ImmutableSet.of())); } ImmutableSet> testFluxConcatMapIterableWithPrefetch() { return ImmutableSet.of( - Flux.just(1).flatMapIterable(ImmutableList::of, 3), - Flux.just(2).map(ImmutableList::of).concatMapIterable(identity(), 3)); + Flux.just(1).flatMapIterable(ImmutableList::of, 5), + Flux.just(2).map(ImmutableList::of).concatMapIterable(identity(), 5), + Flux.just(3).map(ImmutableList::of).concatMapIterable(v -> v, 5), + Flux.just(4).map(ImmutableList::of).concatMapIterable(v -> ImmutableSet.of(), 5)); } Flux testMonoFlatMapToFlux() { @@ -372,25 +382,30 @@ Flux testFluxOfType() { return Flux.just(1).filter(Number.class::isInstance).cast(Number.class); } - Mono testMonoFlatMap() { - return Mono.just("foo").map(Mono::just).flatMap(identity()); + ImmutableSet> testMonoFlatMap() { + return ImmutableSet.of( + Mono.just("foo").map(Mono::just).flatMap(identity()), + Mono.just("bar").map(Mono::just).flatMap(v -> v), + Mono.just("baz").map(Mono::just).flatMap(v -> Mono.empty())); } ImmutableSet> testMonoFlatMapMany() { return ImmutableSet.of( Mono.just(1).map(Mono::just).flatMapMany(identity()), - Mono.just(2).flux().concatMap(Mono::just), - Mono.just(3).flux().concatMap(Mono::just, 2), - Mono.just(4).flux().concatMapDelayError(Mono::just), - Mono.just(5).flux().concatMapDelayError(Mono::just, 2), - Mono.just(6).flux().concatMapDelayError(Mono::just, false, 2), - Mono.just(7).flux().flatMap(Mono::just, 2), - Mono.just(8).flux().flatMap(Mono::just, 2, 3), - Mono.just(9).flux().flatMapDelayError(Mono::just, 2, 3), - Mono.just(10).flux().flatMapSequential(Mono::just, 2), - Mono.just(11).flux().flatMapSequential(Mono::just, 2, 3), - Mono.just(12).flux().flatMapSequentialDelayError(Mono::just, 2, 3), - Mono.just(13).flux().switchMap(Mono::just)); + Mono.just(2).map(Mono::just).flatMapMany(v -> v), + Mono.just(3).map(Mono::just).flatMapMany(v -> Flux.empty()), + Mono.just(4).flux().concatMap(Mono::just), + Mono.just(5).flux().concatMap(Mono::just, 2), + Mono.just(6).flux().concatMapDelayError(Mono::just), + Mono.just(7).flux().concatMapDelayError(Mono::just, 2), + Mono.just(8).flux().concatMapDelayError(Mono::just, false, 2), + Mono.just(9).flux().flatMap(Mono::just, 2), + Mono.just(10).flux().flatMap(Mono::just, 2, 3), + Mono.just(11).flux().flatMapDelayError(Mono::just, 2, 3), + Mono.just(12).flux().flatMapSequential(Mono::just, 2), + Mono.just(13).flux().flatMapSequential(Mono::just, 2, 3), + Mono.just(14).flux().flatMapSequentialDelayError(Mono::just, 2, 3), + Mono.just(15).flux().switchMap(Mono::just)); } ImmutableSet> testConcatMapIterableIdentity() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java index 6bbd71bf7e..0a5bf6a7f8 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestOutput.java @@ -205,20 +205,26 @@ ImmutableSet> testFluxConcatMap() { return ImmutableSet.of( Flux.just(1).concatMap(Mono::just), Flux.just(2).concatMap(Mono::just), - Flux.just(3).concatMap(Mono::just)); + Flux.just(3).concatMap(Mono::just), + Flux.just(4).concatMap(Mono::just), + Flux.just(5).map(Mono::just).concatMap(v -> Mono.empty())); } ImmutableSet> testFluxConcatMapWithPrefetch() { return ImmutableSet.of( Flux.just(1).concatMap(Mono::just, 3), Flux.just(2).concatMap(Mono::just, 4), - Flux.just(3).concatMap(Mono::just, 5)); + Flux.just(3).concatMap(Mono::just, 5), + Flux.just(4).concatMap(Mono::just, 6), + Flux.just(5).map(Mono::just).concatMap(v -> Mono.empty(), 7)); } ImmutableSet> testMonoFlatMapIterable() { return ImmutableSet.of( Mono.just(1).flatMapIterable(ImmutableSet::of), - Mono.just(2).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)); } Flux testMonoFlatMapIterableIdentity() { @@ -228,13 +234,17 @@ Flux testMonoFlatMapIterableIdentity() { ImmutableSet> testFluxConcatMapIterable() { return ImmutableSet.of( Flux.just(1).concatMapIterable(ImmutableList::of), - Flux.just(2).concatMapIterable(ImmutableList::of)); + Flux.just(2).concatMapIterable(ImmutableList::of), + Flux.just(3).concatMapIterable(ImmutableList::of), + Flux.just(4).map(ImmutableList::of).concatMapIterable(v -> ImmutableSet.of())); } ImmutableSet> testFluxConcatMapIterableWithPrefetch() { return ImmutableSet.of( - Flux.just(1).concatMapIterable(ImmutableList::of, 3), - Flux.just(2).concatMapIterable(ImmutableList::of, 3)); + Flux.just(1).concatMapIterable(ImmutableList::of, 5), + Flux.just(2).concatMapIterable(ImmutableList::of, 5), + Flux.just(3).concatMapIterable(ImmutableList::of, 5), + Flux.just(4).map(ImmutableList::of).concatMapIterable(v -> ImmutableSet.of(), 5)); } Flux testMonoFlatMapToFlux() { @@ -365,15 +375,18 @@ Flux testFluxOfType() { return Flux.just(1).ofType(Number.class); } - Mono testMonoFlatMap() { - return Mono.just("foo").flatMap(Mono::just); + ImmutableSet> testMonoFlatMap() { + return ImmutableSet.of( + Mono.just("foo").flatMap(Mono::just), + Mono.just("bar").flatMap(Mono::just), + Mono.just("baz").map(Mono::just).flatMap(v -> Mono.empty())); } ImmutableSet> testMonoFlatMapMany() { return ImmutableSet.of( Mono.just(1).flatMapMany(Mono::just), Mono.just(2).flatMapMany(Mono::just), - Mono.just(3).flatMapMany(Mono::just), + Mono.just(3).map(Mono::just).flatMapMany(v -> Flux.empty()), Mono.just(4).flatMapMany(Mono::just), Mono.just(5).flatMapMany(Mono::just), Mono.just(6).flatMapMany(Mono::just), @@ -383,7 +396,9 @@ ImmutableSet> testMonoFlatMapMany() { Mono.just(10).flatMapMany(Mono::just), Mono.just(11).flatMapMany(Mono::just), Mono.just(12).flatMapMany(Mono::just), - Mono.just(13).flatMapMany(Mono::just)); + Mono.just(13).flatMapMany(Mono::just), + Mono.just(14).flatMapMany(Mono::just), + Mono.just(15).flatMapMany(Mono::just)); } ImmutableSet> testConcatMapIterableIdentity() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java index a03da8b93c..ed6bbc3c19 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java @@ -258,7 +258,8 @@ ImmutableSet testStreamFlatMapCollect() { ImmutableSet> testStreamsConcat() { return ImmutableSet.of( Stream.of(Stream.of(1), Stream.of(2)).flatMap(identity()), - Stream.of(Stream.of(3), Stream.of(4)).flatMap(v -> v)); + Stream.of(Stream.of(3), Stream.of(4)).flatMap(v -> v), + Stream.of(Stream.of(5), Stream.of(6)).flatMap(v -> Stream.empty())); } Stream testStreamTakeWhile() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java index d26a8c589f..9ced3b0dcf 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java @@ -258,7 +258,9 @@ ImmutableSet testStreamFlatMapCollect() { ImmutableSet> testStreamsConcat() { return ImmutableSet.of( - Streams.concat(Stream.of(1), Stream.of(2)), Streams.concat(Stream.of(3), Stream.of(4))); + Streams.concat(Stream.of(1), Stream.of(2)), + Streams.concat(Stream.of(3), Stream.of(4)), + Stream.of(Stream.of(5), Stream.of(6)).flatMap(v -> Stream.empty())); } Stream testStreamTakeWhile() { diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsIdentityOperation.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsIdentityOperation.java new file mode 100644 index 0000000000..706d61428f --- /dev/null +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsIdentityOperation.java @@ -0,0 +1,54 @@ +package tech.picnic.errorprone.refaster.matchers; + +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.staticMethod; +import static com.google.errorprone.matchers.Matchers.toType; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.LambdaExpressionTree.BodyKind; +import java.util.function.DoubleUnaryOperator; +import java.util.function.Function; +import java.util.function.IntUnaryOperator; +import java.util.function.LongUnaryOperator; + +/** A matcher of expressions that represent identity operations. */ +// XXX: In selected contexts many other method invocations can be considered identity operations; +// see the `IdentityConversion` check. Review whether those can/should be captured by this matcher +// as well. +public final class IsIdentityOperation implements Matcher { + private static final long serialVersionUID = 1L; + private static final Matcher DELEGATE = + anyOf( + staticMethod() + .onDescendantOfAny( + DoubleUnaryOperator.class.getName(), + Function.class.getName(), + IntUnaryOperator.class.getName(), + LongUnaryOperator.class.getName()) + .named("identity"), + isIdentityLambdaExpression()); + + /** Instantiates a new {@link IsIdentityOperation} instance. */ + public IsIdentityOperation() {} + + @Override + public boolean matches(ExpressionTree tree, VisitorState state) { + return DELEGATE.matches(tree, state); + } + + // XXX: Also support selected block expressions, including ones that perform a vacuous parameter + // transformation such as those identified by the `IdentityConversion` check. + private static Matcher isIdentityLambdaExpression() { + return toType( + LambdaExpressionTree.class, + (tree, state) -> + tree.getBodyKind() == BodyKind.EXPRESSION + && tree.getParameters().size() == 1 + && ASTHelpers.getSymbol(tree.getParameters().get(0)) + .equals(ASTHelpers.getSymbol(tree.getBody()))); + } +} diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsIdentityOperationTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsIdentityOperationTest.java new file mode 100644 index 0000000000..324a8f784d --- /dev/null +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsIdentityOperationTest.java @@ -0,0 +1,77 @@ +package tech.picnic.errorprone.refaster.matchers; + +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.bugpatterns.BugChecker; +import org.junit.jupiter.api.Test; + +final class IsIdentityOperationTest { + @Test + void matches() { + CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "import java.util.function.BinaryOperator;", + "import java.util.function.DoubleUnaryOperator;", + "import java.util.function.Function;", + "import java.util.function.IntUnaryOperator;", + "import java.util.function.LongUnaryOperator;", + "import java.util.function.UnaryOperator;", + "", + "class A {", + " BinaryOperator negative1() {", + " return (a, b) -> a;", + " }", + "", + " UnaryOperator negative2() {", + " return a -> a + a;", + " }", + "", + " DoubleUnaryOperator positive1() {", + " // BUG: Diagnostic contains:", + " return DoubleUnaryOperator.identity();", + " }", + "", + " Function positive2() {", + " // BUG: Diagnostic contains:", + " return Function.identity();", + " }", + "", + " UnaryOperator positive3() {", + " // BUG: Diagnostic contains:", + " return UnaryOperator.identity();", + " }", + "", + " IntUnaryOperator positive4() {", + " // BUG: Diagnostic contains:", + " return IntUnaryOperator.identity();", + " }", + "", + " LongUnaryOperator positive5() {", + " // BUG: Diagnostic contains:", + " return LongUnaryOperator.identity();", + " }", + "", + " UnaryOperator positive6() {", + " // BUG: Diagnostic contains:", + " return a -> a;", + " }", + "}") + .doTest(); + } + + /** A {@link BugChecker} that simply delegates to {@link IsIdentityOperation}. */ + @BugPattern(summary = "Flags expressions matched by `IsIdentityOperation`", severity = ERROR) + public static final class MatcherTestChecker extends AbstractMatcherTestChecker { + private static final long serialVersionUID = 1L; + + // XXX: This is a false positive reported by Checkstyle. See + // https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120. + @SuppressWarnings("RedundantModifier") + public MatcherTestChecker() { + super(new IsIdentityOperation()); + } + } +}