SONARJAVA-6781 Implement new rule S9344: Bitwise AND operations with zero should be corrected - #5952
Conversation
Detect bitwise AND operations with literal zero (`& 0` and `&= 0`), which always produce zero regardless of the other operand, indicating a likely programming error such as a wrong constant or wrong operator.
S9344 adds one rule to the quality profile, incrementing the total from 465 to 466. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nathsou
left a comment
nathsou
left a comment
There was a problem hiding this comment.
Automated code review found a few issues worth addressing before merge.
| } | ||
| } | ||
|
|
||
| private static boolean isZero(ExpressionTree expression) { |
There was a problem hiding this comment.
isZero() fails to detect a parenthesized zero literal, so flags & (0) is not flagged (false negative). LiteralUtils.longLiteralValue() only unwraps UNARY_MINUS/UNARY_PLUS before checking for INT_LITERAL/LONG_LITERAL — it never unwraps PARENTHESIZED_EXPRESSION. The codebase already has ExpressionUtils.skipParentheses used by other checks (e.g. StringConcatToTextBlockCheck, MathOnFloatCheck, UselessMathematicalComparisonCheck) for exactly this purpose. Also not covered by the test sample.
| } | ||
|
|
||
| @Override | ||
| public void visitNode(Tree tree) { |
There was a problem hiding this comment.
Issue location is inconsistent between the two branches: the & case reports the whole binary expression (reportIssue(tree, MESSAGE)), while the &= case reports only the operator token (reportIssue(assignment.operatorToken(), MESSAGE), line 47). The sibling rule S2437 (UnnecessaryBitOperationCheck) uses operatorToken() uniformly for both binary and assignment forms — worth aligning here too.
| assertThat(actualProfile.isDefault()).isFalse(); | ||
| assertThat(actualProfile.rules()) | ||
| .hasSize(465) | ||
| .hasSize(466) |
There was a problem hiding this comment.
It's added by the rule-api tool. We will be doing a re-run of rule-api for all rules before the release anyway.
| assertThat(actualProfile.isDefault()).isFalse(); | ||
| assertThat(actualProfile.rules()) | ||
| .hasSize(465) | ||
| .hasSize(466) |
There was a problem hiding this comment.
It's added by the rule-api tool. We will be doing a re-run of rule-api for all rules before the release anyway.
…tion - Wrap expression with ExpressionUtils.skipParentheses() in isZero() to detect parenthesized zero literals like `flags & (0)` - Report issue on operator token instead of whole expression for binary `&` to be consistent with `&=` case and sibling rule S2437 - Add test cases for parenthesized zero (compliant and noncompliant) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Code Review ✅ ApprovedImplements rule S9344 to detect bitwise AND operations with literal zero and updates the expected rule count in agentic profile tests. No issues found. Implementation Status ✅ 1 / 1 issues implemented✅ SONARJAVA-6781 — 1 / 1 objectivesThe PR successfully implements the new rule S9344 to check for bitwise AND operations with zero. ✅ 1 complete
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
2 participants
Detect bitwise AND operations with literal zero (
& 0and&= 0), which always produce zero regardless of the other operand, indicating a likely programming error such as a wrong constant or wrong operator.Part of