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

Extend StreamRules Refaster rule collection #605

Merged
merged 4 commits into from
May 4, 2023
Merged

Extend StreamRules Refaster rule collection #605

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
37989ec
Extend `StreamRules` Refaster rule collection
Stephan202 Apr 29, 2023
976725a
Add a comment
Stephan202 May 3, 2023
e146b5e
One more idea
Stephan202 May 3, 2023
70b4554
Drop `this`
rickie May 3, 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
33 changes: 31 additions & 2 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -245,14 +246,18 @@ Optional<S> after(Stream<T> stream, Function<? super T, S> function) {
}

/** In order to test whether a stream has any element, simply try to find one. */
// 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<T> {
@BeforeTemplate
boolean before(Stream<T> stream) {
boolean before(Stream<T> stream, Collector<? super T, ?, ? extends Collection<?>> collector) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it nicer to have a separate before template for this one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion. (Something we could enforce one way or another with automation...)

return Refaster.anyOf(
stream.count() == 0,
stream.count() <= 0,
stream.count() < 1,
stream.findFirst().isEmpty());
stream.findFirst().isEmpty(),
stream.collect(collector).isEmpty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't you theoretically use a filtering collector though?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! We should add a comment calling out this caveat. I'll add a commit later this morning 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added two more comments. Since this kind of is rare (and since we rewrite top-level filtering collectors to a regular filter step), I think this is an acceptable limitation for now.

}

@AfterTemplate
Expand Down Expand Up @@ -347,6 +352,14 @@ boolean before(Stream<T> stream, Predicate<? super T> predicate) {
stream.filter(predicate).findAny().isEmpty());
}

@BeforeTemplate
boolean before2(
Stream<T> stream,
@Matches(IsLambdaExpressionOrMethodReference.class)
Function<? super T, Boolean> predicate) {
return stream.map(predicate).noneMatch(Refaster.anyOf(Boolean::booleanValue, b -> b));
}

@AfterTemplate
boolean after(Stream<T> stream, Predicate<? super T> predicate) {
return stream.noneMatch(predicate);
Expand Down Expand Up @@ -377,6 +390,14 @@ boolean before(Stream<T> stream, Predicate<? super T> predicate) {
!stream.noneMatch(predicate), stream.filter(predicate).findAny().isPresent());
}

@BeforeTemplate
boolean before2(
Stream<T> stream,
@Matches(IsLambdaExpressionOrMethodReference.class)
Function<? super T, Boolean> predicate) {
return stream.map(predicate).anyMatch(Refaster.anyOf(Boolean::booleanValue, b -> b));
}

@AfterTemplate
boolean after(Stream<T> stream, Predicate<? super T> predicate) {
return stream.anyMatch(predicate);
Expand All @@ -389,6 +410,14 @@ boolean before(Stream<T> stream, Predicate<? super T> predicate) {
return stream.noneMatch(Refaster.anyOf(not(predicate), predicate.negate()));
}

@BeforeTemplate
boolean before2(
Stream<T> stream,
@Matches(IsLambdaExpressionOrMethodReference.class)
Function<? super T, Boolean> predicate) {
return stream.map(predicate).allMatch(Refaster.anyOf(Boolean::booleanValue, b -> b));
}

@AfterTemplate
boolean after(Stream<T> stream, Predicate<? super T> predicate) {
return stream.allMatch(predicate);
Expand Down
21 changes: 17 additions & 4 deletions ...contrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ ImmutableSet<Boolean> 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<Boolean> testStreamIsNotEmpty() {
Expand Down Expand Up @@ -146,11 +147,15 @@ ImmutableSet<Optional<String>> testStreamMaxNaturalOrder() {

ImmutableSet<Boolean> testStreamNoneMatch() {
Predicate<String> pred = String::isBlank;
Function<String, Boolean> 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<Boolean> testStreamNoneMatch2() {
Expand All @@ -159,16 +164,24 @@ ImmutableSet<Boolean> testStreamNoneMatch2() {
}

ImmutableSet<Boolean> testStreamAnyMatch() {
Function<String, Boolean> 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<Boolean> testStreamAllMatch() {
Predicate<String> pred = String::isBlank;
Function<String, Boolean> 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() {
Expand Down
23 changes: 19 additions & 4 deletions ...ontrib/src/test/resources/tech/picnic/errorprone/refasterrules/StreamRulesTestOutput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ ImmutableSet<Boolean> 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<Boolean> testStreamIsNotEmpty() {
Expand Down Expand Up @@ -147,11 +148,15 @@ ImmutableSet<Optional<String>> testStreamMaxNaturalOrder() {

ImmutableSet<Boolean> testStreamNoneMatch() {
Predicate<String> pred = String::isBlank;
Function<String, Boolean> 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<Boolean> testStreamNoneMatch2() {
Expand All @@ -160,14 +165,24 @@ ImmutableSet<Boolean> testStreamNoneMatch2() {
}

ImmutableSet<Boolean> testStreamAnyMatch() {
Function<String, Boolean> 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<Boolean> testStreamAllMatch() {
Predicate<String> pred = String::isBlank;
Function<String, Boolean> 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() {
Expand Down