Skip to content

Commit 67e8d8c

Browse files
timtebeekclaude
andauthored
Fix NoGuavaSetsNewHashSet to skip Iterable-only arguments (#882)
* 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]> * Minimize tests --------- Co-authored-by: Claude <[email protected]>
1 parent 92f9a94 commit 67e8d8c

File tree

2 files changed

+93
-8
lines changed

2 files changed

+93
-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: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,78 @@ 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+
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+
void test(Iterable<String> myIterable) {
156+
var result = Sets.newHashSet(myIterable);
157+
}
158+
}
159+
"""
160+
)
161+
);
162+
}
163+
164+
@Test
165+
void setsNewHashSetWithCollectionStillWorks() {
166+
//language=java
167+
rewriteRun(
168+
java(
169+
"""
170+
import com.google.common.collect.Sets;
171+
172+
import java.util.List;
173+
import java.util.Set;
174+
175+
class Test {
176+
public static void test(List<String> myList) {
177+
Set<String> result = Sets.newHashSet(myList);
178+
}
179+
}
180+
""",
181+
"""
182+
import java.util.HashSet;
183+
import java.util.List;
184+
import java.util.Set;
185+
186+
class Test {
187+
public static void test(List<String> myList) {
188+
Set<String> result = new HashSet<>(myList);
189+
}
190+
}
191+
"""
192+
)
193+
);
194+
}
121195
}

0 commit comments

Comments
 (0)