diff --git a/java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java b/java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java index de2d8458fb0..c1fe4846f10 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/OctalValuesCheck.java +++ b/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"); } diff --git a/java-checks/src/test/files/checks/OctalValuesCheck.java b/java-checks/src/test/files/checks/OctalValuesCheck.java deleted file mode 100644 index f96c08f5bdb..00000000000 --- a/java-checks/src/test/files/checks/OctalValuesCheck.java +++ /dev/null @@ -1,13 +0,0 @@ -class A { - int a = 0; - int b = 1; - int c = 42; - int d = 010; // Noncompliant {{Use decimal values instead of octal ones.}} -// ^^^ - int e = 00; // Noncompliant - int f = 0.; - int g = 0x00; - int h = 0X00; - int j = 0b0101; - int k = 0B0101; -} diff --git a/java-checks/src/test/files/checks/OctalValuesCheckSample.java b/java-checks/src/test/files/checks/OctalValuesCheckSample.java new file mode 100644 index 00000000000..a24a411323b --- /dev/null +++ b/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 +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/OctalValuesCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/OctalValuesCheckTest.java index 0e07be63abf..9aa16168091 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/OctalValuesCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/OctalValuesCheckTest.java @@ -24,7 +24,7 @@ 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(); } @@ -32,7 +32,7 @@ void test() { @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(); diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1314.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1314.html index adf30b1fdf0..d306ada71f2 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1314.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1314.html @@ -1,15 +1,80 @@ +

This rule raises an issue when an integer literal starts with a zero followed by other digits, such as 010 or 060. Java +interprets such a literal as a base-8 octal value rather than a base-10 decimal value.

Why is this an issue?

-

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.

-

Noncompliant code example

+

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.

+

For example:

+ +

This syntax is counterintuitive: most developers expect a literal such as 060 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.

+

Octal literals are rarely needed in modern code. When a value is better expressed in another base, Java provides explicit prefixes: 0x +for hexadecimal and 0b for binary.

+

A common legitimate use of octal literals is Unix file permission modes, such as 0644 or 0755. Even there, the symbolic +notation accepted by java.nio.file.attribute.PosixFilePermissions.fromString, such as "rw-r—​r--" or +"rwxr-xr-x", is easier to read and harder to get wrong.

+

For an integer literal whose complete source text matches 0[4-7][04-7]{2}, 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 04755, 0311 or 06_44 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 0700.

+

Exceptions

+

This rule does not raise an issue for:

+ 
-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
+
+

What is the potential impact?

+

Using an octal literal when a decimal value is intended leads to incorrect calculations and logic errors. For example, setting a timeout to +060 seconds results in a 48-second timeout instead of the expected 60 seconds.

+

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.

+

How to fix it

+

Use the decimal value instead. For file permission modes, prefer the symbolic permission string. If an octal value is really intended, use the +0_ prefix, which makes it clear to readers that the literal is octal.

+

Code examples

+

Noncompliant code example

+
+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
-

Compliant solution

+

Compliant solution

+
+int myNumber = 10;
+int timeout = 60;
+int count = 1000;
+
+

Noncompliant code example

+
+int mode = 0640; // Noncompliant: if this is a permission mode, use "rw-r-----" instead
+
+

Compliant solution

+
+Set<PosixFilePermission> mode = PosixFilePermissions.fromString("rw-r-----");
+
+

Or, if an octal value is intended, such as a file permission mode passed to a native API:

-int myNumber = 8;
+int mode = 0_640;

Resources

+

Documentation

+ +

Standards