SONARJAVA-6825: Implement rule S9360 - Constant expressions and comparisons should be simplified - #6036
SONARJAVA-6825: Implement rule S9360 - Constant expressions and comparisons should be simplified#6036romainbrenguier wants to merge 9 commits into
Conversation
…risons should be simplified This rule detects Yoda conditions where a constant literal appears on the left side of a comparison operator (==, !=, <, >). It reports an issue with the message 'Put the variable on the left side of this comparison.' The rule handles all literal types: integers, longs, floats, doubles, booleans, characters, strings, and null. It correctly skips parentheses to detect Yoda conditions in nested expressions like ((0) == count). Test coverage includes: - All literal types with == and != operators - Less than and greater than operators - Nested parentheses handling - Non-comparison contexts (assignments, arithmetic, method calls) - Edge cases: both literals, both variables, ternary operators
Extend YodaConditionCheck to also detect calls to deterministic Math methods where all arguments are compile-time constant literals (e.g., Math.max(5, 10), Math.sqrt(16.0)). Add support for <= and >= comparison operators in Yoda condition detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Compare null to a reference type (Object) instead of a primitive (int) to fix illegal operand types for binary operator '=='. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Code review updated (blocking issues remain unresolved).
|
❌ Ruling needs updating. A fix PR has been created: #6037 Please review and merge it into your branch. |
Ruling Diff SummaryDetected changes in 4 rule files: 0 issues removed, 111 issues added. S9360 (
|
- Extract duplicated "float"/"double" string literals into constants (S1192)
- Fix isLiteral to handle unary minus/plus (e.g. -1 == index now detected)
- Use distinct message for relational Yoda conditions ("...and invert the operator")
- Remove BOOLEAN_LITERAL from Yoda detection (delegate to S1125)
- Remove transcendental Math functions from constant detection (results may vary across JVMs)
- Update ruling expectations and test samples accordingly
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Generated with GitHub Actions
|
❌ Ruling needs updating. A fix PR has been created: #6038 Please review and merge it into your branch. |
- Remove inexact Math functions (cbrt, toDegrees, toRadians) from constant call detection as their results are not exactly specified across JVMs - Fix HTML doc: remove boolean literal examples that conflict with S1125, correct rationale about Java assignment in conditional contexts - Add test cases for newly excluded Math functions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Code Review ✅ Approved 6 resolved / 6 findingsImplements rule S9360 to simplify constant expressions and comparisons, addressing issues with Yoda conditions, constant Math calls, compilation errors, and messaging. All findings were successfully resolved. ✅ 6 resolved✅ Bug: Compiling test sample does not compile: null == int
✅ Bug: Yoda detection misses negative literals and flags constant-only comparisons
✅ Bug: Relational Yoda message suggests a fix that inverts the logic
✅ Quality: Constant Math detection double-reports with S2185 (both in Sonar way)
✅ Quality: Boolean-literal guidance conflicts with S1125 and with Java semantics
...and 1 more resolved from earlier reviews Implementation Status ✅ 1 of 1 objectives covered✅ SONARJAVA-6825 - 1 of 1 objectives coveredThis PR implements rule S9360 for constant expressions and comparisons as specified in the objectives. ✅ 1 covered here
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 |
|
❌ Ruling needs updating. A fix PR has been created: #6040 Please review and merge it into your branch. |
Remove Math.log(2) detections from BloomFilter (method no longer matched), add new negated literal Yoda conditions in LittleEndianDataInputStream and LongMath. 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.
PR #6036 and PR #6022 are competing implementations of the same Jira deliverable, not complementary changes. SONARJAVA-6825 has one stated objective—create S9360—and no description or comments defining separate deliverables. Both PRs share the initial S9360 commits through 36d0e824, then diverge: #6022 implements the Yoda-condition subset, while this PR implements Yoda conditions plus constant Math calls. Both add the same rule key, check class, metadata, documentation, Sonar Way activation, and ruling files, so they cannot reasonably both merge. Please establish one canonical implementation before proceeding. If #6036 supersedes #6022, state that explicitly and close #6022; otherwise explain the intended split and re-scope the work so the two PRs no longer compete for S9360.
2 participants
Summary
0 == count,null != obj) and suggests putting the variable on the leftMathmethods where all arguments are compile-time literals (Math.max(5, 10),Math.sqrt(16.0),Math.abs(-5)) and suggests replacing with precomputed values==,!=,<,>,<=,>=Math.abs(-10))MethodMatchersfor type-safe Math method detection with semantic analysisTest plan
🤖 Generated with Claude Code