From b60d6429385274b86671fe983a661a98f92ca36f Mon Sep 17 00:00:00 2001 From: Vitor Mussatto Date: Wed, 23 Feb 2022 13:12:36 +0100 Subject: [PATCH] CR remarks, adding javadoc, updating tests --- .../ImmutableListTemplates.java | 59 +++++++++++++------ .../ImmutableMapTemplates.java | 27 +++++++++ .../ImmutableSetTemplates.java | 24 ++++++++ .../ImmutableListTemplatesTestInput.java | 10 ++-- .../ImmutableListTemplatesTestOutput.java | 10 ++-- .../ImmutableMapTemplatesTestInput.java | 30 +++++++--- .../ImmutableMapTemplatesTestOutput.java | 30 +++++++--- .../ImmutableSetTemplatesTestInput.java | 11 ++-- .../ImmutableSetTemplatesTestOutput.java | 11 ++-- 9 files changed, 156 insertions(+), 56 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java index 9c5f382f453..52a3f7690b5 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java +++ b/error-prone-contrib/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 after(Stream 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 { @BeforeTemplate List before() { @@ -200,6 +203,10 @@ ImmutableList 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 { @BeforeTemplate List before(T item) { @@ -207,56 +214,72 @@ List before(T item) { } @AfterTemplate - ImmutableCollection after(T item) { + ImmutableList 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 { @BeforeTemplate - List before(T item, T item2) { - return List.of(item, item2); + List before(T e1, T e2) { + return List.of(e1, e2); } @AfterTemplate - ImmutableCollection after(T item, T item2) { - return ImmutableList.of(item, item2); + ImmutableList 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 { @BeforeTemplate - List before(T i1, T i2, T i3) { - return List.of(i1, i2, i3); + List before(T e1, T e2, T e3) { + return List.of(e1, e2, e3); } @AfterTemplate - ImmutableCollection after(T i1, T i2, T i3) { - return ImmutableList.of(i1, i2, i3); + ImmutableList 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 { @BeforeTemplate - List before(T i1, T i2, T i3, T i4) { - return List.of(i1, i2, i3, i4); + List before(T e1, T e2, T e3, T e4) { + return List.of(e1, e2, e3, e4); } @AfterTemplate - ImmutableCollection after(T i1, T i2, T i3, T i4) { - return ImmutableList.of(i1, i2, i3, i4); + ImmutableList 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 { @BeforeTemplate - List before(T i1, T i2, T i3, T i4, T i5) { - return List.of(i1, i2, i3, i4, i5); + List before(T e1, T e2, T e3, T e4, T e5) { + return List.of(e1, e2, e3, e4, e5); } @AfterTemplate - ImmutableCollection after(T i1, T i2, T i3, T i4, T i5) { - return ImmutableList.of(i1, i2, i3, i4, i5); + ImmutableList after(T e1, T e2, T e3, T e4, T e5) { + return ImmutableList.of(e1, e2, e3, e4, e5); } } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java index e5467815027..efd6cf58ec2 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java @@ -255,6 +255,10 @@ ImmutableMap after(ImmutableMap 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 { @BeforeTemplate Map before() { @@ -267,6 +271,10 @@ ImmutableMap 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 { @BeforeTemplate Map before(K k1, V v1) { @@ -279,6 +287,10 @@ ImmutableMap 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 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2) { @@ -291,6 +303,10 @@ ImmutableMap 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 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2, K k3, V v3) { @@ -303,6 +319,11 @@ ImmutableMap 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 { @BeforeTemplate Map before(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4) { @@ -315,6 +336,11 @@ ImmutableMap 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 { @BeforeTemplate Map 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 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/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java index 6b3976a7999..3f7cab17567 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java @@ -152,6 +152,10 @@ ImmutableSet after(SetView 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 { @BeforeTemplate Set before() { @@ -164,6 +168,10 @@ ImmutableSet 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 { @BeforeTemplate Set before(T e1) { @@ -176,6 +184,10 @@ ImmutableSet 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 { @BeforeTemplate Set before(T e1, T e2) { @@ -188,6 +200,10 @@ ImmutableSet 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 { @BeforeTemplate Set before(T e1, T e2, T e3) { @@ -200,6 +216,10 @@ ImmutableSet 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 { @BeforeTemplate Set before(T e1, T e2, T e3, T e4) { @@ -212,6 +232,10 @@ ImmutableSet 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 { @BeforeTemplate Set before(T e1, T e2, T e3, T e4, T e5) { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java index f4b56376dba..498462a147e 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java @@ -90,23 +90,23 @@ List testImmutableListOfTyped() { return Collections.emptyList(); } - Collection testImmutableListOf1() { + List testImmutableListOf1() { return List.of("1"); } - Collection testImmutableListOf2() { + List testImmutableListOf2() { return List.of("1", "2"); } - Collection testImmutableListOf3() { + List testImmutableListOf3() { return List.of("1", "2", "3"); } - Collection testImmutableListOf4() { + List testImmutableListOf4() { return List.of("1", "2", "3", "4"); } - Collection testImmutableListOf5() { + List testImmutableListOf5() { return List.of("1", "2", "3", "4", "5"); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestOutput.java index 18d9b559012..e0373231482 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestOutput.java @@ -85,23 +85,23 @@ List testImmutableListOfTyped() { return ImmutableList.of(); } - Collection testImmutableListOf1() { + List testImmutableListOf1() { return ImmutableList.of("1"); } - Collection testImmutableListOf2() { + List testImmutableListOf2() { return ImmutableList.of("1", "2"); } - Collection testImmutableListOf3() { + List testImmutableListOf3() { return ImmutableList.of("1", "2", "3"); } - Collection testImmutableListOf4() { + List testImmutableListOf4() { return ImmutableList.of("1", "2", "3", "4"); } - Collection testImmutableListOf5() { + List testImmutableListOf5() { return ImmutableList.of("1", "2", "3", "4", "5"); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java index 7d85ac68f75..246b76b6250 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java @@ -95,13 +95,27 @@ Map testImmutableMapOfTyped() { return Collections.emptyMap(); } - ImmutableSet> 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 testImmutableMapOf1() { + return Map.of("k1", "v1"); + } + + Map testImmutableMapOf2() { + return Map.of("k1", "v1", "k2", "v2"); + } + + Map testImmutableMapOf3() { + return Map.of("k1", "v1", "k2", "v2", "k3", "v3"); + } + + Map testImmutableMapOf4() { + return Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"); + } + + Map testImmutableMapOf5() { + return Map.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestOutput.java index 52bff610353..1ca9b27f4a7 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestOutput.java @@ -81,13 +81,27 @@ Map testImmutableMapOfTyped() { return ImmutableMap.of(); } - ImmutableSet> 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 testImmutableMapOf1() { + return ImmutableMap.of("k1", "v1"); + } + + Map testImmutableMapOf2() { + return ImmutableMap.of("k1", "v1", "k2", "v2"); + } + + Map testImmutableMapOf3() { + return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3"); + } + + Map testImmutableMapOf4() { + return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4"); + } + + Map testImmutableMapOf5() { + return ImmutableMap.of("k1", "v1", "k2", "v2", "k3", "v3", "k4", "v4", "k5", "v5"); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestInput.java index 1ab5fa40fcc..63aa2bab3ad 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestInput.java @@ -10,7 +10,6 @@ import com.google.common.collect.Sets; import com.google.common.collect.Streams; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Set; import java.util.stream.Stream; @@ -76,23 +75,23 @@ Set testImmutableSetOfTyped() { return Collections.emptySet(); } - Collection testImmutableSetOfItems1() { + Set testImmutableSetOfItems1() { return Set.of("1"); } - Collection testImmutableSetOfItems2() { + Set testImmutableSetOfItems2() { return Set.of("1", "2"); } - Collection testImmutableSetOfItems3() { + Set testImmutableSetOfItems3() { return Set.of("1", "2", "3"); } - Collection testImmutableSetOfItems4() { + Set testImmutableSetOfItems4() { return Set.of("1", "2", "3", "4"); } - Collection testImmutableSetOfItems5() { + Set testImmutableSetOfItems5() { return Set.of("1", "2", "3", "4", "5"); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestOutput.java index 480eecb3f54..8d2ae98b98f 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestOutput.java @@ -10,7 +10,6 @@ import com.google.common.collect.Sets; import com.google.common.collect.Streams; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Set; import java.util.stream.Stream; @@ -75,23 +74,23 @@ Set testImmutableSetOfTyped() { return ImmutableSet.of(); } - Collection testImmutableSetOfItems1() { + Set testImmutableSetOfItems1() { return ImmutableSet.of("1"); } - Collection testImmutableSetOfItems2() { + Set testImmutableSetOfItems2() { return ImmutableSet.of("1", "2"); } - Collection testImmutableSetOfItems3() { + Set testImmutableSetOfItems3() { return ImmutableSet.of("1", "2", "3"); } - Collection testImmutableSetOfItems4() { + Set testImmutableSetOfItems4() { return ImmutableSet.of("1", "2", "3", "4"); } - Collection testImmutableSetOfItems5() { + Set testImmutableSetOfItems5() { return ImmutableSet.of("1", "2", "3", "4", "5"); } }