add manual_abs_diff lint

Author: yotamofek

CHANGELOG.md

@@ -5784,6 +5784,7 @@ Released 2018-09-13
 [`macro_metavars_in_unsafe`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe
 [`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
 [`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
+[`manual_abs_diff`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff
 [`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits

book/src/lint_configuration.md

@@ -797,6 +797,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
 * [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
 * [`lines_filter_map_ok`](https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok)
+* [`manual_abs_diff`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff)
 * [`manual_bits`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits)
 * [`manual_c_str_literals`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals)
 * [`manual_clamp`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp)

clippy_config/src/conf.rs

@@ -645,6 +645,7 @@ define_Conf! {
         iter_kv_map,
         legacy_numeric_constants,
         lines_filter_map_ok,
+        manual_abs_diff,
         manual_bits,
         manual_c_str_literals,
         manual_clamp,

clippy_lints/src/declared_lints.rs

@@ -313,6 +313,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::macro_metavars_in_unsafe::MACRO_METAVARS_IN_UNSAFE_INFO,
     crate::macro_use::MACRO_USE_IMPORTS_INFO,
     crate::main_recursion::MAIN_RECURSION_INFO,
+    crate::manual_abs_diff::MANUAL_ABS_DIFF_INFO,
     crate::manual_assert::MANUAL_ASSERT_INFO,
     crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
     crate::manual_bits::MANUAL_BITS_INFO,

clippy_lints/src/lib.rs

@@ -205,6 +205,7 @@ mod loops;
 mod macro_metavars_in_unsafe;
 mod macro_use;
 mod main_recursion;
+mod manual_abs_diff;
 mod manual_assert;
 mod manual_async_fn;
 mod manual_bits;
@@ -878,6 +879,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(std_instead_of_core::StdReexports::new(conf)));
     store.register_late_pass(move |_| Box::new(instant_subtraction::InstantSubtraction::new(conf)));
     store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));
+    store.register_late_pass(move |_| Box::new(manual_abs_diff::ManualAbsDiff::new(conf)));
     store.register_late_pass(move |_| Box::new(manual_clamp::ManualClamp::new(conf)));
     store.register_late_pass(|_| Box::new(manual_string_new::ManualStringNew));
     store.register_late_pass(|_| Box::new(unused_peekable::UnusedPeekable));

clippy_lints/src/manual_abs_diff.rs

@@ -0,0 +1,105 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher::If;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::{eq_expr_value, peel_blocks};
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::impl_lint_pass;
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects patterns like `if a > b { a - b } else { b - a }` and suggests using `a.abs_diff(b)`.
+    ///
+    /// ### Why is this bad?
+    /// Using `abs_diff` is shorter, more readable, and avoids control flow.
+    ///
+    /// ### Examples
+    /// ```no_run
+    /// # let (a, b) = (5_usize, 3_usize);
+    /// if a > b {
+    ///     a - b
+    /// } else {
+    ///     b - a
+    /// }
+    /// # ;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # let (a, b) = (5_usize, 3_usize);
+    /// a.abs_diff(b)
+    /// # ;
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub MANUAL_ABS_DIFF,
+    complexity,
+    "using an if-else pattern instead of `abs_diff`"
+}
+
+impl_lint_pass!(ManualAbsDiff => [MANUAL_ABS_DIFF]);
+
+pub struct ManualAbsDiff {
+    msrv: Msrv,
+}
+
+impl ManualAbsDiff {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for ManualAbsDiff {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if !expr.span.from_expansion()
+            && let Some(if_expr) = If::hir(expr)
+            && let Some(r#else) = if_expr.r#else
+            && let ExprKind::Binary(op, rhs, lhs) = if_expr.cond.kind
+            && let (BinOpKind::Gt | BinOpKind::Ge, a, b) | (BinOpKind::Lt | BinOpKind::Le, b, a) = (op.node, rhs, lhs)
+            && self.are_ty_eligible(cx, a, b)
+            && Self::is_sub_expr(cx, if_expr.then, a, b)
+            && Self::is_sub_expr(cx, r#else, b, a)
+        {
+            let a = Sugg::hir(cx, a, "..").maybe_paren();
+            let b = Sugg::hir(cx, b, "..");
+            span_lint_and_sugg(
+                cx,
+                MANUAL_ABS_DIFF,
+                expr.span,
+                "manual absolute difference pattern without using `abs_diff`",
+                "replace with `abs_diff`",
+                format!("{a}.abs_diff({b})"),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
+impl ManualAbsDiff {
+    /// Returns `true` if `a` and `b` are of the same type, and this lint can be applied to that
+    /// type (currently, any primitive unsigned int, or a `Duration`)
+    fn are_ty_eligible(&self, cx: &LateContext<'_>, a: &Expr<'_>, b: &Expr<'_>) -> bool {
+        let a_ty = cx.typeck_results().expr_ty(a).peel_refs();
+        a_ty == cx.typeck_results().expr_ty(b).peel_refs()
+            && ((matches!(a_ty.kind(), ty::Uint(_)) && self.msrv.meets(cx, msrvs::ABS_DIFF))
+                || (is_type_diagnostic_item(cx, a_ty, sym::Duration) && self.msrv.meets(cx, msrvs::DURATION_ABS_DIFF)))
+    }
+
+    /// Checks if the given expression is a subtraction operation between two expected expressions,
+    /// i.e. if `expr` is `{expected_a} - {expected_b}`.
+    fn is_sub_expr(cx: &LateContext<'_>, expr: &Expr<'_>, expected_a: &Expr<'_>, expected_b: &Expr<'_>) -> bool {
+        if let ExprKind::Binary(op, a, b) = peel_blocks(expr).kind
+            && let BinOpKind::Sub = op.node
+            && eq_expr_value(cx, a, expected_a)
+            && eq_expr_value(cx, b, expected_b)
+        {
+            true
+        } else {
+            false
+        }
+    }
+}

clippy_utils/src/msrvs.rs

@@ -27,7 +27,7 @@ msrv_aliases! {
     1,84,0 { CONST_OPTION_AS_SLICE }
     1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
     1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
-    1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION }
+    1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION, DURATION_ABS_DIFF }
     1,80,0 { BOX_INTO_ITER, LAZY_CELL }
     1,77,0 { C_STR_LITERALS }
     1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
@@ -40,6 +40,7 @@ msrv_aliases! {
     1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
     1,63,0 { CLONE_INTO }
     1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN }
+    1,60,0 { ABS_DIFF }
     1,59,0 { THREAD_LOCAL_CONST_INIT }
     1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY, CONST_RAW_PTR_DEREF }
     1,57,0 { MAP_WHILE }

tests/ui/manual_abs_diff.fixed

@@ -0,0 +1,100 @@
+#![warn(clippy::manual_abs_diff)]
+
+use std::time::Duration;
+
+fn main() {
+    let a: usize = 5;
+    let b: usize = 3;
+    let c: usize = 8;
+    let d: usize = 11;
+
+    let _ = a.abs_diff(b);
+    //~^ manual_abs_diff
+    let _ = b.abs_diff(a);
+    //~^ manual_abs_diff
+
+    let _ = a.abs_diff(b);
+    //~^ manual_abs_diff
+    let _ = b.abs_diff(a);
+    //~^ manual_abs_diff
+
+    #[allow(arithmetic_overflow)]
+    {
+        let _ = if a > b { b - a } else { a - b };
+        let _ = if a < b { a - b } else { b - a };
+    }
+
+    let _ = (a + b).abs_diff(c + d);
+    let _ = (c + d).abs_diff(a + b);
+
+    const A: usize = 5;
+    const B: usize = 3;
+    // check const context
+    const _: usize = A.abs_diff(B);
+    //~^ manual_abs_diff
+
+    let a = Duration::from_secs(3);
+    let b = Duration::from_secs(5);
+    let _ = a.abs_diff(b);
+    //~^ manual_abs_diff
+
+    let a: isize = 3;
+    let b: isize = -5;
+    let _ = if a > b { a - b } else { b - a };
+}
+
+// FIXME: bunch of patterns that should be linted
+fn fixme() {
+    let a: usize = 5;
+    let b: usize = 3;
+    let c: usize = 8;
+    let d: usize = 11;
+
+    {
+        let out;
+        if a > b {
+            out = a - b;
+        } else {
+            out = b - a;
+        }
+    }
+
+    {
+        let mut out = 0;
+        if a > b {
+            out = a - b;
+        } else if a < b {
+            out = b - a;
+        }
+    }
+
+    #[allow(clippy::implicit_saturating_sub)]
+    let _ = if a > b {
+        a - b
+    } else if a < b {
+        b - a
+    } else {
+        0
+    };
+
+    let a: i32 = 3;
+    let b: i32 = 5;
+    let _: u32 = if a > b { (a - b) as u32 } else { (b - a) as u32 };
+    let _: u32 = if a > b { a - b } else { b - a } as u32;
+}
+
+fn non_primitive_ty() {
+    #[derive(Eq, PartialEq, PartialOrd)]
+    struct S(i32);
+
+    impl std::ops::Sub for S {
+        type Output = S;
+
+        fn sub(self, rhs: Self) -> Self::Output {
+            Self(self.0 - rhs.0)
+        }
+    }
+
+    let (a, b) = (S(10), S(20));
+    let _ = if a < b { b - a } else { a - b };
+}

tests/ui/manual_abs_diff.rs

@@ -0,0 +1,110 @@
+#![warn(clippy::manual_abs_diff)]
+
+use std::time::Duration;
+
+fn main() {
+    let a: usize = 5;
+    let b: usize = 3;
+    let c: usize = 8;
+    let d: usize = 11;
+
+    let _ = if a > b { a - b } else { b - a };
+    //~^ manual_abs_diff
+    let _ = if a < b { b - a } else { a - b };
+    //~^ manual_abs_diff
+
+    let _ = if a >= b { a - b } else { b - a };
+    //~^ manual_abs_diff
+    let _ = if a <= b { b - a } else { a - b };
+    //~^ manual_abs_diff
+
+    #[allow(arithmetic_overflow)]
+    {
+        let _ = if a > b { b - a } else { a - b };
+        let _ = if a < b { a - b } else { b - a };
+    }
+
+    let _ = if (a + b) > (c + d) {
+        //~^ manual_abs_diff
+        (a + b) - (c + d)
+    } else {
+        (c + d) - (a + b)
+    };
+    let _ = if (a + b) < (c + d) {
+        //~^ manual_abs_diff
+        (c + d) - (a + b)
+    } else {
+        (a + b) - (c + d)
+    };
+
+    const A: usize = 5;
+    const B: usize = 3;
+    // check const context
+    const _: usize = if A > B { A - B } else { B - A };
+    //~^ manual_abs_diff
+
+    let a = Duration::from_secs(3);
+    let b = Duration::from_secs(5);
+    let _ = if a > b { a - b } else { b - a };
+    //~^ manual_abs_diff
+
+    let a: isize = 3;
+    let b: isize = -5;
+    let _ = if a > b { a - b } else { b - a };
+}
+
+// FIXME: bunch of patterns that should be linted
+fn fixme() {
+    let a: usize = 5;
+    let b: usize = 3;
+    let c: usize = 8;
+    let d: usize = 11;
+
+    {
+        let out;
+        if a > b {
+            out = a - b;
+        } else {
+            out = b - a;
+        }
+    }
+
+    {
+        let mut out = 0;
+        if a > b {
+            out = a - b;
+        } else if a < b {
+            out = b - a;
+        }
+    }
+
+    #[allow(clippy::implicit_saturating_sub)]
+    let _ = if a > b {
+        a - b
+    } else if a < b {
+        b - a
+    } else {
+        0
+    };
+
+    let a: i32 = 3;
+    let b: i32 = 5;
+    let _: u32 = if a > b { (a - b) as u32 } else { (b - a) as u32 };
+    let _: u32 = if a > b { a - b } else { b - a } as u32;
+}
+
+fn non_primitive_ty() {
+    #[derive(Eq, PartialEq, PartialOrd)]
+    struct S(i32);
+
+    impl std::ops::Sub for S {
+        type Output = S;
+
+        fn sub(self, rhs: Self) -> Self::Output {
+            Self(self.0 - rhs.0)
+        }
+    }
+
+    let (a, b) = (S(10), S(20));
+    let _ = if a < b { b - a } else { a - b };
+}