Auto merge of #5952 - 1c3t3a:1c3t3a-dev-5819, r=Manishearth

Added a lint which corrects expressions like (a - b) < f32::EPSILON, according to #5819

Fixes #5819
changelog: none

CHANGELOG.md

@@ -1498,6 +1498,7 @@ Released 2018-09-13
 [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic
 [`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp
 [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const
+[`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_equality_without_abs
 [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons
 [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
 [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast

clippy_lints/src/float_equality_without_abs.rs

@@ -0,0 +1,112 @@
+use crate::utils::{match_qpath, paths, snippet, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Spanned;
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for statements of the form `(a - b) < f32::EPSILON` or
+     /// `(a - b) < f64::EPSILON`. Notes the missing `.abs()`.
+     ///
+     /// **Why is this bad?** The code without `.abs()` is more likely to have a bug.
+     ///
+     /// **Known problems:** If the user can ensure that b is larger than a, the `.abs()` is
+     /// technically unneccessary. However, it will make the code more robust and doesn't have any
+     /// large performance implications. If the abs call was deliberately left out for performance
+     /// reasons, it is probably better to state this explicitly in the code, which then can be done
+     /// with an allow.
+     ///
+     /// **Example:**
+     ///
+     /// ```rust
+     /// pub fn is_roughly_equal(a: f32, b: f32) -> bool {
+     ///     (a - b) < f32::EPSILON
+     /// }
+     /// ```
+     /// Use instead:
+     /// ```rust
+     /// pub fn is_roughly_equal(a: f32, b: f32) -> bool {
+     ///     (a - b).abs() < f32::EPSILON
+     /// }
+     /// ```
+    pub FLOAT_EQUALITY_WITHOUT_ABS,
+    correctness,
+    "float equality check without `.abs()`"
+}
+
+declare_lint_pass!(FloatEqualityWithoutAbs => [FLOAT_EQUALITY_WITHOUT_ABS]);
+
+impl<'tcx> LateLintPass<'tcx> for FloatEqualityWithoutAbs {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        let lhs;
+        let rhs;
+
+        // check if expr is a binary expression with a lt or gt operator
+        if let ExprKind::Binary(op, ref left, ref right) = expr.kind {
+            match op.node {
+                BinOpKind::Lt => {
+                    lhs = left;
+                    rhs = right;
+                },
+                BinOpKind::Gt => {
+                    lhs = right;
+                    rhs = left;
+                },
+                _ => return,
+            };
+        } else {
+            return;
+        }
+
+        if_chain! {
+
+            // left hand side is a substraction
+            if let ExprKind::Binary(
+                Spanned {
+                    node: BinOpKind::Sub,
+                    ..
+                },
+                val_l,
+                val_r,
+            ) = lhs.kind;
+
+            // right hand side matches either f32::EPSILON or f64::EPSILON
+            if let ExprKind::Path(ref epsilon_path) = rhs.kind;
+            if match_qpath(epsilon_path, &paths::F32_EPSILON) || match_qpath(epsilon_path, &paths::F64_EPSILON);
+
+            // values of the substractions on the left hand side are of the type float
+            let t_val_l = cx.typeck_results().expr_ty(val_l);
+            let t_val_r = cx.typeck_results().expr_ty(val_r);
+            if let ty::Float(_) = t_val_l.kind;
+            if let ty::Float(_) = t_val_r.kind;
+
+            then {
+                // get the snippet string
+                let lhs_string = snippet(
+                    cx,
+                    lhs.span,
+                    "(...)",
+                );
+                // format the suggestion
+                let suggestion = if lhs_string.starts_with('(') {
+                    format!("{}.abs()", lhs_string)
+                } else {
+                    format!("({}).abs()", lhs_string)
+                };
+                // spans the lint
+                span_lint_and_sugg(
+                    cx,
+                    FLOAT_EQUALITY_WITHOUT_ABS,
+                    expr.span,
+                    "float equality check without `.abs()`",
+                    "add `.abs()`",
+                    suggestion,
+                    Applicability::MaybeIncorrect,
+                );
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -193,6 +193,7 @@ mod excessive_bools;
 mod exit;
 mod explicit_write;
 mod fallible_impl_from;
+mod float_equality_without_abs;
 mod float_literal;
 mod floating_point_arithmetic;
 mod format;
@@ -549,6 +550,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &exit::EXIT,
         &explicit_write::EXPLICIT_WRITE,
         &fallible_impl_from::FALLIBLE_IMPL_FROM,
+        &float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS,
         &float_literal::EXCESSIVE_PRECISION,
         &float_literal::LOSSY_FLOAT_LITERAL,
         &floating_point_arithmetic::IMPRECISE_FLOPS,
@@ -1093,6 +1095,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);
     store.register_late_pass(|| box repeat_once::RepeatOnce);
     store.register_late_pass(|| box self_assignment::SelfAssignment);
+    store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1268,6 +1271,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&eval_order_dependence::DIVERGING_SUB_EXPRESSION),
         LintId::of(&eval_order_dependence::EVAL_ORDER_DEPENDENCE),
         LintId::of(&explicit_write::EXPLICIT_WRITE),
+        LintId::of(&float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
         LintId::of(&float_literal::EXCESSIVE_PRECISION),
         LintId::of(&format::USELESS_FORMAT),
         LintId::of(&formatting::POSSIBLE_MISSING_COMMA),
@@ -1686,6 +1690,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
         LintId::of(&eq_op::EQ_OP),
         LintId::of(&erasing_op::ERASING_OP),
+        LintId::of(&float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
         LintId::of(&formatting::POSSIBLE_MISSING_COMMA),
         LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
         LintId::of(&if_let_mutex::IF_LET_MUTEX),

clippy_lints/src/utils/paths.rs

@@ -35,6 +35,8 @@ pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"];
 pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
 pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
 pub const EXIT: [&str; 3] = ["std", "process", "exit"];
+pub const F32_EPSILON: [&str; 2] = ["f32", "EPSILON"];
+pub const F64_EPSILON: [&str; 2] = ["f64", "EPSILON"];
 pub const FILE: [&str; 3] = ["std", "fs", "File"];
 pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"];
 pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];

src/lintlist/mod.rs

@@ -661,6 +661,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         module: "misc",
     },
+    Lint {
+        name: "float_equality_without_abs",
+        group: "correctness",
+        desc: "float equality check without `.abs()`",
+        deprecation: None,
+        module: "float_equality_without_abs",
+    },
     Lint {
         name: "fn_address_comparisons",
         group: "correctness",

tests/ui/float_equality_without_abs.rs

@@ -0,0 +1,31 @@
+#![warn(clippy::float_equality_without_abs)]
+
+pub fn is_roughly_equal(a: f32, b: f32) -> bool {
+    (a - b) < f32::EPSILON
+}
+
+pub fn main() {
+    // all errors
+    is_roughly_equal(1.0, 2.0);
+    let a = 0.05;
+    let b = 0.0500001;
+
+    let _ = (a - b) < f32::EPSILON;
+    let _ = a - b < f32::EPSILON;
+    let _ = a - b.abs() < f32::EPSILON;
+    let _ = (a as f64 - b as f64) < f64::EPSILON;
+    let _ = 1.0 - 2.0 < f32::EPSILON;
+
+    let _ = f32::EPSILON > (a - b);
+    let _ = f32::EPSILON > a - b;
+    let _ = f32::EPSILON > a - b.abs();
+    let _ = f64::EPSILON > (a as f64 - b as f64);
+    let _ = f32::EPSILON > 1.0 - 2.0;
+
+    // those are correct
+    let _ = (a - b).abs() < f32::EPSILON;
+    let _ = (a as f64 - b as f64).abs() < f64::EPSILON;
+
+    let _ = f32::EPSILON > (a - b).abs();
+    let _ = f64::EPSILON > (a as f64 - b as f64).abs();
+}

tests/ui/float_equality_without_abs.stderr

@@ -0,0 +1,70 @@
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:4:5
+   |
+LL |     (a - b) < f32::EPSILON
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
+   |
+   = note: `-D clippy::float-equality-without-abs` implied by `-D warnings`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:13:13
+   |
+LL |     let _ = (a - b) < f32::EPSILON;
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:14:13
+   |
+LL |     let _ = a - b < f32::EPSILON;
+   |             ^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:15:13
+   |
+LL |     let _ = a - b.abs() < f32::EPSILON;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b.abs()).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:16:13
+   |
+LL |     let _ = (a as f64 - b as f64) < f64::EPSILON;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a as f64 - b as f64).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:17:13
+   |
+LL |     let _ = 1.0 - 2.0 < f32::EPSILON;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(1.0 - 2.0).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:19:13
+   |
+LL |     let _ = f32::EPSILON > (a - b);
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:20:13
+   |
+LL |     let _ = f32::EPSILON > a - b;
+   |             ^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:21:13
+   |
+LL |     let _ = f32::EPSILON > a - b.abs();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a - b.abs()).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:22:13
+   |
+LL |     let _ = f64::EPSILON > (a as f64 - b as f64);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(a as f64 - b as f64).abs()`
+
+error: float equality check without `.abs()`
+  --> $DIR/float_equality_without_abs.rs:23:13
+   |
+LL |     let _ = f32::EPSILON > 1.0 - 2.0;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: add `.abs()`: `(1.0 - 2.0).abs()`
+
+error: aborting due to 11 previous errors
+