Skip to content

Commit

Permalink
Isolate JavaTemplate issue with generic type parameters (#4372)
Browse files Browse the repository at this point in the history
* Isolate JavaTemplate issue with generic type parameters

- For openrewrite/rewrite-spring#284

* Appease the bot

* Inline template method `if`

* Also print preceding comments for J prefixes

* Look at padding before methodInvocation too

* Clear out unintentional whitespace

As this trips up the auto format in IntelliJ

* Restore intentional newlines in `changeFieldToMethod`

* Remove duplicate import for test template
  • Loading branch information
timtebeek authored Aug 11, 2024
1 parent edd7d42 commit eeb43a8
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.openrewrite.Issue;
import org.openrewrite.java.tree.*;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import java.util.List;

Expand Down Expand Up @@ -297,7 +298,7 @@ class T {
void m() {
hashCode();
}
void m2() {
hashCode();
}
Expand Down Expand Up @@ -720,20 +721,20 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
class A {
public enum Type {
One;
public Type(String t) {
}
String t;
public static Type fromType(String type) {
return null;
}
}
public A(Type type) {}
public A() {}
public void method(Type type) {
new A(type);
}
Expand All @@ -743,20 +744,20 @@ public void method(Type type) {
class A {
public enum Type {
One;
public Type(String t) {
}
String t;
public static Type fromType(String type) {
return null;
}
}
public A(Type type) {}
public A() {}
public void method(Type type) {
new A();
}
Expand Down Expand Up @@ -864,7 +865,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
java(
"""
import java.util.Collection;
class Test {
void doSomething(Collection<Object> c) {
assert c.size() > 0;
Expand All @@ -873,7 +874,7 @@ void doSomething(Collection<Object> c) {
""",
"""
import java.util.Collection;
class Test {
void doSomething(Collection<Object> c) {
assert !c.isEmpty();
Expand Down Expand Up @@ -1083,7 +1084,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
"""
import java.util.Map;
import org.junit.jupiter.api.Assertions;
class T {
void m(String one, Map<String, ?> map) {
Assertions.assertEquals(one, map.get("one"));
Expand All @@ -1092,9 +1093,9 @@ void m(String one, Map<String, ?> map) {
""",
"""
import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat;
class T {
void m(String one, Map<String, ?> map) {
assertThat(map.get("one")).isEqualTo(one);
Expand Down Expand Up @@ -1139,7 +1140,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
import java.util.Objects;
import java.util.Map;
import java.util.HashMap;
class T {
void m() {
Map<String, ?> map = new HashMap<>();
Expand All @@ -1150,10 +1151,10 @@ void m() {
"""
import java.util.Objects;
import java.util.Map;
import static java.util.Objects.requireNonNull;
import java.util.HashMap;
class T {
void m() {
Map<String, ?> map = new HashMap<>();
Expand Down Expand Up @@ -1181,13 +1182,13 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, Executi
java(
"""
interface Test {
String a;
}
""",
"""
interface Test {
String a();
}
"""
Expand All @@ -1200,19 +1201,72 @@ void finalMethodParameter() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull", "@lombok.NonNull", null)),
java(
"""
import org.jetbrains.annotations.NotNull;
class A {
String testMethod(@NotNull final String test) {}
}
""", """
import lombok.NonNull;
class A {
String testMethod(@NonNull final String test) {}
"""
import org.jetbrains.annotations.NotNull;
class A {
String testMethod(@NotNull final String test) {}
}
""", """
import lombok.NonNull;
class A {
String testMethod(@NonNull final String test) {}
}
""")
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-spring/pull/284")
void replaceMethodInChainFollowedByGenericTypeParameters() {
rewriteRun(
spec -> spec
.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (new MethodMatcher("batch.StepBuilder create()").matches(method)) {
return JavaTemplate.builder("new StepBuilder()")
//.doBeforeParseTemplate(System.out::println)
.contextSensitive()
.build()
.apply(getCursor(), method.getCoordinates().replace());
}
""")
return super.visitMethodInvocation(method, ctx);
}
}))
.afterTypeValidationOptions(TypeValidation.builder().constructorInvocations(false).build()) // Unclear why
.parser(JavaParser.fromJavaVersion().dependsOn(
"""
package batch;
public class StepBuilder {
public static StepBuilder create() { return new StepBuilder(); }
public StepBuilder() {}
public <T> T method() { return null; }
}
"""
)
),
java(
"""
import batch.StepBuilder;
class Foo {
void test() {
StepBuilder.create()
.<String>method();
}
}
""",
"""
import batch.StepBuilder;
class Foo {
void test() {
new StepBuilder()
.<String>method();
}
}
"""
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@

import org.openrewrite.*;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JLeftPadded;
import org.openrewrite.java.tree.JRightPadded;
import org.openrewrite.java.tree.Space;
import org.openrewrite.java.tree.*;

import java.util.*;
import java.util.stream.Collectors;

import static java.util.stream.Collectors.joining;
import static java.util.stream.StreamSupport.stream;


Expand Down Expand Up @@ -174,7 +172,8 @@ private static String printTreeElement(Tree tree) {
return s != null ? s : "";
}

String[] lines = tree.toString().split("\n");
String precedingComments = tree instanceof J ? printComments(((J) tree).getPrefix().getComments()) : "";
String[] lines = (precedingComments + tree).split("\n");
StringBuilder output = new StringBuilder();
for (int i = 0; i < lines.length; i++) {
output.append(lines[i].trim());
Expand All @@ -197,11 +196,17 @@ private static String printSpace(Space space) {
sb.append(" whitespace=\"")
.append(space.getWhitespace()).append("\"");
sb.append(" comments=\"")
.append(String.join(",", space.getComments().stream().map(c -> c.printComment(new Cursor(null, "root"))).collect(Collectors.toList())))
.append("\"");;
.append(printComments(space.getComments()))
.append("\"");
return sb.toString().replace("\n", "\\s\n");
}

private static String printComments(List<Comment> comments) {
return comments.stream()
.map(c -> c.printComment(new Cursor(null, "root")))
.collect(joining(","));
}

@Override
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,17 @@ public String template(Cursor cursor, String template, Space.Location location,
after.append('}');
}

template(next(cursor), cursor.getValue(), before, after, cursor.getValue(), mode);
if (contextSensitive) {
contextTemplate(next(cursor), cursor.getValue(), before, after, cursor.getValue(), mode);
} else {
contextFreeTemplate(next(cursor), cursor.getValue(), before, after);
}

return before.toString().trim() + "\n/*" + TEMPLATE_COMMENT + "*/" + template + "/*" + STOP_COMMENT + "*/" + "\n" + after.toString().trim();
return before.toString().trim() +
"\n/*" + TEMPLATE_COMMENT + "*/" +
template +
"/*" + STOP_COMMENT + "*/\n" +
after.toString().trim();
});
}

Expand All @@ -84,7 +92,8 @@ public <J2 extends J> List<J2> listTemplatedTrees(JavaSourceFile cu, Class<J2> e

@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Integer integer) {
if (getCursor().getParentTreeCursor().getValue() instanceof SourceFile && (classDecl.getSimpleName().equals("__P__") || classDecl.getSimpleName().equals("__M__"))) {
if (getCursor().getParentTreeCursor().getValue() instanceof SourceFile &&
(classDecl.getSimpleName().equals("__P__") || classDecl.getSimpleName().equals("__M__"))) {
// don't visit the __P__ and __M__ classes declaring stubs
return classDecl;
}
Expand Down Expand Up @@ -193,14 +202,6 @@ private boolean isTemplateStopComment(Comment comment) {
return js;
}

private void template(Cursor cursor, J prior, StringBuilder before, StringBuilder after, J insertionPoint, JavaCoordinates.Mode mode) {
if (contextSensitive) {
contextTemplate(cursor, prior, before, after, insertionPoint, mode);
} else {
contextFreeTemplate(cursor, prior, before, after);
}
}

@SuppressWarnings("DataFlowIssue")
protected void contextFreeTemplate(Cursor cursor, J j, StringBuilder before, StringBuilder after) {
if (j instanceof J.Lambda) {
Expand Down Expand Up @@ -774,11 +775,13 @@ private static class RemoveTreeMarker implements Marker {

private static class TemplatedTreeTrimmerVisitor extends JavaVisitor<Integer> {
private boolean stopCommentExists(@Nullable J j) {
if (j != null) {
for (Comment comment : j.getComments()) {
if (comment instanceof TextComment && ((TextComment) comment).getText().equals(STOP_COMMENT)) {
return true;
}
return j != null && stopCommentExists(j.getComments());
}

private static boolean stopCommentExists(List<Comment> comments) {
for (Comment comment : comments) {
if (comment instanceof TextComment && ((TextComment) comment).getText().equals(STOP_COMMENT)) {
return true;
}
}
return false;
Expand All @@ -803,6 +806,13 @@ public J visitMethodInvocation(J.MethodInvocation method, Integer integer) {
//noinspection ConstantConditions
return mi.getSelect();
}
if (method.getTypeParameters() != null) {
// For method chains return `select` if `STOP_COMMENT` is found before `typeParameters`
if (stopCommentExists(mi.getPadding().getTypeParameters().getBefore().getComments())) {
//noinspection ConstantConditions
return mi.getSelect();
}
}
return mi;
}
}
Expand Down

0 comments on commit eeb43a8

Please sign in to comment.