Preserve throws clause when checked exceptions exist outside assertThrows. #899
+147
−2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's changed?
The
ExpectedExceptionToAssertThrowsrecipe now preserves thethrowsclause when statements before thethrown.expect()call throw checked exceptions.Previously, the recipe unconditionally removed the
throwsclause from test methods when convertingExpectedExceptiontoassertThrows(). This caused compilation errors when code outside theassertThrows()lambda still threw checked exceptions.Key changes:
findPredecessorStatements()to track statements before the firstexpect*()callstatementsBeforeExpectThrowCheckedException()to detect if those statements throw checked exceptionsisCheckedException()to distinguish checked exceptions fromRuntimeException/ErrorvisitMethodDeclaration()to preservethrowswhen pre-expect code throws checked exceptionsWhat's your motivation?
When migrating tests that have setup code before
thrown.expect(), the recipe was incorrectly removing thethrowsdeclaration, causing compilation failures.Example:
The recipe was removing
throws InterruptedExceptioneven thoughsetup()is not wrapped in theassertThrows()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