From da544f83adab0975be2d4cd841fb1a51d42306c8 Mon Sep 17 00:00:00 2001 From: Shivani Sharma Date: Fri, 16 Aug 2024 21:43:14 +1000 Subject: [PATCH] Convert lambda block to expression after adopting `assertThrows` (#582) * Update UpdateTestAnnotationTest.java Test case for inlining valid single line assertthrows * Revert accidental whitespace changes * Also use doAfterVisit LambdaBlockToExpression in ExpectedExceptionToAssertThrows * Pull over commit from parallel PR * Also apply LambdaBlockToExpression after AssertJ migration * Update expected outcome --------- Co-authored-by: Tim te Beek --- ...UnitAssertThrowsToAssertExceptionType.java | 16 ++--- .../ExpectedExceptionToAssertThrows.java | 9 ++- .../testing/junit5/UpdateTestAnnotation.java | 3 + ...AssertThrowsToAssertExceptionTypeTest.java | 15 +++-- .../ExpectedExceptionToAssertThrowsTest.java | 6 +- .../junit5/UpdateTestAnnotationTest.java | 62 +++++++++++++++---- 6 files changed, 77 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionType.java b/src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionType.java index 07f55f5b1..10eb1b5f2 100644 --- a/src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionType.java +++ b/src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionType.java @@ -26,6 +26,7 @@ import org.openrewrite.java.search.UsesMethod; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.staticanalysis.LambdaBlockToExpression; public class JUnitAssertThrowsToAssertExceptionType extends Recipe { @@ -45,18 +46,7 @@ public TreeVisitor getVisitor() { } private static class AssertExceptionTypeVisitor extends JavaIsoVisitor { - private JavaParser.Builder assertionsParser; - - private JavaParser.Builder assertionsParser(ExecutionContext ctx) { - if (assertionsParser == null) { - assertionsParser = JavaParser.fromJavaVersion() - .classpathFromResources(ctx, "assertj-core-3.24"); - } - return assertionsParser; - } - private static final MethodMatcher ASSERT_THROWS_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assertThrows(..)"); - private static final JavaType THROWING_CALLABLE_TYPE = JavaType.buildType("org.assertj.core.api.ThrowableAssert.ThrowingCallable"); @Override @@ -77,7 +67,7 @@ && getCursor().getParentTreeCursor().getValue() instanceof J.Block) { if (executable != null) { mi = JavaTemplate .builder("assertThatExceptionOfType(#{any(java.lang.Class)}).isThrownBy(#{any(org.assertj.core.api.ThrowableAssert.ThrowingCallable)})") - .javaParser(assertionsParser(ctx)) + .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24")) .staticImports("org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType") .build() .apply( @@ -88,6 +78,8 @@ && getCursor().getParentTreeCursor().getValue() instanceof J.Block) { maybeAddImport("org.assertj.core.api.AssertionsForClassTypes", "assertThatExceptionOfType", false); maybeRemoveImport("org.junit.jupiter.api.Assertions.assertThrows"); maybeRemoveImport("org.junit.jupiter.api.Assertions"); + + doAfterVisit(new LambdaBlockToExpression().getVisitor()); } } return mi; diff --git a/src/main/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrows.java b/src/main/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrows.java index fd62333f7..17c4cfd91 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrows.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrows.java @@ -23,6 +23,7 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.*; +import org.openrewrite.staticanalysis.LambdaBlockToExpression; import java.util.Collections; import java.util.List; @@ -158,10 +159,6 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl "Exception.class" : expectMethodInvocation.getArguments().get(0); String templateString = expectedExceptionParam instanceof String ? "#{}assertThrows(#{}, () -> #{any()});" : "#{}assertThrows(#{any()}, () -> #{any()});"; - - Statement statement = bodyWithoutExpectedExceptionCalls.getStatements().size() == 1 && - !(bodyWithoutExpectedExceptionCalls.getStatements().get(0) instanceof J.Throw) ? - bodyWithoutExpectedExceptionCalls.getStatements().get(0) : bodyWithoutExpectedExceptionCalls; m = JavaTemplate.builder(templateString) .contextSensitive() .javaParser(javaParser(ctx)) @@ -172,7 +169,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl m.getCoordinates().replaceBody(), exceptionDeclParam, expectedExceptionParam, - statement + bodyWithoutExpectedExceptionCalls ); // Clear out any declared thrown exceptions @@ -225,6 +222,8 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl maybeAddImport("org.hamcrest.MatcherAssert", "assertThat"); } + doAfterVisit(new LambdaBlockToExpression().getVisitor()); + return m; } diff --git a/src/main/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotation.java b/src/main/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotation.java index 743ccb69d..f7f3314b8 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotation.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotation.java @@ -26,6 +26,7 @@ import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markup; +import org.openrewrite.staticanalysis.LambdaBlockToExpression; import java.util.Collections; import java.util.Comparator; @@ -170,6 +171,8 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex maybeAddImport("org.junit.jupiter.api.Test"); } + doAfterVisit(new LambdaBlockToExpression().getVisitor()); + return super.visitMethodDeclaration(m, ctx); } diff --git a/src/test/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionTypeTest.java b/src/test/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionTypeTest.java index d9651278b..1b5e7fd24 100644 --- a/src/test/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionTypeTest.java +++ b/src/test/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionTypeTest.java @@ -35,6 +35,7 @@ public void defaults(RecipeSpec spec) { .recipe(new JUnitAssertThrowsToAssertExceptionType()); } + @SuppressWarnings({"Convert2MethodRef", "CodeBlock2Expr"}) @DocumentExample @Test void toAssertExceptionOfType() { @@ -47,9 +48,12 @@ void toAssertExceptionOfType() { public class SimpleExpectedExceptionTest { public void throwsExceptionWithSpecificType() { assertThrows(NullPointerException.class, () -> { - throw new NullPointerException(); + foo(); }); } + void foo() { + throw new NullPointerException(); + } } """, """ @@ -57,9 +61,11 @@ public void throwsExceptionWithSpecificType() { public class SimpleExpectedExceptionTest { public void throwsExceptionWithSpecificType() { - assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> { - throw new NullPointerException(); - }); + assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> + foo()); + } + void foo() { + throw new NullPointerException(); } } """ @@ -127,6 +133,7 @@ public void throwsExceptionWithSpecificType() { * A degenerate case showing we need to make sure the assertThrows appears * immediately inside a J.Block. */ + @SuppressWarnings("ThrowableNotThrown") @Test @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/pull/331") void assertThrowsTernaryAssignment() { diff --git a/src/test/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrowsTest.java b/src/test/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrowsTest.java index 55f7ae2a9..c5b4f7835 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrowsTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrowsTest.java @@ -106,7 +106,8 @@ class MyTest { @Test public void testEmptyPath() { - Throwable exception = assertThrows(IllegalArgumentException.class, () -> foo()); + Throwable exception = assertThrows(IllegalArgumentException.class, () -> + foo()); assertTrue(exception.getMessage().contains("Invalid location: gs://")); } void foo() { @@ -459,7 +460,8 @@ class MyTest { @Test public void testEmptyPath() { - assertThrows(IOException.class, () -> foo()); + assertThrows(IOException.class, () -> + foo()); } void foo() throws IOException { throw new IOException(); diff --git a/src/test/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotationTest.java index 79f76ad3c..b6e3c950e 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/UpdateTestAnnotationTest.java @@ -106,6 +106,46 @@ public void test() { ); } + @Test + void assertThrowsSingleLineInlined() { + //language=java + rewriteRun( + java( + """ + import org.junit.Test; + + class MyTest { + + @Test(expected = IllegalArgumentException.class) + public void test() { + foo(); + } + private void foo() { + throw new IllegalArgumentException("boom"); + } + } + """, + """ + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertThrows; + + class MyTest { + + @Test + public void test() { + assertThrows(IllegalArgumentException.class, () -> + foo()); + } + private void foo() { + throw new IllegalArgumentException("boom"); + } + } + """ + ) + ); + } + @SuppressWarnings("ConstantConditions") @Test void assertThrowsSingleStatement() { @@ -113,16 +153,16 @@ void assertThrowsSingleStatement() { rewriteRun( java( """ - import org.junit.Test; - - public class MyTest { - - @Test(expected = IndexOutOfBoundsException.class) - public void test() { - int arr = new int[]{}[0]; - } + import org.junit.Test; + + public class MyTest { + + @Test(expected = IndexOutOfBoundsException.class) + public void test() { + int arr = new int[]{}[0]; } - """, + } + """, """ import org.junit.jupiter.api.Test; @@ -562,7 +602,7 @@ public void testWithThrows() throws IOException { // Second call shows why we wrap the entire method body in the lambda foo(); } - + void foo() throws IOException { throw new IOException(); } @@ -585,7 +625,7 @@ public void testWithThrows() { foo(); }); } - + void foo() throws IOException { throw new IOException(); }