Repository navigation
SONARJAVA-7151: S1314: Ignore short octal literals and common permission patterns - #6327
romainbrenguier wants to merge 3 commits into
Conversation
…on patterns Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| String value = tree.value(); | ||
| if (tree.is(Tree.Kind.INT_LITERAL) && isOctal(value) && value.length() > 2 && !isPermission(value)) { |
There was a problem hiding this comment.
💡 Edge Case: Underscore-separated octal literals bypass the new exemptions
Both new exemptions check the raw token text from tree.value(). Java allows underscores in octal literals, so 0_7 (value 7, same as 07) has length 3 and passes the value.length() > 2 check. A permission written as 0_755 also fails PERMISSION_PATTERN and still gets reported. Fix: strip underscores before both checks, e.g. String digits = value.replace("_", "");, and run the length check and isPermission on digits.
Normalize underscores before applying the length and permission exemptions:
String value = tree.value();
String digits = value.replace("_", "");
if (tree.is(Tree.Kind.INT_LITERAL) && isOctal(value) && digits.length() > 2 && !isPermission(digits)) {
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
… in the issue message Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Suggested RSPEC change for S1314 (not applied to the generated HTML in this PR)The implementation now:
Proposed update to the rule description in RSPEC: <h2>Why is this an issue?</h2>
<p>Integer literals starting with a zero are octal rather than decimal values. While using octal values is fully supported, most developers do not
have experience with them. They may not recognize octal values as such, mistaking them instead for decimal values.</p>
<p>This rule does not raise issues on:</p>
<ul>
<li>single-digit octal literals such as <code>00</code> or <code>07</code>, whose value is the same in decimal,</li>
<li>common Unix file permission modes such as <code>0644</code>, <code>0755</code> or <code>04755</code>,</li>
<li>octal literals written with the <code>0_</code> prefix, such as <code>0_13</code>, which makes the intent explicit.</li>
</ul>
<h2>How to fix it</h2>
<p>Use the decimal value instead. If an octal value is really intended, use the <code>0_</code> prefix, which makes it clear to readers
that the literal is octal.</p>
<h3>Noncompliant code example</h3>
<pre>
int myNumber = 010; // Noncompliant. myNumber will hold 8, not 10 - was this really expected?
</pre>
<h3>Compliant solution</h3>
<pre>
int myNumber = 8;
</pre>
<p>or, if octal is intended:</p>
<pre>
int myNumber = 0_10;
</pre>
<h2>Resources</h2>
<!-- unchanged -->The issue message in RSPEC metadata (if documented) should become: |
…olic permission string Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| if (!tree.is(Tree.Kind.INT_LITERAL) || !isOctal(value) || value.length() <= 2 || hasExplicitOctalPrefix(value)) { | ||
| return; | ||
| } | ||
| if (isPermission(value)) { | ||
| context.reportIssue(this, tree, String.format("Use the symbolic permission string \"%s\" instead of this octal value. %s", toSymbolicPermissions(value), EXPLICIT_PREFIX_MESSAGE)); | ||
| } else { | ||
| context.reportIssue(this, tree, "Use decimal values instead of octal ones. " + EXPLICIT_PREFIX_MESSAGE); | ||
| } |
There was a problem hiding this comment.
💡 Quality: PR description still says permission modes are no longer reported
The PR summary still says S1314 "no longer reports common Unix permission modes (e.g. 0644, 0755, 0777, 04755, 01777)". This commit changes that. visitLiteral now reports every PERMISSION_PATTERN match with a symbolic-permission message, and the special-bit modes (01777, 02755, 04755) no longer match the narrowed pattern, so they get the generic "Use decimal values" message. The test file asserts all of these as Noncompliant. Reviewers and the RSPEC update driven from this description will describe the wrong behavior. Update the description so it says permission modes are still reported, but with a suggested symbolic string, and that special-bit modes get the generic message.
Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 closed / 2 findings🟡 Medium risk · Changing S1314 suppresses short and 0_ octal findings while recommending symbolic permission strings, which could hide warnings or prompt unintended edits. Narrows S1314 so octal literals of length 2 or less and 💡 Edge Case: Underscore-separated octal literals bypass the new exemptions📄 java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java:47-48 📄 java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java:57-59 Both new exemptions check the raw token text from Normalize underscores before applying the length and permission exemptions💡 Quality: PR description still says permission modes are no longer reported📄 java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java:50-57 📄 java-checks/src/test/files/checks/OctalValuesCheck.java:22-36 The PR summary still says S1314 "no longer reports common Unix permission modes (e.g. 0644, 0755, 0777, 04755, 01777)". This commit changes that. 🤖 Prompt for agentsVerification Repos 3 inspectedFunctional validation No results Rules No rules evaluated Cross-repo 3 repositories inspected 0 open impacts
Auto-approval Not enabledOptionsAuto-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 |
|




Summary
OctalValuesCheck) no longer reports octal int literals of length <= 2 (e.g.00,02,07), whose value is identical in decimal.0644,0755,0777,04755,01777): an optional special-bits digit followed by owner/group/others digits, where owner has at least read permission and each digit is one of 0, 4, 5, 6, 7.0_prefix (e.g.0_7,0_13,0_755), and the issue message now suggests this prefix when an octal value is intended.🤖 Generated with Claude Code
Agent workflow
Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR created using
uv run create_with_claude.py Rule S1314, should not throw for int literals that have length <= 2 (e.g. 02), it should also not throw for common permission patterns like 0755 etc... Add samples to the tests for these casesPR updated using
uv run update_with_claude.py --prompt "Implement the action plan described in the document We should allow pattern like 0_7 0_13 0_755 (using prefix 0_) to write octal values as this is more explicit than without the _. Make that explicit in the issue message, that a possible fix if octal is really intended 0_ can be used as it is more explicit. We should still allow common permission patterns like 0755 and 0_755. Don't make change to the html directly but comment on the PR what should be the change to the rspec.." -a "We should allow pattern like 0_7 0_13 0_755 (using prefix 0_) to write octal values as this is more explicit than without the _. Make that explicit in the issue message, that a possible fix if octal is really intended 0_ can be used as it is more explicit. We should still allow common permission patterns like 0755 and 0_755. Don't make change to the html directly but comment on the PR what should be the change to the rspec." -g "claude"PR updated using
uv run update_with_claude.py --prompt "Implement the action plan described in the document Change the implementation to no longer exclude permission patterns from raising. Instead for these cases suggest to use a String equivalent to represent permissions where possible for instance "rwxrwxrwx" instead of 0777.." -a "Change the implementation to no longer exclude permission patterns from raising. Instead for these cases suggest to use a String equivalent to represent permissions where possible for instance "rwxrwxrwx" instead of 0777." -g "claude"