SONARJAVA-6786 Implement new rule S9346: Integer values should not be cast to long for use as timestamps - #5957
Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
❌ Ruling needs updating. A fix PR has been created: #5958 Please review and merge it into your branch. |
Ruling Diff SummaryDetected changes in 1 rule files: 0 issues removed, 2 issues added. S9346 (
|
nathsou
left a comment
nathsou
left a comment
There was a problem hiding this comment.
Request changes requested: preserve narrowing casts, regenerate the rule resources from the corrected RSPEC, and complete the literal exemption. Inline comments follow.
|
|
||
| private void checkArgument(ExpressionTree argument) { | ||
| ExpressionTree arg = ExpressionUtils.skipParentheses(argument); | ||
| if (arg.is(Tree.Kind.TYPE_CAST)) { |
There was a problem hiding this comment.
[P2] Preserve narrowing casts. This unwraps every TYPE_CAST, so new Date((int) millis) inspects millis as long and reports nothing, even though the argument has been truncated to int and then implicitly widened back to long. Only unwrap casts to long; otherwise inspect the cast expression type. Please add this case to the sample.
| "func": "Constant\/Issue", | ||
| "constantCost": "5min" | ||
| }, | ||
| "tags": [], |
There was a problem hiding this comment.
[P2] Regenerate these generated resources from a corrected RSPEC. RSPEC PR 7897 specifies the pitfall and datetime tags plus the LOGICAL attribute, while this file contains no tags and COMPLETE. The resource directory README identifies RSPEC as the source of truth. Align the RSPEC example with the accepted direct-call scope, then regenerate HTML and JSON so the rule ships with the correct metadata.
| if (arg.is(Tree.Kind.TYPE_CAST)) { | ||
| arg = ((TypeCastTree) arg).expression(); | ||
| } | ||
| if (arg.is(Tree.Kind.INT_LITERAL)) { |
There was a problem hiding this comment.
[P3] Complete the literal exemption. new Date(0) is skipped, but equivalent literals such as new Date(-1) and new Date((long) (0)) still report because they are unary or parenthesized AST nodes. Skip parentheses after a long cast and recognize signed integer literals.
Detect 32-bit or smaller integer values (int, short, byte, char) passed as arguments to timestamp-consuming APIs (Date, Timestamp, Instant, Calendar), where the narrow type causes overflow or data corruption.
- Fix secondary location marker alignment in test sample (off by one space) - Exclude int literal arguments (e.g., `new Date(0)`) from detection to reduce false positives on intentional small values - Update HTML noncompliant example to show patterns the rule actually detects (direct int arg and explicit cast) instead of the variable indirection pattern which is not detected Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit excluded int literals from S9346, which means the eclipse-jetty findings at JSONTest.java lines 366 and 433 are no longer raised. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0f0799c to
79b5cb1
Compare
This comment has been minimized.
This comment has been minimized.
…d literals, update metadata - Only unwrap casts to long in checkArgument; narrowing casts (e.g. (int)) are now correctly reported as noncompliant - Recognize negative/positive unary integer literals (-1, +1) as exempt - Strip parentheses after unwrapping long cast for literal check - Update S9346.json: set tags to [pitfall, datetime] and attribute to LOGICAL - Add test cases for narrowing casts and signed/parenthesized literals Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review ✅ Approved 2 resolved / 2 findingsImplements rule S9346 to detect integer values cast to long for use as timestamps, addressing noisy int/long-literal timestamps and missed cases involving stored casts. ✅ 2 resolved✅ Edge Case: Rule flags legitimate int/long-literal timestamps causing noise
✅ Bug: Rule misses documented case: int cast stored in long variable
Implementation Status ✅ 1 / 1 issues implemented✅ SONARJAVA-6786 — 1 / 1 objectivesThe PR successfully implements the new rule S9346 to check that integer values are not cast to long for use as timestamps. ✅ 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 32-bit or smaller integer values (int, short, byte, char) passed as arguments to timestamp-consuming APIs (Date, Timestamp, Instant, Calendar), where the narrow type causes overflow or data corruption.