From df50c878cb01055c885de52f7c3e499175e58bba Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 26 Jul 2025 23:51:36 +0200 Subject: [PATCH 1/3] `ReplaceStreamCollectWithToList` should look at return type --- .../util/ReplaceStreamCollectWithToList.java | 44 +++++++++ .../ReplaceStreamCollectWithToListTest.java | 96 +++++++++++++++++++ 2 files changed, 140 insertions(+) diff --git a/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java b/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java index 42c6da6813..fdfa08c0f2 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java +++ b/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java @@ -27,6 +27,8 @@ import org.openrewrite.java.search.UsesMethod; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.TypeUtils; import java.time.Duration; import java.util.Collections; @@ -95,6 +97,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu Expression command = method.getArguments().get(0); if (COLLECT_TO_UNMODIFIABLE_LIST.matches(command) || convertToList && COLLECT_TO_LIST.matches(command)) { + + // Check if the transformation would result in incompatible types + if (!isTypeCompatible(result)) { + return result; + } + maybeRemoveImport("java.util.stream.Collectors"); J.MethodInvocation toList = JavaTemplate.apply("#{any(java.util.stream.Stream)}.toList()", updateCursor(result), result.getCoordinates().replace(), result.getSelect()); @@ -102,5 +110,41 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } return result; } + + private boolean isTypeCompatible(J.MethodInvocation method) { + // Get the type of the collect method invocation (the resulting List type) + JavaType methodType = method.getType(); + if (!(methodType instanceof JavaType.Parameterized)) { + return true; // Conservative: allow transformation if we can't determine the type + } + + JavaType.Parameterized resultListType = (JavaType.Parameterized) methodType; + if (resultListType.getTypeParameters().isEmpty()) { + return true; // No type parameters to check + } + + // Get the stream type to determine what toList() would return + Expression select = method.getSelect(); + if (select == null || select.getType() == null) { + return true; // Conservative: allow transformation if we can't determine the stream type + } + + JavaType streamType = select.getType(); + if (!(streamType instanceof JavaType.Parameterized)) { + return true; // Conservative: allow transformation if stream type is not parameterized + } + + JavaType.Parameterized parameterizedStream = (JavaType.Parameterized) streamType; + if (parameterizedStream.getTypeParameters().isEmpty()) { + return true; // No type parameters to check + } + + JavaType streamElementType = parameterizedStream.getTypeParameters().get(0); + JavaType expectedElementType = resultListType.getTypeParameters().get(0); + + // Check if the stream element type and expected list element type are exactly the same + // If they differ (e.g., Stream but List), don't transform + return TypeUtils.isOfType(streamElementType, expectedElementType); + } } } diff --git a/src/test/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToListTest.java b/src/test/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToListTest.java index 5e2f09a510..36d9f9db81 100644 --- a/src/test/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToListTest.java +++ b/src/test/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToListTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -180,4 +181,99 @@ List test(Stream stream) { ); } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/791") + @Test + void doesNotReplaceWhenReturnTypeIsIncompatible() { + rewriteRun( + //language=java + java( + """ + import java.util.stream.Collectors; + import java.util.stream.Stream; + import java.util.List; + + class Example { + List foo() { + return Stream.of(Integer.valueOf(1)).collect(Collectors.toUnmodifiableList()); + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/791") + @Test + void replacesWhenTypesAreCompatible() { + rewriteRun( + //language=java + java( + """ + import java.util.stream.Collectors; + import java.util.stream.Stream; + import java.util.List; + + class Example { + List foo() { + return Stream.of(Integer.valueOf(1)).collect(Collectors.toUnmodifiableList()); + } + } + """, + """ + import java.util.stream.Stream; + import java.util.List; + + class Example { + List foo() { + return Stream.of(Integer.valueOf(1)).toList(); + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/791") + @Test + void doesNotReplaceInVariableAssignmentWithIncompatibleTypes() { + rewriteRun( + //language=java + java( + """ + import java.util.stream.Collectors; + import java.util.stream.Stream; + import java.util.List; + + class Example { + void foo() { + List numbers = Stream.of(Integer.valueOf(1)).collect(Collectors.toUnmodifiableList()); + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/791") + @Test + void doesNotReplaceWithToListWhenConvertToListFlagIsTrue() { + rewriteRun( + recipeSpec -> recipeSpec.recipe(new ReplaceStreamCollectWithToList(true)), + //language=java + java( + """ + import java.util.stream.Collectors; + import java.util.stream.Stream; + import java.util.List; + + class Example { + List foo() { + return Stream.of(Integer.valueOf(1)).collect(Collectors.toList()); + } + } + """ + ) + ); + } + } From 90e04c1420a05f598fd7411fe6d63badc07c0bef Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 26 Jul 2025 23:55:44 +0200 Subject: [PATCH 2/3] Condense --- .../util/ReplaceStreamCollectWithToList.java | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java b/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java index fdfa08c0f2..447a489b18 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java +++ b/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java @@ -97,12 +97,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu Expression command = method.getArguments().get(0); if (COLLECT_TO_UNMODIFIABLE_LIST.matches(command) || convertToList && COLLECT_TO_LIST.matches(command)) { - + // Check if the transformation would result in incompatible types if (!isTypeCompatible(result)) { return result; } - + maybeRemoveImport("java.util.stream.Collectors"); J.MethodInvocation toList = JavaTemplate.apply("#{any(java.util.stream.Stream)}.toList()", updateCursor(result), result.getCoordinates().replace(), result.getSelect()); @@ -117,31 +117,21 @@ private boolean isTypeCompatible(J.MethodInvocation method) { if (!(methodType instanceof JavaType.Parameterized)) { return true; // Conservative: allow transformation if we can't determine the type } - - JavaType.Parameterized resultListType = (JavaType.Parameterized) methodType; - if (resultListType.getTypeParameters().isEmpty()) { - return true; // No type parameters to check - } - + // Get the stream type to determine what toList() would return Expression select = method.getSelect(); if (select == null || select.getType() == null) { return true; // Conservative: allow transformation if we can't determine the stream type } - + JavaType streamType = select.getType(); if (!(streamType instanceof JavaType.Parameterized)) { return true; // Conservative: allow transformation if stream type is not parameterized } - - JavaType.Parameterized parameterizedStream = (JavaType.Parameterized) streamType; - if (parameterizedStream.getTypeParameters().isEmpty()) { - return true; // No type parameters to check - } - - JavaType streamElementType = parameterizedStream.getTypeParameters().get(0); - JavaType expectedElementType = resultListType.getTypeParameters().get(0); - + + JavaType streamElementType = ((JavaType.Parameterized) streamType).getTypeParameters().get(0); + JavaType expectedElementType = ((JavaType.Parameterized) methodType).getTypeParameters().get(0); + // Check if the stream element type and expected list element type are exactly the same // If they differ (e.g., Stream but List), don't transform return TypeUtils.isOfType(streamElementType, expectedElementType); From 3d632ed5cd7142ff24f1c96d5318092f43d3ad61 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 26 Jul 2025 23:59:03 +0200 Subject: [PATCH 3/3] Condense further --- .../util/ReplaceStreamCollectWithToList.java | 32 ++++++------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java b/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java index 447a489b18..67c1bef233 100644 --- a/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java +++ b/src/main/java/org/openrewrite/java/migrate/util/ReplaceStreamCollectWithToList.java @@ -99,7 +99,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu convertToList && COLLECT_TO_LIST.matches(command)) { // Check if the transformation would result in incompatible types - if (!isTypeCompatible(result)) { + if (!areTypesCompatible(result)) { return result; } @@ -111,30 +111,18 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu return result; } - private boolean isTypeCompatible(J.MethodInvocation method) { - // Get the type of the collect method invocation (the resulting List type) - JavaType methodType = method.getType(); - if (!(methodType instanceof JavaType.Parameterized)) { - return true; // Conservative: allow transformation if we can't determine the type + private boolean areTypesCompatible(J.MethodInvocation method) { + if (method.getSelect() == null || + method.getSelect().getType() == null || + !(method.getSelect().getType() instanceof JavaType.Parameterized) || + !(method.getType() instanceof JavaType.Parameterized)) { + return false; } - - // Get the stream type to determine what toList() would return - Expression select = method.getSelect(); - if (select == null || select.getType() == null) { - return true; // Conservative: allow transformation if we can't determine the stream type - } - - JavaType streamType = select.getType(); - if (!(streamType instanceof JavaType.Parameterized)) { - return true; // Conservative: allow transformation if stream type is not parameterized - } - - JavaType streamElementType = ((JavaType.Parameterized) streamType).getTypeParameters().get(0); - JavaType expectedElementType = ((JavaType.Parameterized) methodType).getTypeParameters().get(0); - // Check if the stream element type and expected list element type are exactly the same // If they differ (e.g., Stream but List), don't transform - return TypeUtils.isOfType(streamElementType, expectedElementType); + return TypeUtils.isOfType( + ((JavaType.Parameterized) method.getSelect().getType()).getTypeParameters().get(0), + ((JavaType.Parameterized) method.getType()).getTypeParameters().get(0)); } } }