Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
|
❌ Ruling needs updating. A fix PR has been created: #6198 Please review and merge it into your branch. |
🤖 Generated with GitHub Actions
|
❌ Ruling needs updating. A fix PR has been created: #6198 Please review and merge it into your branch. |
Ruling Diff SummaryDetected changes in 5 rule files: 0 issues removed, 182 issues added. S3949 (
|
|
❌ Ruling needs updating. A fix PR has been created: #6200 Please review and merge it into your branch. |
| private static boolean isUnsafeMidpoint(BinaryExpressionTree addition) { | ||
| if (!addition.symbolType().isPrimitive(Type.Primitives.INT)) { | ||
| return false; | ||
| } | ||
| Tree parent = ExpressionUtils.skipParenthesesUpwards(addition.parent()); | ||
| if (!(parent instanceof BinaryExpressionTree division) || !division.is(Tree.Kind.DIVIDE) | ||
| || ExpressionUtils.skipParentheses(division.leftOperand()) != addition) { | ||
| return false; | ||
| } | ||
| Integer denominator = ExpressionUtils.skipParentheses(division.rightOperand()).asConstant(Integer.class).orElse(null); | ||
| Long longDenominator = ExpressionUtils.skipParentheses(division.rightOperand()).asConstant(Long.class).orElse(null); | ||
| Double floatingDenominator = LiteralUtils.doubleLiteralValue(ExpressionUtils.skipParentheses(division.rightOperand())); | ||
| if (!Integer.valueOf(2).equals(denominator) && !Long.valueOf(2L).equals(longDenominator) | ||
| && !Double.valueOf(2.0).equals(floatingDenominator)) { | ||
| return false; |
There was a problem hiding this comment.
💡 Bug: Midpoint heuristic still flags non-overflowing narrow operands
The new guard only requires the addition's own type to be int, but byte/short/char operands are promoted to int, so (low + high) / 2 with two short (or byte/char) parameters still reaches the heuristic; IntegerOverflowRange.rangeOf returns null for such unknown identifiers and isUnsafeMidpoint treats left == null || right == null as unsafe, raising an issue on a sum that mathematically cannot exceed int range. Restrict the heuristic to additions whose operands are themselves int/long typed so promoted narrow operands are not reported.
Require both operands to be int before applying the midpoint heuristic:
private static boolean isUnsafeMidpoint(BinaryExpressionTree addition) {
if (!addition.symbolType().isPrimitive(Type.Primitives.INT)
|| !addition.leftOperand().symbolType().isPrimitive(Type.Primitives.INT)
|| !addition.rightOperand().symbolType().isPrimitive(Type.Primitives.INT)) {
return false;
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
❌ Ruling needs updating. A fix PR has been created: #6200 Please review and merge it into your branch. |
Code Review 👍 Approved with suggestions 8 closed / 9 findings🟡 Medium risk Implements S3949 to detect integer and long overflow with exact and bounded range evaluation, including unsafe midpoint formulas while suppressing overlap with related rules. The implementation is comprehensive with semantic and non-semantic tests, but the midpoint heuristic still flags non-overflowing narrow operands ( 💡 Bug: Midpoint heuristic still flags non-overflowing narrow operands📄 java-checks/src/main/java/org/sonar/java/checks/S3949Check.java:121-135 The new guard only requires the addition's own type to be Require both operands to be int before applying the midpoint heuristic✅ 8 closed✅ Bug: Bounded ranges cause FPs on
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|
1 participant
Summary
intandlongoverflow with exact and bounded range evaluationLinks
Validation
mvn -pl java-checks -am test -Dtest=S3949CheckTest -Dsurefire.failIfNoSpecifiedTests=falseAI disclosure
LLM model used for implementation: GPT-5.6 Sol