Add a few ImmutableCollection Refaster templates (#8)

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<Integer> getReplacementRanges(
       Description description, EndPosTable endPositions) {
     return getReplacements(description, endPositions)
         .map(Replacement::range)
+        .filter(not(Range::isEmpty))
         .collect(toImmutableRangeSet());
   }
 

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<T> collection) {
           Iterables.isEmpty(collection));
     }
 
+    @BeforeTemplate
+    boolean before(ImmutableCollection<T> collection) {
+      return collection.asList().isEmpty();
+    }
+
     @AfterTemplate
     @AlsoNegation
     boolean after(Collection<T> collection) {
@@ -51,6 +58,11 @@ int before(Collection<T> collection) {
       return Iterables.size(collection);
     }
 
+    @BeforeTemplate
+    int before(ImmutableCollection<T> collection) {
+      return collection.asList().size();
+    }
+
     @AfterTemplate
     int after(Collection<T> collection) {
       return collection.size();
@@ -175,16 +187,6 @@ ImmutableList<T> after(ImmutableCollection<T> 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<T> {
     @BeforeTemplate
     Stream<T> before(ImmutableCollection<T> collection) {
@@ -197,6 +199,121 @@ Stream<T> after(ImmutableCollection<T> collection) {
     }
   }
 
+  /**
+   * Don't call {@link ImmutableCollection#asList()} if {@link Collection#contains(Object)} is
+   * called on the result; call it directly.
+   */
+  static final class ImmutableCollectionContains<T, S> {
+    @BeforeTemplate
+    boolean before(ImmutableCollection<T> collection, S elem) {
+      return collection.asList().contains(elem);
+    }
+
+    @AfterTemplate
+    boolean after(ImmutableCollection<T> 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<T> {
+    @BeforeTemplate
+    Stream<T> before(ImmutableCollection<T> collection) {
+      return collection.asList().parallelStream();
+    }
+
+    @AfterTemplate
+    Stream<T> after(ImmutableCollection<T> 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<T> {
+    @BeforeTemplate
+    String before(ImmutableCollection<T> collection) {
+      return collection.asList().toString();
+    }
+
+    @AfterTemplate
+    String after(ImmutableCollection<T> collection) {
+      return collection.toString();
+    }
+  }
+
+  /** Prefer calling {@link Collection#toArray()} over more contrived alternatives. */
+  static final class CollectionToArray<T> {
+    @BeforeTemplate
+    Object[] before(Collection<T> collection, int size) {
+      return Refaster.anyOf(
+          collection.toArray(new Object[size]), collection.toArray(Object[]::new));
+    }
+
+    @BeforeTemplate
+    Object[] before(ImmutableCollection<T> collection) {
+      return collection.asList().toArray();
+    }
+
+    @AfterTemplate
+    Object[] after(Collection<T> 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<T, S> {
+    @BeforeTemplate
+    Object[] before(ImmutableCollection<T> collection, S[] array) {
+      return collection.asList().toArray(array);
+    }
+
+    @AfterTemplate
+    Object[] after(ImmutableCollection<T> 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<T, S> {
+    @BeforeTemplate
+    S[] before(ImmutableCollection<T> collection, IntFunction<S[]> generator) {
+      return collection.asList().toArray(generator);
+    }
+
+    @AfterTemplate
+    S[] after(ImmutableCollection<T> collection, IntFunction<S[]> 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<T> {
+    @BeforeTemplate
+    Iterator<T> before(ImmutableCollection<T> collection) {
+      return collection.asList().iterator();
+    }
+
+    @AfterTemplate
+    Iterator<T> after(ImmutableCollection<T> 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}.

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<Boolean> 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<Integer> testCollectionSize() {
+    return ImmutableSet.of(Iterables.size(ImmutableSet.of(1)), ImmutableSet.of(2).asList().size());
   }
 
   boolean testCollectionAddAllToCollectionExpression() {
@@ -64,14 +66,45 @@ ArrayList<String> testNewArrayListFromCollection() {
     return Lists.newArrayList(ImmutableList.of("foo"));
   }
 
-  Stream<Integer> testImmutableCollectionAsListToStream() {
+  Stream<Integer> testImmutableCollectionStream() {
     return ImmutableSet.of(1).asList().stream();
   }
 
   ImmutableList<Integer> testImmutableCollectionAsList() {
     return ImmutableList.copyOf(ImmutableSet.of(1));
   }
 
+  boolean testImmutableCollectionContains() {
+    return ImmutableSet.of(1).asList().contains("foo");
+  }
+
+  Stream<Integer> testImmutableCollectionParallelStream() {
+    return ImmutableSet.of(1).asList().parallelStream();
+  }
+
+  String testImmutableCollectionToString() {
+    return ImmutableSet.of(1).asList().toString();
+  }
+
+  ImmutableSet<Object[]> 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<Integer> testImmutableCollectionIterator() {
+    return ImmutableSet.of(1).asList().iterator();
+  }
+
   ImmutableSet<Optional<Integer>> testOptionalFirstCollectionElement() {
     return ImmutableSet.of(
         ImmutableSet.of(0).stream().findAny(),

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<Boolean> 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<Integer> testCollectionSize() {
+    return ImmutableSet.of(ImmutableSet.of(1).size(), ImmutableSet.of(2).size());
   }
 
   boolean testCollectionAddAllToCollectionExpression() {
@@ -56,14 +58,43 @@ ArrayList<String> testNewArrayListFromCollection() {
     return new ArrayList<>(ImmutableList.of("foo"));
   }
 
-  Stream<Integer> testImmutableCollectionAsListToStream() {
+  Stream<Integer> testImmutableCollectionStream() {
     return ImmutableSet.of(1).stream();
   }
 
   ImmutableList<Integer> testImmutableCollectionAsList() {
     return ImmutableSet.of(1).asList();
   }
 
+  boolean testImmutableCollectionContains() {
+    return ImmutableSet.of(1).contains("foo");
+  }
+
+  Stream<Integer> testImmutableCollectionParallelStream() {
+    return ImmutableSet.of(1).parallelStream();
+  }
+
+  String testImmutableCollectionToString() {
+    return ImmutableSet.of(1).toString();
+  }
+
+  ImmutableSet<Object[]> 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<Integer> testImmutableCollectionIterator() {
+    return ImmutableSet.of(1).iterator();
+  }
+
   ImmutableSet<Optional<Integer>> testOptionalFirstCollectionElement() {
     return ImmutableSet.of(
         ImmutableSet.of(0).stream().findFirst(),

pom.xml

@@ -1014,7 +1014,7 @@
                     <artifactId>pitest-maven</artifactId>
                     <version>1.6.2</version>
                     <configuration>
-                        <!-- Use multple threads to speed things up. Extend
+                        <!-- Use multiple threads to speed things up. Extend
                         timeouts to prevent false positives as a result of
                         contention. -->
                         <threads>4</threads>