SONARJAVA-6759: Avoid overlap between S1244 and S9147 for NaN comparisons

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,22 @@ 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 (f == (Float.NaN)) {} // Compliant - covered by S9147
+    if (d == NaN) {} // Compliant - covered by S9147 (static import)
+    if (Double.NaN == Double.NaN) {} // Compliant - covered by S9147 (NaN on both sides)
+    if (f == f) {} // Compliant - NaN self-test pattern
+    if (d == Double.MAX_VALUE) {} // Noncompliant {{Equality tests should not be made with floating point values.}}
+    if (Double.NaN == Float.NaN) {} // Compliant - covered by S9147
+  }
 }

java-checks/src/main/java/org/sonar/java/checks/FloatEqualityCheck.java

@@ -19,6 +19,7 @@
 import java.util.Arrays;
 import java.util.List;
 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;
@@ -87,7 +88,9 @@ 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())
+      || ExpressionUtils.getNanOwnerTypeName(binaryExpressionTree.leftOperand()) != null
+      || ExpressionUtils.getNanOwnerTypeName(binaryExpressionTree.rightOperand()) != null;
   }
 
   private static boolean hasFloatingType(ExpressionTree expressionTree) {

java-checks/src/main/java/org/sonar/java/checks/NanEqualityCheck.java

@@ -17,16 +17,10 @@
 package org.sonar.java.checks;
 
 import java.util.List;
-import javax.annotation.Nullable;
 import org.sonar.check.Rule;
 import org.sonar.java.model.ExpressionUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
-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.Tree;
 
 @Rule(key = "S9147")
@@ -40,47 +34,14 @@ public List<Tree.Kind> nodesToVisit() {
   @Override
   public void visitNode(Tree tree) {
     BinaryExpressionTree binaryExpression = (BinaryExpressionTree) tree;
-    String typeName = getNanTypeName(binaryExpression.leftOperand());
+    String typeName = ExpressionUtils.getNanOwnerTypeName(binaryExpression.leftOperand());
     if (typeName == null) {
-      typeName = getNanTypeName(binaryExpression.rightOperand());
+      typeName = ExpressionUtils.getNanOwnerTypeName(binaryExpression.rightOperand());
     }
     if (typeName != null) {
       reportIssue(binaryExpression.operatorToken(),
         String.format("Use \"%s.isNaN()\" instead of comparison with \"%s.NaN\".", typeName, typeName));
     }
   }
 
-  @Nullable
-  private static String getNanTypeName(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 resolveNanOwnerType(memberSelect.identifier().symbol());
-      }
-    } else if (expr.is(Tree.Kind.IDENTIFIER)) {
-      IdentifierTree identifier = (IdentifierTree) expr;
-      if ("NaN".equals(identifier.name())) {
-        return resolveNanOwnerType(identifier.symbol());
-      }
-    }
-    return null;
-  }
-
-  @Nullable
-  private static String resolveNanOwnerType(Symbol symbol) {
-    if (symbol.isUnknown()) {
-      return null;
-    }
-    Symbol owner = symbol.owner();
-    if (owner == null || owner.type() == null) {
-      return null;
-    }
-    Type ownerType = owner.type();
-    if (ownerType.is("java.lang.Double") || ownerType.is("java.lang.Float")) {
-      return ownerType.name();
-    }
-    return null;
-  }
-
 }

java-frontend/src/main/java/org/sonar/java/model/ExpressionUtils.java

@@ -23,6 +23,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.semantic.Type;
 import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
 import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
 import org.sonar.plugins.java.api.tree.ExpressionTree;
@@ -403,4 +404,37 @@ public static String annotationAttributeName(ExpressionTree expression) {
     return "value";
   }
 
