Skip to content

Commit

Permalink
Keep whitespace between arguments in Guava immutable recipes (#583)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
knutwannheden authored Oct 20, 2024
1 parent d67ebf8 commit 88e8ff2
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -158,6 +158,7 @@ void method() {
}

@Test
@DocumentExample
void replaceArguments() {
//language=java
rewriteRun(
Expand Down Expand Up @@ -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<String> m = ImmutableSet.of();
}
"""
import com.google.common.collect.ImmutableSet;
)
);
}

class Test {
ImmutableSetp<String> 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<String> m = ImmutableSet.of(
"foo",
"bar"
);
}
""",
"""
import java.util.Set;
class Test {
Set<String> m = Set.of(
"foo",
"bar"
);
}
"""
)
);
}
Expand Down

0 comments on commit 88e8ff2

Please sign in to comment.