Skip to content

Commit 5acb803

Browse files
timtebeekclaude
andcommitted
Fix NoGuavaSetsNewHashSet to skip Iterable-only arguments
Addresses issue #881 where Sets.newHashSet(Iterables.filter(...)) was incorrectly transformed to broken code using Arrays.asList(). The recipe now: - Only transforms Sets.newHashSet(Collection) to new HashSet<>(Collection) - Skips transformation for Iterable-only arguments (not Collection) - Preserves the original Guava code when a safe transformation is not possible This conservative approach prevents generating code with type inference failures and compilation errors. Test cases added: - setsNewHashSetWithIterablesFilter: Verifies Iterables.filter() is skipped - setsNewHashSetWithCustomIterable: Verifies custom Iterable is skipped - setsNewHashSetWithCollectionStillWorks: Verifies Collection case still works 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 92f9a94 commit 5acb803

File tree

2 files changed

+98
-8
lines changed

2 files changed

+98
-8
lines changed

src/main/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSet.java

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,33 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
5555
@Override
5656
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
5757
if (NEW_HASH_SET.matches(method)) {
58-
maybeRemoveImport("com.google.common.collect.Sets");
59-
maybeAddImport("java.util.HashSet");
6058
if (method.getArguments().isEmpty() || method.getArguments().get(0) instanceof J.Empty) {
59+
maybeRemoveImport("com.google.common.collect.Sets");
60+
maybeAddImport("java.util.HashSet");
6161
return JavaTemplate.builder("new HashSet<>()")
6262
.contextSensitive()
6363
.imports("java.util.HashSet")
6464
.build()
6565
.apply(getCursor(), method.getCoordinates().replace());
6666
}
67-
if (method.getArguments().size() == 1 && TypeUtils.isAssignableTo("java.util.Collection", method.getArguments().get(0).getType())) {
68-
return JavaTemplate.builder("new HashSet<>(#{any(java.util.Collection)})")
69-
.contextSensitive()
70-
.imports("java.util.HashSet")
71-
.build()
72-
.apply(getCursor(), method.getCoordinates().replace(), method.getArguments().get(0));
67+
if (method.getArguments().size() == 1) {
68+
// Only handle if it's a Collection (not just any Iterable)
69+
if (TypeUtils.isAssignableTo("java.util.Collection", method.getArguments().get(0).getType())) {
70+
maybeRemoveImport("com.google.common.collect.Sets");
71+
maybeAddImport("java.util.HashSet");
72+
return JavaTemplate.builder("new HashSet<>(#{any(java.util.Collection)})")
73+
.contextSensitive()
74+
.imports("java.util.HashSet")
75+
.build()
76+
.apply(getCursor(), method.getCoordinates().replace(), method.getArguments().get(0));
77+
}
78+
// Skip Iterable-only cases to avoid generating broken code
79+
if (TypeUtils.isAssignableTo("java.lang.Iterable", method.getArguments().get(0).getType())) {
80+
return method;
81+
}
7382
}
83+
maybeRemoveImport("com.google.common.collect.Sets");
84+
maybeAddImport("java.util.HashSet");
7485
maybeAddImport("java.util.Arrays");
7586
JavaTemplate newHashSetVarargs = JavaTemplate.builder("new HashSet<>(Arrays.asList(" + method.getArguments().stream().map(a -> "#{any()}").collect(joining(",")) + "))")
7687
.contextSensitive()

src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,83 @@ class Test {
118118
)
119119
);
120120
}
121+
122+
@Test
123+
void setsNewHashSetWithIterablesFilter() {
124+
//language=java
125+
rewriteRun(
126+
java(
127+
"""
128+
import java.util.ArrayList;
129+
import java.util.List;
130+
131+
import com.google.common.collect.Iterables;
132+
import com.google.common.collect.Sets;
133+
134+
class Test {
135+
public static void test() {
136+
final List<ClassCastException> result = new ArrayList<ClassCastException>();
137+
List<Exception> myExceptions = new ArrayList<Exception>();
138+
result.addAll(Sets.newHashSet(Iterables.filter(myExceptions, ClassCastException.class)));
139+
}
140+
}
141+
"""
142+
)
143+
);
144+
}
145+
146+
@Test
147+
void setsNewHashSetWithCustomIterable() {
148+
//language=java
149+
rewriteRun(
150+
java(
151+
"""
152+
import com.google.common.collect.Sets;
153+
154+
class Test {
155+
public static void test() {
156+
Iterable<String> myIterable = () -> java.util.List.of("a", "b").iterator();
157+
var result = Sets.newHashSet(myIterable);
158+
}
159+
}
160+
"""
161+
)
162+
);
163+
}
164+
165+
@Test
166+
void setsNewHashSetWithCollectionStillWorks() {
167+
//language=java
168+
rewriteRun(
169+
java(
170+
"""
171+
import com.google.common.collect.Sets;
172+
173+
import java.util.ArrayList;
174+
import java.util.List;
175+
import java.util.Set;
176+
177+
class Test {
178+
public static void test() {
179+
List<String> myList = new ArrayList<>();
180+
Set<String> result = Sets.newHashSet(myList);
181+
}
182+
}
183+
""",
184+
"""
185+
import java.util.ArrayList;
186+
import java.util.HashSet;
187+
import java.util.List;
188+
import java.util.Set;
189+
190+
class Test {
191+
public static void test() {
192+
List<String> myList = new ArrayList<>();
193+
Set<String> result = new HashSet<>(myList);
194+
}
195+
}
196+
"""
197+
)
198+
);
199+
}
121200
}

0 commit comments

Comments
 (0)