+  @Nullable
+  public static String getNanOwnerTypeName(ExpressionTree expression) {
+    ExpressionTree expr = skipParentheses(expression);
+    if (expr.is(Tree.Kind.MEMBER_SELECT)) {
+      MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expr;
+      if ("NaN".equals(memberSelect.identifier().name())) {
+        return resolveNanOwnerType(memberSelect.identifier().symbol());
+      }
+    } else if (expr.is(Tree.Kind.IDENTIFIER)) {
+      IdentifierTree identifier = (IdentifierTree) expr;
+      if ("NaN".equals(identifier.name())) {
+        return resolveNanOwnerType(identifier.symbol());
+      }
+    }
+    return null;
+  }
+
+  @Nullable
+  private static String resolveNanOwnerType(Symbol symbol) {
+    if (symbol.isUnknown()) {
+      return null;
+    }
+    Symbol owner = symbol.owner();
+    if (owner == null || owner.type() == null) {
+      return null;
+    }
+    Type ownerType = owner.type();
+    if (ownerType.is("java.lang.Double") || ownerType.is("java.lang.Float")) {
+      return ownerType.name();
+    }
+    return null;
+  }
+
 }

java-frontend/src/test/java/org/sonar/java/model/ExpressionUtilsTest.java

@@ -41,11 +41,15 @@
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.VariableTree;
 
+import org.sonar.plugins.java.api.tree.IdentifierTree;
+
 import static java.lang.reflect.Modifier.isFinal;
 import static java.lang.reflect.Modifier.isPrivate;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 import static org.sonar.java.model.ExpressionUtils.isInvocationOnVariable;
 import static org.sonar.java.model.ExpressionUtils.skipParenthesesUpwards;
 import static org.sonar.java.model.assertions.TreeAssert.assertThat;
@@ -470,6 +474,130 @@ void areVariablesSame_unknown_symbol() {
     assertThat(ExpressionUtils.areVariablesSame(initializer.falseExpression(), condition.rightOperand(), false)).isFalse();
   }
 
