From 989d4c5c69caf960df8dc277eccfc8a6c739ec7c Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 11 Feb 2024 11:16:40 +0100 Subject: [PATCH] Compact and replace `StreamIs{,Not}Empty` Refaster rules 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. --- .../errorprone/refasterrules/StreamRules.java | 17 +++++++++-------- .../refasterrules/StreamRulesTestInput.java | 15 +++++++-------- .../refasterrules/StreamRulesTestOutput.java | 15 +++++++-------- 3 files changed, 23 insertions(+), 24 deletions(-) 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 5ab7f9de537..e7bea741423 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 01cd245601d..5126726c3bf 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 0d865cba59b..a11b7ca644c 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() {