SONARJAVA-7151: S1314: Ignore short octal literals and common permission patterns

java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java

@@ -16,6 +16,7 @@
  */
 package org.sonar.java.checks;
 
+import java.util.regex.Pattern;
 import org.sonar.check.Rule;
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
@@ -26,6 +27,15 @@
 @Rule(key = "S1314")
 public class OctalValuesCheck extends BaseTreeVisitor implements JavaFileScanner {
 
+  /**
+   * Matches Unix file permission modes such as {@code 0644} or {@code 0755}:
+   * owner, group and others digits, where owner has at least read permission
+   * and each digit is one of the common values 0, 4, 5, 6 or 7.
+   */
+  private static final Pattern PERMISSION_PATTERN = Pattern.compile("0[4-7][04-7]{2}");
+
+  private static final String EXPLICIT_PREFIX_MESSAGE = "If an octal value is intended, use the more explicit \"0_\" prefix.";
+
   private JavaFileScannerContext context;
 
   @Override
@@ -36,15 +46,48 @@ public void scanFile(final JavaFileScannerContext context) {
 
   @Override
   public void visitLiteral(LiteralTree tree) {
-    if (tree.is(Tree.Kind.INT_LITERAL) && isOctal(tree.value())) {
-      context.reportIssue(this, tree, "Use decimal values instead of octal ones.");
+    String value = tree.value();
+    if (!tree.is(Tree.Kind.INT_LITERAL) || !isOctal(value) || value.length() <= 2 || hasExplicitOctalPrefix(value)) {
+      return;
+    }
+    if (isPermission(value)) {
+      String message = String.format(
+        "If this is a file permission mode, use the symbolic permission string \"%s\"; otherwise use the decimal value. %s",
+        toSymbolicPermissions(value),
+        EXPLICIT_PREFIX_MESSAGE);
+      context.reportIssue(this, tree, message);
+    } else {
+      context.reportIssue(this, tree, "Use decimal values instead of octal ones. " + EXPLICIT_PREFIX_MESSAGE);
+    }
+  }
+
+  /**
+   * Converts an octal permission mode such as {@code 0755} into its symbolic representation such as {@code rwxr-xr-x},
+   * as accepted by {@code java.nio.file.attribute.PosixFilePermissions.fromString}.
+   */
+  private static String toSymbolicPermissions(String value) {
+    StringBuilder sb = new StringBuilder();
+    for (char digit : value.substring(1).toCharArray()) {
+      int bits = digit - '0';
+      sb.append((bits & 4) != 0 ? 'r' : '-')
+        .append((bits & 2) != 0 ? 'w' : '-')
+        .append((bits & 1) != 0 ? 'x' : '-');
     }
+    return sb.toString();
   }
 
   private static boolean isOctal(String value) {
     return value.charAt(0) == '0'  && !"0".equals(value) && !isHexadecimal(value) && !isBinary(value);
   }
 
+  private static boolean hasExplicitOctalPrefix(String value) {
+    return value.startsWith("0_");
+  }
+
+  private static boolean isPermission(String value) {
+    return PERMISSION_PATTERN.matcher(value).matches();
+  }
+
   private static boolean isHexadecimal(String value) {
     return value.startsWith("0x") || value.startsWith("0X");
   }

java-checks/src/test/files/checks/OctalValuesCheckSample.java

@@ -0,0 +1,51 @@
+class A {
+  int a = 0;
+  int b = 1;
+  int c = 42;
+  int d = 010; // Noncompliant {{Use decimal values instead of octal ones. If an octal value is intended, use the more explicit "0_" prefix.}}
+//        ^^^
+  int e = 00;
+  int f = 0.;
+  int g = 0x00;
+  int h = 0X00;
+  int j = 0b0101;
+  int k = 0B0101;
+  int l = 01;
+  int m = 02;
+  int n = 07;
+  int o = 012; // Noncompliant
+  int p = 0123; // Noncompliant
+  int q = 0012; // Noncompliant
+  int r = 0311; // Noncompliant
+  int s = 01234; // Noncompliant
+
+  int perm1 = 0400; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "r--------"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+//            ^^^^
+  int perm2 = 0444; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "r--r--r--"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm3 = 0600; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rw-------"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm4 = 0640; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rw-r-----"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm5 = 0644; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rw-r--r--"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm6 = 0660; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rw-rw----"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm7 = 0664; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rw-rw-r--"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm8 = 0666; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rw-rw-rw-"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm9 = 0700; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rwx------"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm10 = 0750; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rwxr-x---"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm11 = 0755; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rwxr-xr-x"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm12 = 0770; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rwxrwx---"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm13 = 0775; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rwxrwxr-x"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm14 = 0777; // Noncompliant {{If this is a file permission mode, use the symbolic permission string "rwxrwxrwx"; otherwise use the decimal value. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm15 = 01777; // Noncompliant {{Use decimal values instead of octal ones. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm16 = 02755; // Noncompliant {{Use decimal values instead of octal ones. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm17 = 04755; // Noncompliant {{Use decimal values instead of octal ones. If an octal value is intended, use the more explicit "0_" prefix.}}
+  int perm18 = 0_755;
+  int perm19 = 0_644;
+
+  int explicit1 = 0_7;
+  int explicit2 = 0_13;
+  int explicit3 = 0_123;
+  int explicit4 = 0_311;
+  int explicit5 = 0_1234;
+  int explicit6 = 0_12_34;
+  int underscore1 = 01_234; // Noncompliant
+  int underscore2 = 00_13; // Noncompliant
+}

java-checks/src/test/java/org/sonar/java/checks/OctalValuesCheckTest.java

@@ -24,15 +24,15 @@ class OctalValuesCheckTest {
   @Test
   void test() {
     CheckVerifier.newVerifier()
-      .onFile("src/test/files/checks/OctalValuesCheck.java")
+      .onFile("src/test/files/checks/OctalValuesCheckSample.java")
       .withCheck(new OctalValuesCheck())
       .verifyIssues();
   }
 
   @Test
   void test_without_semantic() {
     CheckVerifier.newVerifier()
-      .onFile("src/test/files/checks/OctalValuesCheck.java")
+      .onFile("src/test/files/checks/OctalValuesCheckSample.java")
       .withCheck(new OctalValuesCheck())
       .withoutSemantic()
       .verifyIssues();

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1314.html

@@ -1,15 +1,80 @@
+<p>This rule raises an issue when an integer literal starts with a zero followed by other digits, such as <code>010</code> or <code>060</code>. Java
+interprets such a literal as a base-8 octal value rather than a base-10 decimal value.</p>
 <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>
-<h3>Noncompliant code example</h3>
+<p>In Java, integer literals that begin with a leading zero are interpreted as octal numbers, not decimal numbers. This is a legacy feature inherited
+from C.</p>
+<p>For example:</p>
+<ul>
+  <li><code>010</code> represents 8 in decimal, computed as 1×8 + 0</li>
+  <li><code>060</code> represents 48 in decimal, computed as 6×8 + 0</li>
+  <li><code>01000</code> represents 512 in decimal, computed as 1×512 + 0×64 + 0×8 + 0</li>
+</ul>
+<p>This syntax is counterintuitive: most developers expect a literal such as <code>060</code> to mean sixty, with the leading zero being mere padding.
+There is no visual cue that octal interpretation is happening, and code reviewers can easily miss the leading zero.</p>
+<p>Octal literals are rarely needed in modern code. When a value is better expressed in another base, Java provides explicit prefixes: <code>0x</code>
+for hexadecimal and <code>0b</code> for binary.</p>
+<p>A common legitimate use of octal literals is Unix file permission modes, such as <code>0644</code> or <code>0755</code>. Even there, the symbolic
+notation accepted by <code>java.nio.file.attribute.PosixFilePermissions.fromString</code>, such as <code>"rw-r—​r--"</code> or
+<code>"rwxr-xr-x"</code>, is easier to read and harder to get wrong.</p>
+<p>For an integer literal whose complete source text matches <code>0[4-7][04-7]{2}</code>, the rule message suggests the equivalent symbolic
+permission string. This means exactly four characters: a leading zero, an owner digit from 4 to 7, and group and others digits each equal to 0, 4, 5,
+6 or 7. Literals such as <code>04755</code>, <code>0311</code> or <code>06_44</code> do not match this criterion and receive the general octal message
+instead. Permission-mode literals are still reported, not exempted. The dedicated message also mentions the decimal value as an alternative, since
+such a literal may be a zero-padded decimal like a time of day <code>0700</code>.</p>
+<h3>Exceptions</h3>
+<p>This rule does not raise an issue for:</p>
+<ul>
+  <li>Literals with at most two characters, such as <code>00</code> or <code>07</code>. Their octal and decimal values are identical, so there is no
+  risk of confusion.</li>
+  <li>Octal literals written with the <code>0_</code> prefix, such as <code>0_7</code>, <code>0_13</code> or <code>0_755</code>, which makes the
+  intent explicit.</li>
+</ul>
 <pre>
-int myNumber = 010; // Noncompliant. myNumber will hold 8, not 10 - was this really expected?
+int zero = 00;            // Compliant: same value in octal and decimal
+int seven = 07;           // Compliant: same value in octal and decimal
+int explicitOctal = 0_13; // Compliant: explicit octal intent
+int mode = 0_755;         // Compliant: explicit octal intent
+</pre>
+<h3>What is the potential impact?</h3>
+<p>Using an octal literal when a decimal value is intended leads to incorrect calculations and logic errors. For example, setting a timeout to
+<code>060</code> seconds results in a 48-second timeout instead of the expected 60 seconds.</p>
+<p>Such bugs are difficult to diagnose because the code compiles without any warning, and the discrepancy between the expected and actual values may
+only show up under specific conditions.</p>
+<h2>How to fix it</h2>
+<p>Use the decimal value instead. For file permission modes, prefer the symbolic permission string. 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>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+int myNumber = 010;    // Noncompliant: myNumber holds 8, not 10
+int timeout = 060;     // Noncompliant: timeout holds 48, not 60
+int count = 01000;     // Noncompliant: count holds 512, not 1000
 </pre>
-<h3>Compliant solution</h3>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+int myNumber = 10;
+int timeout = 60;
+int count = 1000;
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+int mode = 0640; // Noncompliant: if this is a permission mode, use "rw-r-----" instead
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+Set&lt;PosixFilePermission&gt; mode = PosixFilePermissions.fromString("rw-r-----");
+</pre>
+<p>Or, if an octal value is intended, such as a file permission mode passed to a native API:</p>
 <pre>
-int myNumber = 8;
+int mode = 0_640;
 </pre>
 <h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Java Language Specification - <a href="https://docs.oracle.com/javase/specs/jls/se21/html/jls-3.html#jls-3.10.1">§3.10.1 Integer
+  Literals</a></li>
+</ul>
+<h3>Standards</h3>
 <ul>
   <li><a
   href="https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/recommendations/declarations-and-initialization-dcl/dcl18-c">CERT, DCL18-C.</a> - Do not begin integer constants with 0 when specifying a decimal value</li>