diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterCheck.java index 01c2ce9eaa..7cd69a5afd 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterCheck.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterCheck.java @@ -2,6 +2,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableRangeSet.toImmutableRangeSet; +import static java.util.function.Predicate.not; import com.google.auto.service.AutoService; import com.google.common.annotations.VisibleForTesting; @@ -10,6 +11,7 @@ import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableRangeSet; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; import com.google.common.reflect.ClassPath; @@ -143,6 +145,7 @@ private static ImmutableRangeSet getReplacementRanges( Description description, EndPosTable endPositions) { return getReplacements(description, endPositions) .map(Replacement::range) + .filter(not(Range::isEmpty)) .collect(toImmutableRangeSet()); } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java index 803b8013c7..aea3761bb7 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java @@ -10,11 +10,13 @@ import com.google.errorprone.refaster.annotation.BeforeTemplate; import java.util.ArrayList; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.NavigableSet; import java.util.Optional; import java.util.Queue; import java.util.SortedSet; +import java.util.function.IntFunction; import java.util.stream.Stream; /** Refaster templates related to expressions dealing with (arbitrary) collections. */ @@ -37,6 +39,11 @@ boolean before(Collection collection) { Iterables.isEmpty(collection)); } + @BeforeTemplate + boolean before(ImmutableCollection collection) { + return collection.asList().isEmpty(); + } + @AfterTemplate @AlsoNegation boolean after(Collection collection) { @@ -51,6 +58,11 @@ int before(Collection collection) { return Iterables.size(collection); } + @BeforeTemplate + int before(ImmutableCollection collection) { + return collection.asList().size(); + } + @AfterTemplate int after(Collection collection) { return collection.size(); @@ -175,16 +187,6 @@ ImmutableList after(ImmutableCollection collection) { * Don't call {@link ImmutableCollection#asList()} if the result is going to be streamed; stream * directly. */ - // XXX: Similar rules could be implemented for the following variants: - // collection.asList().contains(null); - // collection.asList().isEmpty(); - // collection.asList().iterator(); - // collection.asList().parallelStream(); - // collection.asList().size(); - // collection.asList().toArray(); - // collection.asList().toArray(Object[]::new); - // collection.asList().toArray(new Object[0]); - // collection.asList().toString(); static final class ImmutableCollectionAsListToStream { @BeforeTemplate Stream before(ImmutableCollection collection) { @@ -197,6 +199,121 @@ Stream after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if {@link Collection#contains(Object)} is + * called on the result; call it directly. + */ + static final class ImmutableCollectionContains { + @BeforeTemplate + boolean before(ImmutableCollection collection, S elem) { + return collection.asList().contains(elem); + } + + @AfterTemplate + boolean after(ImmutableCollection collection, S elem) { + return collection.contains(elem); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#parallelStream()} + * is called on the result; call it directly. + */ + static final class ImmutableCollectionParallelStream { + @BeforeTemplate + Stream before(ImmutableCollection collection) { + return collection.asList().parallelStream(); + } + + @AfterTemplate + Stream after(ImmutableCollection collection) { + return collection.parallelStream(); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toString()} is + * called on the result; call it directly. + */ + static final class ImmutableCollectionToString { + @BeforeTemplate + String before(ImmutableCollection collection) { + return collection.asList().toString(); + } + + @AfterTemplate + String after(ImmutableCollection collection) { + return collection.toString(); + } + } + + /** Prefer calling {@link Collection#toArray()} over more contrived alternatives. */ + static final class CollectionToArray { + @BeforeTemplate + Object[] before(Collection collection, int size) { + return Refaster.anyOf( + collection.toArray(new Object[size]), collection.toArray(Object[]::new)); + } + + @BeforeTemplate + Object[] before(ImmutableCollection collection) { + return collection.asList().toArray(); + } + + @AfterTemplate + Object[] after(Collection collection) { + return collection.toArray(); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if {@link + * ImmutableCollection#toArray(Object[])}` is called on the result; call it directly. + */ + static final class ImmutableCollectionToArrayWithArray { + @BeforeTemplate + Object[] before(ImmutableCollection collection, S[] array) { + return collection.asList().toArray(array); + } + + @AfterTemplate + Object[] after(ImmutableCollection collection, S[] array) { + return collection.toArray(array); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if {@link + * ImmutableCollection#toArray(IntFunction)}} is called on the result; call it directly. + */ + static final class ImmutableCollectionToArrayWithGenerator { + @BeforeTemplate + S[] before(ImmutableCollection collection, IntFunction generator) { + return collection.asList().toArray(generator); + } + + @AfterTemplate + S[] after(ImmutableCollection collection, IntFunction generator) { + return collection.toArray(generator); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#iterator()} is + * called on the result; call it directly. + */ + static final class ImmutableCollectionIterator { + @BeforeTemplate + Iterator before(ImmutableCollection collection) { + return collection.asList().iterator(); + } + + @AfterTemplate + Iterator after(ImmutableCollection collection) { + return collection.iterator(); + } + } + /** * Don't use the ternary operator to extract the first element of a possibly-empty {@link * Collection} as an {@link Optional}. diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestInput.java index d8ab773139..3e67f56412 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestInput.java @@ -6,6 +6,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedList; import java.util.Optional; import java.util.TreeSet; @@ -25,11 +26,12 @@ ImmutableSet testCollectionIsEmpty() { ImmutableSet.of(4).size() != 0, ImmutableSet.of(5).size() > 0, ImmutableSet.of(6).size() >= 1, - Iterables.isEmpty(ImmutableSet.of(7))); + Iterables.isEmpty(ImmutableSet.of(7)), + ImmutableSet.of(8).asList().isEmpty()); } - int testCollectionSize() { - return Iterables.size(ImmutableSet.of()); + ImmutableSet testCollectionSize() { + return ImmutableSet.of(Iterables.size(ImmutableSet.of(1)), ImmutableSet.of(2).asList().size()); } boolean testCollectionAddAllToCollectionExpression() { @@ -64,7 +66,7 @@ ArrayList testNewArrayListFromCollection() { return Lists.newArrayList(ImmutableList.of("foo")); } - Stream testImmutableCollectionAsListToStream() { + Stream testImmutableCollectionStream() { return ImmutableSet.of(1).asList().stream(); } @@ -72,6 +74,37 @@ ImmutableList testImmutableCollectionAsList() { return ImmutableList.copyOf(ImmutableSet.of(1)); } + boolean testImmutableCollectionContains() { + return ImmutableSet.of(1).asList().contains("foo"); + } + + Stream testImmutableCollectionParallelStream() { + return ImmutableSet.of(1).asList().parallelStream(); + } + + String testImmutableCollectionToString() { + return ImmutableSet.of(1).asList().toString(); + } + + ImmutableSet testCollectionToArray() { + return ImmutableSet.of( + ImmutableSet.of(1).toArray(new Object[1]), + ImmutableSet.of(2).toArray(Object[]::new), + ImmutableSet.of(3).asList().toArray()); + } + + Integer[] testImmutableCollectionToArrayWithArray() { + return ImmutableSet.of(1).asList().toArray(new Integer[0]); + } + + Integer[] testImmutableCollectionToArrayWithGenerator() { + return ImmutableSet.of(1).asList().toArray(Integer[]::new); + } + + Iterator testImmutableCollectionIterator() { + return ImmutableSet.of(1).asList().iterator(); + } + ImmutableSet> testOptionalFirstCollectionElement() { return ImmutableSet.of( ImmutableSet.of(0).stream().findAny(), diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestOutput.java index e461402a68..970e5e346d 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/CollectionTemplatesTestOutput.java @@ -6,6 +6,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedList; import java.util.Optional; import java.util.TreeSet; @@ -25,11 +26,12 @@ ImmutableSet testCollectionIsEmpty() { !ImmutableSet.of(4).isEmpty(), !ImmutableSet.of(5).isEmpty(), !ImmutableSet.of(6).isEmpty(), - ImmutableSet.of(7).isEmpty()); + ImmutableSet.of(7).isEmpty(), + ImmutableSet.of(8).isEmpty()); } - int testCollectionSize() { - return ImmutableSet.of().size(); + ImmutableSet testCollectionSize() { + return ImmutableSet.of(ImmutableSet.of(1).size(), ImmutableSet.of(2).size()); } boolean testCollectionAddAllToCollectionExpression() { @@ -56,7 +58,7 @@ ArrayList testNewArrayListFromCollection() { return new ArrayList<>(ImmutableList.of("foo")); } - Stream testImmutableCollectionAsListToStream() { + Stream testImmutableCollectionStream() { return ImmutableSet.of(1).stream(); } @@ -64,6 +66,35 @@ ImmutableList testImmutableCollectionAsList() { return ImmutableSet.of(1).asList(); } + boolean testImmutableCollectionContains() { + return ImmutableSet.of(1).contains("foo"); + } + + Stream testImmutableCollectionParallelStream() { + return ImmutableSet.of(1).parallelStream(); + } + + String testImmutableCollectionToString() { + return ImmutableSet.of(1).toString(); + } + + ImmutableSet testCollectionToArray() { + return ImmutableSet.of( + ImmutableSet.of(1).toArray(), ImmutableSet.of(2).toArray(), ImmutableSet.of(3).toArray()); + } + + Integer[] testImmutableCollectionToArrayWithArray() { + return ImmutableSet.of(1).toArray(new Integer[0]); + } + + Integer[] testImmutableCollectionToArrayWithGenerator() { + return ImmutableSet.of(1).toArray(Integer[]::new); + } + + Iterator testImmutableCollectionIterator() { + return ImmutableSet.of(1).iterator(); + } + ImmutableSet> testOptionalFirstCollectionElement() { return ImmutableSet.of( ImmutableSet.of(0).stream().findFirst(), diff --git a/pom.xml b/pom.xml index 01fff48fea..c5aced757e 100644 --- a/pom.xml +++ b/pom.xml @@ -1014,7 +1014,7 @@ pitest-maven 1.6.2 - 4