From 37989ec33e4b0244376c9ebc08a3d71c57f6b677 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 29 Apr 2023 16:16:48 +0200 Subject: [PATCH 1/4] Extend `StreamRules` Refaster rule collection All changes suggested by SonarCloud's `java:s4034` rule, as well as the examples mentioned in openrewrite/rewrite#2984 are now covered. (In a number of cases through composition of more generic rules.) See https://rules.sonarsource.com/java/RSPEC-4034 --- .../errorprone/refasterrules/StreamRules.java | 30 +++++++++++++++++-- .../refasterrules/StreamRulesTestInput.java | 21 ++++++++++--- .../refasterrules/StreamRulesTestOutput.java | 23 +++++++++++--- 3 files changed, 64 insertions(+), 10 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 c1b1180bcc..50e701519e 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 @@ -28,6 +28,7 @@ import com.google.errorprone.refaster.annotation.Placeholder; import com.google.errorprone.refaster.annotation.UseImportPolicy; import java.util.Arrays; +import java.util.Collection; import java.util.Comparator; import java.util.DoubleSummaryStatistics; import java.util.IntSummaryStatistics; @@ -247,12 +248,13 @@ Optional after(Stream stream, Function function) { /** In order to test whether a stream has any element, simply try to find one. */ static final class StreamIsEmpty { @BeforeTemplate - boolean before(Stream stream) { + boolean before(Stream stream, Collector> collector) { return Refaster.anyOf( stream.count() == 0, stream.count() <= 0, stream.count() < 1, - stream.findFirst().isEmpty()); + stream.findFirst().isEmpty(), + stream.collect(collector).isEmpty()); } @AfterTemplate @@ -347,6 +349,14 @@ boolean before(Stream stream, Predicate predicate) { stream.filter(predicate).findAny().isEmpty()); } + @BeforeTemplate + boolean before2( + Stream stream, + @Matches(IsLambdaExpressionOrMethodReference.class) + Function predicate) { + return stream.map(predicate).noneMatch(Refaster.anyOf(Boolean::booleanValue, b -> b)); + } + @AfterTemplate boolean after(Stream stream, Predicate predicate) { return stream.noneMatch(predicate); @@ -377,6 +387,14 @@ boolean before(Stream stream, Predicate predicate) { !stream.noneMatch(predicate), stream.filter(predicate).findAny().isPresent()); } + @BeforeTemplate + boolean before2( + Stream stream, + @Matches(IsLambdaExpressionOrMethodReference.class) + Function predicate) { + return stream.map(predicate).anyMatch(Refaster.anyOf(Boolean::booleanValue, b -> b)); + } + @AfterTemplate boolean after(Stream stream, Predicate predicate) { return stream.anyMatch(predicate); @@ -389,6 +407,14 @@ boolean before(Stream stream, Predicate predicate) { return stream.noneMatch(Refaster.anyOf(not(predicate), predicate.negate())); } + @BeforeTemplate + boolean before2( + Stream stream, + @Matches(IsLambdaExpressionOrMethodReference.class) + Function predicate) { + return stream.map(predicate).allMatch(Refaster.anyOf(Boolean::booleanValue, b -> b)); + } + @AfterTemplate boolean after(Stream stream, Predicate predicate) { return stream.allMatch(predicate); 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 c568e49507..48f0de13a7 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 @@ -109,7 +109,8 @@ ImmutableSet testStreamIsEmpty() { Stream.of(1).count() == 0, Stream.of(2).count() <= 0, Stream.of(3).count() < 1, - Stream.of(4).findFirst().isEmpty()); + Stream.of(4).findFirst().isEmpty(), + Stream.of(5).collect(toImmutableSet()).isEmpty()); } ImmutableSet testStreamIsNotEmpty() { @@ -146,11 +147,15 @@ ImmutableSet> testStreamMaxNaturalOrder() { ImmutableSet testStreamNoneMatch() { Predicate pred = String::isBlank; + Function toBooleanFunction = Boolean::valueOf; return ImmutableSet.of( !Stream.of("foo").anyMatch(s -> s.length() > 1), Stream.of("bar").allMatch(not(String::isBlank)), Stream.of("baz").allMatch(pred.negate()), - Stream.of("qux").filter(String::isEmpty).findAny().isEmpty()); + Stream.of("qux").filter(String::isEmpty).findAny().isEmpty(), + Stream.of("quux").map(s -> s.isBlank()).noneMatch(Boolean::booleanValue), + Stream.of("quuz").map(Boolean::valueOf).noneMatch(r -> r), + Stream.of("corge").map(toBooleanFunction).noneMatch(Boolean::booleanValue)); } ImmutableSet testStreamNoneMatch2() { @@ -159,16 +164,24 @@ ImmutableSet testStreamNoneMatch2() { } ImmutableSet testStreamAnyMatch() { + Function toBooleanFunction = Boolean::valueOf; return ImmutableSet.of( !Stream.of("foo").noneMatch(s -> s.length() > 1), - Stream.of("bar").filter(String::isEmpty).findAny().isPresent()); + Stream.of("bar").filter(String::isEmpty).findAny().isPresent(), + Stream.of("baz").map(s -> s.isBlank()).anyMatch(Boolean::booleanValue), + Stream.of("qux").map(Boolean::valueOf).anyMatch(r -> r), + Stream.of("quux").map(toBooleanFunction).anyMatch(Boolean::booleanValue)); } ImmutableSet testStreamAllMatch() { Predicate pred = String::isBlank; + Function toBooleanFunction = Boolean::valueOf; return ImmutableSet.of( Stream.of("foo").noneMatch(not(String::isBlank)), - Stream.of("bar").noneMatch(pred.negate())); + Stream.of("bar").noneMatch(pred.negate()), + Stream.of("baz").map(s -> s.isBlank()).allMatch(Boolean::booleanValue), + Stream.of("qux").map(Boolean::valueOf).allMatch(r -> r), + Stream.of("quux").map(toBooleanFunction).anyMatch(Boolean::booleanValue)); } boolean testStreamAllMatch2() { 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 fb7484f6b5..7ed6bbd4ae 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 @@ -110,7 +110,8 @@ ImmutableSet testStreamIsEmpty() { Stream.of(1).findAny().isEmpty(), Stream.of(2).findAny().isEmpty(), Stream.of(3).findAny().isEmpty(), - Stream.of(4).findAny().isEmpty()); + Stream.of(4).findAny().isEmpty(), + Stream.of(5).findAny().isEmpty()); } ImmutableSet testStreamIsNotEmpty() { @@ -147,11 +148,15 @@ ImmutableSet> testStreamMaxNaturalOrder() { ImmutableSet testStreamNoneMatch() { Predicate pred = String::isBlank; + Function toBooleanFunction = Boolean::valueOf; return ImmutableSet.of( Stream.of("foo").noneMatch(s -> s.length() > 1), Stream.of("bar").noneMatch(String::isBlank), Stream.of("baz").noneMatch(pred), - Stream.of("qux").noneMatch(String::isEmpty)); + Stream.of("qux").noneMatch(String::isEmpty), + Stream.of("quux").noneMatch(s -> s.isBlank()), + Stream.of("quuz").noneMatch(Boolean::valueOf), + Stream.of("corge").map(toBooleanFunction).noneMatch(Boolean::booleanValue)); } ImmutableSet testStreamNoneMatch2() { @@ -160,14 +165,24 @@ ImmutableSet testStreamNoneMatch2() { } ImmutableSet testStreamAnyMatch() { + Function toBooleanFunction = Boolean::valueOf; return ImmutableSet.of( - Stream.of("foo").anyMatch(s -> s.length() > 1), Stream.of("bar").anyMatch(String::isEmpty)); + Stream.of("foo").anyMatch(s -> s.length() > 1), + Stream.of("bar").anyMatch(String::isEmpty), + Stream.of("baz").anyMatch(s -> s.isBlank()), + Stream.of("qux").anyMatch(Boolean::valueOf), + Stream.of("quux").map(toBooleanFunction).anyMatch(Boolean::booleanValue)); } ImmutableSet testStreamAllMatch() { Predicate pred = String::isBlank; + Function toBooleanFunction = Boolean::valueOf; return ImmutableSet.of( - Stream.of("foo").allMatch(String::isBlank), Stream.of("bar").allMatch(pred)); + Stream.of("foo").allMatch(String::isBlank), + Stream.of("bar").allMatch(pred), + Stream.of("baz").allMatch(s -> s.isBlank()), + Stream.of("qux").allMatch(Boolean::valueOf), + Stream.of("quux").map(toBooleanFunction).anyMatch(Boolean::booleanValue)); } boolean testStreamAllMatch2() { From 976725addb79a7b8d41809b37be557c83e2afcd0 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 3 May 2023 10:35:26 +0200 Subject: [PATCH 2/4] Add a comment --- .../java/tech/picnic/errorprone/refasterrules/StreamRules.java | 1 + 1 file changed, 1 insertion(+) 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 50e701519e..e3bbe9c2c0 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 @@ -246,6 +246,7 @@ Optional after(Stream stream, Function function) { } /** In order to test whether a stream has any element, simply try to find one. */ + // XXX: This this rule assumes that any matched `Collector` does not perform any filtering. static final class StreamIsEmpty { @BeforeTemplate boolean before(Stream stream, Collector> collector) { From e146b5e79c5eb625c5fbe12a25a3f6aedb9c9800 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 3 May 2023 10:39:23 +0200 Subject: [PATCH 3/4] One more idea --- .../java/tech/picnic/errorprone/refasterrules/StreamRules.java | 2 ++ 1 file changed, 2 insertions(+) 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 e3bbe9c2c0..a256574534 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 @@ -247,6 +247,8 @@ Optional after(Stream stream, Function function) { /** In order to test whether a stream has any element, simply try to find one. */ // XXX: This 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 { @BeforeTemplate boolean before(Stream stream, Collector> collector) { From 70b45543a4bc4734ea78391e607041abe6bc2bfe Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Wed, 3 May 2023 14:31:23 +0200 Subject: [PATCH 4/4] Drop `this` --- .../java/tech/picnic/errorprone/refasterrules/StreamRules.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a256574534..604abf1f74 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 @@ -246,7 +246,7 @@ Optional after(Stream stream, Function function) { } /** In order to test whether a stream has any element, simply try to find one. */ - // XXX: This this rule assumes that any matched `Collector` does not perform any filtering. + // 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 {