ExpectedExceptionToAssertThrows: Handle branching and var usage in except methods

[amishra-u]

What's Changed?

Previously, the ExpectedExceptionToAssertThrows recipe failed to correctly handle:

• Conditional exception registration (commonly seen in parameterized tests)
• Variable usage in expectMessage arguments

When variables declared before the thrown.expect(...) calls were referenced in assertions, results in uncompilable code after transformation.

Problem Example (Before)

Input:

void test() {
    int[] a = new int[] { 1 };
    thrown.expect(IndexOutOfBoundsException.class);
    thrown.expectMessage("Index 1 out of bounds for length 1");
    thrown.expectMessage("Index 1 out of bounds for length " + a.length);
    int b = a[1];
}

Output (broken):

void test() {
    Throwable exception = assertThrows(IndexOutOfBoundsException.class, () -> {
        int[] a = new int[]{1};
        int b = a[1];
    });
    assertTrue(exception.getMessage().contains("Index 1 out of bounds for length 1"));
    assertThat(exception.getMessage(), containsString("Index 1 out of bounds for length " + a.length)); //  'a' is undefined
}

The updated recipe now wraps only the statements that appear after the thrown.expect call. Such that all the variables declared will be available in assert call scope.

Conditional Exception Handling

The updated recipe now also supports conditionally triggered exceptions, which were previously unhandled. These often appear in parameterized tests.

1. If-Else Conditional:

if (shouldThrow) {
    this.thrown.expect(IllegalArgumentException.class);
    this.thrown.expectMessage("input must be greater than or equal to zero");
    foo(-1);
} else {
    assertThat(foo(2), equalTo(4));
}

2. Single If Block:

public void testConditionalThrow() {
    int x = 2;
    if (x < 0) {
        this.thrown.expect(IllegalArgumentException.class);
        this.thrown.expectMessage("input must be greater than or equal to zero");
    }
    int y = foo(x);
    assertThat(y, equalTo(4));
}

The updated recipe correctly handles both patterns, including nested conditional blocks. (refer unit test to understand the transformation)

Additional Improvements

• Enhanced transformation of thrown.expectMessage(String) to use Hamcrest matchers, aligning with the original behavior.

Before:

assertTrue(exception.getMessage().contains("Index 1 out of bounds for length 1"));

After:

assertThat(exception.getMessage(), containsString("Invalid location: gs://"));

Anyone you would like to review specifically?

@timtebeek

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[github-actions]

Some suggestions could not be made:

• src/main/resources/META-INF/rewrite/examples.yml
  • lines 2399-2399
  • lines 2408-2408
  • lines 2583-2598
  • lines 3392-3391
  • lines 3427-3445

[amishra-u]

@timtebeek updated the PR summary to clearly justify the changes introduced in this update.

[timtebeek]

Hi thanks! Didn't get to this one; there's been a lot to get to with codemix.ai next week, but we hope to get this in soon still.