From 09d870e26be80c4d4cf790a2e937727d83b5c239 Mon Sep 17 00:00:00 2001 From: Gonzalo Amestoy Date: Mon, 5 Dec 2022 23:43:19 +0100 Subject: [PATCH 1/2] Introduce CollectionForEach Refaster rule --- .../errorprone/refasterrules/CollectionRules.java | 14 ++++++++++++++ .../refasterrules/CollectionRulesTestInput.java | 4 ++++ .../refasterrules/CollectionRulesTestOutput.java | 4 ++++ 3 files changed, 22 insertions(+) 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 19df9107db..796da37d5a 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 @@ -17,6 +17,7 @@ import java.util.Queue; import java.util.Set; import java.util.SortedSet; +import java.util.function.Consumer; import java.util.function.IntFunction; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; @@ -404,6 +405,19 @@ Optional after(Queue queue) { } } + /** Prefer calling {@link Collection#forEach} ()} over more contrived alternatives. */ + static final class CollectionForEach { + @BeforeTemplate + void before(Collection collection, Consumer consumer) { + collection.stream().forEach(consumer); + } + + @AfterTemplate + void after(Collection collection, Consumer consumer) { + collection.forEach(consumer); + } + } + // XXX: collection.stream().noneMatch(e -> e.equals(other)) // ^ This is !collection.contains(other). Do we already rewrite variations on this? } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java index c5c3f686d5..68f6f79bfb 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java @@ -186,4 +186,8 @@ ImmutableSet> testRemoveOptionalFirstQueueElement() { ? Optional.ofNullable(new LinkedList().remove()) : Optional.empty()); } + + void testCollectionForEach() { + ImmutableSet.of(1).stream().forEach(String::valueOf); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java index 7d580729cc..0ebaa40f3b 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java @@ -136,4 +136,8 @@ ImmutableSet> testRemoveOptionalFirstQueueElement() { Optional.ofNullable(new LinkedList().poll()), Optional.ofNullable(new LinkedList().poll())); } + + void testCollectionForEach() { + ImmutableSet.of(1).forEach(String::valueOf); + } } From 67b4b9a7990e9f02baa8a85b934ba43f1d502078 Mon Sep 17 00:00:00 2001 From: Gonzalo Amestoy Date: Tue, 6 Dec 2022 08:24:34 +0100 Subject: [PATCH 2/2] Define a more generic rule for Consumer --- .../picnic/errorprone/refasterrules/CollectionRules.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 796da37d5a..90dc4f183f 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 @@ -405,15 +405,15 @@ Optional after(Queue queue) { } } - /** Prefer calling {@link Collection#forEach} ()} over more contrived alternatives. */ + /** Prefer {@link Collection#forEach(Consumer)} over more contrived alternatives. */ static final class CollectionForEach { @BeforeTemplate - void before(Collection collection, Consumer consumer) { + void before(Collection collection, Consumer consumer) { collection.stream().forEach(consumer); } @AfterTemplate - void after(Collection collection, Consumer consumer) { + void after(Collection collection, Consumer consumer) { collection.forEach(consumer); } }