From 59d56a6bfb0ee86438aa1067de1121fe21ec3154 Mon Sep 17 00:00:00 2001 From: BramliAK <39566577+BramliAK@users.noreply.github.com> Date: Mon, 25 Nov 2024 23:12:07 +0100 Subject: [PATCH] Do not convert return types in NoGuavaJava21 by default (#601) * [HotFix] NoGuavaJava21 causing invalid code since v2.28.0 * Apply formatter to minimize diff * add Ability To Convert Return Type as an option * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix conflicts * Apply formatter * Minimize diff by restoring original method order * Minimize diff by restoring original method order * Move tests into nested sub class * Correctly evaluate nullable Boolean --------- Co-authored-by: Tim te Beek Co-authored-by: Tim te Beek Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../guava/AbstractNoGuavaImmutableOf.java | 35 ++- .../migrate/guava/NoGuavaImmutableListOf.java | 6 + .../migrate/guava/NoGuavaImmutableMapOf.java | 6 + .../migrate/guava/NoGuavaImmutableSetOf.java | 6 + .../guava/NoGuavaImmutableListOfTest.java | 274 +++++++++++------- .../guava/NoGuavaImmutableMapOfTest.java | 251 +++++++++------- .../guava/NoGuavaImmutableSetOfTest.java | 139 +++++---- .../java/migrate/guava/NoGuavaJava21Test.java | 27 +- 8 files changed, 461 insertions(+), 283 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java b/src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java index 1a3984d32b..1a6ef36b43 100644 --- a/src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java +++ b/src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java @@ -37,11 +37,25 @@ abstract class AbstractNoGuavaImmutableOf extends Recipe { private final String guavaType; private final String javaType; + @Option(displayName = "Whether to convert return type (the default value is false).", + description = "converting the return type from Guava Type to Java Type " + + "The default value is false.", + example = "true", + required = false) + @Nullable + Boolean convertReturnType; + AbstractNoGuavaImmutableOf(String guavaType, String javaType) { this.guavaType = guavaType; this.javaType = javaType; } + AbstractNoGuavaImmutableOf(String guavaType, String javaType, @Nullable Boolean convertReturnType) { + this.guavaType = guavaType; + this.javaType = javaType; + this.convertReturnType = convertReturnType; + } + private String getShortType(String fullyQualifiedType) { return fullyQualifiedType.substring(javaType.lastIndexOf(".") + 1); } @@ -103,8 +117,9 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) @Override public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { J.VariableDeclarations mv = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx); - - if (multiVariable != mv && TypeUtils.isOfClassType(mv.getType(), guavaType)) { + if (Boolean.TRUE.equals(convertReturnType) && + multiVariable != mv && + TypeUtils.isOfClassType(mv.getType(), guavaType)) { JavaType newType = JavaType.buildType(javaType); mv = mv.withTypeExpression(mv.getTypeExpression() == null ? null : createNewTypeExpression(mv.getTypeExpression(), newType)); @@ -117,7 +132,6 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return variable; })); } - return mv; } @@ -148,7 +162,6 @@ private TypeTree createNewTypeExpression(TypeTree typeTree, JavaType newType) { ); } - private boolean isParentTypeDownCast(MethodCall immutableMethod) { J parent = getCursor().dropParentUntil(J.class::isInstance).getValue(); boolean isParentTypeDownCast = false; @@ -173,10 +186,12 @@ private boolean isParentTypeDownCast(MethodCall immutableMethod) { } else if (parent instanceof J.MethodInvocation) { J.MethodInvocation m = (J.MethodInvocation) parent; int index = m.getArguments().indexOf(immutableMethod); - if (m.getMethodType() != null && index != -1 && !m.getMethodType().getParameterTypes().isEmpty()) { - isParentTypeDownCast = isParentTypeMatched(m.getMethodType().getParameterTypes().get(index)); - } else { - isParentTypeDownCast = true; + if (m.getMethodType() != null) { + if (index != -1 && !m.getMethodType().getParameterTypes().isEmpty()) { + isParentTypeDownCast = isParentTypeMatched(m.getMethodType().getParameterTypes().get(index)); + } else { + isParentTypeDownCast = !TypeUtils.isOfClassType(m.getMethodType().getReturnType(), guavaType); + } } } else if (parent instanceof J.NewClass) { J.NewClass c = (J.NewClass) parent; @@ -207,8 +222,8 @@ private boolean isParentTypeDownCast(MethodCall immutableMethod) { private boolean isParentTypeMatched(@Nullable JavaType type) { JavaType.FullyQualified fq = TypeUtils.asFullyQualified(type); return TypeUtils.isOfClassType(fq, javaType) || - TypeUtils.isOfClassType(fq, "java.lang.Object") || - TypeUtils.isOfClassType(fq, guavaType); + (Boolean.TRUE.equals(convertReturnType) && TypeUtils.isOfClassType(fq, guavaType)) || + TypeUtils.isOfClassType(fq, "java.lang.Object"); } }); } diff --git a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOf.java b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOf.java index 69e71f1b1f..f83fb83717 100644 --- a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOf.java +++ b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOf.java @@ -15,8 +15,14 @@ */ package org.openrewrite.java.migrate.guava; +import org.jspecify.annotations.Nullable; + public class NoGuavaImmutableListOf extends AbstractNoGuavaImmutableOf { public NoGuavaImmutableListOf() { super("com.google.common.collect.ImmutableList", "java.util.List"); } + + public NoGuavaImmutableListOf(@Nullable Boolean convertReturnType) { + super("com.google.common.collect.ImmutableList", "java.util.List", convertReturnType); + } } diff --git a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOf.java b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOf.java index 55866046a9..6418031867 100644 --- a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOf.java +++ b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOf.java @@ -15,8 +15,14 @@ */ package org.openrewrite.java.migrate.guava; +import org.jspecify.annotations.Nullable; + public class NoGuavaImmutableMapOf extends AbstractNoGuavaImmutableOf { public NoGuavaImmutableMapOf() { super("com.google.common.collect.ImmutableMap", "java.util.Map"); } + + public NoGuavaImmutableMapOf(@Nullable Boolean convertReturnType) { + super("com.google.common.collect.ImmutableMap", "java.util.Map", convertReturnType); + } } diff --git a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOf.java b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOf.java index 17fc5649e8..37ccf18ab3 100644 --- a/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOf.java +++ b/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOf.java @@ -15,8 +15,14 @@ */ package org.openrewrite.java.migrate.guava; +import org.jspecify.annotations.Nullable; + public class NoGuavaImmutableSetOf extends AbstractNoGuavaImmutableOf { public NoGuavaImmutableSetOf() { super("com.google.common.collect.ImmutableSet", "java.util.Set"); } + + public NoGuavaImmutableSetOf(@Nullable Boolean convertReturnType) { + super("com.google.common.collect.ImmutableSet", "java.util.Set", convertReturnType); + } } diff --git a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOfTest.java b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOfTest.java index 98b286fbb8..79420d7293 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOfTest.java @@ -35,18 +35,20 @@ public void defaults(RecipeSpec spec) { @Test void doNotChangeReturnsImmutableList() { rewriteRun( - //language=java - java( - """ - import com.google.common.collect.ImmutableList; - - class Test { - ImmutableList getList() { - return ImmutableList.of(); - } - } + version( + //language=java + java( """ - ) + import com.google.common.collect.ImmutableList; + + class Test { + ImmutableList getList() { + return ImmutableList.of(); + } + } + """ + ) + , 9) ); } @@ -54,40 +56,44 @@ ImmutableList getList() { @Test void doNotChangeFieldAssignmentToImmutableList() { rewriteRun( - //language=java - java( - """ - import com.google.common.collect.ImmutableList; - - class Test { - ImmutableList m; - - { - this.m = ImmutableList.of(); - } - } + version( + //language=java + java( """ - ) + import com.google.common.collect.ImmutableList; + + class Test { + ImmutableList m; + + { + this.m = ImmutableList.of(); + } + } + """ + ) + , 9) ); } @Test void doNotChangeAssignsToImmutableList() { rewriteRun( - //language=java - java( - """ - import com.google.common.collect.ImmutableList; - - class Test { - ImmutableList m; - - void init() { - m = ImmutableList.of(); - } - } + version( + //language=java + java( """ - ) + import com.google.common.collect.ImmutableList; + + class Test { + ImmutableList m; + + void init() { + m = ImmutableList.of(); + } + } + """ + ) + , 9) ); } @@ -98,7 +104,7 @@ void doNotChangeNewClass() { java( """ import com.google.common.collect.ImmutableList; - + public class A { ImmutableList immutableList; public A(ImmutableList immutableList) { @@ -107,15 +113,17 @@ public A(ImmutableList immutableList) { } """ ), - //language=java - java( - """ - import com.google.common.collect.ImmutableList; - - class Test { - A a = new A(ImmutableList.of()); - } - """) + version( + //language=java + java( + """ + import com.google.common.collect.ImmutableList; + + class Test { + A a = new A(ImmutableList.of()); + } + """) + , 9) ); } @@ -129,7 +137,7 @@ void doNotChangeMethodInvocation() { //language=java """ import com.google.common.collect.ImmutableList; - + public class A { ImmutableList immutableList; public void method(ImmutableList immutableList) { @@ -139,19 +147,21 @@ public void method(ImmutableList immutableList) { """ ) ), - //language=java - java( - """ - import com.google.common.collect.ImmutableList; - - class Test { - void method() { - A a = new A(); - a.method(ImmutableList.of()); - } - } + version( + //language=java + java( """ - ) + import com.google.common.collect.ImmutableList; + + class Test { + void method() { + A a = new A(); + a.method(ImmutableList.of()); + } + } + """ + ) + , 9) ); } @@ -164,14 +174,14 @@ void replaceArguments() { """ import java.util.List; import com.google.common.collect.ImmutableList; - + class Test { List m = ImmutableList.of("A", "B", "C", "D"); } """, """ import java.util.List; - + class Test { List m = List.of("A", "B", "C", "D"); } @@ -191,7 +201,7 @@ void fieldAssignmentToList() { """ import java.util.List; import com.google.common.collect.ImmutableList; - + class Test { List m; { @@ -201,7 +211,7 @@ class Test { """, """ import java.util.List; - + class Test { List m; { @@ -224,14 +234,14 @@ void assignmentToList() { """ import java.util.List; import com.google.common.collect.ImmutableList; - + class Test { List m = ImmutableList.of(); } """, """ import java.util.List; - + class Test { List m = List.of(); } @@ -251,7 +261,7 @@ void returnsList() { """ import java.util.List; import com.google.common.collect.ImmutableList; - + class Test { List list() { return ImmutableList.of(); @@ -260,7 +270,7 @@ List list() { """, """ import java.util.List; - + class Test { List list() { return List.of(); @@ -280,7 +290,7 @@ void newClassWithListArgument() { java( """ import java.util.List; - + public class A { List list; public A(List list) { @@ -294,14 +304,14 @@ public A(List list) { java( """ import com.google.common.collect.ImmutableList; - + class Test { A a = new A(ImmutableList.of()); } """, """ import java.util.List; - + class Test { A a = new A(List.of()); } @@ -319,7 +329,7 @@ void doChangeMethodInvocationWithSelect() { java( """ import java.util.List; - + public class A { Object[] list; public void method(Object[] list ) { @@ -333,7 +343,7 @@ public void method(Object[] list ) { java( """ import com.google.common.collect.ImmutableList; - + class Test { void method() { A a = new A(); @@ -343,7 +353,7 @@ void method() { """, """ import java.util.List; - + class Test { void method() { A a = new A(); @@ -364,7 +374,7 @@ void methodInvocationWithListArgument() { java( """ import java.util.List; - + public class A { List list; public void method(List list) { @@ -378,7 +388,7 @@ public void method(List list) { java( """ import com.google.common.collect.ImmutableList; - + class Test { void method() { A a = new A(); @@ -388,7 +398,7 @@ void method() { """, """ import java.util.List; - + class Test { void method() { A a = new A(); @@ -412,7 +422,7 @@ void listOfInts() { """ import java.util.List; import com.google.common.collect.ImmutableList; - + class Test { List list() { return ImmutableList.of(1, 2, 3); @@ -421,7 +431,7 @@ List list() { """, """ import java.util.List; - + class Test { List list() { return List.of(1, 2, 3); @@ -443,7 +453,7 @@ void insideAnonymousArrayInitializer() { java( """ import com.google.common.collect.ImmutableList; - + class A { Object[] o = new Object[] { ImmutableList.of(1, 2, 3) @@ -452,7 +462,7 @@ class A { """, """ import java.util.List; - + class A { Object[] o = new Object[] { List.of(1, 2, 3) @@ -474,14 +484,14 @@ void assignToMoreGeneralType() { java( """ import com.google.common.collect.ImmutableList; - + class A { Object o = ImmutableList.of(1, 2, 3); } """, """ import java.util.List; - + class A { Object o = List.of(1, 2, 3); } @@ -502,42 +512,90 @@ void doChangeAssignToImmutableMap() { """ import com.google.common.collect.ImmutableList; import java.util.List; - + class A { Object o = List.of(ImmutableList.of(1, 2), ImmutableList.of(2, 3)); } + """ + ), + 9 + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/489") + @Test + void doChangeWithStreamOperation() { + rewriteRun( + version( + //language=java + java( + """ + import com.google.common.collect.ImmutableList; + + class A { + void test() { + final List list = ImmutableList.of(1, 2).stream().toList(); + } + } """, """ import java.util.List; - + class A { - Object o = List.of(List.of(1, 2), List.of(2, 3)); + void test() { + final List list = List.of(1, 2).stream().toList(); + } } """ ), - 9 + 11 ) ); } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/489") @Test - void doChangeNestedListsWithVariableType() { + void doNotChangeWithImmutableListMethod() { + rewriteRun( + version( + //language=java + java( + """ + import com.google.common.collect.ImmutableList; + + class A { + void test() { + ImmutableList.of(1, 2).asList(); + } + } + """ + ), + 11 + ) + ); + } + + @Test + void doChangeAllAssignToImmutableMap() { //language=java rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableListOf(true)), version( java( """ import com.google.common.collect.ImmutableList; - + import java.util.List; + class A { - ImmutableList> o = ImmutableList.of(ImmutableList.of(1, 2), ImmutableList.of(2, 3)); + Object o = List.of(ImmutableList.of(1, 2), ImmutableList.of(2, 3)); } """, """ import java.util.List; - + class A { - List> o = List.of(List.of(1, 2), List.of(2, 3)); + Object o = List.of(List.of(1, 2), List.of(2, 3)); } """ ), @@ -547,57 +605,54 @@ class A { } @Test - void doChangeAssignFromImmutableListToList() { + void doChangeNestedListsWithVariableType() { + //language=java rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableListOf(true)), version( - //language=java java( """ import com.google.common.collect.ImmutableList; - + class A { - void test() { - ImmutableList o = ImmutableList.of(1, 2); - } + ImmutableList> o = ImmutableList.of(ImmutableList.of(1, 2), ImmutableList.of(2, 3)); } """, """ import java.util.List; - + class A { - void test() { - List o = List.of(1, 2); - } + List> o = List.of(List.of(1, 2), List.of(2, 3)); } """ ), - 11 + 9 ) ); } - @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/489") @Test - void doChangeWithStreamOperation() { + void doChangeAssignFromImmutableListToList() { rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableListOf(true)), version( //language=java java( """ import com.google.common.collect.ImmutableList; - + class A { void test() { - final List list = ImmutableList.of(1, 2).stream().toList(); + ImmutableList o = ImmutableList.of(1, 2); } } """, """ import java.util.List; - + class A { void test() { - final List list = List.of(1, 2).stream().toList(); + List o = List.of(1, 2); } } """ @@ -606,5 +661,4 @@ void test() { ) ); } - } diff --git a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOfTest.java b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOfTest.java index 0c3af4c267..8f5989e4bd 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOfTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.java.migrate.guava; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.openrewrite.Issue; import org.openrewrite.java.JavaParser; @@ -39,7 +40,7 @@ void doNotChangeReturnsImmutableMap() { java( """ import com.google.common.collect.ImmutableMap; - + class Test { ImmutableMap getMap() { return ImmutableMap.of(); @@ -57,10 +58,10 @@ void doNotChangeFieldAssignmentToImmutableMap() { java( """ import com.google.common.collect.ImmutableMap; - + class Test { ImmutableMap m; - + { this.m = ImmutableMap.of(); } @@ -77,10 +78,10 @@ void doNotChangeAssignsToImmutableMap() { java( """ import com.google.common.collect.ImmutableMap; - + class Test { ImmutableMap m; - + void init() { m = ImmutableMap.of(); } @@ -97,7 +98,7 @@ void doNotChangeNewClass() { java( """ import com.google.common.collect.ImmutableMap; - + public class A { ImmutableMap immutableMap; public A(ImmutableMap immutableMap) { @@ -109,7 +110,7 @@ public A(ImmutableMap immutableMap) { java( """ import com.google.common.collect.ImmutableMap; - + class Test { A a = new A(ImmutableMap.of()); } @@ -125,7 +126,7 @@ void doNotChangeMethodInvocation() { java( """ import com.google.common.collect.ImmutableMap; - + public class A { ImmutableMap immutableMap; public void method(ImmutableMap immutableMap) { @@ -137,7 +138,7 @@ public void method(ImmutableMap immutableMap) { java( """ import com.google.common.collect.ImmutableMap; - + class Test { void method() { A a = new A(); @@ -158,14 +159,14 @@ void replaceArguments() { """ import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { Map m = ImmutableMap.of("A", "B", "C", "D"); } """, """ import java.util.Map; - + class Test { Map m = Map.of("A", "B", "C", "D"); } @@ -185,7 +186,7 @@ void fieldAssignmentToMap() { """ import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { Map m; { @@ -195,7 +196,7 @@ class Test { """, """ import java.util.Map; - + class Test { Map m; { @@ -218,14 +219,14 @@ void assignmentToMap() { """ import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { Map m = ImmutableMap.of(); } """, """ import java.util.Map; - + class Test { Map m = Map.of(); } @@ -245,7 +246,7 @@ void returnsMap() { """ import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { Map map() { return ImmutableMap.of(); @@ -254,7 +255,7 @@ Map map() { """, """ import java.util.Map; - + class Test { Map map() { return Map.of(); @@ -277,7 +278,7 @@ void mapOfInts() { """ import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { Map map() { return ImmutableMap.of(1, 1, 2, 2, 3, 3); @@ -286,7 +287,7 @@ Map map() { """, """ import java.util.Map; - + class Test { Map map() { return Map.of(1, 1, 2, 2, 3, 3); @@ -306,7 +307,7 @@ void newClassWithMapArgument() { java( """ import java.util.Map; - + public class A { Map map; public A(Map map) { @@ -319,14 +320,14 @@ public A(Map map) { java( """ import com.google.common.collect.ImmutableMap; - + class Test { A a = new A(ImmutableMap.of()); } """, """ import java.util.Map; - + class Test { A a = new A(Map.of()); } @@ -344,7 +345,7 @@ void methodInvocationWithMapArgument() { java( """ import java.util.Map; - + public class A { Map map; public void method(Map map) { @@ -357,7 +358,7 @@ public void method(Map map) { java( """ import com.google.common.collect.ImmutableMap; - + class Test { void method() { A a = new A(); @@ -367,7 +368,7 @@ void method() { """, """ import java.util.Map; - + class Test { void method() { A a = new A(); @@ -391,7 +392,7 @@ void variableIsMap() { import java.util.HashMap; import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { Map> map = new HashMap<>(); void setMap(String value) { @@ -404,7 +405,7 @@ void setMap(String value) { """ import java.util.HashMap; import java.util.Map; - + class Test { Map> map = new HashMap<>(); void setMap(String value) { @@ -429,7 +430,7 @@ void insideAnonymousArrayInitializer() { java( """ import com.google.common.collect.ImmutableMap; - + class A { Object[] o = new Object[] { ImmutableMap.of(1, 1, 2, 2, 3, 3) @@ -438,7 +439,7 @@ class A { """, """ import java.util.Map; - + class A { Object[] o = new Object[] { Map.of(1, 1, 2, 2, 3, 3) @@ -460,14 +461,14 @@ void assignToMoreGeneralType() { java( """ import com.google.common.collect.ImmutableMap; - + class A { Object o = ImmutableMap.of(1, 1, 2, 2, 3, 3); } """, """ import java.util.Map; - + class A { Object o = Map.of(1, 1, 2, 2, 3, 3); } @@ -480,7 +481,7 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doChangeNestedMaps() { + void doNotChangeNestedMaps() { //language=java rewriteRun( version( @@ -488,42 +489,9 @@ void doChangeNestedMaps() { """ import com.google.common.collect.ImmutableMap; import java.util.Map; - - class A { - Object o = Map.of(1, ImmutableMap.of(2, 3)); - } - """, - """ - import java.util.Map; - - class A { - Object o = Map.of(1, Map.of(2, 3)); - } - """ - ), - 11 - ) - ); - } - @Test - void doChangeNestedMaps2() { - //language=java - rewriteRun( - version( - java( - """ - import com.google.common.collect.ImmutableMap; - - class A { - Object o = ImmutableMap.of(1, ImmutableMap.of(2, 3)); - } - """, - """ - import java.util.Map; - class A { - Object o = Map.of(1, Map.of(2, 3)); + Object o = Map.of(1, ImmutableMap.of(2, 3)); } """ ), @@ -534,7 +502,7 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doChangeAssignToImmutableMap() { + void doNotChangeAssignToImmutableMap() { //language=java rewriteRun( version( @@ -542,17 +510,10 @@ void doChangeAssignToImmutableMap() { """ import java.util.Map; import com.google.common.collect.ImmutableMap; - + class Test { ImmutableMap m = ImmutableMap.of(); } - """, - """ - import java.util.Map; - - class Test { - Map m = Map.of(); - } """ ), 9 @@ -560,34 +521,122 @@ class Test { ); } - @Test - void doChangeNestedAssignReturnType() { - rewriteRun( - version( + @Nested + class ReturnTypeTrue { + + @Test + void doChangeNestedMaps() { //language=java - java( - """ - import com.google.common.collect.ImmutableMap; - - class A { - public void getMap() { - ImmutableMap> s = ImmutableMap.of("key", ImmutableMap.of("value1", "value2")); + rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableMapOf(true)), + version( + java( + """ + import com.google.common.collect.ImmutableMap; + import java.util.Map; + + class A { + Object o = Map.of(1, ImmutableMap.of(2, 3)); } - } - """, - """ - import java.util.Map; - - class A { - public void getMap() { - Map> s = Map.of("key", Map.of("value1", "value2")); + """, + """ + import java.util.Map; + + class A { + Object o = Map.of(1, Map.of(2, 3)); } - } - """ - ), - 9 - ) - ); - } + """ + ), + 11 + ) + ); + } + + @Test + void doChangeNestedMaps2() { + //language=java + rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableMapOf(true)), + version( + java( + """ + import com.google.common.collect.ImmutableMap; + + class A { + Object o = ImmutableMap.of(1, ImmutableMap.of(2, 3)); + } + """, + """ + import java.util.Map; + + class A { + Object o = Map.of(1, Map.of(2, 3)); + } + """ + ), + 11 + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") + @Test + void doChangeAssignToImmutableMap() { + //language=java + rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableMapOf(true)), + version( + java( + """ + import java.util.Map; + import com.google.common.collect.ImmutableMap; + + class Test { + ImmutableMap m = ImmutableMap.of(); + } + """, + """ + import java.util.Map; + + class Test { + Map m = Map.of(); + } + """ + ), + 9 + ) + ); + } + + @Test + void doChangeNestedAssignReturnType() { + rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableMapOf(true)), + version( + //language=java + java( + """ + import com.google.common.collect.ImmutableMap; + + class A { + public void getMap() { + ImmutableMap> s = ImmutableMap.of("key", ImmutableMap.of("value1", "value2")); + } + } + """, + """ + import java.util.Map; + + class A { + public void getMap() { + Map> s = Map.of("key", Map.of("value1", "value2")); + } + } + """ + ), + 9 + ) + ); + } + } } diff --git a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOfTest.java b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOfTest.java index d868741261..6804d8873e 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOfTest.java @@ -39,7 +39,7 @@ void doNotChangeReturnsImmutableSet() { java( """ import com.google.common.collect.ImmutableSet; - + class Test { ImmutableSet getSet() { return ImmutableSet.of(); @@ -57,10 +57,10 @@ void doNotChangeFieldAssignmentToImmutableSet() { java( """ import com.google.common.collect.ImmutableSet; - + class Test { ImmutableSet m; - + { this.m = ImmutableSet.of(); } @@ -77,10 +77,10 @@ void doNotChangeAssignsToImmutableSet() { java( """ import com.google.common.collect.ImmutableSet; - + class Test { ImmutableSet m; - + void init() { m = ImmutableSet.of(); } @@ -100,7 +100,7 @@ void doNotChangeNewClass() { //language=java """ import com.google.common.collect.ImmutableSet; - + public class A { ImmutableSet immutableSet; public A(ImmutableSet immutableSet) { @@ -113,7 +113,7 @@ public A(ImmutableSet immutableSet) { java( """ import com.google.common.collect.ImmutableSet; - + class Test { A a = new A(ImmutableSet.of()); } @@ -132,7 +132,7 @@ void doNotChangeMethodInvocation() { //language=java """ import com.google.common.collect.ImmutableSet; - + public class A { ImmutableSet immutableSet; public void method(ImmutableSet immutableSet) { @@ -145,7 +145,7 @@ public void method(ImmutableSet immutableSet) { java( """ import com.google.common.collect.ImmutableSet; - + class Test { void method() { A a = new A(); @@ -167,14 +167,14 @@ void replaceArguments() { """ import java.util.Set; import com.google.common.collect.ImmutableSet; - + class Test { Set m = ImmutableSet.of("A", "B", "C", "D"); } """, """ import java.util.Set; - + class Test { Set m = Set.of("A", "B", "C", "D"); } @@ -194,7 +194,7 @@ void fieldAssignmentToSet() { """ import java.util.Set; import com.google.common.collect.ImmutableSet; - + class Test { Set m; { @@ -204,7 +204,7 @@ class Test { """, """ import java.util.Set; - + class Test { Set m; { @@ -227,14 +227,14 @@ void assignmentToSet() { """ import java.util.Set; import com.google.common.collect.ImmutableSet; - + class Test { Set m = ImmutableSet.of(); } """, """ import java.util.Set; - + class Test { Set m = Set.of(); } @@ -254,7 +254,7 @@ void returnsSet() { """ import java.util.Set; import com.google.common.collect.ImmutableSet; - + class Test { Set set() { return ImmutableSet.of(); @@ -263,7 +263,7 @@ Set set() { """, """ import java.util.Set; - + class Test { Set set() { return Set.of(); @@ -286,7 +286,7 @@ void setOfInts() { """ import java.util.Set; import com.google.common.collect.ImmutableSet; - + class Test { Set set() { return ImmutableSet.of(1, 2, 3); @@ -295,7 +295,7 @@ Set set() { """, """ import java.util.Set; - + class Test { Set set() { return Set.of(1, 2, 3); @@ -315,7 +315,7 @@ void newClassWithSetArgument() { java( """ import java.util.Set; - + public class A { Set set; public A(Set set) { @@ -329,14 +329,14 @@ public A(Set set) { java( """ import com.google.common.collect.ImmutableSet; - + class Test { A a = new A(ImmutableSet.of()); } """, """ import java.util.Set; - + class Test { A a = new A(Set.of()); } @@ -354,7 +354,7 @@ void methodInvocationWithSetArgument() { java( """ import java.util.Set; - + public class A { Set set; public void method(Set set) { @@ -367,7 +367,7 @@ public void method(Set set) { java( """ import com.google.common.collect.ImmutableSet; - + class Test { void method() { A a = new A(); @@ -377,7 +377,7 @@ void method() { """, """ import java.util.Set; - + class Test { void method() { A a = new A(); @@ -400,7 +400,7 @@ void insideAnonymousArrayInitializer() { java( """ import com.google.common.collect.ImmutableSet; - + class A { Object[] o = new Object[] { ImmutableSet.of(1, 2, 3) @@ -409,7 +409,7 @@ class A { """, """ import java.util.Set; - + class A { Object[] o = new Object[] { Set.of(1, 2, 3) @@ -431,14 +431,14 @@ void assignToMoreGeneralType() { java( """ import com.google.common.collect.ImmutableSet; - + class A { Object o = ImmutableSet.of(1, 2, 3); } """, """ import java.util.Set; - + class A { Object o = Set.of(1, 2, 3); } @@ -451,7 +451,7 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doChangeNestedSets() { + void doNotChangeNestedSets() { //language=java rewriteRun( version( @@ -459,17 +459,10 @@ void doChangeNestedSets() { """ import com.google.common.collect.ImmutableSet; import java.util.Set; - + class A { Object o = Set.of(ImmutableSet.of(1, 2)); } - """, - """ - import java.util.Set; - - class A { - Object o = Set.of(Set.of(1, 2)); - } """ ), 9 @@ -479,24 +472,17 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doChangeAssignToImmutableSet() { + void doNotChangeAssignToImmutableSet() { //language=java rewriteRun( spec -> spec.allSources(all -> all.markers(javaVersion(9))), java( """ import com.google.common.collect.ImmutableSet; - + class Test { ImmutableSet m = ImmutableSet.of(); } - """, - """ - import java.util.Set; - - class Test { - Set m = Set.of(); - } """ ) ); @@ -511,7 +497,7 @@ void multiLine() { """ import com.google.common.collect.ImmutableSet; import java.util.Set; - + class Test { Set m = ImmutableSet.of( "foo", @@ -521,7 +507,7 @@ class Test { """, """ import java.util.Set; - + class Test { Set m = Set.of( "foo", @@ -532,4 +518,59 @@ class Test { ) ); } + + @Test + void doChangeNestedSets() { + //language=java + rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableSetOf(true)), + version( + java( + """ + import com.google.common.collect.ImmutableSet; + import java.util.Set; + + class A { + Object o = Set.of(ImmutableSet.of(1, 2)); + } + """, + """ + import java.util.Set; + + class A { + Object o = Set.of(Set.of(1, 2)); + } + """ + ), + 9 + ) + ); + } + + @Test + void doChangeAssignToImmutableSet() { + //language=java + rewriteRun( + spec -> spec.recipe(new NoGuavaImmutableSetOf(true)), + version( + java( + """ + import com.google.common.collect.ImmutableSet; + + class Test { + ImmutableSet m = ImmutableSet.of(); + } + """, + """ + import java.util.Set; + + class Test { + Set m = Set.of(); + } + """ + ), + 11) + ); + } + } diff --git a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaJava21Test.java b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaJava21Test.java index b49fac08f0..a2a7359b71 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaJava21Test.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaJava21Test.java @@ -43,16 +43,16 @@ void preferMathClampForDouble() { rewriteRun( version( java( - """ + """ import com.google.common.primitives.Doubles; - + class Test { public double testMethod() { return Doubles.constrainToRange(20D, 10D, 100D); } } """, - """ + """ class Test { public double testMethod() { return Math.clamp(20D, 10D, 100D); @@ -70,16 +70,16 @@ void preferMathClampForLongs() { rewriteRun( version( java( - """ + """ import com.google.common.primitives.Longs; - + class Test { public long testMethod() { return Longs.constrainToRange(20L, 10L, 100L); } } """, - """ + """ class Test { public long testMethod() { return Math.clamp(20L, 10L, 100L); @@ -97,16 +97,16 @@ void preferMathClampForFloats() { rewriteRun( version( java( - """ + """ import com.google.common.primitives.Floats; - + class Test { public float testMethod() { return Floats.constrainToRange(20F, 10F, 100F); } } """, - """ + """ class Test { public float testMethod() { return Math.clamp(20F, 10F, 100F); @@ -128,7 +128,7 @@ void noGuavaImmutableOfException() { """ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableMap; - + class A { public Object getMap() { return ImmutableMap.of("key", ImmutableSet.of("value1", "value2")); @@ -136,12 +136,13 @@ public Object getMap() { } """, """ + import com.google.common.collect.ImmutableSet; + import java.util.Map; - import java.util.Set; - + class A { public Object getMap() { - return Map.of("key", Set.of("value1", "value2")); + return Map.of("key", ImmutableSet.of("value1", "value2")); } } """