CR remarks, adding javadoc, updating tests

PicnicSupermarket · Feb 23, 2022 · b60d642 · b60d642
1 parent 0b6b9f4
commit b60d642
Show file tree

Hide file tree

Showing 9 changed files with 156 additions and 56 deletions.
diff --git a/...ontrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java b/...ontrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java
@@ -6,7 +6,6 @@
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toList;
 
-import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Streams;
@@ -188,6 +187,10 @@ ImmutableList<T> after(Stream<T> stream) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableList#of(Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
   static final class ImmutableListOf<T> {
     @BeforeTemplate
     List<T> before() {
@@ -200,63 +203,83 @@ ImmutableList<T> after() {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableList#of(Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
   static final class ImmutableListOf1<T> {
     @BeforeTemplate
     List<T> before(T item) {
       return List.of(item);
     }
 
     @AfterTemplate
-    ImmutableCollection<T> after(T item) {
+    ImmutableList<T> after(T item) {
       return ImmutableList.of(item);
     }
   }
 
+  /**
+   * Prefer {@link ImmutableList#of(Object, Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
   static final class ImmutableListOf2<T> {
     @BeforeTemplate
-    List<T> before(T item, T item2) {
-      return List.of(item, item2);
+    List<T> before(T e1, T e2) {
+      return List.of(e1, e2);
     }
 
     @AfterTemplate
-    ImmutableCollection<T> after(T item, T item2) {
-      return ImmutableList.of(item, item2);
+    ImmutableList<T> after(T e1, T e2) {
+      return ImmutableList.of(e1, e2);
     }
   }
 
+  /**
+   * Prefer {@link ImmutableList#of(Object, Object, Object)} over alternatives that don't
+   * communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableListOf3<T> {
     @BeforeTemplate
-    List<T> before(T i1, T i2, T i3) {
-      return List.of(i1, i2, i3);
+    List<T> before(T e1, T e2, T e3) {
+      return List.of(e1, e2, e3);
     }
 
     @AfterTemplate
-    ImmutableCollection<T> after(T i1, T i2, T i3) {
-      return ImmutableList.of(i1, i2, i3);
+    ImmutableList<T> after(T e1, T e2, T e3) {
+      return ImmutableList.of(e1, e2, e3);
     }
   }
 
+  /**
+   * Prefer {@link ImmutableList#of(Object, Object, Object, Object)} over alternatives that don't
+   * communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableListOf4<T> {
     @BeforeTemplate
-    List<T> before(T i1, T i2, T i3, T i4) {
-      return List.of(i1, i2, i3, i4);
+    List<T> before(T e1, T e2, T e3, T e4) {
+      return List.of(e1, e2, e3, e4);
     }
 
     @AfterTemplate
-    ImmutableCollection<T> after(T i1, T i2, T i3, T i4) {
-      return ImmutableList.of(i1, i2, i3, i4);
+    ImmutableList<T> after(T e1, T e2, T e3, T e4) {
+      return ImmutableList.of(e1, e2, e3, e4);
     }
   }
 
+  /**
+   * Prefer {@link ImmutableList#of(Object, Object, Object, Object, Object)} over alternatives that
+   * don't communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableListOf5<T> {
     @BeforeTemplate
-    List<T> before(T i1, T i2, T i3, T i4, T i5) {
-      return List.of(i1, i2, i3, i4, i5);
+    List<T> before(T e1, T e2, T e3, T e4, T e5) {
+      return List.of(e1, e2, e3, e4, e5);
     }
 
     @AfterTemplate
-    ImmutableCollection<T> after(T i1, T i2, T i3, T i4, T i5) {
-      return ImmutableList.of(i1, i2, i3, i4, i5);
+    ImmutableList<T> after(T e1, T e2, T e3, T e4, T e5) {
+      return ImmutableList.of(e1, e2, e3, e4, e5);
     }
   }
 }
diff --git a/...contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java b/...contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java
@@ -255,6 +255,10 @@ ImmutableMap<K, V> after(ImmutableMap<K, V> map) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableMap#of()} over alternatives that don't communicate the immutability of
+   * the resulting list at the type level.
+   */
   static final class ImmutableMapOf<K, V> {
     @BeforeTemplate
     Map<K, V> before() {
@@ -267,6 +271,10 @@ ImmutableMap<K, V> after() {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableMap#of(Object, Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
   static final class ImmutableMapOf1<K, V> {
     @BeforeTemplate
     Map<K, V> before(K k1, V v1) {
@@ -279,6 +287,10 @@ ImmutableMap<K, V> after(K k1, V v1) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableMap#of(Object, Object, Object, Object)} over alternatives that don't
+   * communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableMapOf2<K, V> {
     @BeforeTemplate
     Map<K, V> before(K k1, V v1, K k2, V v2) {
@@ -291,6 +303,10 @@ ImmutableMap<K, V> after(K k1, V v1, K k2, V v2) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableMap#of(Object, Object, Object, Object, Object, Object)} over
+   * alternatives that don't communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableMapOf3<K, V> {
     @BeforeTemplate
     Map<K, V> before(K k1, V v1, K k2, V v2, K k3, V v3) {
@@ -303,6 +319,11 @@ ImmutableMap<K, V> after(K k1, V v1, K k2, V v2, K k3, V v3) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableMap#of(Object, Object, Object, Object, Object, Object, Object, Object)}
+   * over alternatives that don't communicate the immutability of the resulting list at the type
+   * level.
+   */
   static final class ImmutableMapOf4<K, V> {
     @BeforeTemplate
     Map<K, V> before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4) {
@@ -315,6 +336,11 @@ ImmutableMap<K, V> after(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableMap#of(Object, Object, Object, Object, Object, Object, Object, Object,
+   * Object, Object)} over alternatives that don't communicate the immutability of the resulting
+   * list at the type level.
+   */
   static final class ImmutableMapOf5<K, V> {
     @BeforeTemplate
     Map<K, V> before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V v5) {
@@ -326,6 +352,7 @@ ImmutableMap<K, V> after(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V
       return ImmutableMap.of(k1, v1, k2, v2, k3, v3, k4, v4, k5, v5);
     }
   }
+
   // XXX: Add a template for this:
   // Maps.transformValues(streamOfEntries.collect(groupBy(fun)), ImmutableMap::copyOf)
   // ->

diff --git a/...contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java b/...contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java
@@ -152,6 +152,10 @@ ImmutableSet<T> after(SetView<T> set) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableSet#of()} over alternatives that don't communicate the immutability of
+   * the resulting list at the type level.
+   */
   static final class ImmutableSetOf<T> {
     @BeforeTemplate
     Set<T> before() {
@@ -164,6 +168,10 @@ ImmutableSet<T> after() {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableSet#of(Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
   static final class ImmutableSetOfItems1<T> {
     @BeforeTemplate
     Set<T> before(T e1) {
@@ -176,6 +184,10 @@ ImmutableSet<T> after(T e1) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableSet#of(Object, Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
   static final class ImmutableSetOfItems2<T> {
     @BeforeTemplate
     Set<T> before(T e1, T e2) {
@@ -188,6 +200,10 @@ ImmutableSet<T> after(T e1, T e2) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableSet#of(Object, Object, Object)} over alternatives that don't communicate
+   * the immutability of the resulting list at the type level.
+   */
   static final class ImmutableSetOfItems3<T> {
     @BeforeTemplate
     Set<T> before(T e1, T e2, T e3) {
@@ -200,6 +216,10 @@ ImmutableSet<T> after(T e1, T e2, T e3) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableSet#of(Object, Object, Object, Object)} over alternatives that don't
+   * communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableSetOfItems4<T> {
     @BeforeTemplate
     Set<T> before(T e1, T e2, T e3, T e4) {
@@ -212,6 +232,10 @@ ImmutableSet<T> after(T e1, T e2, T e3, T e4) {
     }
   }
 
+  /**
+   * Prefer {@link ImmutableSet#of(Object, Object, Object, Object, Object)} over alternatives that
+   * don't communicate the immutability of the resulting list at the type level.
+   */
   static final class ImmutableSetOfItems5<T> {
     @BeforeTemplate
     Set<T> before(T e1, T e2, T e3, T e4, T e5) {

diff --git a/...rc/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java b/...rc/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java
@@ -90,23 +90,23 @@ List<String> testImmutableListOfTyped() {
     return Collections.emptyList();
   }
 
-  Collection<String> testImmutableListOf1() {
+  List<String> testImmutableListOf1() {
     return List.of("1");
   }
 
-  Collection<String> testImmutableListOf2() {
+  List<String> testImmutableListOf2() {
     return List.of("1", "2");
   }
 
-  Collection<String> testImmutableListOf3() {
+  List<String> testImmutableListOf3() {
     return List.of("1", "2", "3");
   }
 
-  Collection<String> testImmutableListOf4() {
+  List<String> testImmutableListOf4() {
     return List.of("1", "2", "3", "4");
   }
 
-  Collection<String> testImmutableListOf5() {
+  List<String> testImmutableListOf5() {
     return List.of("1", "2", "3", "4", "5");
   }
 }
diff --git a/...c/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestOutput.java b/...c/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestOutput.java
@@ -85,23 +85,23 @@ List<String> testImmutableListOfTyped() {
     return ImmutableList.of();
   }
 
-  Collection<String> testImmutableListOf1() {
+  List<String> testImmutableListOf1() {
     return ImmutableList.of("1");
   }
 
-  Collection<String> testImmutableListOf2() {
+  List<String> testImmutableListOf2() {
     return ImmutableList.of("1", "2");
   }
 
-  Collection<String> testImmutableListOf3() {
+  List<String> testImmutableListOf3() {
     return ImmutableList.of("1", "2", "3");
   }
 
-  Collection<String> testImmutableListOf4() {
+  List<String> testImmutableListOf4() {
     return ImmutableList.of("1", "2", "3", "4");
   }
 
-  Collection<String> testImmutableListOf5() {
+  List<String> testImmutableListOf5() {
     return ImmutableList.of("1", "2", "3", "4", "5");
   }
 }
diff --git a/...src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java b/...src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java
@@ -95,13 +95,27 @@ Map<String, String> testImmutableMapOfTyped() {
     return Collections.emptyMap();
   }
 
-  ImmutableSet<Map<?, ?>> testImmutableMapOfN() {
-    return ImmutableSet.of(
-        Collections.emptyMap(),
-        Map.of("k1", "v1"),
-        Map.of("k1", "v1", "k2", "v2"),
-        Map.of("k1", "v1", "k2", "v2", "k3", "v3"),
-        Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"),
-        Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"));
+  Map<?, ?> testImmutableMapOf() {
+    return Collections.emptyMap();
+  }
+
+  Map<String, String> testImmutableMapOf1() {
+    return Map.of("k1", "v1");
+  }
+
+  Map<String, String> testImmutableMapOf2() {
+    return Map.of("k1", "v1", "k2", "v2");
+  }
+
+  Map<String, String> testImmutableMapOf3() {
+    return Map.of("k1", "v1", "k2", "v2", "k3", "v3");
+  }
+
+  Map<String, String> testImmutableMapOf4() {
+    return Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4");
+  }
+
+  Map<String, String> testImmutableMapOf5() {
+    return Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5");
   }
 }
diff --git a/...rc/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestOutput.java b/...rc/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestOutput.java
@@ -81,13 +81,27 @@ Map<String, String> testImmutableMapOfTyped() {
     return ImmutableMap.of();
   }
 
-  ImmutableSet<Map<?, ?>> testImmutableMapOfN() {
-    return ImmutableSet.of(
-        ImmutableMap.of(),
-        ImmutableMap.of("k1", "v1"),
-        ImmutableMap.of("k1", "v1", "k2", "v2"),
-        ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3"),
-        ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"),
-        ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"));
+  Map<?, ?> testImmutableMapOf() {
+    return ImmutableMap.of();
+  }
+
+  Map<String, String> testImmutableMapOf1() {
+    return ImmutableMap.of("k1", "v1");
+  }
+
+  Map<String, String> testImmutableMapOf2() {
+    return ImmutableMap.of("k1", "v1", "k2", "v2");
+  }
+
+  Map<String, String> testImmutableMapOf3() {
+    return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3");
+  }
+
+  Map<String, String> testImmutableMapOf4() {
+    return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4");
+  }
+
+  Map<String, String> testImmutableMapOf5() {
+    return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5");
   }
 }