Skip to content

SONARJAVA-6703 Implement new rule S9133 - #5862

Draft
romainbrenguier wants to merge 3 commits into
masterfrom
new-rule/SONARJAVA-6703-S9133
Draft

SONARJAVA-6703 Implement new rule S9133#5862
romainbrenguier wants to merge 3 commits into
masterfrom
new-rule/SONARJAVA-6703-S9133

Conversation

@romainbrenguier

Copy link
Copy Markdown
Contributor

Detect hard-coded floating-point literals that approximate well-known mathematical constants (pi, e, sqrt(2), ln(2)) and suggest using the corresponding Math class constants or expressions instead.

Part of

Detect hard-coded floating-point literals that approximate well-known
mathematical constants (pi, e, sqrt(2), ln(2)) and suggest using the
corresponding Math class constants or expressions instead.
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6703

Comment thread java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java Outdated
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5863

Please review and merge it into your branch.

romainbrenguier pushed a commit that referenced this pull request Aug 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 1 rule files: 0 issues removed, 18 issues added.

S9133 (java) on sonar-server - 0 issues removed, 18 issues added - new ruling file

Added src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java (line 103)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java (line 116)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java (line 117)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java (line 106)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java (line 144)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/step/CustomMeasuresCopyStepTest.java)

Added src/test/java/org/sonar/server/issue/IssueDocTesting.java (line 43)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueDocTesting.java)

Added src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java (line 352)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java)

Added src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java (line 355)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java)

Added src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java (line 361)

(source file not found at this revision: src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java)

Added src/test/java/org/sonar/server/measure/custom/ws/CustomMeasureValidatorTest.java (line 73)

(source file not found at this revision: src/test/java/org/sonar/server/measure/custom/ws/CustomMeasureValidatorTest.java)

Added src/test/java/org/sonar/server/util/RubyUtilsTest.java (line 91)

(source file not found at this revision: src/test/java/org/sonar/server/util/RubyUtilsTest.java)

Added src/test/java/org/sonar/server/util/RubyUtilsTest.java (line 92)

(source file not found at this revision: src/test/java/org/sonar/server/util/RubyUtilsTest.java)

Replace fixed relative tolerance with dynamic precision-based tolerance
derived from the literal's significant digit count. Raise minimum
significant digits from 3 to 4 to avoid false positives on common
domain values like 0.693. Add test cases for underscore-separated
literals, D/d suffixes, leading-dot literals, and zero value.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@romainbrenguier
romainbrenguier force-pushed the new-rule/SONARJAVA-6703-S9133 branch from 4925f8b to 0c2cf07 Compare August 3, 2026 13:49
Remove unreachable '-' and '+' handling from countSignificantDigits
since Java literal tokens never contain sign characters. Add test
cases for uppercase 'E' scientific notation, '0X' hex prefix, 'F'
float suffix, and leading-dot noncompliant literal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Implements rule S9133 to detect hard-coded floating-point approximations of mathematical constants, addressing the loose tolerance and normalize() edge case test coverage findings.

✅ 2 resolved
Edge Case: Loose tolerance may flag legitimate 3-digit domain values

📄 java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java:28-29 📄 java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java:79-85
With RELATIVE_TOLERANCE=0.002 and only MIN_SIGNIFICANT_DIGITS=3, a literal that matches a constant only in its first ~3 digits (e.g. 2.72 for E, or 0.693 which is a common domain value) is flagged even though the author clearly did not intend the constant's full precision. This can create false positives on genuine domain literals. Consider requiring the literal to match the constant across all of its own significant digits (i.e., the error should be on the order of the literal's own rounding) rather than a fixed relative window, to reduce noise while still catching approximations like 3.14159.

Quality: No tests for normalize() edge cases (underscores, D suffix, leading dot)

📄 java-checks/src/main/java/org/sonar/java/checks/HardcodedMathConstantCheck.java:88-102 📄 java-checks-test-sources/default/src/main/java/checks/HardcodedMathConstantCheckSample.java:1-15
normalize() handles digit separators ("3.14_159"), upper/lower 'd'/'D'/'F' suffixes, and the sample covers 'f' and 'e'/hex, but there is no test exercising an underscore-separated literal, a 'D'/'d' suffixed double, or a leading-dot literal (e.g. ".314159e1"). Add sample lines such as double a = 3.14_159; and double b = 3.14159d; to lock in this normalization behavior against regressions.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

sonarqube-next Bot commented Aug 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant