-
Notifications
You must be signed in to change notification settings - Fork 723
SONARJAVA-6703 Implement new rule S9133 #5862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
b2a4577
0c2cf07
876d1a1
d50a663
ceb5f87
141debe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,14 @@ | ||
| { | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/ce/log/CeLoggingTest.java": [ | ||
| 64 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/component/ws/TreeActionTest.java": [ | ||
| 88 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/language/ws/LanguageWsTest.java": [ | ||
| 51 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/ws/ShowActionTest.java": [ | ||
| 150 | ||
| ] | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/ce/log/CeLoggingTest.java": [ | ||
| 64 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/component/ws/TreeActionTest.java": [ | ||
| 88 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/language/ws/LanguageWsTest.java": [ | ||
| 51 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/ws/ShowActionTest.java": [ | ||
| 150 | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| { | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java": [ | ||
| 103, | ||
| 116, | ||
| 117, | ||
| 136 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java": [ | ||
| 106, | ||
| 144 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/IssueDocTesting.java": [ | ||
| 43 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java": [ | ||
| 352, | ||
| 355, | ||
| 361, | ||
| 362, | ||
| 365, | ||
| 371, | ||
| 374 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/measure/custom/ws/CustomMeasureValidatorTest.java": [ | ||
| 73 | ||
| ], | ||
| "org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/RubyUtilsTest.java": [ | ||
| 91, | ||
| 92, | ||
| 92 | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| package checks; | ||
|
|
||
| class HardcodedMathConstantCheckSample { | ||
|
|
||
| // Pi approximations at various precisions (3+ significant digits) | ||
| double pi1 = 3.14; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| double pi2 = 3.14159; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| double pi3 = 3.14159265; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| double pi4 = 3.14159265358979; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| double pi5 = 3.141; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
|
|
||
| // E approximations (3+ significant digits) | ||
| double e0 = 2.72; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
| double e1 = 2.718; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
| double e2 = 2.71828; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
| double e3 = 2.71828182845; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
|
|
||
| // sqrt(2) approximations (3+ significant digits) | ||
| double sqrt2_0 = 1.41; // Noncompliant {{Use "Math.sqrt(2)" instead of this approximation of the square root of 2.}} | ||
| double sqrt2_1 = 1.414; // Noncompliant {{Use "Math.sqrt(2)" instead of this approximation of the square root of 2.}} | ||
| double sqrt2_2 = 1.41421; // Noncompliant {{Use "Math.sqrt(2)" instead of this approximation of the square root of 2.}} | ||
| double sqrt2_3 = 1.4142135; // Noncompliant {{Use "Math.sqrt(2)" instead of this approximation of the square root of 2.}} | ||
|
|
||
| // ln(2) approximations (3+ significant digits) | ||
| double ln2_1 = 0.693; // Noncompliant {{Use "Math.log(2)" instead of this approximation of the natural logarithm of 2.}} | ||
| double ln2_2 = 0.6931; // Noncompliant {{Use "Math.log(2)" instead of this approximation of the natural logarithm of 2.}} | ||
| double ln2_3 = 0.69314; // Noncompliant {{Use "Math.log(2)" instead of this approximation of the natural logarithm of 2.}} | ||
| double ln2_4 = 0.693147; // Noncompliant {{Use "Math.log(2)" instead of this approximation of the natural logarithm of 2.}} | ||
|
|
||
| // Float literals | ||
| float piFloat = 3.14159f; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| float eFloat = 2.718f; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
|
|
||
| // Underscore-separated literal (normalize strips underscores) | ||
| double piUnderscore = 3.14_159; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
|
|
||
| // D-suffix double literal (normalize strips suffix) | ||
| double piDsuffix = 3.14159d; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| double eDsuffix = 2.71828D; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
|
|
||
| // Static final field | ||
| private static final double MY_PI = 3.14159265358979; // Noncompliant | ||
|
|
||
| // In arithmetic expressions | ||
| double area(double r) { | ||
| return 3.14159 * r * r; // Noncompliant | ||
| } | ||
|
|
||
| double circumference(double r) { | ||
| return 2 * 3.14159 * r; // Noncompliant | ||
| } | ||
|
|
||
| double volume(double r) { | ||
| return (4.0 / 3.0) * 3.14159 * r * r * r; // Noncompliant | ||
| } | ||
|
|
||
| // Method arguments | ||
| double shifted(double x) { | ||
| return Math.sin(x + 3.14159); // Noncompliant | ||
| } | ||
|
|
||
| // Ternary expression | ||
| double pick(boolean b, double x) { | ||
| return b ? 3.14159 : x; // Noncompliant | ||
| } | ||
|
|
||
| // Compliant - standard library constants | ||
| double compliantPi = Math.PI; | ||
| double compliantE = Math.E; | ||
| double compliantSqrt2 = Math.sqrt(2); | ||
| double compliantLn2 = Math.log(2); | ||
| double compliantStrictPi = StrictMath.PI; | ||
| double compliantStrictE = StrictMath.E; | ||
|
|
||
| // Compliant - unrelated values | ||
| double unrelated1 = 3.0; | ||
| double unrelated2 = 2.0; | ||
| double unrelated3 = 1.5; | ||
| double unrelated4 = 0.5; | ||
| double unrelated5 = 100.0; | ||
| double unrelated6 = 0.001; | ||
|
|
||
| // Compliant - zero value (covers absoluteValue == 0.0 branch) | ||
| double zero = 0.0; | ||
|
|
||
| // Compliant - too few significant digits (fewer than 3) | ||
| double tooImprecise2 = 3.1; | ||
| double tooImprecise3 = 2.7; | ||
| double tooImprecise4 = 1.4; | ||
| double tooImprecise5 = 0.69; | ||
|
|
||
| // Compliant - outside tolerance (with 3 significant digits) | ||
| double outsideTolerance1 = 3.16; | ||
|
|
||
| // Compliant - scientific notation (skipped) | ||
| double sci1 = 3.14e0; | ||
| double sci2 = 314E-2; | ||
|
|
||
| // Compliant - hex float literals (skipped) | ||
| double hex1 = 0x1.0p0; | ||
| double hex2 = 0X1.0p0; | ||
|
|
||
| // Leading-zero float literal (not octal — octal notation does not apply to floating-point) | ||
| double leadingZeroPi = 03.14159; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
|
|
||
| // Leading-dot literal with enough significant digits (3) | ||
| double leadingDot = .693; // Noncompliant {{Use "Math.log(2)" instead of this approximation of the natural logarithm of 2.}} | ||
|
|
||
| // Float literal with F suffix | ||
| float piFloatF = 3.14159F; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
|
|
||
| // Leading-dot literal with enough significant digits | ||
| double leadingDotLn2 = .6931; // Noncompliant {{Use "Math.log(2)" instead of this approximation of the natural logarithm of 2.}} | ||
|
|
||
| // Negative numbers (unary minus applied to literal - still detected) | ||
| double negativePi = -3.14159; // Noncompliant {{Use "Math.PI" instead of this approximation of pi.}} | ||
| double negativeE = -2.71828; // Noncompliant {{Use "Math.E" instead of this approximation of Euler's number.}} | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package org.sonar.java.checks; | ||
|
|
||
| import java.util.List; | ||
| import org.sonar.check.Rule; | ||
| import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||
| import org.sonar.plugins.java.api.tree.LiteralTree; | ||
| import org.sonar.plugins.java.api.tree.Tree; | ||
|
|
||
| @Rule(key = "S9133") | ||
| public class HardcodedMathConstantCheck extends IssuableSubscriptionVisitor { | ||
|
|
||
| private static final int MIN_SIGNIFICANT_DIGITS = 3; | ||
|
|
||
| private enum MathConstant { | ||
| PI(Math.PI, "Math.PI", "pi"), | ||
| E(Math.E, "Math.E", "Euler's number"), | ||
| SQRT2(Math.sqrt(2), "Math.sqrt(2)", "the square root of 2"), | ||
| LN2(Math.log(2), "Math.log(2)", "the natural logarithm of 2"); | ||
|
|
||
| final double value; | ||
| final String replacement; | ||
| final String description; | ||
|
|
||
| MathConstant(double value, String replacement, String description) { | ||
| this.value = value; | ||
| this.replacement = replacement; | ||
| this.description = description; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public List<Tree.Kind> nodesToVisit() { | ||
| return List.of(Tree.Kind.DOUBLE_LITERAL, Tree.Kind.FLOAT_LITERAL); | ||
| } | ||
|
|
||
| @Override | ||
| public void visitNode(Tree tree) { | ||
| LiteralTree literalTree = (LiteralTree) tree; | ||
| String rawValue = literalTree.value(); | ||
|
|
||
| String normalized = normalize(rawValue); | ||
| if (normalized == null) { | ||
| return; | ||
| } | ||
|
|
||
| double parsedValue; | ||
| try { | ||
| parsedValue = Double.parseDouble(normalized); | ||
| } catch (NumberFormatException e) { | ||
| return; | ||
| } | ||
|
|
||
| double absoluteValue = Math.abs(parsedValue); | ||
| if (absoluteValue == 0.0) { | ||
| return; | ||
| } | ||
|
|
||
| int significantDigits = countSignificantDigits(normalized); | ||
| if (significantDigits < MIN_SIGNIFICANT_DIGITS) { | ||
| return; | ||
| } | ||
|
|
||
| // Tolerance is based on the literal's own precision: half a unit in the last significant digit. | ||
| // This ensures we only flag values that match the constant across all their significant digits. | ||
| double relativeTolerance = 5.0 * Math.pow(10, -significantDigits); | ||
|
|
||
| for (MathConstant constant : MathConstant.values()) { | ||
| double relativeError = Math.abs(absoluteValue - constant.value) / constant.value; | ||
| if (relativeError < relativeTolerance) { | ||
| reportIssue(tree, "Use \"" + constant.replacement + "\" instead of this approximation of " + constant.description + "."); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static String normalize(String rawValue) { | ||
| String value = rawValue.replace("_", ""); | ||
| // Strip type suffix | ||
| char last = value.charAt(value.length() - 1); | ||
| if ("fd".indexOf(Character.toLowerCase(last)) >= 0) { | ||
| value = value.substring(0, value.length() - 1); | ||
| } | ||
| // Skip hex float literals | ||
| if (value.startsWith("0x") || value.startsWith("0X")) { | ||
| return null; | ||
| } | ||
| // Skip scientific notation | ||
| if (value.indexOf('e') >= 0 || value.indexOf('E') >= 0) { | ||
| return null; | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| private static int countSignificantDigits(String normalized) { | ||
| String sig = normalized.replace("-", "").replace(".", "").replaceFirst("^0+", ""); | ||
| return sig.isEmpty() ? 1 : sig.length(); | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks to be incorrect on negative numbers at first sight, the reproducer should contain a negative nubmer case to verify
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it could also be rewritten in a easier-to-understand way : private static int countSignificantDigits(String normalized) {
String sig = normalized.replace("-", "").replace(".", "").replaceFirst("^0+", "");
return sig.isEmpty() ? 1 : sig.length();
} |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package org.sonar.java.checks; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.sonar.java.checks.verifier.CheckVerifier; | ||
|
|
||
| import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; | ||
|
|
||
| class HardcodedMathConstantCheckTest { | ||
|
|
||
| @Test | ||
| void test() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile(mainCodeSourcesPath("checks/HardcodedMathConstantCheckSample.java")) | ||
| .withCheck(new HardcodedMathConstantCheck()) | ||
| .verifyIssues(); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| <h2>Why is this an issue?</h2> | ||
| <p>Hard-coding approximate values of well-known mathematical constants creates several problems in your codebase.</p> | ||
| <p>First, these hard-coded values often lack the full precision available in predefined library constants. For example, writing <code>3.14</code> instead of | ||
| using a library-provided constant for π loses significant decimal places, potentially leading to calculation errors in scientific or engineering | ||
| applications.</p> | ||
| <p>Second, hard-coded values make your code less readable. When other developers see <code>3.14159</code>, they must recognize it as π themselves. | ||
| Using named constants from standard libraries makes the intent immediately clear.</p> | ||
| <p>Third, maintenance becomes harder. If you need to change the precision or update the value across your codebase, you must find and modify multiple | ||
| literal values. With named constants, the meaning is centralized and clear.</p> | ||
| <p>In Java, these constants and functions are available in the <code>Math</code> class: <code>Math.PI</code>, <code>Math.E</code>, | ||
| <code>Math.sqrt(2)</code>, and <code>Math.log(2)</code>.</p> | ||
| <h2>How to fix it</h2> | ||
| <p>Replace hard-coded numeric approximations with the appropriate predefined constant from the <code>Math</code> class. For π, use | ||
| <code>Math.PI</code>. For Euler's number, use <code>Math.E</code>. For other common constants that don't have direct equivalents, use mathematical | ||
| expressions like <code>Math.sqrt(2)</code> or <code>Math.log(2)</code>.</p> | ||
| <h3>Noncompliant code example</h3> | ||
| <pre> | ||
| public class CircleCalculator { | ||
| public double calculateArea(double radius) { | ||
| return 3.14 * radius * radius; // Noncompliant | ||
| } | ||
|
|
||
| public double calculateCircumference(double radius) { | ||
| return 2 * 3.14159 * radius; // Noncompliant | ||
| } | ||
| } | ||
| </pre> | ||
| <h3>Compliant solution</h3> | ||
| <pre> | ||
| public class CircleCalculator { | ||
| public double calculateArea(double radius) { | ||
| return Math.PI * radius * radius; | ||
| } | ||
|
|
||
| public double calculateCircumference(double radius) { | ||
| return 2 * Math.PI * radius; | ||
| } | ||
| } | ||
| </pre> | ||
| <h2>Resources</h2> | ||
| <h3>Documentation</h3> | ||
| <ul> | ||
| <li>Java Documentation - <a href="https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Math.html">Math (Java SE 17 & JDK | ||
| 17)</a></li> | ||
| <li>Java Documentation - <a href="https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StrictMath.html">StrictMath (Java SE 17 | ||
| & JDK 17)</a></li> | ||
| </ul> | ||
| <h3>Standards</h3> | ||
| <ul> | ||
| <li>CWE - <a href="https://cwe.mitre.org/data/definitions/1106.html">CWE-1106: Insufficient Use of Symbolic Constants</a></li> | ||
| </ul> |
Uh oh!
There was an error while loading. Please reload this page.