From 88e8ff2bc0cd461d42586f1f4633eab65a2a2eaf Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Sun, 20 Oct 2024 11:44:46 +0200 Subject: [PATCH] Keep whitespace between arguments in Guava immutable recipes (#583) * Keep whitespace between arguments in Guava immutable recipes When transforming a call like `ImmutableSet.of(/*comment*/ "foo")` make sure to keep the formatting of the arguments the way it was. It could be formatted with line breaks or even contain comments. Instead of templating all arguments, just template the first one (or the first two in the case of `ImmutableMap`) and rely on `#{any()}` to extract the type from the template parameter, rather than mapping and supplying it manually. * Add `@DocumentExample` to one test case --- .../guava/AbstractNoGuavaImmutableOf.java | 51 ++++++------------ .../guava/NoGuavaImmutableListOfTest.java | 2 +- .../guava/NoGuavaImmutableMapOfTest.java | 2 +- .../guava/NoGuavaImmutableSetOfTest.java | 54 ++++++++++++++----- 4 files changed, 59 insertions(+), 50 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 7da4e70d26..0492c0c6d8 100644 --- a/src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java +++ b/src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java @@ -28,8 +28,6 @@ import org.openrewrite.java.tree.*; import java.time.Duration; -import java.util.Objects; -import java.util.stream.Collectors; abstract class AbstractNoGuavaImmutableOf extends Recipe { @@ -72,45 +70,26 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) maybeRemoveImport(guavaType); maybeAddImport(javaType); - String template = method.getArguments().stream() - .map(arg -> { - if (arg.getType() instanceof JavaType.Primitive) { - String type = ""; - if (JavaType.Primitive.Boolean == arg.getType()) { - type = "Boolean"; - } else if (JavaType.Primitive.Byte == arg.getType()) { - type = "Byte"; - } else if (JavaType.Primitive.Char == arg.getType()) { - type = "Character"; - } else if (JavaType.Primitive.Double == arg.getType()) { - type = "Double"; - } else if (JavaType.Primitive.Float == arg.getType()) { - type = "Float"; - } else if (JavaType.Primitive.Int == arg.getType()) { - type = "Integer"; - } else if (JavaType.Primitive.Long == arg.getType()) { - type = "Long"; - } else if (JavaType.Primitive.Short == arg.getType()) { - type = "Short"; - } else if (JavaType.Primitive.String == arg.getType()) { - type = "String"; - } - return TypeUtils.asFullyQualified(JavaType.buildType("java.lang." + type)); - } else { - return TypeUtils.asFullyQualified(arg.getType()); - } - }) - .filter(Objects::nonNull) - .map(type -> "#{any(" + type.getFullyQualifiedName() + ")}") - .collect(Collectors.joining(",", getShortType(javaType) + ".of(", ")")); + String template; + Object[] args; + if (method.getArguments().isEmpty() || method.getArguments().get(0) instanceof J.Empty) { + template = getShortType(javaType) + ".of()"; + args = new Object[]{}; + } else if ("com.google.common.collect.ImmutableMap".equals(guavaType)) { + template = getShortType(javaType) + ".of(#{any()}, #{any()})"; + args = new Object[]{method.getArguments().get(0), method.getArguments().get(1)}; + } else { + template = getShortType(javaType) + ".of(#{any()})"; + args = new Object[]{method.getArguments().get(0)}; + } - return JavaTemplate.builder(template) - .contextSensitive() + J.MethodInvocation templated = JavaTemplate.builder(template) .imports(javaType) .build() .apply(getCursor(), method.getCoordinates().replace(), - method.getArguments().get(0) instanceof J.Empty ? new Object[]{} : method.getArguments().toArray()); + args); + return templated.getPadding().withArguments(method.getPadding().getArguments()); } return super.visitMethodInvocation(method, ctx); } 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 3bf5b474b3..69381789ec 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableListOfTest.java @@ -505,7 +505,7 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doNotchangeAssignToImmutableList() { + void doNotChangeAssignToImmutableList() { //language=java rewriteRun( version( 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 4166a18f68..ae047798a0 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableMapOfTest.java @@ -501,7 +501,7 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doNotchangeAssignToImmutableMap() { + void doNotChangeAssignToImmutableMap() { //language=java rewriteRun( version( 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 5f5fafb4ee..36aa28b110 100644 --- a/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOfTest.java +++ b/src/test/java/org/openrewrite/java/migrate/guava/NoGuavaImmutableSetOfTest.java @@ -16,13 +16,13 @@ package org.openrewrite.java.migrate.guava; import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; import org.openrewrite.Issue; import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; -import static org.openrewrite.java.Assertions.java; -import static org.openrewrite.java.Assertions.version; +import static org.openrewrite.java.Assertions.*; class NoGuavaImmutableSetOfTest implements RewriteTest { @Override @@ -158,6 +158,7 @@ void method() { } @Test + @DocumentExample void replaceArguments() { //language=java rewriteRun( @@ -471,20 +472,49 @@ class A { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/256") @Test - void doNotchangeAssignToImmutableSet() { + void doNotChangeAssignToImmutableSet() { //language=java rewriteRun( - version( - java( + spec -> spec.allSources(all -> all.markers(javaVersion(9))), + java( + """ + import com.google.common.collect.ImmutableSet; + + class Test { + ImmutableSet m = ImmutableSet.of(); + } """ - import com.google.common.collect.ImmutableSet; + ) + ); + } - class Test { - ImmutableSetp m = ImmutableSet.of(); - } - """ - ), - 9 + @Test + void multiLine() { + //language=java + rewriteRun( + spec -> spec.allSources(all -> all.markers(javaVersion(11))), + java( + """ + import com.google.common.collect.ImmutableSet; + import java.util.Set; + + class Test { + Set m = ImmutableSet.of( + "foo", + "bar" + ); + } + """, + """ + import java.util.Set; + + class Test { + Set m = Set.of( + "foo", + "bar" + ); + } + """ ) ); }