From 5acb803834125d592771acb4d4a5c205633d6e58 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 20 Oct 2025 17:49:33 +0200 Subject: [PATCH 1/2] Fix NoGuavaSetsNewHashSet to skip Iterable-only arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../migrate/guava/NoGuavaSetsNewHashSet.java | 27 +++++-- .../guava/NoGuavaSetsNewHashSetTest.java | 79 +++++++++++++++++++ 2 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSet.java b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSet.java index 45e3528ca0..7938c1b3eb 100644 --- a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSet.java +++ b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSet.java @@ -55,22 +55,33 @@ public TreeVisitor getVisitor() { @Override public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { if (NEW_HASH_SET.matches(method)) { - maybeRemoveImport("com.google.common.collect.Sets"); - maybeAddImport("java.util.HashSet"); if (method.getArguments().isEmpty() || method.getArguments().get(0) instanceof J.Empty) { + maybeRemoveImport("com.google.common.collect.Sets"); + maybeAddImport("java.util.HashSet"); return JavaTemplate.builder("new HashSet<>()") .contextSensitive() .imports("java.util.HashSet") .build() .apply(getCursor(), method.getCoordinates().replace()); } - if (method.getArguments().size() == 1 && TypeUtils.isAssignableTo("java.util.Collection", method.getArguments().get(0).getType())) { - return JavaTemplate.builder("new HashSet<>(#{any(java.util.Collection)})") - .contextSensitive() - .imports("java.util.HashSet") - .build() - .apply(getCursor(), method.getCoordinates().replace(), method.getArguments().get(0)); + if (method.getArguments().size() == 1) { + // Only handle if it's a Collection (not just any Iterable) + if (TypeUtils.isAssignableTo("java.util.Collection", method.getArguments().get(0).getType())) { + maybeRemoveImport("com.google.common.collect.Sets"); + maybeAddImport("java.util.HashSet"); + return JavaTemplate.builder("new HashSet<>(#{any(java.util.Collection)})") + .contextSensitive() + .imports("java.util.HashSet") + .build() + .apply(getCursor(), method.getCoordinates().replace(), method.getArguments().get(0)); + } + // Skip Iterable-only cases to avoid generating broken code + if (TypeUtils.isAssignableTo("java.lang.Iterable", method.getArguments().get(0).getType())) { + return method; + } } + maybeRemoveImport("com.google.common.collect.Sets"); + maybeAddImport("java.util.HashSet"); maybeAddImport("java.util.Arrays"); JavaTemplate newHashSetVarargs = JavaTemplate.builder("new HashSet<>(Arrays.asList(" + method.getArguments().stream().map(a -> "#{any()}").collect(joining(",")) + "))") .contextSensitive() diff --git a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java index aed1a27630..bee90cfa08 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java @@ -118,4 +118,83 @@ class Test { ) ); } + + @Test + void setsNewHashSetWithIterablesFilter() { + //language=java + rewriteRun( + java( + """ + import java.util.ArrayList; + import java.util.List; + + import com.google.common.collect.Iterables; + import com.google.common.collect.Sets; + + class Test { + public static void test() { + final List result = new ArrayList(); + List myExceptions = new ArrayList(); + result.addAll(Sets.newHashSet(Iterables.filter(myExceptions, ClassCastException.class))); + } + } + """ + ) + ); + } + + @Test + void setsNewHashSetWithCustomIterable() { + //language=java + rewriteRun( + java( + """ + import com.google.common.collect.Sets; + + class Test { + public static void test() { + Iterable myIterable = () -> java.util.List.of("a", "b").iterator(); + var result = Sets.newHashSet(myIterable); + } + } + """ + ) + ); + } + + @Test + void setsNewHashSetWithCollectionStillWorks() { + //language=java + rewriteRun( + java( + """ + import com.google.common.collect.Sets; + + import java.util.ArrayList; + import java.util.List; + import java.util.Set; + + class Test { + public static void test() { + List myList = new ArrayList<>(); + Set result = Sets.newHashSet(myList); + } + } + """, + """ + import java.util.ArrayList; + import java.util.HashSet; + import java.util.List; + import java.util.Set; + + class Test { + public static void test() { + List myList = new ArrayList<>(); + Set result = new HashSet<>(myList); + } + } + """ + ) + ); + } } From 7d825292b6f15d994029d8e7b09eac39197a419a Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 20 Oct 2025 22:34:09 +0200 Subject: [PATCH 2/2] Minimize tests --- .../migrate/guava/NoGuavaSetsNewHashSetTest.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java index bee90cfa08..99f8b21294 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaSetsNewHashSetTest.java @@ -132,7 +132,7 @@ void setsNewHashSetWithIterablesFilter() { import com.google.common.collect.Sets; class Test { - public static void test() { + void test() { final List result = new ArrayList(); List myExceptions = new ArrayList(); result.addAll(Sets.newHashSet(Iterables.filter(myExceptions, ClassCastException.class))); @@ -152,8 +152,7 @@ void setsNewHashSetWithCustomIterable() { import com.google.common.collect.Sets; class Test { - public static void test() { - Iterable myIterable = () -> java.util.List.of("a", "b").iterator(); + void test(Iterable myIterable) { var result = Sets.newHashSet(myIterable); } } @@ -170,26 +169,22 @@ void setsNewHashSetWithCollectionStillWorks() { """ import com.google.common.collect.Sets; - import java.util.ArrayList; import java.util.List; import java.util.Set; class Test { - public static void test() { - List myList = new ArrayList<>(); + public static void test(List myList) { Set result = Sets.newHashSet(myList); } } """, """ - import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; class Test { - public static void test() { - List myList = new ArrayList<>(); + public static void test(List myList) { Set result = new HashSet<>(myList); } }