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 <tim@moderne.io>
openrewrite · Aug 16, 2024 · da544f8 · da544f8
1 parent 17b0427
commit da544f8
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 34 deletions.
diff --git a/...ain/java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionType.java b/...ain/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<?, ExecutionContext> getVisitor() {
     }
 
     private static class AssertExceptionTypeVisitor extends JavaIsoVisitor<ExecutionContext> {
-        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
@@ -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
@@ -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/...java/org/openrewrite/java/testing/assertj/JUnitAssertThrowsToAssertExceptionTypeTest.java b/...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,19 +48,24 @@ void toAssertExceptionOfType() {
               public class SimpleExpectedExceptionTest {
                   public void throwsExceptionWithSpecificType() {
                       assertThrows(NullPointerException.class, () -> {
-                          throw new NullPointerException();
+                          foo();
                       });
                   }
+                  void foo() {
+                      throw new NullPointerException();
+                  }
               }
               """,
             """
               import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
               
               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 <code>assertThrows</code> 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
@@ -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
@@ -106,23 +106,63 @@ 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() {
         //language=java
         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();
                   }