From d29eae755be6af2d06418f040ddcb5aed2b93620 Mon Sep 17 00:00:00 2001 From: nathsou Date: Fri, 28 Aug 2026 12:11:30 +0200 Subject: [PATCH 1/2] RC-289 S9351: Fix FPs on normalized BigDecimals --- .../BigDecimalEqualsCheckGuavaSample.java | 5 +++ .../checks/BigDecimalEqualsCheckSample.java | 19 +++++++- .../java/checks/BigDecimalEqualsCheck.java | 43 ++++++++++++++++++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckGuavaSample.java b/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckGuavaSample.java index 2246839b5d9..0c8225431e8 100644 --- a/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckGuavaSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckGuavaSample.java @@ -14,6 +14,11 @@ void method(BigDecimal a, BigDecimal b, Object o, String s) { // Compliant res = com.google.common.base.Objects.equal(s, "hello"); + res = com.google.common.base.Objects.equal(null, s); + res = com.google.common.base.Objects.equal(s, null); + res = com.google.common.base.Objects.equal(null, a); + res = com.google.common.base.Objects.equal(a, null); + res = com.google.common.base.Objects.equal(a.setScale(2), b.setScale(2)); } static class AccountWithGuavaEquals { diff --git a/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java index 7d66cb41507..d192917b382 100644 --- a/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java @@ -5,7 +5,9 @@ class BigDecimalEqualsCheckSample { - void method(BigDecimal a, BigDecimal b, Object o, String s) { + private static final int SCALE = 2; + + void method(BigDecimal a, BigDecimal b, Object o, String s, int runtimeScaleA, int runtimeScaleB) { boolean res; res = a.equals(b); // Noncompliant [["BigDecimal.equals()" compares scale as well as value; use "compareTo() == 0" for numerical comparison.]] @@ -18,6 +20,11 @@ void method(BigDecimal a, BigDecimal b, Object o, String s) { // ^^^^^^ res = Objects.equals(a, o); // Noncompliant res = Objects.equals(o, a); // Noncompliant + res = a.setScale(2).equals(b); // Noncompliant + res = a.equals(b.setScale(2)); // Noncompliant + res = a.setScale(2).equals(b.setScale(3)); // Noncompliant + res = a.setScale(runtimeScaleA).equals(b.setScale(runtimeScaleB)); // Noncompliant + res = Objects.equals(a.setScale(2), b); // Noncompliant // Compliant res = a.compareTo(b) == 0; @@ -26,6 +33,16 @@ void method(BigDecimal a, BigDecimal b, Object o, String s) { res = s.equals(a); res = s.equals("hello"); res = Objects.equals(s, "hello"); + res = Objects.equals(null, s); + res = Objects.equals(s, null); + res = Objects.equals(null, a); + res = Objects.equals(a, null); + res = Objects.equals((null), s); + res = a.setScale(2).equals(b.setScale(2)); + res = a.setScale(2, java.math.RoundingMode.UP).equals(b.setScale(2, java.math.RoundingMode.DOWN)); + res = a.setScale(SCALE, java.math.RoundingMode.HALF_UP).equals(b.setScale(SCALE, java.math.RoundingMode.HALF_UP)); + res = (a.setScale(2)).equals((b.setScale(2))); + res = Objects.equals(a.setScale(2), b.setScale(2)); } static class Account { diff --git a/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java index 5845603d28a..12dd6655f2d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java @@ -18,6 +18,7 @@ import java.util.Collections; import java.util.List; +import java.util.Optional; import org.sonar.check.Rule; import org.sonar.java.checks.helpers.MethodTreeUtils; import org.sonar.java.model.ExpressionUtils; @@ -25,6 +26,8 @@ import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.Arguments; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.MethodTree; import org.sonar.plugins.java.api.tree.Tree; @@ -48,6 +51,14 @@ public class BigDecimalEqualsCheck extends IssuableSubscriptionVisitor { .addParametersMatcher(JAVA_LANG_OBJECT, JAVA_LANG_OBJECT) .build(); + private static final MethodMatchers SET_SCALE = MethodMatchers.create() + .ofTypes(BIG_DECIMAL) + .names("setScale") + .addParametersMatcher("int") + .addParametersMatcher("int", "int") + .addParametersMatcher("int", "java.math.RoundingMode") + .build(); + @Override public List nodesToVisit() { return Collections.singletonList(Tree.Kind.METHOD_INVOCATION); @@ -60,17 +71,45 @@ public void visitNode(Tree tree) { return; } if (INSTANCE_EQUALS.matches(mit)) { - reportIssue(ExpressionUtils.methodName(mit), MESSAGE); + if (!hasSameKnownScale(mit)) { + reportIssue(ExpressionUtils.methodName(mit), MESSAGE); + } } else if (STATIC_EQUALS.matches(mit)) { Arguments arguments = mit.arguments(); Type firstType = arguments.get(0).symbolType(); Type secondType = arguments.get(1).symbolType(); - if (isBigDecimal(firstType) || isBigDecimal(secondType)) { + if (!hasNullArgument(arguments) + && (isBigDecimal(firstType) || isBigDecimal(secondType)) + && !hasSameKnownScale(arguments.get(0), arguments.get(1))) { reportIssue(ExpressionUtils.methodName(mit), MESSAGE); } } } + private static boolean hasNullArgument(Arguments arguments) { + return arguments.stream().anyMatch(ExpressionUtils::isNullLiteral); + } + + private static boolean hasSameKnownScale(MethodInvocationTree invocation) { + if (invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect) { + return hasSameKnownScale(memberSelect.expression(), invocation.arguments().get(0)); + } + return false; + } + + private static boolean hasSameKnownScale(ExpressionTree first, ExpressionTree second) { + Optional firstScale = setScale(first); + return firstScale.isPresent() && firstScale.equals(setScale(second)); + } + + private static Optional setScale(ExpressionTree expression) { + ExpressionTree unwrapped = ExpressionUtils.skipParentheses(expression); + if (unwrapped instanceof MethodInvocationTree invocation && SET_SCALE.matches(invocation)) { + return invocation.arguments().get(0).asConstant(Integer.class); + } + return Optional.empty(); + } + private static boolean isBigDecimal(Type type) { return !type.isUnknown() && type.isSubtypeOf(BIG_DECIMAL); } From 6d0061ae9ce375eaedb804d87cfa7d21afeb0f08 Mon Sep 17 00:00:00 2001 From: nathsou Date: Fri, 28 Aug 2026 12:27:34 +0200 Subject: [PATCH 2/2] RC-289 Handle inherited BigDecimal setScale calls --- .../src/main/java/checks/BigDecimalEqualsCheckSample.java | 1 + .../main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java index d192917b382..97815845508 100644 --- a/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/BigDecimalEqualsCheckSample.java @@ -84,6 +84,7 @@ public MyBigDecimal(String val) { void testCustom(MyBigDecimal other) { boolean r = this.equals(other); // Noncompliant + r = this.setScale(2).equals(other.setScale(2)); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java index 12dd6655f2d..e24734a6c01 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/BigDecimalEqualsCheck.java @@ -104,7 +104,7 @@ private static boolean hasSameKnownScale(ExpressionTree first, ExpressionTree se private static Optional setScale(ExpressionTree expression) { ExpressionTree unwrapped = ExpressionUtils.skipParentheses(expression); - if (unwrapped instanceof MethodInvocationTree invocation && SET_SCALE.matches(invocation)) { + if (unwrapped instanceof MethodInvocationTree invocation && SET_SCALE.matches(invocation.methodSymbol())) { return invocation.arguments().get(0).asConstant(Integer.class); } return Optional.empty();