+  @Test
+  void getNanOwnerTypeName_member_select() {
+    // Double.NaN -> "Double"
+    assertNanOwnerTypeName("Double.NaN", "Double");
+    // Float.NaN -> "Float"
+    assertNanOwnerTypeName("Float.NaN", "Float");
+    // Non-NaN member select -> null
+    assertNanOwnerTypeName("Double.MAX_VALUE", null);
+    // Parenthesized expression (Double.NaN) -> "Double"
+    assertNanOwnerTypeName("(Double.NaN)", "Double");
+    // Parenthesized expression (Float.NaN) -> "Float"
+    assertNanOwnerTypeName("(Float.NaN)", "Float");
+  }
+
+  @Test
+  void getNanOwnerTypeName_identifier_static_import() {
+    // Statically imported NaN from Double -> "Double"
+    CompilationUnitTree unit = JParserTestUtils.parse("""
+      import static java.lang.Double.NaN;
+      class A { Object f = NaN; }
+      """);
+    ExpressionTree expr = ((VariableTree) ((ClassTree) unit.types().get(0)).members().get(0)).initializer();
+    assertThat(ExpressionUtils.getNanOwnerTypeName(expr)).isEqualTo("Double");
+  }
+
+  @Test
+  void getNanOwnerTypeName_identifier_static_import_float() {
+    // Statically imported NaN from Float -> "Float"
+    CompilationUnitTree unit = JParserTestUtils.parse("""
+      import static java.lang.Float.NaN;
+      class A { Object f = NaN; }
+      """);
+    ExpressionTree expr = ((VariableTree) ((ClassTree) unit.types().get(0)).members().get(0)).initializer();
+    assertThat(ExpressionUtils.getNanOwnerTypeName(expr)).isEqualTo("Float");
+  }
+
+  @Test
+  void getNanOwnerTypeName_other_expression_types() {
+    // Literal -> null
+    assertNanOwnerTypeName("42.0", null);
+    // Method call -> null
+    assertNanOwnerTypeName("Double.valueOf(0.0)", null);
+  }
+
+  @Test
+  void getNanOwnerTypeName_unknown_nan() {
+    // Unknown class NaN field -> null (symbol is unknown)
+    CompilationUnitTree unit = JParserTestUtils.parse("""
+      class A { Object f = UnknownClass.NaN; }
+      """);
+    ExpressionTree expr = ((VariableTree) ((ClassTree) unit.types().get(0)).members().get(0)).initializer();
+    assertThat(ExpressionUtils.getNanOwnerTypeName(expr)).isNull();
+  }
+
+  private void assertNanOwnerTypeName(String code, @Nullable String expected) {
+    CompilationUnitTree unit = JParserTestUtils.parse("class A { Object f = " + code + "; }");
+    ExpressionTree expression = ((VariableTree) ((ClassTree) unit.types().get(0)).members().get(0)).initializer();
+    String actual = ExpressionUtils.getNanOwnerTypeName(expression);
+    if (expected == null) {
+      assertThat(actual).isNull();
+    } else {
+      assertThat(actual).isEqualTo(expected);
+    }
+  }
+
+  @Test
+  void getNanOwnerTypeName_identifier_not_nan() {
+    // An identifier that is not named "NaN" should return null (line 417 false branch)
+    assertNanOwnerTypeName("someVariable", null);
+  }
+
+  @Test
+  void getNanOwnerTypeName_custom_class_nan_field() {
+    // A NaN field on a custom class (not Double/Float) should return null (line 437)
+    CompilationUnitTree unit = JParserTestUtils.parse("""
+      class MyClass {
+        static double NaN = 0.0;
+      }
+      class A { Object f = MyClass.NaN; }
+      """);
+    ExpressionTree expr = ((VariableTree) ((ClassTree) unit.types().get(1)).members().get(0)).initializer();
+    assertThat(ExpressionUtils.getNanOwnerTypeName(expr)).isNull();
+  }
+
+  @Test
+  void getNanOwnerTypeName_member_select_not_nan() {
+    // A member select where the identifier is not "NaN" should return null (line 412 false branch)
+    assertNanOwnerTypeName("Double.MAX_VALUE", null);
+  }
+
+  @Test
+  void getNanOwnerTypeName_null_owner_type_via_mock() {
+    // Test resolveNanOwnerType returns null when owner.type() is null (line 431)
+    Symbol nanSymbol = mock(Symbol.class);
+    when(nanSymbol.isUnknown()).thenReturn(false);
+    Symbol ownerSymbol = mock(Symbol.class);
+    when(ownerSymbol.type()).thenReturn(null);
+    when(nanSymbol.owner()).thenReturn(ownerSymbol);
+
+    IdentifierTree identifier = mock(IdentifierTree.class);
+    when(identifier.name()).thenReturn("NaN");
+    when(identifier.symbol()).thenReturn(nanSymbol);
+    when(identifier.is(Tree.Kind.MEMBER_SELECT)).thenReturn(false);
+    when(identifier.is(Tree.Kind.IDENTIFIER)).thenReturn(true);
+
+    assertThat(ExpressionUtils.getNanOwnerTypeName(identifier)).isNull();
+  }
+
+  @Test
+  void getNanOwnerTypeName_null_owner_via_mock() {
+    // Test resolveNanOwnerType returns null when owner is null (line 431)
+    Symbol nanSymbol = mock(Symbol.class);
+    when(nanSymbol.isUnknown()).thenReturn(false);
+    when(nanSymbol.owner()).thenReturn(null);
+
+    IdentifierTree identifier = mock(IdentifierTree.class);
+    when(identifier.name()).thenReturn("NaN");
+    when(identifier.symbol()).thenReturn(nanSymbol);
+    when(identifier.is(Tree.Kind.MEMBER_SELECT)).thenReturn(false);
+    when(identifier.is(Tree.Kind.IDENTIFIER)).thenReturn(true);
+
+    assertThat(ExpressionUtils.getNanOwnerTypeName(identifier)).isNull();
+  }
+
   @Test
   void testAnnotationAttributeName(){
     var unit = JParserTestUtils.parse("""

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9147.html

@@ -56,6 +56,7 @@ <h3>Documentation</h3>
 </ul>
 <h3>Related rules</h3>
 <ul>
+  <li>{rule:java:S1244} - Floating point numbers should not be tested for equality</li>
   <li>{rule:java:S6725} - Python equivalent: NaN should not be tested for equality</li>
   <li>{rule:java:S6679} - JavaScript equivalent: NaN should not be tested for equality</li>
   <li>{rule:java:S2688} - JavaScript: NaN should not be used in comparisons</li>