Merge pull request #2203 from clippered/float_cmp_const

Fix #1142 float constant comparison lint

clippy_lints/src/lib.rs

@@ -367,6 +367,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         arithmetic::INTEGER_ARITHMETIC,
         array_indexing::INDEXING_SLICING,
         assign_ops::ASSIGN_OPS,
+        misc::FLOAT_CMP_CONST,
     ]);
 
     reg.register_lint_group("clippy_pedantic", vec![

clippy_lints/src/misc.rs

@@ -13,6 +13,7 @@ use utils::{get_item_name, get_parent_expr, implements_trait, in_constant, in_ma
             span_lint_and_then, walk_ptrs_ty};
 use utils::sugg::Sugg;
 use syntax::ast::{FloatTy, LitKind, CRATE_NODE_ID};
+use consts::constant;
 
 /// **What it does:** Checks for function arguments and let bindings denoted as
 /// `ref`.
@@ -200,6 +201,27 @@ declare_lint! {
     "using 0 as *{const, mut} T"
 }
 
+/// **What it does:** Checks for (in-)equality comparisons on floating-point
+/// value and constant, except in functions called `*eq*` (which probably
+/// implement equality for a type involving floats).
+///
+/// **Why is this bad?** Floating point calculations are usually imprecise, so
+/// asking if two values are *exactly* equal is asking for trouble. For a good
+/// guide on what to do, see [the floating point
+/// guide](http://www.floating-point-gui.de/errors/comparison).
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// const ONE == 1.00f64
+/// x == ONE  // where both are floats
+/// ```
+declare_restriction_lint! {
+    pub FLOAT_CMP_CONST,
+    "using `==` or `!=` on float constants instead of comparing difference with an epsilon"
+}
+
 #[derive(Copy, Clone)]
 pub struct Pass;
 
@@ -214,7 +236,8 @@ impl LintPass for Pass {
             REDUNDANT_PATTERN,
             USED_UNDERSCORE_BINDING,
             SHORT_CIRCUIT_STATEMENT,
-            ZERO_PTR
+            ZERO_PTR,
+            FLOAT_CMP_CONST
         )
     }
 }
@@ -334,7 +357,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                             return;
                         }
                     }
-                    span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| {
+                    let (lint, msg) = if is_named_constant(cx, left) || is_named_constant(cx, right) {
+                        (FLOAT_CMP_CONST, "strict comparison of f32 or f64 constant")
+                    } else {
+                        (FLOAT_CMP, "strict comparison of f32 or f64")
+                    };
+                    span_lint_and_then(cx, lint, expr.span, msg, |db| {
                         let lhs = Sugg::hir(cx, left, "..");
                         let rhs = Sugg::hir(cx, right, "..");
 
@@ -423,6 +451,14 @@ fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) {
     }
 }
 
+fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool {
+    if let Some((_, res)) = constant(cx, expr) {
+        res
+    } else {
+       false
+    }
+}
+
 fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool {
     let parent_item = cx.tcx.hir.get_parent(expr.id);
     let parent_def_id = cx.tcx.hir.local_def_id(parent_item);

tests/ui/float_cmp_const.rs

@@ -0,0 +1,45 @@
+
+
+
+#![warn(float_cmp_const)]
+#![allow(float_cmp)]
+#![allow(unused, no_effect, unnecessary_operation)]
+
+const ONE: f32 = 1.0;
+const TWO: f32 = 2.0;
+
+fn eq_one(x: f32) -> bool {
+    if x.is_nan() { false } else { x == ONE } // no error, inside "eq" fn
+}
+
+fn main() {
+    // has errors
+    1f32 == ONE;
+    TWO == ONE;
+    TWO != ONE;
+    ONE + ONE == TWO;
+    1 as f32 == ONE;
+
+    let v = 0.9;
+    v == ONE;
+    v != ONE;
+
+    // no errors, lower than or greater than comparisons
+    v < ONE;
+    v > ONE;
+    v <= ONE;
+    v >= ONE;
+
+    // no errors, zero and infinity values
+    ONE != 0f32;
+    TWO == 0f32;
+    ONE != ::std::f32::INFINITY;
+    ONE == ::std::f32::NEG_INFINITY;
+
+    // no errors, but will warn float_cmp if '#![allow(float_cmp)]' above is removed
+    let w = 1.1;
+    v == w;
+    v != w;
+    v == 1.0;
+    v != 1.0;
+}

tests/ui/float_cmp_const.stderr

@@ -0,0 +1,85 @@
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:17:5
+   |
+17 |     1f32 == ONE;
+   |     ^^^^^^^^^^^ help: consider comparing them within some error: `(1f32 - ONE).abs() < error`
+   |
+   = note: `-D float-cmp-const` implied by `-D warnings`
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:17:5
+   |
+17 |     1f32 == ONE;
+   |     ^^^^^^^^^^^
+
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:18:5
+   |
+18 |     TWO == ONE;
+   |     ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() < error`
+   |
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:18:5
+   |
+18 |     TWO == ONE;
+   |     ^^^^^^^^^^
+
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:19:5
+   |
+19 |     TWO != ONE;
+   |     ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() < error`
+   |
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:19:5
+   |
+19 |     TWO != ONE;
+   |     ^^^^^^^^^^
+
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:20:5
+   |
+20 |     ONE + ONE == TWO;
+   |     ^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(ONE + ONE - TWO).abs() < error`
+   |
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:20:5
+   |
+20 |     ONE + ONE == TWO;
+   |     ^^^^^^^^^^^^^^^^
+
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:21:5
+   |
+21 |     1 as f32 == ONE;
+   |     ^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(1 as f32 - ONE).abs() < error`
+   |
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:21:5
+   |
+21 |     1 as f32 == ONE;
+   |     ^^^^^^^^^^^^^^^
+
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:24:5
+   |
+24 |     v == ONE;
+   |     ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() < error`
+   |
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:24:5
+   |
+24 |     v == ONE;
+   |     ^^^^^^^^
+
+error: strict comparison of f32 or f64 constant
+  --> $DIR/float_cmp_const.rs:25:5
+   |
+25 |     v != ONE;
+   |     ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() < error`
+   |
+note: std::f32::EPSILON and std::f64::EPSILON are available.
+  --> $DIR/float_cmp_const.rs:25:5
+   |
+25 |     v != ONE;
+   |     ^^^^^^^^
+