From 50f38d56ebb94ad606361066d4ac7a1c96c233e5 Mon Sep 17 00:00:00 2001 From: Anton Pinsky <145679039+api-from-the-ion@users.noreply.github.com> Date: Mon, 27 Nov 2023 13:58:39 +0100 Subject: [PATCH] =?UTF-8?q?[#3716]=20Test=20for=20an=20IllegalArgumentExce?= =?UTF-8?q?ption=20in=20JavaTemplateJava=E2=80=A6=20(#3717)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [#3716] WIP: Test for an IllegalArgumentException in JavaTemplateJavaExtension; not running * [#3716] Fix for the case the placeholder is generic with unknown type; tests for the case * [#3716] Put more context in the exception message * Fix some formatting in tests --------- Co-authored-by: Knut Wannheden --- .../java/JavaTemplateSubstitutionsTest.java | 33 +++++ .../openrewrite/java/JavaTemplateTest.java | 126 +++++++++++++++++- .../template/JavaTemplateJavaExtension.java | 3 +- .../java/internal/template/Substitutions.java | 3 +- 4 files changed, 162 insertions(+), 3 deletions(-) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateSubstitutionsTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateSubstitutionsTest.java index 35bff44425e..f53098e0e9a 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateSubstitutionsTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateSubstitutionsTest.java @@ -424,4 +424,37 @@ void test(boolean condition) { ) ); } + + @Test + void testAnyIsGenericWithUnknownType() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() { + @Override + public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + return JavaTemplate.builder("System.out.println(#{any()})") + .contextSensitive() + .build() + .apply(getCursor(), method.getCoordinates().replace(), method); + } + })).cycles(1).expectedCyclesThatMakeChanges(1), + java( + """ + import java.util.Map; + class Test { + void test(Map map) { + map.get("test"); + } + } + """, + """ + import java.util.Map; + class Test { + void test(Map map) { + System.out.println(map.get("test")); + } + } + """ + ) + ); + } } diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java index df1bfcf0288..a9a8cfba304 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java @@ -23,6 +23,8 @@ import org.openrewrite.java.tree.*; import org.openrewrite.test.RewriteTest; +import java.util.List; + import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.RewriteTest.toRecipe; @@ -995,7 +997,9 @@ class T { void addStatementInIfBlock() { rewriteRun( spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() { - final MethodMatcher lowerCaseMatcher = new MethodMatcher("java.lang.String toLowerCase()"); @Override + final MethodMatcher lowerCaseMatcher = new MethodMatcher("java.lang.String toLowerCase()"); + + @Override public J.Block visitBlock(J.Block block, ExecutionContext ctx) { J.Block newBlock = super.visitBlock(block, ctx); if (newBlock.getStatements().stream().noneMatch(J.VariableDeclarations.class::isInstance)) { @@ -1041,4 +1045,124 @@ void test() { ) ); } + + @Test + void replaceMethodCallWithGenericParameterWithUnknownType() { + rewriteRun( + spec -> spec.parser(JavaParser.fromJavaVersion().classpath("junit-jupiter-api")) + .recipe(toRecipe(() -> new JavaIsoVisitor<>() { + private final JavaParser.Builder assertionsParser = JavaParser.fromJavaVersion() + .classpath("assertj-core"); + + private static final MethodMatcher JUNIT_ASSERT_EQUALS = new MethodMatcher("org.junit.jupiter.api.Assertions assertEquals(..)"); + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + if (!JUNIT_ASSERT_EQUALS.matches(method)) { + return method; + } + + List args = method.getArguments(); + Expression expected = args.get(0); + Expression actual = args.get(1); + + maybeAddImport("org.assertj.core.api.Assertions", "assertThat"); + maybeRemoveImport("org.junit.jupiter.api.Assertions"); + + if (args.size() == 2) { + return JavaTemplate.builder("assertThat(#{any()}).isEqualTo(#{any()});") + .staticImports("org.assertj.core.api.Assertions.assertThat") + .javaParser(assertionsParser) + .build() + .apply(getCursor(), method.getCoordinates().replace(), actual, expected); + } else { + return super.visitMethodInvocation(method, ctx); + } + } + })), + java( + """ + import java.util.Map; + import org.junit.jupiter.api.Assertions; + + class T { + void m(String one, Map map) { + Assertions.assertEquals(one, map.get("one")); + } + } + """, + """ + import java.util.Map; + + import static org.assertj.core.api.Assertions.assertThat; + + class T { + void m(String one, Map map) { + assertThat(map.get("one")).isEqualTo(one); + } + } + """ + ) + ); + } + + @Test + void replaceCallWithUnknownGenericReturnValue() { + rewriteRun( + spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() { + private static final MethodMatcher OBJECTS_EQUALS = new MethodMatcher("java.util.Objects equals(..)"); + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + if (!OBJECTS_EQUALS.matches(method)) { + return method; + } + + List args = method.getArguments(); + Expression expected = args.get(0); + Expression actual = args.get(1); + + maybeAddImport("java.util.Objects", "requireNonNull"); + + if (args.size() == 2) { + return JavaTemplate.builder("requireNonNull(#{any()}).equals(#{any()});") + .staticImports("java.util.Objects.requireNonNull") + .javaParser(JavaParser.fromJavaVersion()) + .build() + .apply(getCursor(), method.getCoordinates().replace(), actual, expected); + } else { + return super.visitMethodInvocation(method, ctx); + } + } + })), + java( + """ + import java.util.Objects; + import java.util.Map; + import java.util.HashMap; + + class T { + void m() { + Map map = new HashMap<>(); + Objects.equals("", map.get("one")); + } + } + """, + """ + import java.util.Objects; + import java.util.Map; + + import static java.util.Objects.requireNonNull; + import java.util.HashMap; + + class T { + void m() { + Map map = new HashMap<>(); + requireNonNull(map.get("one")).equals(""); + } + } + """ + ) + ); + } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java index 7aadad30349..b38e91180e6 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java @@ -474,7 +474,8 @@ private J3 maybeReplaceStatement(Statement statement, Class e } throw new IllegalArgumentException("Expected a template that would generate exactly one " + "statement to replace one statement, but generated " + gen.size() + - ". Template:\n" + substitutedTemplate); + ". Template:\n" + substitutedTemplate + "\nSubstitutions:\n" + substitutions + + "\nStatement:\n" + statement); } return autoFormat(gen.get(0).withPrefix(statement.getPrefix()), p); diff --git a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/Substitutions.java b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/Substitutions.java index 0db385f9eb5..34eee1891bb 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/internal/template/Substitutions.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/internal/template/Substitutions.java @@ -175,7 +175,8 @@ private String getTypeName(@Nullable JavaType type) { } return ((JavaType.Parameterized) type).getFullyQualifiedName() + joiner; } else if (type instanceof JavaType.GenericTypeVariable) { - return ((JavaType.GenericTypeVariable) type).getName(); + String name = ((JavaType.GenericTypeVariable) type).getName(); + return "?".equals(name) ? "Object" : name; } else if (type instanceof JavaType.FullyQualified) { return ((JavaType.FullyQualified) type).getFullyQualifiedName(); } else if (type instanceof JavaType.Primitive) {