From 2c2d0dfd70754e2ff20d138cf2103bea7b0659cf Mon Sep 17 00:00:00 2001 From: mohamedsamehsalah Date: Sat, 10 Aug 2024 13:15:02 +0200 Subject: [PATCH 1/2] Introduce `ArraysAsList` refaster rule --- .../errorprone/refasterrules/StreamRules.java | 16 ++++++++++++++++ .../refasterrules/StreamRulesTestInput.java | 5 +++++ .../refasterrules/StreamRulesTestOutput.java | 4 ++++ 3 files changed, 25 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 e7bea74142..656f3b2c75 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 @@ -38,6 +38,7 @@ import java.util.Comparator; import java.util.DoubleSummaryStatistics; import java.util.IntSummaryStatistics; +import java.util.List; import java.util.LongSummaryStatistics; import java.util.Map; import java.util.Objects; @@ -122,6 +123,21 @@ Stream after(T[] array) { } } + /** + * Prefer {@link Arrays#asList(Object[])} over {@link Stream#toList()} as the former is clearer. + */ + static final class ArraysAsList { + @BeforeTemplate + List before(@NotMatches(IsRefasterAsVarargs.class) T[] array) { + return Arrays.stream(array).toList(); + } + + @AfterTemplate + List after(T[] array) { + return Arrays.asList(array); + } + } + /** Don't unnecessarily call {@link Streams#concat(Stream...)}. */ static final class ConcatOneStream { @BeforeTemplate 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 5126726c3b..8a73ecf4d9 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 @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Streams; +import java.util.Arrays; import java.util.DoubleSummaryStatistics; import java.util.IntSummaryStatistics; import java.util.List; @@ -86,6 +87,10 @@ Stream testStreamOfArray() { return Stream.of(new String[] {"foo", "bar"}); } + List testArraysAsList() { + return Arrays.stream(new String[] {"foo", "bar"}).toList(); + } + Stream testConcatOneStream() { return Streams.concat(Stream.of(1)); } 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 a11b7ca644..52a32ce5ff 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 @@ -87,6 +87,10 @@ Stream testStreamOfArray() { return Arrays.stream(new String[] {"foo", "bar"}); } + List testArraysAsList() { + return Arrays.asList(new String[] {"foo", "bar"}); + } + Stream testConcatOneStream() { return Stream.of(1); } From 47016e50e54aa433d3a6204bc82aba8e1efed28c Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 10 Aug 2024 16:06:33 +0200 Subject: [PATCH 2/2] Suggestions --- .../refasterrules/CollectionRules.java | 20 +++++++++++++++++++ .../errorprone/refasterrules/StreamRules.java | 16 --------------- .../CollectionRulesTestInput.java | 6 ++++++ .../CollectionRulesTestOutput.java | 6 ++++++ .../refasterrules/StreamRulesTestInput.java | 5 ----- .../refasterrules/StreamRulesTestOutput.java | 4 ---- 6 files changed, 32 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 ecbc7e51f4..4ee37189e1 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 @@ -8,7 +8,9 @@ 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.NotMatches; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -21,6 +23,7 @@ import java.util.function.IntFunction; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; +import tech.picnic.errorprone.refaster.matchers.IsRefasterAsVarargs; /** Refaster rules related to expressions dealing with (arbitrary) collections. */ // XXX: There are other Guava `Iterables` methods that should not be called if the input is known to @@ -294,6 +297,23 @@ String after(ImmutableCollection collection) { } } + /** Prefer {@link Arrays#asList(Object[])} over more contrived alternatives. */ + // XXX: Consider moving this rule to `ImmutableListRules` and having it suggest + // `ImmutableList#copyOf`. That would retain immutability, at the cost of no longer handling + // `null`s. + static final class ArraysAsList { + // XXX: This expression produces an unmodifiable list, while the alternative doesn't. + @BeforeTemplate + List before(@NotMatches(IsRefasterAsVarargs.class) T[] array) { + return Arrays.stream(array).toList(); + } + + @AfterTemplate + List after(T[] array) { + return Arrays.asList(array); + } + } + /** Prefer calling {@link Collection#toArray()} over more contrived alternatives. */ static final class CollectionToArray { @BeforeTemplate 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 656f3b2c75..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 @@ -38,7 +38,6 @@ import java.util.Comparator; import java.util.DoubleSummaryStatistics; import java.util.IntSummaryStatistics; -import java.util.List; import java.util.LongSummaryStatistics; import java.util.Map; import java.util.Objects; @@ -123,21 +122,6 @@ Stream after(T[] array) { } } - /** - * Prefer {@link Arrays#asList(Object[])} over {@link Stream#toList()} as the former is clearer. - */ - static final class ArraysAsList { - @BeforeTemplate - List before(@NotMatches(IsRefasterAsVarargs.class) T[] array) { - return Arrays.stream(array).toList(); - } - - @AfterTemplate - List after(T[] array) { - return Arrays.asList(array); - } - } - /** Don't unnecessarily call {@link Streams#concat(Stream...)}. */ static final class ConcatOneStream { @BeforeTemplate 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 2968c3a093..c667a80895 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 @@ -6,9 +6,11 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; +import java.util.List; import java.util.Optional; import java.util.TreeSet; import java.util.stream.Stream; @@ -98,6 +100,10 @@ String testImmutableCollectionToString() { return ImmutableSet.of(1).asList().toString(); } + List testArraysAsList() { + return Arrays.stream(new String[0]).toList(); + } + ImmutableSet testCollectionToArray() { return ImmutableSet.of( ImmutableSet.of(1).toArray(new Object[1]), 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 1eb0abc0c3..756135c889 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 @@ -6,9 +6,11 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; +import java.util.List; import java.util.Optional; import java.util.TreeSet; import java.util.stream.Stream; @@ -90,6 +92,10 @@ String testImmutableCollectionToString() { return ImmutableSet.of(1).toString(); } + List testArraysAsList() { + return Arrays.asList(new String[0]); + } + ImmutableSet testCollectionToArray() { return ImmutableSet.of( ImmutableSet.of(1).toArray(), ImmutableSet.of(2).toArray(), ImmutableSet.of(3).toArray()); 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 8a73ecf4d9..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 @@ -27,7 +27,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Streams; -import java.util.Arrays; import java.util.DoubleSummaryStatistics; import java.util.IntSummaryStatistics; import java.util.List; @@ -87,10 +86,6 @@ Stream testStreamOfArray() { return Stream.of(new String[] {"foo", "bar"}); } - List testArraysAsList() { - return Arrays.stream(new String[] {"foo", "bar"}).toList(); - } - Stream testConcatOneStream() { return Streams.concat(Stream.of(1)); } 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 52a32ce5ff..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 @@ -87,10 +87,6 @@ Stream testStreamOfArray() { return Arrays.stream(new String[] {"foo", "bar"}); } - List testArraysAsList() { - return Arrays.asList(new String[] {"foo", "bar"}); - } - Stream testConcatOneStream() { return Stream.of(1); }