Skip to content

SONARJAVA-6743 Implement new rule S9147: "NaN" should not be tested for equality using "==" or "!=" - #5908

Draft
romainbrenguier wants to merge 2 commits into
masterfrom
new-rule/SONARJAVA-6743-S9147
Draft

SONARJAVA-6743 Implement new rule S9147: "NaN" should not be tested for equality using "==" or "!="#5908
romainbrenguier wants to merge 2 commits into
masterfrom
new-rule/SONARJAVA-6743-S9147

Conversation

@romainbrenguier

Copy link
Copy Markdown
Contributor

Detect equality and inequality comparisons (== and !=) with Double.NaN and Float.NaN constants, which always produce incorrect results due to IEEE 754 semantics. Suggests using Double.isNaN() or Float.isNaN() instead.

Detect equality and inequality comparisons (== and !=) with Double.NaN
and Float.NaN constants, which always produce incorrect results due to
IEEE 754 semantics. Suggests using Double.isNaN() or Float.isNaN()
instead.
@hashicorp-vault-sonar-prod

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

Copy link
Copy Markdown
Contributor

SONARJAVA-6743

if (expr.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expr;
if ("NaN".equals(memberSelect.identifier().name())) {
String ownerType = memberSelect.identifier().symbol().owner().type().fullyQualifiedName();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Possible NPE on unknown symbol owner in getNanTypeName

memberSelect.identifier().symbol().owner().type().fullyQualifiedName() chains through owner(), which is declared @Nullable (returns null for package/unknown symbols per the Symbol interface contract). When the NaN identifier's symbol cannot be resolved (e.g. incomplete classpath/semantics), this can throw a NullPointerException, causing the check to fail on that file. Guard against unresolved symbols before dereferencing. Note similar checks (e.g. SillyEqualsCheck) use isUnknown() guards.

Bail out when the symbol is unknown or its owner is null before dereferencing.:

if ("NaN".equals(memberSelect.identifier().name())) {
  Symbol symbol = memberSelect.identifier().symbol();
  Symbol owner = symbol.owner();
  if (symbol.isUnknown() || owner == null) {
    return null;
  }
  String ownerType = owner.type().fullyQualifiedName();
  if ("java.lang.Double".equals(ownerType)) {
    return "Double";
  }
  if ("java.lang.Float".equals(ownerType)) {
    return "Float";
  }
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment on lines +53 to +64
if (expr.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expr;
if ("NaN".equals(memberSelect.identifier().name())) {
String ownerType = memberSelect.identifier().symbol().owner().type().fullyQualifiedName();
if ("java.lang.Double".equals(ownerType)) {
return "Double";
}
if ("java.lang.Float".equals(ownerType)) {
return "Float";
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: Static-imported NaN not detected (MEMBER_SELECT-only match)

getNanTypeName only matches when the operand is a MEMBER_SELECT (e.g. Double.NaN). Code using import static java.lang.Double.NaN; and referencing bare NaN yields an IDENTIFIER node and is silently missed (false negative). Consider also handling identifier references whose symbol owner is java.lang.Double/Float, or document this limitation.

Was this helpful? React with 👍 / 👎

@github-actions

Copy link
Copy Markdown
Contributor

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

Please review and merge it into your branch.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@gitar-bot

gitar-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown
CI failed: Integration and ruling tests failed due to test output mismatches for the new rule S9147 and a Windows file access error during scanner cache movement.

Overview

Analysis of 3 CI logs revealed 2 distinct test and execution failures related to the implementation of new rule S9147 ("NaN" should not be tested for equality using "==" or "!="). Specifically, integration/autoscan diff expectations require updating to match the new rule outputs, and a Windows runner encountered a file locking/access denial when caching SonarScanner engine files.

Failures

Ruling and Autoscan Test Output Mismatches (confidence: high)

  • Type: test
  • Affected jobs: 93421513093, 93431933615
  • Related to change: yes
  • Root cause: The PR introduces rule S9147, but expected ruling and autoscan test outputs (its/autoscan and test expectations) do not match the actual analysis output generated during the test run.
  • Suggested fix: Review the generated diff report artifacts (diff_autoscan), update the expected test resources and ruling JSON files to match the new rule's output, and commit the updated expectations.

Windows Scanner Cache Access Denied Error (confidence: high)

  • Type: test
  • Affected jobs: 93421513332
  • Related to change: yes
  • Root cause: An AccessDeniedException occurred on Windows when SonarScanner attempted to move a temporary cache file into the .sonar/cache directory (sonar-scanner-engine-enterprise), causing the ruling integration test to fail.
  • Suggested fix: Ensure there are no conflicting concurrent processes holding locks on the SonarScanner engine jar cache files in the Windows runner home directory.

Summary

  • Change-related failures: 2 failures (ruling/autoscan output mismatches and Windows scanner cache permission/lock error directly tied to testing the new rule)
  • Infrastructure/flaky failures: 0 infrastructure or flaky failures
  • Recommended action: Update the expected ruling/autoscan results for rule S9147 and verify Windows cache runner permissions.
Code Review ⚠️ Changes requested 0 resolved / 2 findings

Implements rule S9147 to detect NaN equality and inequality comparisons using == and !=. This introduces a possible NPE on unknown symbol owner in getNanTypeName and misses static-imported NaN constants due to MEMBER_SELECT-only matching.

⚠️ Bug: Possible NPE on unknown symbol owner in getNanTypeName

📄 java-checks/src/main/java/org/sonar/java/checks/NanEqualityCheck.java:56

memberSelect.identifier().symbol().owner().type().fullyQualifiedName() chains through owner(), which is declared @Nullable (returns null for package/unknown symbols per the Symbol interface contract). When the NaN identifier's symbol cannot be resolved (e.g. incomplete classpath/semantics), this can throw a NullPointerException, causing the check to fail on that file. Guard against unresolved symbols before dereferencing. Note similar checks (e.g. SillyEqualsCheck) use isUnknown() guards.

Bail out when the symbol is unknown or its owner is null before dereferencing.
if ("NaN".equals(memberSelect.identifier().name())) {
  Symbol symbol = memberSelect.identifier().symbol();
  Symbol owner = symbol.owner();
  if (symbol.isUnknown() || owner == null) {
    return null;
  }
  String ownerType = owner.type().fullyQualifiedName();
  if ("java.lang.Double".equals(ownerType)) {
    return "Double";
  }
  if ("java.lang.Float".equals(ownerType)) {
    return "Float";
  }
}
💡 Edge Case: Static-imported NaN not detected (MEMBER_SELECT-only match)

📄 java-checks/src/main/java/org/sonar/java/checks/NanEqualityCheck.java:53-64

getNanTypeName only matches when the operand is a MEMBER_SELECT (e.g. Double.NaN). Code using import static java.lang.Double.NaN; and referencing bare NaN yields an IDENTIFIER node and is silently missed (false negative). Consider also handling identifier references whose symbol owner is java.lang.Double/Float, or document this limitation.

🤖 Prompt for agents
Code Review: Implements rule S9147 to detect NaN equality and inequality comparisons using `==` and `!=`. This introduces a possible NPE on unknown symbol owner in getNanTypeName and misses static-imported NaN constants due to MEMBER_SELECT-only matching.

1. ⚠️ Bug: Possible NPE on unknown symbol owner in getNanTypeName
   Files: java-checks/src/main/java/org/sonar/java/checks/NanEqualityCheck.java:56

   `memberSelect.identifier().symbol().owner().type().fullyQualifiedName()` chains through `owner()`, which is declared `@Nullable` (returns null for package/unknown symbols per the Symbol interface contract). When the `NaN` identifier's symbol cannot be resolved (e.g. incomplete classpath/semantics), this can throw a NullPointerException, causing the check to fail on that file. Guard against unresolved symbols before dereferencing. Note similar checks (e.g. SillyEqualsCheck) use `isUnknown()` guards.

   Fix (Bail out when the symbol is unknown or its owner is null before dereferencing.):
   if ("NaN".equals(memberSelect.identifier().name())) {
     Symbol symbol = memberSelect.identifier().symbol();
     Symbol owner = symbol.owner();
     if (symbol.isUnknown() || owner == null) {
       return null;
     }
     String ownerType = owner.type().fullyQualifiedName();
     if ("java.lang.Double".equals(ownerType)) {
       return "Double";
     }
     if ("java.lang.Float".equals(ownerType)) {
       return "Float";
     }
   }

2. 💡 Edge Case: Static-imported NaN not detected (MEMBER_SELECT-only match)
   Files: java-checks/src/main/java/org/sonar/java/checks/NanEqualityCheck.java:53-64

   `getNanTypeName` only matches when the operand is a `MEMBER_SELECT` (e.g. `Double.NaN`). Code using `import static java.lang.Double.NaN;` and referencing bare `NaN` yields an IDENTIFIER node and is silently missed (false negative). Consider also handling identifier references whose symbol owner is java.lang.Double/Float, or document this limitation.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

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

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

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

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