From beb96f0f4b1474283e13ff8cb32d519644436383 Mon Sep 17 00:00:00 2001
From: Mohamed Sameh <110535847+mohamedsamehsalah@users.noreply.github.com>
Date: Tue, 1 Oct 2024 10:05:33 +0200
Subject: [PATCH] Introduce `CollectionIterator` Refaster rule (#1347)

This rule supersedes the more specific `ImmutableCollectionIterator` rule.
---
 .../errorprone/refasterrules/CollectionRules.java  | 14 ++++++++------
 .../refasterrules/CollectionRulesTestInput.java    |  5 +++--
 .../refasterrules/CollectionRulesTestOutput.java   |  4 ++--
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java
index 4ee37189e1..3a02f6fe5e 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java
@@ -365,18 +365,20 @@ S[] after(ImmutableCollection<T> collection, IntFunction<S[]> generator) {
     }
   }
 
-  /**
-   * Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#iterator()} is
-   * called on the result; call it directly.
-   */
-  static final class ImmutableCollectionIterator<T> {
+  /** Prefer {@link Collection#iterator()} over more contrived or less efficient alternatives. */
+  static final class CollectionIterator<T> {
+    @BeforeTemplate
+    Iterator<T> before(Collection<T> collection) {
+      return collection.stream().iterator();
+    }
+
     @BeforeTemplate
     Iterator<T> before(ImmutableCollection<T> collection) {
       return collection.asList().iterator();
     }
 
     @AfterTemplate
-    Iterator<T> after(ImmutableCollection<T> collection) {
+    Iterator<T> after(Collection<T> collection) {
       return collection.iterator();
     }
   }
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java
index c667a80895..6936af3223 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java
@@ -119,8 +119,9 @@ Integer[] testImmutableCollectionToArrayWithGenerator() {
     return ImmutableSet.of(1).asList().toArray(Integer[]::new);
   }
 
-  Iterator<Integer> testImmutableCollectionIterator() {
-    return ImmutableSet.of(1).asList().iterator();
+  ImmutableSet<Iterator<Integer>> testCollectionIterator() {
+    return ImmutableSet.of(
+        ImmutableSet.of(1).stream().iterator(), ImmutableSet.of(2).asList().iterator());
   }
 
   ImmutableSet<Optional<Integer>> testOptionalFirstCollectionElement() {
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java
index 756135c889..59367ba255 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java
@@ -109,8 +109,8 @@ Integer[] testImmutableCollectionToArrayWithGenerator() {
     return ImmutableSet.of(1).toArray(Integer[]::new);
   }
 
-  Iterator<Integer> testImmutableCollectionIterator() {
-    return ImmutableSet.of(1).iterator();
+  ImmutableSet<Iterator<Integer>> testCollectionIterator() {
+    return ImmutableSet.of(ImmutableSet.of(1).iterator(), ImmutableSet.of(2).iterator());
   }
 
   ImmutableSet<Optional<Integer>> testOptionalFirstCollectionElement() {