From 052e02c71e776eab92dc0fc4d5b99b506dbb8507 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 11:48:34 +0100 Subject: [PATCH 01/15] Add first refaster template with tests, immutableCollectionAsListIsEmpty --- .../CollectionTemplates.java | 19 ++++++++++++++++++- .../CollectionTemplatesTestInput.java | 4 ++++ .../CollectionTemplatesTestOutput.java | 4 ++++ 3 files changed, 26 insertions(+), 1 deletion(-) 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..22dc77bffb 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 @@ -177,7 +177,6 @@ ImmutableList after(ImmutableCollection collection) { */ // 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(); @@ -197,6 +196,24 @@ Stream after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#isEmpty()} is + * called on the result; call it directly. + */ + static final class ImmutableCollectionAsListIsEmpty { + @BeforeTemplate + boolean before(ImmutableCollection collection) { + // XXX: @Stephan this one can also fit in at: CollectionIsEmpty in the @Before, ~line 25. + // I don't know what the convention would be for that. + return collection.asList().isEmpty(); + } + + @AfterTemplate + boolean after(ImmutableCollection collection) { + return collection.isEmpty(); + } + } + /** * 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..aaa7a4062c 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 @@ -68,6 +68,10 @@ Stream testImmutableCollectionAsListToStream() { return ImmutableSet.of(1).asList().stream(); } + boolean testImmutableCollectionAsListIsEmpty() { + return ImmutableSet.of(1).asList().isEmpty(); + } + ImmutableList testImmutableCollectionAsList() { return ImmutableList.copyOf(ImmutableSet.of(1)); } 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..56de3ee8e4 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 @@ -60,6 +60,10 @@ Stream testImmutableCollectionAsListToStream() { return ImmutableSet.of(1).stream(); } + boolean testImmutableCollectionAsListIsEmpty() { + return ImmutableSet.of(1).isEmpty(); + } + ImmutableList testImmutableCollectionAsList() { return ImmutableSet.of(1).asList(); } From a8fd6e8f21cb4bc1269549b886bab4c22136dc21 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 12:57:04 +0100 Subject: [PATCH 02/15] Add ImmutableCollectionsAsListContainsNull and improve the previous one --- .../CollectionTemplates.java | 22 +++++++++++++++---- .../CollectionTemplatesTestInput.java | 14 ++++++++---- .../CollectionTemplatesTestOutput.java | 12 ++++++---- 3 files changed, 36 insertions(+), 12 deletions(-) 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 22dc77bffb..7e4572df96 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 @@ -176,9 +176,7 @@ ImmutableList after(ImmutableCollection collection) { * directly. */ // XXX: Similar rules could be implemented for the following variants: - // collection.asList().contains(null); // collection.asList().iterator(); - // collection.asList().parallelStream(); // collection.asList().size(); // collection.asList().toArray(); // collection.asList().toArray(Object[]::new); @@ -197,8 +195,8 @@ Stream after(ImmutableCollection collection) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#isEmpty()} is - * called on the result; call it directly. + * Don't call {@link ImmutableCollection#asList()} if `isEmpty()` is called on the result; call it + * directly. */ static final class ImmutableCollectionAsListIsEmpty { @BeforeTemplate @@ -214,6 +212,22 @@ boolean after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if `contains(null)` is called on the result; + * call it directly. + */ + static final class ImmutableCollectionAsListContainsNull { + @BeforeTemplate + boolean before(ImmutableCollection collection) { + return collection.asList().contains(null); + } + + @AfterTemplate + boolean after(ImmutableCollection collection) { + return collection.contains(null); + } + } + /** * 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 aaa7a4062c..d514e99dc4 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 @@ -64,16 +64,22 @@ ArrayList testNewArrayListFromCollection() { return Lists.newArrayList(ImmutableList.of("foo")); } + ImmutableList testImmutableCollectionAsList() { + return ImmutableList.copyOf(ImmutableSet.of(1)); + } + Stream testImmutableCollectionAsListToStream() { return ImmutableSet.of(1).asList().stream(); } - boolean testImmutableCollectionAsListIsEmpty() { - return ImmutableSet.of(1).asList().isEmpty(); + ImmutableSet testImmutableCollectionAsListIsEmpty() { + return ImmutableSet.of( + ImmutableSet.of(1).asList().isEmpty(), ImmutableSet.of().asList().isEmpty()); } - ImmutableList testImmutableCollectionAsList() { - return ImmutableList.copyOf(ImmutableSet.of(1)); + ImmutableSet testImmutableCollectionAsListContainsNull() { + return ImmutableSet.of( + ImmutableSet.of(1).asList().contains(null), ImmutableSet.of().asList().contains(null)); } ImmutableSet> testOptionalFirstCollectionElement() { 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 56de3ee8e4..49472a0a8b 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 @@ -56,16 +56,20 @@ ArrayList testNewArrayListFromCollection() { return new ArrayList<>(ImmutableList.of("foo")); } + ImmutableList testImmutableCollectionAsList() { + return ImmutableSet.of(1).asList(); + } + Stream testImmutableCollectionAsListToStream() { return ImmutableSet.of(1).stream(); } - boolean testImmutableCollectionAsListIsEmpty() { - return ImmutableSet.of(1).isEmpty(); + ImmutableSet testImmutableCollectionAsListIsEmpty() { + return ImmutableSet.of(ImmutableSet.of(1).isEmpty(), ImmutableSet.of().isEmpty()); } - ImmutableList testImmutableCollectionAsList() { - return ImmutableSet.of(1).asList(); + ImmutableSet testImmutableCollectionAsListContainsNull() { + return ImmutableSet.of(ImmutableSet.of(1).contains(null), ImmutableSet.of().contains(null)); } ImmutableSet> testOptionalFirstCollectionElement() { From b63ca0b63ba1f5bd83298abb0e47bf64749b0fc3 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 14:59:01 +0100 Subject: [PATCH 03/15] Add two new templates ToString and Size --- .../CollectionTemplates.java | 54 +++++++++++++++++-- .../CollectionTemplatesTestInput.java | 12 +++++ .../CollectionTemplatesTestOutput.java | 12 +++++ 3 files changed, 73 insertions(+), 5 deletions(-) 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 7e4572df96..8503a7598a 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 @@ -177,11 +177,6 @@ ImmutableList after(ImmutableCollection collection) { */ // XXX: Similar rules could be implemented for the following variants: // collection.asList().iterator(); - // 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) { @@ -228,6 +223,55 @@ boolean after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if `parallelStream()` is called on the result; + * call it directly. + */ + static final class ImmutableCollectionAsListParallelStream { + @BeforeTemplate + Stream before(ImmutableCollection collection) { + return collection.asList().parallelStream(); + } + + @AfterTemplate + Stream after(ImmutableCollection collection) { + return collection.parallelStream(); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if `size()` is called on the result; call it + * directly. + */ + static final class ImmutableCollectionAsListSize { + @BeforeTemplate + int before(ImmutableCollection collection) { + // XXX: Again, this one can also be added to the CollectionSize example, ~line 44. + return collection.asList().size(); + } + + @AfterTemplate + int after(ImmutableCollection collection) { + return collection.size(); + } + } + + /** + * Don't call {@link ImmutableCollection#asList()} if `toString()` is called on the result; call + * it directly. + */ + static final class ImmutableCollectionAsListToString { + @BeforeTemplate + String before(ImmutableCollection collection) { + return collection.asList().toString(); + } + + @AfterTemplate + String after(ImmutableCollection collection) { + return collection.toString(); + } + } + /** * 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 d514e99dc4..77166e0a81 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 @@ -82,6 +82,18 @@ ImmutableSet testImmutableCollectionAsListContainsNull() { ImmutableSet.of(1).asList().contains(null), ImmutableSet.of().asList().contains(null)); } + Stream testImmutableCollectionAsListParallelStream() { + return ImmutableSet.of(1).asList().parallelStream(); + } + + int testImmutableCollectionAsListSize() { + return ImmutableSet.of(1).asList().size(); + } + + String testImmutableCollectionAsListToString() { + return ImmutableSet.of(1).asList().toString(); + } + 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 49472a0a8b..4f2b6af3dd 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 @@ -72,6 +72,18 @@ ImmutableSet testImmutableCollectionAsListContainsNull() { return ImmutableSet.of(ImmutableSet.of(1).contains(null), ImmutableSet.of().contains(null)); } + Stream testImmutableCollectionAsListParallelStream() { + return ImmutableSet.of(1).parallelStream(); + } + + int testImmutableCollectionAsListSize() { + return ImmutableSet.of(1).size(); + } + + String testImmutableCollectionAsListToString() { + return ImmutableSet.of(1).toString(); + } + ImmutableSet> testOptionalFirstCollectionElement() { return ImmutableSet.of( ImmutableSet.of(0).stream().findFirst(), From 9c791b9aa903f196693c4f86e8b082c12c4b633c Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 15:04:01 +0100 Subject: [PATCH 04/15] Improve todo list --- .../errorprone/refastertemplates/CollectionTemplates.java | 3 +++ 1 file changed, 3 insertions(+) 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 8503a7598a..c95a3d76f5 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 @@ -177,6 +177,9 @@ ImmutableList after(ImmutableCollection collection) { */ // XXX: Similar rules could be implemented for the following variants: // collection.asList().iterator(); + // collection.asList().toArray(); + // collection.asList().toArray(Object[]::new); + // collection.asList().toArray(new Object[0]); static final class ImmutableCollectionAsListToStream { @BeforeTemplate Stream before(ImmutableCollection collection) { From d23382cfc5f4266b26dde6ab1b75c002269764ed Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 15:14:20 +0100 Subject: [PATCH 05/15] Add object[]::new templates to collection --- .../refastertemplates/CollectionTemplates.java | 17 +++++++++++++++++ .../CollectionTemplatesTestInput.java | 16 ++++++++++++---- .../CollectionTemplatesTestOutput.java | 16 ++++++++++++---- 3 files changed, 41 insertions(+), 8 deletions(-) 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 c95a3d76f5..9b240d3fcc 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 @@ -275,6 +275,23 @@ String after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if `toArray(...)` is called on the result; call + * it directly. + */ + static final class ImmutableCollectionAsListToNewArrayObject { + @BeforeTemplate + Object[] before(ImmutableCollection collection) { + return Refaster.anyOf( + collection.asList().toArray(Object[]::new), collection.asList().toArray(new Object[0])); + } + + @AfterTemplate + Object[] after(ImmutableCollection collection) { + return collection.toArray(Object[]::new); + } + } + /** * 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 77166e0a81..836f1a9c0f 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 @@ -64,14 +64,14 @@ ArrayList testNewArrayListFromCollection() { return Lists.newArrayList(ImmutableList.of("foo")); } - ImmutableList testImmutableCollectionAsList() { - return ImmutableList.copyOf(ImmutableSet.of(1)); - } - Stream testImmutableCollectionAsListToStream() { return ImmutableSet.of(1).asList().stream(); } + ImmutableList testImmutableCollectionAsList() { + return ImmutableList.copyOf(ImmutableSet.of(1)); + } + ImmutableSet testImmutableCollectionAsListIsEmpty() { return ImmutableSet.of( ImmutableSet.of(1).asList().isEmpty(), ImmutableSet.of().asList().isEmpty()); @@ -94,6 +94,14 @@ String testImmutableCollectionAsListToString() { return ImmutableSet.of(1).asList().toString(); } + ImmutableSet testImmutableCollectionAsListToNewArrayObject() { + return ImmutableSet.of( + ImmutableSet.of(1).asList().toArray(Object[]::new), + ImmutableSet.of().asList().toArray(Object[]::new), + ImmutableSet.of(1).asList().toArray(new Object[0]), + ImmutableSet.of().asList().toArray(new Object[0])); + } + 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 4f2b6af3dd..4f21d508b7 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 @@ -56,14 +56,14 @@ ArrayList testNewArrayListFromCollection() { return new ArrayList<>(ImmutableList.of("foo")); } - ImmutableList testImmutableCollectionAsList() { - return ImmutableSet.of(1).asList(); - } - Stream testImmutableCollectionAsListToStream() { return ImmutableSet.of(1).stream(); } + ImmutableList testImmutableCollectionAsList() { + return ImmutableSet.of(1).asList(); + } + ImmutableSet testImmutableCollectionAsListIsEmpty() { return ImmutableSet.of(ImmutableSet.of(1).isEmpty(), ImmutableSet.of().isEmpty()); } @@ -84,6 +84,14 @@ String testImmutableCollectionAsListToString() { return ImmutableSet.of(1).toString(); } + ImmutableSet testImmutableCollectionAsListToNewArrayObject() { + return ImmutableSet.of( + ImmutableSet.of(1).toArray(Object[]::new), + ImmutableSet.of().toArray(Object[]::new), + ImmutableSet.of(1).toArray(Object[]::new), + ImmutableSet.of().toArray(Object[]::new)); + } + ImmutableSet> testOptionalFirstCollectionElement() { return ImmutableSet.of( ImmutableSet.of(0).stream().findFirst(), From ee72a3fad81ba46c8fb10de7679c1e4816b7e2f5 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 15:24:39 +0100 Subject: [PATCH 06/15] Add ImmutableCollectionAsListToArray method and improve todos --- .../CollectionTemplates.java | 21 +++++++++++++++---- .../CollectionTemplatesTestInput.java | 4 ++++ .../CollectionTemplatesTestOutput.java | 4 ++++ 3 files changed, 25 insertions(+), 4 deletions(-) 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 9b240d3fcc..0cb3e7e754 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 @@ -177,10 +177,7 @@ ImmutableList after(ImmutableCollection collection) { */ // XXX: Similar rules could be implemented for the following variants: // collection.asList().iterator(); - // collection.asList().toArray(); - // collection.asList().toArray(Object[]::new); - // collection.asList().toArray(new Object[0]); - static final class ImmutableCollectionAsListToStream { + static final class ImmutableCollectionAsListToStream { @BeforeTemplate Stream before(ImmutableCollection collection) { return collection.asList().stream(); @@ -292,6 +289,22 @@ Object[] after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if `toArray()` is called on the result; call it + * directly. + */ + static final class ImmutableCollectionAsListToArray { + @BeforeTemplate + Object[] before(ImmutableCollection collection) { + return collection.asList().toArray(); + } + + @AfterTemplate + Object[] after(ImmutableCollection collection) { + return collection.toArray(); + } + } + /** * 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 836f1a9c0f..91993c1dd1 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 @@ -102,6 +102,10 @@ ImmutableSet testImmutableCollectionAsListToNewArrayObject() { ImmutableSet.of().asList().toArray(new Object[0])); } + Object[] testImmutableCollectionAsListToArray() { + return ImmutableSet.of(1).asList().toArray(); + } + 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 4f21d508b7..79ab366ba8 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 @@ -92,6 +92,10 @@ ImmutableSet testImmutableCollectionAsListToNewArrayObject() { ImmutableSet.of().toArray(Object[]::new)); } + Object[] testImmutableCollectionAsListToArray() { + return ImmutableSet.of(1).toArray(); + } + ImmutableSet> testOptionalFirstCollectionElement() { return ImmutableSet.of( ImmutableSet.of(0).stream().findFirst(), From 7965cfcc9d73c03875c69e78da3fcdedf8ee815a Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Dec 2020 15:46:12 +0100 Subject: [PATCH 07/15] Add ImmutableListAsListIterator template and remove last todo --- .../CollectionTemplates.java | 22 ++++++++++++++++--- .../CollectionTemplatesTestInput.java | 5 +++++ .../CollectionTemplatesTestOutput.java | 5 +++++ 3 files changed, 29 insertions(+), 3 deletions(-) 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 0cb3e7e754..5276849e1f 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,6 +10,7 @@ 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; @@ -175,9 +176,7 @@ 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().iterator(); - static final class ImmutableCollectionAsListToStream { + static final class ImmutableCollectionAsListToStream { @BeforeTemplate Stream before(ImmutableCollection collection) { return collection.asList().stream(); @@ -305,6 +304,23 @@ Object[] after(ImmutableCollection collection) { } } + /** + * Don't call {@link ImmutableCollection#asList()} if `iterator()` is called on the result; call + * it directly. + */ + static final class ImmutableCollectionAsListIterator { + @BeforeTemplate + Iterator before(ImmutableCollection collection) { + // XXX: @Stephan, I'm not sure about this one. Since it is actually an UnmodifiableIterator... + 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 91993c1dd1..3d4d965fdc 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; @@ -106,6 +107,10 @@ Object[] testImmutableCollectionAsListToArray() { return ImmutableSet.of(1).asList().toArray(); } + Iterator testImmutableCollectionAsListIterator() { + 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 79ab366ba8..901d6c07c1 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; @@ -96,6 +97,10 @@ Object[] testImmutableCollectionAsListToArray() { return ImmutableSet.of(1).toArray(); } + Iterator testImmutableCollectionAsListIterator() { + return ImmutableSet.of(1).iterator(); + } + ImmutableSet> testOptionalFirstCollectionElement() { return ImmutableSet.of( ImmutableSet.of(0).stream().findFirst(), From d30b6c1b80b8768ccda3272cb3552582a99f6d65 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 30 Dec 2020 13:52:36 +0100 Subject: [PATCH 08/15] Pair programming --- .../refastertemplates/CollectionTemplates.java | 14 +++++++------- .../bugpatterns/CollectionTemplatesTestInput.java | 4 ++-- .../bugpatterns/CollectionTemplatesTestOutput.java | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) 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 5276849e1f..efd8dfa950 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 @@ -207,18 +207,18 @@ boolean after(ImmutableCollection collection) { } /** - * Don't call {@link ImmutableCollection#asList()} if `contains(null)` is called on the result; - * call it directly. + * Don't call {@link ImmutableCollection#asList()} if {@link Collection#contains(Object)} is + * called on the result; call it directly. */ - static final class ImmutableCollectionAsListContainsNull { + static final class ImmutableCollectionContains { @BeforeTemplate - boolean before(ImmutableCollection collection) { - return collection.asList().contains(null); + boolean before(ImmutableCollection collection, S elem) { + return collection.asList().contains(elem); } @AfterTemplate - boolean after(ImmutableCollection collection) { - return collection.contains(null); + boolean after(ImmutableCollection collection, S elem) { + return collection.contains(elem); } } 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 3d4d965fdc..2c759f3b59 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 @@ -78,9 +78,9 @@ ImmutableSet testImmutableCollectionAsListIsEmpty() { ImmutableSet.of(1).asList().isEmpty(), ImmutableSet.of().asList().isEmpty()); } - ImmutableSet testImmutableCollectionAsListContainsNull() { + ImmutableSet testImmutableCollectionContains() { return ImmutableSet.of( - ImmutableSet.of(1).asList().contains(null), ImmutableSet.of().asList().contains(null)); + ImmutableSet.of(1).asList().contains("foo"), ImmutableSet.of(2).asList().contains("bar")); } Stream testImmutableCollectionAsListParallelStream() { 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 901d6c07c1..3a106a07c0 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 @@ -69,8 +69,8 @@ ImmutableSet testImmutableCollectionAsListIsEmpty() { return ImmutableSet.of(ImmutableSet.of(1).isEmpty(), ImmutableSet.of().isEmpty()); } - ImmutableSet testImmutableCollectionAsListContainsNull() { - return ImmutableSet.of(ImmutableSet.of(1).contains(null), ImmutableSet.of().contains(null)); + ImmutableSet testImmutableCollectionContains() { + return ImmutableSet.of(ImmutableSet.of(1).contains("foo"), ImmutableSet.of(2).contains("bar")); } Stream testImmutableCollectionAsListParallelStream() { From d25419bf299d766cb315f4a16d9549e9627cbb70 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Sun, 3 Jan 2021 14:19:04 +0100 Subject: [PATCH 09/15] Change names and try to generalize the toArray things --- .../CollectionTemplates.java | 93 ++++++++----------- 1 file changed, 37 insertions(+), 56 deletions(-) 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 efd8dfa950..2ed2bf0974 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 @@ -189,20 +189,18 @@ Stream after(ImmutableCollection collection) { } /** - * Don't call {@link ImmutableCollection#asList()} if `isEmpty()` is called on the result; call it + * Don't call {@link ImmutableCollection#asList()} if {@link Collection#isEmpty()} is called on the result; call it * directly. */ - static final class ImmutableCollectionAsListIsEmpty { + static final class ImmutableCollectionIsEmpty { @BeforeTemplate - boolean before(ImmutableCollection collection) { - // XXX: @Stephan this one can also fit in at: CollectionIsEmpty in the @Before, ~line 25. - // I don't know what the convention would be for that. - return collection.asList().isEmpty(); + boolean before(ImmutableCollection immutableCollection) { + return immutableCollection.asList().isEmpty(); } @AfterTemplate - boolean after(ImmutableCollection collection) { - return collection.isEmpty(); + boolean after(ImmutableCollection immutableCollection) { + return immutableCollection.isEmpty(); } } @@ -223,101 +221,84 @@ boolean after(ImmutableCollection collection, S elem) { } /** - * Don't call {@link ImmutableCollection#asList()} if `parallelStream()` is called on the result; + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#parallelStream()} is called on the result; * call it directly. */ - static final class ImmutableCollectionAsListParallelStream { + static final class ImmutableCollectionParallelStream { @BeforeTemplate - Stream before(ImmutableCollection collection) { - return collection.asList().parallelStream(); + Stream before(ImmutableCollection immutableCollection) { + return immutableCollection.asList().parallelStream(); } @AfterTemplate - Stream after(ImmutableCollection collection) { - return collection.parallelStream(); + Stream after(ImmutableCollection immutableCollection) { + return immutableCollection.parallelStream(); } } /** - * Don't call {@link ImmutableCollection#asList()} if `size()` is called on the result; call it + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#size()} is called on the result; call it * directly. */ - static final class ImmutableCollectionAsListSize { + static final class ImmutableCollectionSize { @BeforeTemplate - int before(ImmutableCollection collection) { - // XXX: Again, this one can also be added to the CollectionSize example, ~line 44. - return collection.asList().size(); + int before(ImmutableCollection immutableCollection) { + return immutableCollection.asList().size(); } @AfterTemplate - int after(ImmutableCollection collection) { - return collection.size(); + int after(ImmutableCollection immutableCollection) { + return immutableCollection.size(); } } /** - * Don't call {@link ImmutableCollection#asList()} if `toString()` is called on the result; call + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toString()} is called on the result; call * it directly. */ - static final class ImmutableCollectionAsListToString { + static final class ImmutableCollectionToString { @BeforeTemplate - String before(ImmutableCollection collection) { - return collection.asList().toString(); + String before(ImmutableCollection immutableCollection) { + return immutableCollection.asList().toString(); } @AfterTemplate - String after(ImmutableCollection collection) { - return collection.toString(); + String after(ImmutableCollection immutableCollection) { + return immutableCollection.toString(); } } /** - * Don't call {@link ImmutableCollection#asList()} if `toArray(...)` is called on the result; call + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] a)}` is called on the result; call * it directly. */ - static final class ImmutableCollectionAsListToNewArrayObject { + static final class ImmutableCollectionToArray { @BeforeTemplate - Object[] before(ImmutableCollection collection) { + Object[] before(ImmutableCollection immutableCollection, S[] elem) { return Refaster.anyOf( - collection.asList().toArray(Object[]::new), collection.asList().toArray(new Object[0])); - } - - @AfterTemplate - Object[] after(ImmutableCollection collection) { - return collection.toArray(Object[]::new); - } - } - - /** - * Don't call {@link ImmutableCollection#asList()} if `toArray()` is called on the result; call it - * directly. - */ - static final class ImmutableCollectionAsListToArray { - @BeforeTemplate - Object[] before(ImmutableCollection collection) { - return collection.asList().toArray(); + immutableCollection.asList().toArray(elem), immutableCollection.asList().toArray(elem)); } @AfterTemplate - Object[] after(ImmutableCollection collection) { - return collection.toArray(); + Object[] after(ImmutableCollection immutableCollection, S[] elem) { + return immutableCollection.toArray(elem); } } /** - * Don't call {@link ImmutableCollection#asList()} if `iterator()` is called on the result; call + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#iterator()} is called on the result; call * it directly. */ - static final class ImmutableCollectionAsListIterator { + static final class ImmutableCollectionIterator { @BeforeTemplate - Iterator before(ImmutableCollection collection) { - // XXX: @Stephan, I'm not sure about this one. Since it is actually an UnmodifiableIterator... - return collection.asList().iterator(); + Iterator before(ImmutableCollection immutableCollection) { + // XXX: @Stephan, I'm still not sure about this one. Since it is actually an UnmodifiableIterator... + return immutableCollection.asList().iterator(); } @AfterTemplate - Iterator after(ImmutableCollection collection) { - return collection.iterator(); + Iterator after(ImmutableCollection immutableCollection) { + return immutableCollection.iterator(); } } From b690b6da67bbe3918e18b6f09edd7668fb165e52 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Sun, 3 Jan 2021 14:42:04 +0100 Subject: [PATCH 10/15] Add back a template for the case where toArray does not have a parameter --- .../CollectionTemplates.java | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) 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 2ed2bf0974..21668b68ee 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 @@ -189,8 +189,8 @@ Stream after(ImmutableCollection collection) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link Collection#isEmpty()} is called on the result; call it - * directly. + * Don't call {@link ImmutableCollection#asList()} if {@link Collection#isEmpty()} is called on + * the result; call it directly. */ static final class ImmutableCollectionIsEmpty { @BeforeTemplate @@ -221,8 +221,8 @@ boolean after(ImmutableCollection collection, S elem) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#parallelStream()} is called on the result; - * call it directly. + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#parallelStream()} + * is called on the result; call it directly. */ static final class ImmutableCollectionParallelStream { @BeforeTemplate @@ -237,8 +237,8 @@ Stream after(ImmutableCollection immutableCollection) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#size()} is called on the result; call it - * directly. + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#size()} is called + * on the result; call it directly. */ static final class ImmutableCollectionSize { @BeforeTemplate @@ -253,8 +253,8 @@ int after(ImmutableCollection immutableCollection) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toString()} is called on the result; call - * it directly. + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toString()} is + * called on the result; call it directly. */ static final class ImmutableCollectionToString { @BeforeTemplate @@ -269,10 +269,10 @@ String after(ImmutableCollection immutableCollection) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] a)}` is called on the result; call - * it directly. + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] + * a)}` is called on the result; call it directly. */ - static final class ImmutableCollectionToArray { + static final class ImmutableCollectionToArrayWithObject { @BeforeTemplate Object[] before(ImmutableCollection immutableCollection, S[] elem) { return Refaster.anyOf( @@ -286,13 +286,30 @@ Object[] after(ImmutableCollection immutableCollection, S[] elem) { } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#iterator()} is called on the result; call - * it directly. + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray()} is + * called on the result; call it directly. + */ + static final class ImmutableCollectionToArray { + @BeforeTemplate + Object[] before(ImmutableCollection collection) { + return collection.asList().toArray(); + } + + @AfterTemplate + Object[] after(ImmutableCollection collection) { + return collection.toArray(); + } + } + + /** + * 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 immutableCollection) { - // XXX: @Stephan, I'm still not sure about this one. Since it is actually an UnmodifiableIterator... + // XXX: @Stephan, I'm still not sure about this one. Since it is actually an + // UnmodifiableIterator... return immutableCollection.asList().iterator(); } From 39a71533ce94079d99a329051bb6d6c38b1e552a Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Thu, 7 Jan 2021 11:00:57 +0100 Subject: [PATCH 11/15] Fix typo --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index e18644f222..cc59e828f3 100644 --- a/pom.xml +++ b/pom.xml @@ -1014,7 +1014,7 @@ pitest-maven 1.6.2 - 4 From 6df102b47a02aa98081a8438b31dfa1e760bded8 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 8 Jan 2021 21:25:19 +0100 Subject: [PATCH 12/15] Add the pair programming things of yesterday - but the tests needs to be updated and checked - that is for tomorrow --- .../CollectionTemplates.java | 36 +++++++++++++++++++ .../CollectionTemplatesTestInput.java | 2 ++ .../CollectionTemplatesTestOutput.java | 2 ++ 3 files changed, 40 insertions(+) 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 21668b68ee..0872f242f9 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 @@ -16,6 +16,7 @@ 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. */ @@ -268,6 +269,41 @@ String after(ImmutableCollection immutableCollection) { } } + static final class CollectionToArray { + @BeforeTemplate + Object[] before(Collection collection, int size) { + return collection.toArray(new Object[size]); + } + + @AfterTemplate + Object[] after(Collection collection, int size) { + return collection.toArray(Object[]::new); + } + } + + static final class CollectionToObjectArray { + @BeforeTemplate + Object[] before(Collection collection) { + return collection.toArray(Object[]::new); + } + + @AfterTemplate + Object[] after(Collection collection) { + return collection.toArray(); + } + } + + static final class ImmutableCollectionAsListToNewArrayObject { + @BeforeTemplate + S[] before(ImmutableCollection collection, IntFunction generator) { + return collection.asList().toArray(generator); + } + + S[] after(ImmutableCollection collection, IntFunction generator) { + return collection.toArray(generator); + } + } + /** * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] * a)}` is called on the result; call it directly. 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 2c759f3b59..906e0e6e0d 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 @@ -99,6 +99,8 @@ ImmutableSet testImmutableCollectionAsListToNewArrayObject() { return ImmutableSet.of( ImmutableSet.of(1).asList().toArray(Object[]::new), ImmutableSet.of().asList().toArray(Object[]::new), + ImmutableSet.of(1).asList().toArray(Integer[]::new), + ImmutableSet.of().asList().toArray(Integer[]::new), ImmutableSet.of(1).asList().toArray(new Object[0]), ImmutableSet.of().asList().toArray(new Object[0])); } 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 3a106a07c0..2a23659927 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 @@ -89,6 +89,8 @@ ImmutableSet testImmutableCollectionAsListToNewArrayObject() { return ImmutableSet.of( ImmutableSet.of(1).toArray(Object[]::new), ImmutableSet.of().toArray(Object[]::new), + ImmutableSet.of(1).toArray(Integer[]::new), + ImmutableSet.of().toArray(Integer[]::new), ImmutableSet.of(1).toArray(Object[]::new), ImmutableSet.of().toArray(Object[]::new)); } From 929e068abdb31067bdae8452a16de5bb662879e9 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Sun, 10 Jan 2021 14:24:04 +0100 Subject: [PATCH 13/15] Add tests for the array cases and try to see why it does not apply found matches for some cases... --- .../errorprone/bugpatterns/RefasterCheck.java | 5 ++ .../CollectionTemplates.java | 58 ++++++++++--------- .../CollectionTemplatesTestInput.java | 21 ++++++- .../CollectionTemplatesTestOutput.java | 25 +++++++- 4 files changed, 79 insertions(+), 30 deletions(-) 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..8c2ec678dc 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 @@ -143,6 +143,11 @@ private static ImmutableRangeSet getReplacementRanges( Description description, EndPosTable endPositions) { return getReplacements(description, endPositions) .map(Replacement::range) + .filter(e -> !e.isEmpty()) + // If I add this, the test wont crash, but not succeed. + // Later on an ImmutableRangeSet adds an empty range + // which causes it to throw an IllegalArgumentException. `An exception was thrown by EP: + // range must not be empty but was was [3774..3774) .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 0872f242f9..0efda94196 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 @@ -269,55 +269,61 @@ String after(ImmutableCollection immutableCollection) { } } - static final class CollectionToArray { + /** + * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] + * a)}` is called on the result; call it directly. + */ + static final class ImmutableCollectionToArrayWithObject { @BeforeTemplate - Object[] before(Collection collection, int size) { - return collection.toArray(new Object[size]); + Object[] before(ImmutableCollection immutableCollection, S[] elem) { + return Refaster.anyOf( + immutableCollection.asList().toArray(elem), immutableCollection.asList().toArray(elem)); } @AfterTemplate - Object[] after(Collection collection, int size) { - return collection.toArray(Object[]::new); + Object[] after(ImmutableCollection immutableCollection, S[] elem) { + return immutableCollection.toArray(elem); } } - static final class CollectionToObjectArray { + /** + * Don't call {@link ImmutableCollection#asList()} if {@link + * ImmutableCollection#toArray(IntFunction)} ()} is called on the result; call it directly. + */ + static final class ImmutableCollectionToArrayGenerator { @BeforeTemplate - Object[] before(Collection collection) { - return collection.toArray(Object[]::new); + S[] before(ImmutableCollection collection, IntFunction generator) { + return collection.asList().toArray(generator); } - @AfterTemplate - Object[] after(Collection collection) { - return collection.toArray(); + S[] after(ImmutableCollection collection, IntFunction generator) { + return collection.toArray(generator); } } - static final class ImmutableCollectionAsListToNewArrayObject { + /** Prefer calling {@link Collection#toArray(Object[])} without a size parameter. */ + static final class CollectionToObjectArray { @BeforeTemplate - S[] before(ImmutableCollection collection, IntFunction generator) { - return collection.asList().toArray(generator); + Object[] before(Collection collection, int size) { + return collection.toArray(new Object[size]); } - S[] after(ImmutableCollection collection, IntFunction generator) { - return collection.toArray(generator); + @AfterTemplate + Object[] after(Collection collection, int size) { + return collection.toArray(Object[]::new); } } - /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] - * a)}` is called on the result; call it directly. - */ - static final class ImmutableCollectionToArrayWithObject { + /** Prefer {@link Collection#toArray()} over {@link Collection#toArray(Object[])}. */ + static final class CollectionToArray { @BeforeTemplate - Object[] before(ImmutableCollection immutableCollection, S[] elem) { - return Refaster.anyOf( - immutableCollection.asList().toArray(elem), immutableCollection.asList().toArray(elem)); + Object[] before(Collection collection) { + return collection.toArray(Object[]::new); } @AfterTemplate - Object[] after(ImmutableCollection immutableCollection, S[] elem) { - return immutableCollection.toArray(elem); + Object[] after(Collection collection) { + return collection.toArray(); } } 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 906e0e6e0d..284726e827 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 @@ -95,7 +95,26 @@ String testImmutableCollectionAsListToString() { return ImmutableSet.of(1).asList().toString(); } - ImmutableSet testImmutableCollectionAsListToNewArrayObject() { + ImmutableSet testCollectionToObjectArray() { + return ImmutableSet.of( + ImmutableSet.of(1, 2).toArray(new Object[4]), + ImmutableSet.of().toArray(new Object[0]), + ImmutableSet.of(1).toArray(new Object[1])); + } + + ImmutableSet testCollectionToArray() { + return ImmutableSet.of( + ImmutableSet.of(1, 2).toArray(Object[]::new), + ImmutableSet.of().toArray(Object[]::new), + ImmutableSet.of(1).toArray(Object[]::new)); + } + + ImmutableSet testImmutableCollectionToArrayGenerator() { + return ImmutableSet.of(ImmutableSet.of(1).asList().toArray(size -> new Integer[size + 1]), + ImmutableSet.of().asList().toArray(size -> new String[size])); + } + + ImmutableSet testImmutableCollectionToArrayWithObject() { return ImmutableSet.of( ImmutableSet.of(1).asList().toArray(Object[]::new), ImmutableSet.of().asList().toArray(Object[]::new), 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 2a23659927..daed5153e7 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 @@ -85,14 +85,33 @@ String testImmutableCollectionAsListToString() { return ImmutableSet.of(1).toString(); } - ImmutableSet testImmutableCollectionAsListToNewArrayObject() { + ImmutableSet testCollectionToObjectArray() { + return ImmutableSet.of( + ImmutableSet.of(1, 2).toArray(Object[]::new), + ImmutableSet.of().toArray(Object[]::new), + ImmutableSet.of(1).toArray(Object[]::new)); + } + + ImmutableSet testCollectionToArray() { + return ImmutableSet.of( + ImmutableSet.of(1, 2).toArray(), + ImmutableSet.of().toArray(), + ImmutableSet.of(1).toArray()); + } + + ImmutableSet testImmutableCollectionToArrayGenerator() { + return ImmutableSet.of(ImmutableSet.of(1).toArray(size -> new Integer[size + 1]), + ImmutableSet.of().toArray(size -> new String[size])); + } + + ImmutableSet testImmutableCollectionToArrayWithObject() { return ImmutableSet.of( ImmutableSet.of(1).toArray(Object[]::new), ImmutableSet.of().toArray(Object[]::new), ImmutableSet.of(1).toArray(Integer[]::new), ImmutableSet.of().toArray(Integer[]::new), - ImmutableSet.of(1).toArray(Object[]::new), - ImmutableSet.of().toArray(Object[]::new)); + ImmutableSet.of(1).toArray(new Object[0]), + ImmutableSet.of().toArray(new Object[0])); } Object[] testImmutableCollectionAsListToArray() { From 9e0b3f37f438669e42715bf6997104e949501f70 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Thu, 14 Jan 2021 15:17:15 +0100 Subject: [PATCH 14/15] Suggestions --- .../errorprone/bugpatterns/RefasterCheck.java | 8 +- .../CollectionTemplates.java | 126 +++++++----------- .../CollectionTemplatesTestInput.java | 55 +++----- .../CollectionTemplatesTestOutput.java | 51 ++----- 4 files changed, 78 insertions(+), 162 deletions(-) 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 8c2ec678dc..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,11 +145,7 @@ private static ImmutableRangeSet getReplacementRanges( Description description, EndPosTable endPositions) { return getReplacements(description, endPositions) .map(Replacement::range) - .filter(e -> !e.isEmpty()) - // If I add this, the test wont crash, but not succeed. - // Later on an ImmutableRangeSet adds an empty range - // which causes it to throw an IllegalArgumentException. `An exception was thrown by EP: - // range must not be empty but was was [3774..3774) + .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 0efda94196..1e4de0dafe 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 @@ -39,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) { @@ -189,22 +194,6 @@ Stream after(ImmutableCollection collection) { } } - /** - * Don't call {@link ImmutableCollection#asList()} if {@link Collection#isEmpty()} is called on - * the result; call it directly. - */ - static final class ImmutableCollectionIsEmpty { - @BeforeTemplate - boolean before(ImmutableCollection immutableCollection) { - return immutableCollection.asList().isEmpty(); - } - - @AfterTemplate - boolean after(ImmutableCollection immutableCollection) { - return immutableCollection.isEmpty(); - } - } - /** * Don't call {@link ImmutableCollection#asList()} if {@link Collection#contains(Object)} is * called on the result; call it directly. @@ -227,13 +216,13 @@ boolean after(ImmutableCollection collection, S elem) { */ static final class ImmutableCollectionParallelStream { @BeforeTemplate - Stream before(ImmutableCollection immutableCollection) { - return immutableCollection.asList().parallelStream(); + Stream before(ImmutableCollection collection) { + return collection.asList().parallelStream(); } @AfterTemplate - Stream after(ImmutableCollection immutableCollection) { - return immutableCollection.parallelStream(); + Stream after(ImmutableCollection collection) { + return collection.parallelStream(); } } @@ -243,13 +232,13 @@ Stream after(ImmutableCollection immutableCollection) { */ static final class ImmutableCollectionSize { @BeforeTemplate - int before(ImmutableCollection immutableCollection) { - return immutableCollection.asList().size(); + int before(ImmutableCollection collection) { + return collection.asList().size(); } @AfterTemplate - int after(ImmutableCollection immutableCollection) { - return immutableCollection.size(); + int after(ImmutableCollection collection) { + return collection.size(); } } @@ -259,87 +248,64 @@ int after(ImmutableCollection immutableCollection) { */ static final class ImmutableCollectionToString { @BeforeTemplate - String before(ImmutableCollection immutableCollection) { - return immutableCollection.asList().toString(); + String before(ImmutableCollection collection) { + return collection.asList().toString(); } @AfterTemplate - String after(ImmutableCollection immutableCollection) { - return immutableCollection.toString(); + String after(ImmutableCollection collection) { + return collection.toString(); } } - /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray(Object[] - * a)}` is called on the result; call it directly. - */ - static final class ImmutableCollectionToArrayWithObject { + /** Prefer calling {@link Collection#toArray()} over more contrived alternatives. */ + static final class CollectionToArray { @BeforeTemplate - Object[] before(ImmutableCollection immutableCollection, S[] elem) { + Object[] before(Collection collection, int size) { return Refaster.anyOf( - immutableCollection.asList().toArray(elem), immutableCollection.asList().toArray(elem)); - } - - @AfterTemplate - Object[] after(ImmutableCollection immutableCollection, S[] elem) { - return immutableCollection.toArray(elem); - } - } - - /** - * Don't call {@link ImmutableCollection#asList()} if {@link - * ImmutableCollection#toArray(IntFunction)} ()} is called on the result; call it directly. - */ - static final class ImmutableCollectionToArrayGenerator { - @BeforeTemplate - S[] before(ImmutableCollection collection, IntFunction generator) { - return collection.asList().toArray(generator); - } - - S[] after(ImmutableCollection collection, IntFunction generator) { - return collection.toArray(generator); + collection.toArray(new Object[size]), collection.toArray(Object[]::new)); } - } - /** Prefer calling {@link Collection#toArray(Object[])} without a size parameter. */ - static final class CollectionToObjectArray { @BeforeTemplate - Object[] before(Collection collection, int size) { - return collection.toArray(new Object[size]); + Object[] before(ImmutableCollection collection) { + return collection.asList().toArray(); } @AfterTemplate - Object[] after(Collection collection, int size) { - return collection.toArray(Object[]::new); + Object[] after(Collection collection) { + return collection.toArray(); } } - /** Prefer {@link Collection#toArray()} over {@link Collection#toArray(Object[])}. */ - static final class CollectionToArray { + /** + * 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(Collection collection) { - return collection.toArray(Object[]::new); + Object[] before(ImmutableCollection collection, S[] array) { + return collection.asList().toArray(array); } @AfterTemplate - Object[] after(Collection collection) { - return collection.toArray(); + Object[] after(ImmutableCollection collection, S[] array) { + return collection.toArray(array); } } /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toArray()} is - * called on the result; call it directly. + * Don't call {@link ImmutableCollection#asList()} if {@link + * ImmutableCollection#toArray(IntFunction)}} is called on the result; call it directly. */ - static final class ImmutableCollectionToArray { + static final class ImmutableCollectionToArrayWithGenerator { @BeforeTemplate - Object[] before(ImmutableCollection collection) { - return collection.asList().toArray(); + S[] before(ImmutableCollection collection, IntFunction generator) { + return collection.asList().toArray(generator); } @AfterTemplate - Object[] after(ImmutableCollection collection) { - return collection.toArray(); + S[] after(ImmutableCollection collection, IntFunction generator) { + return collection.toArray(generator); } } @@ -349,15 +315,13 @@ Object[] after(ImmutableCollection collection) { */ static final class ImmutableCollectionIterator { @BeforeTemplate - Iterator before(ImmutableCollection immutableCollection) { - // XXX: @Stephan, I'm still not sure about this one. Since it is actually an - // UnmodifiableIterator... - return immutableCollection.asList().iterator(); + Iterator before(ImmutableCollection collection) { + return collection.asList().iterator(); } @AfterTemplate - Iterator after(ImmutableCollection immutableCollection) { - return immutableCollection.iterator(); + Iterator after(ImmutableCollection collection) { + return collection.iterator(); } } 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 284726e827..df1efa8dba 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 @@ -26,7 +26,8 @@ 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() { @@ -65,7 +66,7 @@ ArrayList testNewArrayListFromCollection() { return Lists.newArrayList(ImmutableList.of("foo")); } - Stream testImmutableCollectionAsListToStream() { + Stream testImmutableCollectionStream() { return ImmutableSet.of(1).asList().stream(); } @@ -73,62 +74,38 @@ ImmutableList testImmutableCollectionAsList() { return ImmutableList.copyOf(ImmutableSet.of(1)); } - ImmutableSet testImmutableCollectionAsListIsEmpty() { - return ImmutableSet.of( - ImmutableSet.of(1).asList().isEmpty(), ImmutableSet.of().asList().isEmpty()); + boolean testImmutableCollectionContains() { + return ImmutableSet.of(1).asList().contains("foo"); } - ImmutableSet testImmutableCollectionContains() { - return ImmutableSet.of( - ImmutableSet.of(1).asList().contains("foo"), ImmutableSet.of(2).asList().contains("bar")); - } - - Stream testImmutableCollectionAsListParallelStream() { + Stream testImmutableCollectionParallelStream() { return ImmutableSet.of(1).asList().parallelStream(); } - int testImmutableCollectionAsListSize() { + int testImmutableCollectionSize() { return ImmutableSet.of(1).asList().size(); } - String testImmutableCollectionAsListToString() { + String testImmutableCollectionToString() { return ImmutableSet.of(1).asList().toString(); } - ImmutableSet testCollectionToObjectArray() { - return ImmutableSet.of( - ImmutableSet.of(1, 2).toArray(new Object[4]), - ImmutableSet.of().toArray(new Object[0]), - ImmutableSet.of(1).toArray(new Object[1])); - } - ImmutableSet testCollectionToArray() { return ImmutableSet.of( - ImmutableSet.of(1, 2).toArray(Object[]::new), - ImmutableSet.of().toArray(Object[]::new), - ImmutableSet.of(1).toArray(Object[]::new)); + ImmutableSet.of(1).toArray(new Object[1]), + ImmutableSet.of(2).toArray(Object[]::new), + ImmutableSet.of(3).asList().toArray()); } - ImmutableSet testImmutableCollectionToArrayGenerator() { - return ImmutableSet.of(ImmutableSet.of(1).asList().toArray(size -> new Integer[size + 1]), - ImmutableSet.of().asList().toArray(size -> new String[size])); - } - - ImmutableSet testImmutableCollectionToArrayWithObject() { - return ImmutableSet.of( - ImmutableSet.of(1).asList().toArray(Object[]::new), - ImmutableSet.of().asList().toArray(Object[]::new), - ImmutableSet.of(1).asList().toArray(Integer[]::new), - ImmutableSet.of().asList().toArray(Integer[]::new), - ImmutableSet.of(1).asList().toArray(new Object[0]), - ImmutableSet.of().asList().toArray(new Object[0])); + Integer[] testImmutableCollectionToArrayWithArray() { + return ImmutableSet.of(1).asList().toArray(new Integer[0]); } - Object[] testImmutableCollectionAsListToArray() { - return ImmutableSet.of(1).asList().toArray(); + Integer[] testImmutableCollectionToArrayWithGenerator() { + return ImmutableSet.of(1).asList().toArray(Integer[]::new); } - Iterator testImmutableCollectionAsListIterator() { + Iterator testImmutableCollectionIterator() { return ImmutableSet.of(1).asList().iterator(); } 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 daed5153e7..fde4be21f9 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 @@ -26,7 +26,8 @@ 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() { @@ -57,7 +58,7 @@ ArrayList testNewArrayListFromCollection() { return new ArrayList<>(ImmutableList.of("foo")); } - Stream testImmutableCollectionAsListToStream() { + Stream testImmutableCollectionStream() { return ImmutableSet.of(1).stream(); } @@ -65,60 +66,36 @@ ImmutableList testImmutableCollectionAsList() { return ImmutableSet.of(1).asList(); } - ImmutableSet testImmutableCollectionAsListIsEmpty() { - return ImmutableSet.of(ImmutableSet.of(1).isEmpty(), ImmutableSet.of().isEmpty()); + boolean testImmutableCollectionContains() { + return ImmutableSet.of(1).contains("foo"); } - ImmutableSet testImmutableCollectionContains() { - return ImmutableSet.of(ImmutableSet.of(1).contains("foo"), ImmutableSet.of(2).contains("bar")); - } - - Stream testImmutableCollectionAsListParallelStream() { + Stream testImmutableCollectionParallelStream() { return ImmutableSet.of(1).parallelStream(); } - int testImmutableCollectionAsListSize() { + int testImmutableCollectionSize() { return ImmutableSet.of(1).size(); } - String testImmutableCollectionAsListToString() { + String testImmutableCollectionToString() { return ImmutableSet.of(1).toString(); } - ImmutableSet testCollectionToObjectArray() { - return ImmutableSet.of( - ImmutableSet.of(1, 2).toArray(Object[]::new), - ImmutableSet.of().toArray(Object[]::new), - ImmutableSet.of(1).toArray(Object[]::new)); - } - ImmutableSet testCollectionToArray() { return ImmutableSet.of( - ImmutableSet.of(1, 2).toArray(), - ImmutableSet.of().toArray(), - ImmutableSet.of(1).toArray()); - } - - ImmutableSet testImmutableCollectionToArrayGenerator() { - return ImmutableSet.of(ImmutableSet.of(1).toArray(size -> new Integer[size + 1]), - ImmutableSet.of().toArray(size -> new String[size])); + ImmutableSet.of(1).toArray(), ImmutableSet.of(2).toArray(), ImmutableSet.of(3).toArray()); } - ImmutableSet testImmutableCollectionToArrayWithObject() { - return ImmutableSet.of( - ImmutableSet.of(1).toArray(Object[]::new), - ImmutableSet.of().toArray(Object[]::new), - ImmutableSet.of(1).toArray(Integer[]::new), - ImmutableSet.of().toArray(Integer[]::new), - ImmutableSet.of(1).toArray(new Object[0]), - ImmutableSet.of().toArray(new Object[0])); + Integer[] testImmutableCollectionToArrayWithArray() { + return ImmutableSet.of(1).toArray(new Integer[0]); } - Object[] testImmutableCollectionAsListToArray() { - return ImmutableSet.of(1).toArray(); + Integer[] testImmutableCollectionToArrayWithGenerator() { + return ImmutableSet.of(1).toArray(Integer[]::new); } - Iterator testImmutableCollectionAsListIterator() { + Iterator testImmutableCollectionIterator() { return ImmutableSet.of(1).iterator(); } From 0d11b92ad905295c389b398a9dec9688b19ff32e Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 14 Feb 2021 14:24:37 +0100 Subject: [PATCH 15/15] Simplify --- .../CollectionTemplates.java | 21 +++++-------------- .../CollectionTemplatesTestInput.java | 8 ++----- .../CollectionTemplatesTestOutput.java | 8 ++----- 3 files changed, 9 insertions(+), 28 deletions(-) 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 1e4de0dafe..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 @@ -58,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(); @@ -226,22 +231,6 @@ Stream after(ImmutableCollection collection) { } } - /** - * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#size()} is called - * on the result; call it directly. - */ - static final class ImmutableCollectionSize { - @BeforeTemplate - int before(ImmutableCollection collection) { - return collection.asList().size(); - } - - @AfterTemplate - int after(ImmutableCollection collection) { - return collection.size(); - } - } - /** * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#toString()} is * called on the result; call it directly. 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 df1efa8dba..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 @@ -30,8 +30,8 @@ ImmutableSet testCollectionIsEmpty() { 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() { @@ -82,10 +82,6 @@ Stream testImmutableCollectionParallelStream() { return ImmutableSet.of(1).asList().parallelStream(); } - int testImmutableCollectionSize() { - return ImmutableSet.of(1).asList().size(); - } - String testImmutableCollectionToString() { return ImmutableSet.of(1).asList().toString(); } 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 fde4be21f9..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 @@ -30,8 +30,8 @@ ImmutableSet testCollectionIsEmpty() { 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() { @@ -74,10 +74,6 @@ Stream testImmutableCollectionParallelStream() { return ImmutableSet.of(1).parallelStream(); } - int testImmutableCollectionSize() { - return ImmutableSet.of(1).size(); - } - String testImmutableCollectionToString() { return ImmutableSet.of(1).toString(); }