`ExpectedExceptionToAssertThrows` leads to compilation issue when variables from lambda are referenced after `assertThrows`

[TomChatt2]

What version of OpenRewrite are you using?

I am using openrewrite-maven-plugin 5.11.0 with rewrite-spring 5.1.1

How are you running OpenRewrite?

I am using the Maven plugin. Apologies, my project is on an offline network with no Internet access, so I'm not able to share or copy anything.

This is the gist of the initial code

import my.MyAppException;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class ApplicationTest {

  @Rule
  public ExpectedException expectedException = ExpectedException.none();

  @Test
  public void sampleTest() throws Exception {
    final String EXPECTED_MSG = "this is what i expected";
    expectedException.expect(MyAppException.class);
    expectedException.expectMessage(EXPECTED_MSG);
    doThrow(new MyAppException(EXPECTED_MSG)).when(...)...
    // some app code expected to throw exception here
  }
}

What did you expect to see?

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

import my.MyAppException;
import org.junit.jupiter.api.Test;

class ApplicationTest {

  @Test
  void sampleTest() throws Exception {
    final String EXPECTED_MSG = "this is what i expected";
    ThrowableException ex = assertThrows(MyAppException.class, () -> {
        doThrow(new MyAppException(EXPECTED_MSG)).when(...)...
        // some app code expected to throw exception here
    });
    assertTrue(ex.getMessage().contains(EXPECTED_MSG));
  }
}

What did you see instead?

import static org.junit.jupiter.api.Assertions.assertEquals;
// the import for assertThrows was missing

import my.MyAppException;
import org.junit.jupiter.api.Test;

class ApplicationTest {

  @Test
  void sampleTest() throws Exception {
    ThrowableException ex = assertThrows(MyAppException.class, () -> {  // ERROR: assertThrows is undefined (missing import)
        final String EXPECTED_MSG = "this is what i expected";  // this shouldn't have been put inside the lambda expression
        doThrow(new MyAppException(EXPECTED_MSG)).when(...)...
        // some app code expected to throw exception here
    });
    assertTrue(ex.getMessage().contains(EXPECTED_MSG)); // ERROR: EXPECTED_MSG is undefined here
  }
}

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

No errors. The rewrite completed "successfully" but just produced code with syntax errors.

Notes

This bug is in the same context and similar conditions as Issue openrewrite/rewrite-spring#363 but seems to be a different problem.

Are you interested in contributing a fix to OpenRewrite?

Wish I could. Sorry, not able at this time.

[timtebeek]

Thanks for the report @TomChatt2 ; Looks like a recipe bug indeed, and thanks for referencing that similar issue as well!

[bsmahi]

Hi @TomChatt2 Recently have migrated my personal project from JUnit4 to JUnit5 using SpringBoot2JUnit4to5Migration recipe and have used the following dependencies with the versions rewrite-maven-plugin as 5.27.2 rewrite-spring as 5.7.0

It was migrated successfully without any issues, infact assertThrows static import added successfully without any issues

Could you please try with the following. For more details, please go through the below link https://docs.openrewrite.org/recipes/java/spring/boot2/springboot2junit4to5migration

Thanks, Mahi

[timtebeek]

We've had another look at this one; in short there's no guaranteed way to improve things for one case without potentially making things worse for other cases.

We'd considered keeping everything before the last call to org.junit.rules.ExpectedException expect*(..) outside/before the assertThrows block, but that might then fail to compile if mutable variables are then used in the assertThrows block. Also; this would only be necessary if any of the variables before the assertThrows are even used beyond the assertThrows in an assertion on the message. Otherwise, we might needlessly make things worse for those reasonable cases.

Perhaps it's best then not to change the recipe here; as we can't reliably do so without also potentially doing harm to other cases.