Skip to content

Comments

Preserve throws clause when checked exceptions exist outside assertThrows.#899

Merged
timtebeek merged 2 commits intoopenrewrite:mainfrom
motlin:expected-exception-preserve-throws
Feb 21, 2026
Merged

Preserve throws clause when checked exceptions exist outside assertThrows.#899
timtebeek merged 2 commits intoopenrewrite:mainfrom
motlin:expected-exception-preserve-throws

Conversation

@motlin
Copy link
Contributor

@motlin motlin commented Jan 28, 2026

What's changed?

The ExpectedExceptionToAssertThrows recipe now preserves the throws clause when statements before the thrown.expect() call throw checked exceptions.

Previously, the recipe unconditionally removed the throws clause from test methods when converting ExpectedException to assertThrows(). This caused compilation errors when code outside the assertThrows() lambda still threw checked exceptions.

Key changes:

  • Added findPredecessorStatements() to track statements before the first expect*() call
  • Added statementsBeforeExpectThrowCheckedException() to detect if those statements throw checked exceptions
  • Added isCheckedException() to distinguish checked exceptions from RuntimeException/Error
  • Modified visitMethodDeclaration() to preserve throws when pre-expect code throws checked exceptions

What's your motivation?

When migrating tests that have setup code before thrown.expect(), the recipe was incorrectly removing the throws declaration, causing compilation failures.

Example:

@Test
public void testMethod() throws InterruptedException {
    setup();  // throws InterruptedException - NOT wrapped in assertThrows
    this.thrown.expect(IllegalArgumentException.class);
    doSomething();
}

The recipe was removing throws InterruptedException even though setup() is not wrapped in the assertThrows() lambda.

Anything in particular you'd like reviewers to focus on?

The logic for detecting checked exceptions in statementThrowsCheckedException() - it recursively visits method invocations and constructor calls to check their thrown exception types. Please verify this covers all cases.

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

The fix uses JavaType.Method.getThrownExceptions() to inspect method types, which requires proper type attribution. This should work in typical recipe execution contexts.

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-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Jan 28, 2026
@timtebeek timtebeek self-requested a review January 29, 2026 00:54
@motlin motlin force-pushed the expected-exception-preserve-throws branch 3 times, most recently from fd06b9c to e63c8f1 Compare February 3, 2026 14:57
@motlin motlin force-pushed the expected-exception-preserve-throws branch from e63c8f1 to c5d641e Compare February 5, 2026 18:44
@motlin
Copy link
Contributor Author

motlin commented Feb 20, 2026

@timtebeek I wanted to check what you think of this one. I've been using it in a SNAPSHOT build against parts of my codebase and it's working well for me.

@motlin motlin force-pushed the expected-exception-preserve-throws branch from c5d641e to 877cc37 Compare February 20, 2026 16:24
@timtebeek
Copy link
Member

Glad to hear that's working well for you already! I'm admittedly behind on reviews, but that comes with landing Kotlin v2 and Python in the same week, as well as three online trainings. 😅 It's at the top of my list now, but first I'm out climbing. 🧗🏻

Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! Great to not have compilation issues when other exceptions are declared thrown.

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Feb 21, 2026
@timtebeek timtebeek merged commit 36ebbba into openrewrite:main Feb 21, 2026
1 check passed
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Feb 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants