SONARJAVA-6053 Fix S112 false positive for checked exception#5648
SONARJAVA-6053 Fix S112 false positive for checked exception#5648rombirli wants to merge 4 commits into
Conversation
Agentic Analysis: Early ResultsAgentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action. 25 issue(s) found across 1 file(s):
Analyzed by SonarQube Agentic Analysis in 3.1 s |
hashicorp-vault-sonar-prod
Bot
changed the title
rombirli
force-pushed
the
rombirli/SONARJAVA-6053-fp-s112-runtimeexception-wrapping
branch
from
2720f71 to
a8740d8
Compare
…ic raw exception wrapped)
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| private static boolean isSimpleWrapping(NewClassTree tree) { | ||
| return WRAPPING_EXCEPTIONS.stream().anyMatch(tree.identifier().symbolType()::is) && | ||
| tree.arguments().stream().anyMatch(argument -> | ||
| argument.symbolType().isSubtypeOf(THROWABLE ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
⚠️ Edge Case: isSimpleWrapping suppresses S112 even when wrapping a generic/raw exception
isSimpleWrapping (RawExceptionCheck.java:131-136) treats any throw new RuntimeException(x) / throw new Error(x) as compliant as long as one argument is a subtype of java.lang.Throwable. It does not verify that the wrapped argument is a specific exception. As a result, wrapping a generic/raw exception is now considered compliant, e.g.:
catch (Exception e) { throw new RuntimeException(e); } // now Compliant (was Noncompliant)
catch (Throwable e) { throw new RuntimeException(e); } // now Compliant (was Noncompliant)This exactly matches the limitation the author flagged in the commit message ("allow all wrappings, without considering generic raw exception wrapped"). The corresponding test cases wraps_generic_exception and wraps_throwable, which were previously // Noncompliant, were deleted in RawExceptionCheckSample.java rather than kept as expected detections. Since the whole purpose of S112 is to discourage propagating generic exceptions, this re-introduces false negatives for precisely that pattern. The fix should keep wrapping compliant only when the wrapped cause is itself a non-raw (specific) exception.
Only treat the throw as a compliant wrapping when the wrapped cause is a specific (non-raw) exception, so wrapping a generic Exception/Throwable is still reported.:
private static boolean isSimpleWrapping(NewClassTree tree) {
return WRAPPING_EXCEPTIONS.stream().anyMatch(tree.identifier().symbolType()::is) &&
tree.arguments().stream().anyMatch(argument -> {
Type argType = argument.symbolType();
return argType.isSubtypeOf(THROWABLE) && !isRawException(argType);
});
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| private static boolean isSimpleWrapping(NewClassTree tree) { | ||
| return WRAPPING_EXCEPTIONS.stream().anyMatch(tree.identifier().symbolType()::is) && | ||
| tree.arguments().stream().anyMatch(argument -> | ||
| argument.symbolType().isSubtypeOf(THROWABLE ) |
There was a problem hiding this comment.
💡 Quality: Trailing whitespace in isSubtypeOf(THROWABLE )
Minor: argument.symbolType().isSubtypeOf(THROWABLE ) (RawExceptionCheck.java:134) has a stray space before the closing parenthesis. Harmless but worth cleaning up while editing this method.
Was this helpful? React with 👍 / 👎
|
CI failed: Integration tests failed because the rule S112 changes altered the number of reported issues, causing a baseline mismatch in the ruling regression tests.OverviewAnalysis of the CI logs indicates that the PR changes to rule S112 have triggered regressions in the integration test suite. Two failures were identified across different environments (Linux and Windows), both stemming from discrepancies between actual analysis output and existing test baselines. FailuresRuling Integration Test Mismatch (confidence: high)
Summary
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
1 participant
Summary by Gitar
RawExceptionCheckto avoid reporting false positives when wrapping checked exceptions inRuntimeExceptionorError.isSimpleWrappinglogic to detect when aThrowableis passed as an argument to aNewClassTreeconstructor.RawExceptionCheckSample.javato reproduce S112 false positives.eclipse-jetty,guava,jboss-ejb3-tutorial, andsonar-serverto reflect the improved rule logic.This will update automatically on new commits.