LambdaBlockToExpression does not work in assertThrows (or generally when no value is returned)

[timo-abele]

What version of OpenRewrite are you using?

I am using

• Maven plugin v5.18.0
• rewrite-static-analysis v1.2.0

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

<plugin>
    <groupId>org.openrewrite.maven</groupId>
    <artifactId>rewrite-maven-plugin</artifactId>
    <version>5.18.0</version>
    <configuration>
        <activeRecipes>
            <recipe>org.openrewrite.staticanalysis.LambdaBlockToExpression</recipe>
        </activeRecipes>
    </configuration>
    <dependencies>
        <dependency>
            <groupId>org.openrewrite.recipe</groupId>
            <artifactId>rewrite-static-analysis</artifactId>
            <version>1.2.0</version>
        </dependency>
    </dependencies>
</plugin>

What is the smallest, simplest way to reproduce the problem?

import static org.junit.jupiter.api.Assertions.assertThrows;

import java.math.BigDecimal;
import org.junit.jupiter.api.Test;

public class SimpleTest {

  @Test
  void shouldFailOnDivisionByZero() {
    assertThrows(ArithmeticException.class, () -> {
      BigDecimal.ONE.divide(BigDecimal.ZERO);
    });
  }

}

What did you expect to see?

import static org.junit.jupiter.api.Assertions.assertThrows;

import java.math.BigDecimal;
import org.junit.jupiter.api.Test;

public class SimpleTest {

  @Test
  void shouldFailOnDivisionByZero() {
    assertThrows(ArithmeticException.class, () -> BigDecimal.ONE.divide(BigDecimal.ZERO));
  }

}

What did you see instead?

no change whatsoever.

Notes

The example above is how I encountered this bug. The examplee below are a bit more minimal but less common in my opinion.

package org.example;

import javax.swing.JButton;

public class Main {

  public static void main(String[] args) {
    Runnable runHelloWorld = () -> {
      System.out.println("Hello world!");
    };
    runHelloWorld.run();

    JButton jb = new JButton();
    jb.addActionListener(event -> {
      System.out.println(7);
    });
  }
}

All three examples have in common that they don't return a value, so the description in the docs:

Single-line statement lambdas returning a value can be replaced with expression lambdas.

would render this issue a feature request. However, if I were to write a separate recipe I wouldn't know what else to call it but "LambdaBlockToExpression", so I declare the description part of the bug 😃. In addition, IntelliJ suggests for all three examples to "Replace with expression lambda", just like for lambdas that return a value.

What is the full stack trace of any errors you encountered?

No errors, works as (IMO incompletely) designed

Are you interested in contributing a fix to OpenRewrite?

I'm somewhat interested in contributing, but not sure if I have the time. It would help me to know if the the functionality described does indeed belong in this recipe, or what to call such a separate recipe.

[timtebeek]

Hi @timo-abele Thanks for the offer to help! I think it ought to be easy to covert some of the examples you've given into a minimal unit test as a start, as for example seen here https://github.com/openrewrite/rewrite-static-analysis/blob/3c3fdedf86e0c60782c591ac78e28346f3e4fcd5/src/test/java/org/openrewrite/staticanalysis/LambdaBlockToExpressionTest.java#L37-L58

When you then run that new test in a debugger you'll quite quickly find that you're not getting past the second if statement here https://github.com/openrewrite/rewrite-static-analysis/blob/e57dac0991cf3e57edf99826d975c079d0971baf/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java#L47-L57

That statements.get(0) instanceof J.Return can then likely be selectively expanded to also allow your use case to pass & make changes. More than happy to help review any attempts you make towards this. Feel free to open up a draft PR containing just a test if you want to get started.

[timo-abele]

Could you please take a look at my draft PR @timtebeek?

[timo-abele]

Hi @timtebeek, can you please reopen this issue? I just tried 1.3.0-SNAPSHOT and my original use case with assertThrows is not transformed. I tried to reproduce it with the standard library but this is transformed as desired:

import java.math.BigDecimal;
import org.junit.jupiter.api.Test;

public class SimpleTestHomeMade {

  @Test
  void shouldFailOnDivisionByZero() {

    myAssertThrows(UnsupportedOperationException.class, () -> {
      BigDecimal.ONE.divide(BigDecimal.ZERO);
    });
  }

  public static <T extends Throwable> T myAssertThrows(Class<T> expectedType, SimpleTestHomeMade.MyExecutable executable) {
    return (T) null;
  }

  public interface MyExecutable {
    void execute() throws java.lang.Throwable;
  }

}

whereas this is not:

import static org.junit.jupiter.api.Assertions.assertThrows;

import java.math.BigDecimal;
import org.junit.jupiter.api.Test;

public class SimpleTest {

  @Test
  void shouldFailOnDivisionByZero() {
    assertThrows(ArithmeticException.class, () -> {
      BigDecimal.ONE.divide(BigDecimal.ZERO);
    });
  }

}

[timtebeek]

Sorry to hear it's not yet working as expected ; guess we'd need to refine with another unit test that more closely matches your intended use. Would you want to get that started?