Reland "[analyzer] add UNNECESSARY_NAN_COMPARISON hint"

This is a reland of commit 254da67

Original change's description:
> [analyzer] add `UNNECESSARY_NAN_COMPARISON` hint
>
> Fixes #44649
>
> Change-Id: I3784f7222a217e5b6ca485c02e9a889964ffcc8c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269881
> Commit-Queue: Brian Wilkerson <[email protected]>
> Reviewed-by: Brian Wilkerson <[email protected]>

Change-Id: I868d46df69f178094742d9193de5d347ea2cc60b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278440
Reviewed-by: Oleh Prypin <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: asashour

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -1637,6 +1637,14 @@ HintCode.UNNECESSARY_IGNORE:
     Remove the ignore comment (or one code in the comment).
 HintCode.UNNECESSARY_IMPORT:
   status: hasFix
+HintCode.UNNECESSARY_NAN_COMPARISON_FALSE:
+  status: needsFix
+  notes: |-
+    Use 'isNaN', or remove the comparison.
+HintCode.UNNECESSARY_NAN_COMPARISON_TRUE:
+  status: needsFix
+  notes: |-
+    Use 'isNaN', or remove the comparison.
 HintCode.UNNECESSARY_NO_SUCH_METHOD:
   status: needsFix
   notes: |-

pkg/analyzer/lib/src/dart/error/hint_codes.g.dart

@@ -1233,6 +1233,22 @@ class HintCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  ///  No parameters.
+  static const HintCode UNNECESSARY_NAN_COMPARISON_FALSE = HintCode(
+    'UNNECESSARY_NAN_COMPARISON',
+    "A double can't equal NaN, so the condition is always 'false'.",
+    correctionMessage: "Try using 'isNan', or removing the condition.",
+    uniqueName: 'UNNECESSARY_NAN_COMPARISON_FALSE',
+  );
+
+  ///  No parameters.
+  static const HintCode UNNECESSARY_NAN_COMPARISON_TRUE = HintCode(
+    'UNNECESSARY_NAN_COMPARISON',
+    "A double can't equal NaN, so the condition is always 'true'.",
+    correctionMessage: "Try using 'isNan', or removing the condition.",
+    uniqueName: 'UNNECESSARY_NAN_COMPARISON_TRUE',
+  );
+
   ///  No parameters.
   static const HintCode UNNECESSARY_NO_SUCH_METHOD = HintCode(
     'UNNECESSARY_NO_SUCH_METHOD',
@@ -1244,7 +1260,7 @@ class HintCode extends AnalyzerErrorCode {
   ///  No parameters.
   static const HintCode UNNECESSARY_NULL_COMPARISON_FALSE = HintCode(
     'UNNECESSARY_NULL_COMPARISON',
-    "The operand can't be null, so the condition is always false.",
+    "The operand can't be null, so the condition is always 'false'.",
     correctionMessage:
         "Try removing the condition, an enclosing condition, or the whole "
         "conditional statement.",
@@ -1255,7 +1271,7 @@ class HintCode extends AnalyzerErrorCode {
   ///  No parameters.
   static const HintCode UNNECESSARY_NULL_COMPARISON_TRUE = HintCode(
     'UNNECESSARY_NULL_COMPARISON',
-    "The operand can't be null, so the condition is always true.",
+    "The operand can't be null, so the condition is always 'true'.",
     correctionMessage: "Remove the condition.",
     hasPublishedDocs: true,
     uniqueName: 'UNNECESSARY_NULL_COMPARISON_TRUE',

pkg/analyzer/lib/src/error/best_practices_verifier.dart

@@ -372,6 +372,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
   void visitBinaryExpression(BinaryExpression node) {
     _checkForDivisionOptimizationHint(node);
     _deprecatedVerifier.binaryExpression(node);
+    _checkForInvariantNanComparison(node);
     _checkForInvariantNullComparison(node);
     _invalidAccessVerifier.verifyBinary(node);
     super.visitBinaryExpression(node);
@@ -1207,6 +1208,40 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  void _checkForInvariantNanComparison(BinaryExpression node) {
+    void reportStartEnd(
+      HintCode errorCode,
+      SyntacticEntity startEntity,
+      SyntacticEntity endEntity,
+    ) {
+      var offset = startEntity.offset;
+      _errorReporter.reportErrorForOffset(
+        errorCode,
+        offset,
+        endEntity.end - offset,
+      );
+    }
+
+    bool isDoubleNan(Expression expression) =>
+        expression is PrefixedIdentifier &&
+        expression.prefix.name == 'double' &&
+        expression.identifier.name == 'nan';
+
+    void checkLeftRight(HintCode errorCode) {
+      if (isDoubleNan(node.leftOperand)) {
+        reportStartEnd(errorCode, node.leftOperand, node.operator);
+      } else if (isDoubleNan(node.rightOperand)) {
+        reportStartEnd(errorCode, node.operator, node.rightOperand);
+      }
+    }
+
+    if (node.operator.type == TokenType.BANG_EQ) {
+      checkLeftRight(HintCode.UNNECESSARY_NAN_COMPARISON_TRUE);
+    } else if (node.operator.type == TokenType.EQ_EQ) {
+      checkLeftRight(HintCode.UNNECESSARY_NAN_COMPARISON_FALSE);
+    }
+  }
+
   void _checkForInvariantNullComparison(BinaryExpression node) {
     if (!_isNonNullableByDefault) return;
 

pkg/analyzer/lib/src/error/error_code_values.g.dart

@@ -670,6 +670,8 @@ const List<ErrorCode> errorCodeValues = [
   HintCode.UNNECESSARY_FINAL,
   HintCode.UNNECESSARY_IGNORE,
   HintCode.UNNECESSARY_IMPORT,
+  HintCode.UNNECESSARY_NAN_COMPARISON_FALSE,
+  HintCode.UNNECESSARY_NAN_COMPARISON_TRUE,
   HintCode.UNNECESSARY_NO_SUCH_METHOD,
   HintCode.UNNECESSARY_NULL_COMPARISON_FALSE,
   HintCode.UNNECESSARY_NULL_COMPARISON_TRUE,

pkg/analyzer/messages.yaml

@@ -20962,6 +20962,18 @@ HintCode:
       If some of the names imported by this import are intended to be used but
       aren't yet, and if those names aren't imported by other imports, then add
       the missing references to those names.
+  UNNECESSARY_NAN_COMPARISON_FALSE:
+    sharedName: UNNECESSARY_NAN_COMPARISON
+    problemMessage: "A double can't equal NaN, so the condition is always 'false'."
+    correctionMessage: Try using 'isNan', or removing the condition.
+    hasPublishedDocs: false
+    comment: No parameters.
+  UNNECESSARY_NAN_COMPARISON_TRUE:
+    sharedName: UNNECESSARY_NAN_COMPARISON
+    problemMessage: "A double can't equal NaN, so the condition is always 'true'."
+    correctionMessage: Try using 'isNan', or removing the condition.
+    hasPublishedDocs: false
+    comment: No parameters.
   UNNECESSARY_NO_SUCH_METHOD:
     problemMessage: "Unnecessary 'noSuchMethod' declaration."
     correctionMessage: "Try removing the declaration of 'noSuchMethod'."
@@ -21013,7 +21025,7 @@ HintCode:
       ```
   UNNECESSARY_NULL_COMPARISON_FALSE:
     sharedName: UNNECESSARY_NULL_COMPARISON
-    problemMessage: "The operand can't be null, so the condition is always false."
+    problemMessage: "The operand can't be null, so the condition is always 'false'."
     correctionMessage: Try removing the condition, an enclosing condition, or the whole conditional statement.
     hasPublishedDocs: true
     comment: No parameters.
@@ -21071,7 +21083,7 @@ HintCode:
       ```
   UNNECESSARY_NULL_COMPARISON_TRUE:
     sharedName: UNNECESSARY_NULL_COMPARISON
-    problemMessage: "The operand can't be null, so the condition is always true."
+    problemMessage: "The operand can't be null, so the condition is always 'true'."
     correctionMessage: Remove the condition.
     hasPublishedDocs: true
     comment: No parameters.

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -794,6 +794,7 @@ import 'unnecessary_cast_test.dart' as unnecessary_cast;
 import 'unnecessary_final_test.dart' as unnecessary_final;
 import 'unnecessary_ignore_test.dart' as unnecessary_ignore;
 import 'unnecessary_import_test.dart' as unnecessary_import;
+import 'unnecessary_nan_comparison_test.dart' as unnecessary_nan_comparison;
 import 'unnecessary_no_such_method_test.dart' as unnecessary_no_such_method;
 import 'unnecessary_non_null_assertion_test.dart'
     as unnecessary_non_null_assertion;
@@ -1362,6 +1363,7 @@ main() {
     unnecessary_cast.main();
     unnecessary_final.main();
     unnecessary_ignore.main();
+    unnecessary_nan_comparison.main();
     unnecessary_no_such_method.main();
     unnecessary_non_null_assertion.main();
     unnecessary_null_comparison.main();

pkg/analyzer/test/src/diagnostics/unnecessary_nan_comparison_test.dart

@@ -0,0 +1,57 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(UnnecessaryNanComparisonTest);
+  });
+}
+
+@reflectiveTest
+class UnnecessaryNanComparisonTest extends PubPackageResolutionTest {
+  test_equal() async {
+    await assertErrorsInCode('''
+void f(double d) {
+  d == double.nan;
+}
+''', [
+      error(HintCode.UNNECESSARY_NAN_COMPARISON_FALSE, 23, 13),
+    ]);
+  }
+
+  test_equal_nanFirst() async {
+    await assertErrorsInCode('''
+void f(double d) {
+  double.nan == d;
+}
+''', [
+      error(HintCode.UNNECESSARY_NAN_COMPARISON_FALSE, 21, 13),
+    ]);
+  }
+
+  test_notEqual() async {
+    await assertErrorsInCode('''
+void f(double d) {
+  d != double.nan;
+}
+''', [
+      error(HintCode.UNNECESSARY_NAN_COMPARISON_TRUE, 23, 13),
+    ]);
+  }
+
+  test_notEqual_nanFirst() async {
+    await assertErrorsInCode('''
+void f(double d) {
+  double.nan != d;
+}
+''', [
+      error(HintCode.UNNECESSARY_NAN_COMPARISON_TRUE, 21, 13),
+    ]);
+  }
+}

pkg/analyzer/tool/diagnostics/diagnostics.md

@@ -19429,9 +19429,9 @@ class B extends A {}
 
 ### unnecessary_null_comparison
 
-_The operand can't be null, so the condition is always false._
+_The operand can't be null, so the condition is always 'false'._
 
-_The operand can't be null, so the condition is always true._
+_The operand can't be null, so the condition is always 'true'._
 
 #### Description