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

rewrite-static-analysis/src/test/java/org/openrewrite/staticanalysis/LambdaBlockToExpressionTest.java

Lines 37 to 58 in 3c3fded

	@Test
	void simplifyLambdaBlockToExpression() {
	rewriteRun(
	//language=java
	java(
	"""
	import java.util.function.Function;
	class Test {
	Function<Integer, Integer> f = n -> {
	return n+1;
	};
	}
	""",
	"""
	import java.util.function.Function;
	class Test {
	Function<Integer, Integer> f = n -> n+1;
	}
	"""
	)
	);
	}

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

rewrite-static-analysis/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java

Lines 47 to 57 in e57dac0

	if (lambda.getBody() instanceof J.Block) {
	List<Statement> statements = ((J.Block) lambda.getBody()).getStatements();
	if (statements.size() == 1 && statements.get(0) instanceof J.Return) {
	Space prefix = statements.get(0).getPrefix();
	if (prefix.getComments().isEmpty()) {
	return l.withBody(((J.Return) statements.get(0)).getExpression());
	} else {
	return l.withBody(((J.Return) statements.get(0)).getExpression().withPrefix(prefix));
	}
	}
	}

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);
    });
  }

}