From 1946eb2404bb5f99351dd5d086c4b21ae07b6903 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 18 Aug 2026 09:45:46 +0200 Subject: [PATCH 1/5] SONARJAVA-6759: Avoid overlap between S1244 and S9147 for NaN comparisons S1244 no longer raises issues when a float/double is compared to Double.NaN or Float.NaN, since this case is already covered by S9147. Also adds S1244 as a related rule in the S9147 Rspec. Co-Authored-By: Claude Opus 4.6 --- .../src/main/java/checks/FloatEquality.java | 15 ++++++++ .../sonar/java/checks/FloatEqualityCheck.java | 37 ++++++++++++++++++- .../org/sonar/l10n/java/rules/java/S9147.html | 1 + 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/FloatEquality.java b/java-checks-test-sources/default/src/main/java/checks/FloatEquality.java index 75cd035df59..0852562bbb1 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FloatEquality.java +++ b/java-checks-test-sources/default/src/main/java/checks/FloatEquality.java @@ -1,5 +1,7 @@ package checks; +import static java.lang.Double.NaN; + class FloatEquality { void foo() { float f1 = 0.1f; @@ -51,4 +53,17 @@ void method(Double d1, Double d2, Float f1, Float f2) { if (f1.equals(f2)) { } // Noncompliant if (new Object().equals(f2)) { } //compliant } + + void nanComparisons() { + double d = 0.1d; + float f = 0.1f; + if (d == Double.NaN) {} // Compliant - covered by S9147 + if (d != Double.NaN) {} // Compliant - covered by S9147 + if (Double.NaN == d) {} // Compliant - covered by S9147 + if (f == Float.NaN) {} // Compliant - covered by S9147 + if (f != Float.NaN) {} // Compliant - covered by S9147 + if (Float.NaN == f) {} // Compliant - covered by S9147 + if (d == (Double.NaN)) {} // Compliant - covered by S9147 + if (d == NaN) {} // Compliant - covered by S9147 (static import) + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FloatEqualityCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FloatEqualityCheck.java index 61560ef41b2..48fcf54e803 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FloatEqualityCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FloatEqualityCheck.java @@ -18,13 +18,18 @@ import java.util.Arrays; import java.util.List; +import javax.annotation.Nullable; import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; import org.sonar.java.model.SyntacticEquivalence; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.Tree; @@ -87,7 +92,37 @@ private static boolean isIndirectEquality(BinaryExpressionTree binaryExpressionT private static boolean isNanTest(BinaryExpressionTree binaryExpressionTree) { - return SyntacticEquivalence.areEquivalent(binaryExpressionTree.leftOperand(), binaryExpressionTree.rightOperand()); + return SyntacticEquivalence.areEquivalent(binaryExpressionTree.leftOperand(), binaryExpressionTree.rightOperand()) + || isNanExpression(binaryExpressionTree.leftOperand()) + || isNanExpression(binaryExpressionTree.rightOperand()); + } + + private static boolean isNanExpression(ExpressionTree expression) { + ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + if (expr.is(Tree.Kind.MEMBER_SELECT)) { + MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expr; + if ("NaN".equals(memberSelect.identifier().name())) { + return isFloatingPointNan(memberSelect.identifier().symbol()); + } + } else if (expr.is(Tree.Kind.IDENTIFIER)) { + IdentifierTree identifier = (IdentifierTree) expr; + if ("NaN".equals(identifier.name())) { + return isFloatingPointNan(identifier.symbol()); + } + } + return false; + } + + private static boolean isFloatingPointNan(@Nullable Symbol symbol) { + if (symbol == null || symbol.isUnknown()) { + return false; + } + Symbol owner = symbol.owner(); + if (owner == null || owner.type() == null) { + return false; + } + Type ownerType = owner.type(); + return ownerType.is("java.lang.Double") || ownerType.is("java.lang.Float"); } private static boolean hasFloatingType(ExpressionTree expressionTree) { diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9147.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9147.html index 3b090e2ff63..d6dcf7143a8 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9147.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9147.html @@ -56,6 +56,7 @@

Documentation

Related rules