SONARJAVA-6787 Implement new rule S9351: "BigDecimal.compareTo()" should be used instead of "equals()" for numerical comparison

[nathsou]

Description

Implements rule S9351: "BigDecimal.compareTo()\" should be used instead of \"equals()\" for numerical comparison.

• Jira Issue: SONARJAVA-6787
• RSPEC PR: SonarSource/rspec#7902

Implementation Summary

• Added BigDecimalEqualsCheck which detects calls to BigDecimal.equals(Object) and Objects.equals(...) / com.google.common.base.Objects.equal(...) involving BigDecimal operands.
• Exempts equals(Object) method definitions overriding Object.equals(Object) where scale-sensitive comparison is required for hashCode() consistency.
• Added comprehensive unit tests in BigDecimalEqualsCheckTest and BigDecimalEqualsCheckSample.
• Added generated rule metadata JSON/HTML and profile inclusion for "Sonar way".

🤖 Generated with AI assistance.

---

Summary by Gitar

• New checks:
  • Implemented CompilationOrPreparationInLoopCheck to detect regex compilation or statement preparation inside loops
• New rule S9351:
  • Added HTML documentation explaining the // NOSONAR exception rationale for BigDecimal.compareTo() usage

_{This will update automatically on new commits.}

[gitar-bot]

💡 Performance: Loop-invariance check re-traverses entire loop per matched call

In isLoopInvariant, for every identifier argument of every matched invocation inside a loop, a fresh DeclaredOrAssignedLocalsCollector traverses the whole loop subtree (loop.accept(collector)). For loops containing many matched invocations this is repeated O(matches) times over the loop body, which is wasteful on large loops. Consider computing the assigned/declared-locals set once per loop and caching it.

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Implements rule S9351 to flag BigDecimal.equals() usage in favor of compareTo(), resolving incorrect receiver matching and metadata issues. Consider optimizing loop traversal performance in isLoopInvariant to avoid redundant subtree visits.

💡 Performance: Loop-invariance check re-traverses entire loop per matched call

📄 java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java:152-166

In isLoopInvariant, for every identifier argument of every matched invocation inside a loop, a fresh DeclaredOrAssignedLocalsCollector traverses the whole loop subtree (loop.accept(collector)). For loops containing many matched invocations this is repeated O(matches) times over the loop body, which is wasteful on large loops. Consider computing the assigned/declared-locals set once per loop and caching it.

✅ 2 resolved

✅ Quality: S9351.json declares quickfix "targeted" but no quick fix implemented

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9351.json:17 📄 java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java:34-48
S9351.json sets "quickfix": "targeted", but BigDecimalEqualsCheck implements no quick fix (no withQuickFix / JavaQuickFix usage), and the test only calls verifyIssues(). This advertises an IDE quick fix that does not exist. Either implement a targeted quick fix (replace .equals(x) with .compareTo(x) == 0) or set the field to "infeasible".

✅ Edge Case: BigDecimalEqualsCheck flags equals() on any receiver when arg is BigDecimal

📄 java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java:62-68
INSTANCE_EQUALS uses ofAnyType() and reports whenever either the receiver or the argument is a BigDecimal. This means calls like someString.equals(bigDecimal) or arbitraryObject.equals(bigDecimal) are flagged even though the receiver is unrelated to BigDecimal and the comparison is not a BigDecimal-vs-BigDecimal numerical comparison. If this breadth is unintended it will yield false positives; if intended per RSPEC keep it, otherwise require the receiver type to also be BigDecimal for the instance-equals branch.

🤖 Prompt for agents

Code Review: Implements rule S9351 to flag BigDecimal.equals() usage in favor of compareTo(), resolving incorrect receiver matching and metadata issues. Consider optimizing loop traversal performance in isLoopInvariant to avoid redundant subtree visits.

1. 💡 Performance: Loop-invariance check re-traverses entire loop per matched call
   Files: java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java:152-166

   In isLoopInvariant, for every identifier argument of every matched invocation inside a loop, a fresh DeclaredOrAssignedLocalsCollector traverses the whole loop subtree (loop.accept(collector)). For loops containing many matched invocations this is repeated O(matches) times over the loop body, which is wasteful on large loops. Consider computing the assigned/declared-locals set once per loop and caching it.

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]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
96.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube