Auto merge of #4910 - krishna-veerareddy:issue-1205-cmp-nan-against-consts, r=phansch

Detect comparisons with NAN constants

Currently `cmp_nan` lint doesn't detect comparisons with NaN's if the operands are consts variables so to fix this we evaluate the const variables first before testing for NaN.

changelog: Detect comparisons with NaN constants in `cmp_nan` lint

Fixes #1205

clippy_lints/src/misc.rs

@@ -343,12 +343,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
             ExprKind::Binary(ref cmp, ref left, ref right) => {
                 let op = cmp.node;
                 if op.is_comparison() {
-                    if let ExprKind::Path(QPath::Resolved(_, ref path)) = left.kind {
-                        check_nan(cx, path, expr);
-                    }
-                    if let ExprKind::Path(QPath::Resolved(_, ref path)) = right.kind {
-                        check_nan(cx, path, expr);
-                    }
+                    check_nan(cx, left, expr);
+                    check_nan(cx, right, expr);
                     check_to_owned(cx, left, right);
                     check_to_owned(cx, right, left);
                 }
@@ -444,14 +440,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
     }
 }
 
-fn check_nan(cx: &LateContext<'_, '_>, path: &Path, expr: &Expr) {
-    if !in_constant(cx, expr.hir_id) {
-        if let Some(seg) = path.segments.last() {
-            if seg.ident.name == sym!(NAN) {
+fn check_nan(cx: &LateContext<'_, '_>, expr: &Expr, cmp_expr: &Expr) {
+    if_chain! {
+        if !in_constant(cx, cmp_expr.hir_id);
+        if let Some((value, _)) = constant(cx, cx.tables, expr);
+        then {
+            let needs_lint = match value {
+                Constant::F32(num) => num.is_nan(),
+                Constant::F64(num) => num.is_nan(),
+                _ => false,
+            };
+
+            if needs_lint {
                 span_lint(
                     cx,
                     CMP_NAN,
-                    expr.span,
+                    cmp_expr.span,
                     "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead",
                 );
             }

tests/ui/cmp_nan.rs

@@ -1,3 +1,6 @@
+const NAN_F32: f32 = std::f32::NAN;
+const NAN_F64: f64 = std::f64::NAN;
+
 #[warn(clippy::cmp_nan)]
 #[allow(clippy::float_cmp, clippy::no_effect, clippy::unnecessary_operation)]
 fn main() {
@@ -8,6 +11,12 @@ fn main() {
     x > std::f32::NAN;
     x <= std::f32::NAN;
     x >= std::f32::NAN;
+    x == NAN_F32;
+    x != NAN_F32;
+    x < NAN_F32;
+    x > NAN_F32;
+    x <= NAN_F32;
+    x >= NAN_F32;
 
     let y = 0f64;
     y == std::f64::NAN;
@@ -16,4 +25,10 @@ fn main() {
     y > std::f64::NAN;
     y <= std::f64::NAN;
     y >= std::f64::NAN;
+    y == NAN_F64;
+    y != NAN_F64;
+    y < NAN_F64;
+    y > NAN_F64;
+    y <= NAN_F64;
+    y >= NAN_F64;
 }

tests/ui/cmp_nan.stderr

@@ -1,5 +1,5 @@
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:5:5
+  --> $DIR/cmp_nan.rs:8:5
    |
 LL |     x == std::f32::NAN;
    |     ^^^^^^^^^^^^^^^^^^
@@ -7,70 +7,142 @@ LL |     x == std::f32::NAN;
    = note: `-D clippy::cmp-nan` implied by `-D warnings`
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:6:5
+  --> $DIR/cmp_nan.rs:9:5
    |
 LL |     x != std::f32::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:7:5
+  --> $DIR/cmp_nan.rs:10:5
    |
 LL |     x < std::f32::NAN;
    |     ^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:8:5
+  --> $DIR/cmp_nan.rs:11:5
    |
 LL |     x > std::f32::NAN;
    |     ^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:9:5
+  --> $DIR/cmp_nan.rs:12:5
    |
 LL |     x <= std::f32::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:10:5
+  --> $DIR/cmp_nan.rs:13:5
    |
 LL |     x >= std::f32::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:13:5
+  --> $DIR/cmp_nan.rs:14:5
+   |
+LL |     x == NAN_F32;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:15:5
+   |
+LL |     x != NAN_F32;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:16:5
+   |
+LL |     x < NAN_F32;
+   |     ^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:17:5
+   |
+LL |     x > NAN_F32;
+   |     ^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:18:5
+   |
+LL |     x <= NAN_F32;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:19:5
+   |
+LL |     x >= NAN_F32;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:22:5
    |
 LL |     y == std::f64::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:14:5
+  --> $DIR/cmp_nan.rs:23:5
    |
 LL |     y != std::f64::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:15:5
+  --> $DIR/cmp_nan.rs:24:5
    |
 LL |     y < std::f64::NAN;
    |     ^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:16:5
+  --> $DIR/cmp_nan.rs:25:5
    |
 LL |     y > std::f64::NAN;
    |     ^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:17:5
+  --> $DIR/cmp_nan.rs:26:5
    |
 LL |     y <= std::f64::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
 error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
-  --> $DIR/cmp_nan.rs:18:5
+  --> $DIR/cmp_nan.rs:27:5
    |
 LL |     y >= std::f64::NAN;
    |     ^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 12 previous errors
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:28:5
+   |
+LL |     y == NAN_F64;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:29:5
+   |
+LL |     y != NAN_F64;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:30:5
+   |
+LL |     y < NAN_F64;
+   |     ^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:31:5
+   |
+LL |     y > NAN_F64;
+   |     ^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:32:5
+   |
+LL |     y <= NAN_F64;
+   |     ^^^^^^^^^^^^
+
+error: doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead
+  --> $DIR/cmp_nan.rs:33:5
+   |
+LL |     y >= NAN_F64;
+   |     ^^^^^^^^^^^^
+
+error: aborting due to 24 previous errors