From f8cac193302c79b8c1432f7caef639e0966e73d7 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Fri, 22 Mar 2024 08:23:18 +0100 Subject: [PATCH] Compact and replace `StreamIs{,Not}Empty` Refaster rules (#1028) The new `StreamFindAnyIs{Empty,Present}` rules are simpler thanks to the use of `@AlsoNegation`. In some cases an additional application of the `OptionalIsEmpty` rule will be required. --- .../refasterrules/CollectionRules.java | 2 +- .../errorprone/refasterrules/StreamRules.java | 17 +++++++++-------- .../refasterrules/StreamRulesTestInput.java | 15 +++++++-------- .../refasterrules/StreamRulesTestOutput.java | 15 +++++++-------- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java index cd341e791d..08edc53a5b 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java @@ -39,7 +39,7 @@ static final class CollectionIsEmpty { "java:S1155" /* This violation will be rewritten. */, "LexicographicalAnnotationAttributeListing" /* `key-*` entry must remain last. */, "OptionalFirstCollectionElement" /* This is a more specific template. */, - "StreamIsEmpty" /* This is a more specific template. */, + "StreamFindAnyIsEmpty" /* This is a more specific template. */, "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict" }) boolean before(Collection collection) { 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 5ab7f9de53..e7bea74142 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 @@ -25,6 +25,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.AlsoNegation; import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.google.errorprone.refaster.annotation.Matches; import com.google.errorprone.refaster.annotation.MayOptionallyUse; @@ -256,7 +257,7 @@ Optional after(Stream stream, Function function) { // XXX: This rule assumes that any matched `Collector` does not perform any filtering. // (Perhaps we could add a `@Matches` guard that validates that the collector expression does not // contain a `Collectors#filtering` call. That'd still not be 100% accurate, though.) - static final class StreamIsEmpty, M extends Map> { + static final class StreamFindAnyIsEmpty, M extends Map> { @BeforeTemplate boolean before(Stream stream, Collector collector) { return Refaster.anyOf( @@ -274,20 +275,20 @@ boolean before2(Stream stream, Collector collector } @AfterTemplate + @AlsoNegation boolean after(Stream stream) { return stream.findAny().isEmpty(); } } - /** In order to test whether a stream has any element, simply try to find one. */ - static final class StreamIsNotEmpty { + /** + * Prefer {@link Stream#findAny()} over {@link Stream#findFirst()} if one only cares whether the + * stream is nonempty. + */ + static final class StreamFindAnyIsPresent { @BeforeTemplate boolean before(Stream stream) { - return Refaster.anyOf( - stream.count() != 0, - stream.count() > 0, - stream.count() >= 1, - stream.findFirst().isPresent()); + return stream.findFirst().isPresent(); } @AfterTemplate 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 01cd245601..5126726c3b 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 @@ -120,7 +120,7 @@ ImmutableSet> testStreamMapFirst() { Stream.of("bar").map(String::length).findFirst()); } - ImmutableSet testStreamIsEmpty() { + ImmutableSet testStreamFindAnyIsEmpty() { return ImmutableSet.of( Stream.of(1).count() == 0, Stream.of(2).count() <= 0, @@ -131,15 +131,14 @@ ImmutableSet testStreamIsEmpty() { Stream.of(7).collect(collectingAndThen(toImmutableList(), ImmutableList::isEmpty)), Stream.of(8).collect(collectingAndThen(toImmutableMap(k -> k, v -> v), Map::isEmpty)), Stream.of(9) - .collect(collectingAndThen(toImmutableMap(k -> k, v -> v), ImmutableMap::isEmpty))); + .collect(collectingAndThen(toImmutableMap(k -> k, v -> v), ImmutableMap::isEmpty)), + Stream.of(10).count() != 0, + Stream.of(11).count() > 0, + Stream.of(12).count() >= 1); } - ImmutableSet testStreamIsNotEmpty() { - return ImmutableSet.of( - Stream.of(1).count() != 0, - Stream.of(2).count() > 0, - Stream.of(3).count() >= 1, - Stream.of(4).findFirst().isPresent()); + boolean testStreamFindAnyIsPresent() { + return Stream.of(1).findFirst().isPresent(); } ImmutableSet> testStreamMin() { 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 0d865cba59..a11b7ca644 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 @@ -121,7 +121,7 @@ ImmutableSet> testStreamMapFirst() { Stream.of("bar").findFirst().map(String::length)); } - ImmutableSet testStreamIsEmpty() { + ImmutableSet testStreamFindAnyIsEmpty() { return ImmutableSet.of( Stream.of(1).findAny().isEmpty(), Stream.of(2).findAny().isEmpty(), @@ -131,15 +131,14 @@ ImmutableSet testStreamIsEmpty() { Stream.of(6).findAny().isEmpty(), Stream.of(7).findAny().isEmpty(), Stream.of(8).findAny().isEmpty(), - Stream.of(9).findAny().isEmpty()); + Stream.of(9).findAny().isEmpty(), + !Stream.of(10).findAny().isEmpty(), + !Stream.of(11).findAny().isEmpty(), + !Stream.of(12).findAny().isEmpty()); } - ImmutableSet testStreamIsNotEmpty() { - return ImmutableSet.of( - Stream.of(1).findAny().isPresent(), - Stream.of(2).findAny().isPresent(), - Stream.of(3).findAny().isPresent(), - Stream.of(4).findAny().isPresent()); + boolean testStreamFindAnyIsPresent() { + return Stream.of(1).findAny().isPresent(); } ImmutableSet> testStreamMin() {