feat(manual_saturating_arithmetic): lint x.saturating_sub(y).unwrap_or_default()

clippy_lints/src/methods/manual_saturating_arithmetic.rs

@@ -3,18 +3,20 @@ use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{path_res, sym};
 use rustc_ast::ast;
 use rustc_errors::Applicability;
-use rustc_hir as hir;
 use rustc_hir::def::Res;
+use rustc_hir::{self as hir, Expr};
 use rustc_lint::LateContext;
+use rustc_middle::ty::Ty;
 use rustc_middle::ty::layout::LayoutOf;
+use rustc_span::Symbol;
 
-pub fn check(
+pub fn check_unwrap_or(
     cx: &LateContext<'_>,
-    expr: &hir::Expr<'_>,
-    arith_lhs: &hir::Expr<'_>,
-    arith_rhs: &hir::Expr<'_>,
-    unwrap_arg: &hir::Expr<'_>,
-    arith: &str,
+    expr: &Expr<'_>,
+    arith_lhs: &Expr<'_>,
+    arith_rhs: &Expr<'_>,
+    unwrap_arg: &Expr<'_>,
+    arith: Symbol,
 ) {
     let ty = cx.typeck_results().expr_ty(arith_lhs);
     if !ty.is_integral() {
@@ -25,49 +27,120 @@ pub fn check(
         return;
     };
 
+    let Some(checked_arith) = CheckedArith::new(arith) else {
+        return;
+    };
+
+    check(cx, expr, arith_lhs, arith_rhs, ty, mm, checked_arith);
+}
+
+/// Precondition: should be called on `x.saturating_sub(y).unwrap_or_default()`
+pub(super) fn check_unwrap_or_default(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    arith_lhs: &Expr<'_>,
+    arith_rhs: &Expr<'_>,
+) {
+    let ty = cx.typeck_results().expr_ty(arith_lhs);
+    if !ty.is_integral() {
+        return;
+    }
+
+    let mm = if ty.is_signed() {
+        return; // iN::default() is 0, which is neither MIN nor MAX
+    } else {
+        MinMax::Min // uN::default() is 0, which is also the MIN
+    };
+
+    // See precondition
+    let checked_arith = CheckedArith::Sub;
+
+    check(cx, expr, arith_lhs, arith_rhs, ty, mm, checked_arith);
+}
+
+fn check(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    arith_lhs: &Expr<'_>,
+    arith_rhs: &Expr<'_>,
+    ty: Ty<'_>,
+    mm: MinMax,
+    checked_arith: CheckedArith,
+) {
     if ty.is_signed() {
         use self::MinMax::{Max, Min};
         use self::Sign::{Neg, Pos};
+        use CheckedArith::{Add, Sub};
 
         let Some(sign) = lit_sign(arith_rhs) else {
             return;
         };
 
-        match (arith, sign, mm) {
-            ("add", Pos, Max) | ("add", Neg, Min) | ("sub", Neg, Max) | ("sub", Pos, Min) => (),
+        match (checked_arith, sign, mm) {
+            (Add, Pos, Max) | (Add, Neg, Min) | (Sub, Neg, Max) | (Sub, Pos, Min) => (),
             // "mul" is omitted because lhs can be negative.
             _ => return,
         }
     } else {
-        match (mm, arith) {
-            (MinMax::Max, "add" | "mul") | (MinMax::Min, "sub") => (),
+        use self::MinMax::{Max, Min};
+        use CheckedArith::{Add, Mul, Sub};
+        match (mm, checked_arith) {
+            (Max, Add | Mul) | (Min, Sub) => (),
             _ => return,
         }
     }
 
     let mut applicability = Applicability::MachineApplicable;
+    let saturating_arith = checked_arith.as_saturating();
     span_lint_and_sugg(
         cx,
         super::MANUAL_SATURATING_ARITHMETIC,
         expr.span,
         "manual saturating arithmetic",
-        format!("consider using `saturating_{arith}`"),
+        format!("consider using `{saturating_arith}`"),
         format!(
-            "{}.saturating_{arith}({})",
+            "{}.{saturating_arith}({})",
             snippet_with_applicability(cx, arith_lhs.span, "..", &mut applicability),
             snippet_with_applicability(cx, arith_rhs.span, "..", &mut applicability),
         ),
         applicability,
     );
 }
 
+#[derive(Clone, Copy)]
+enum CheckedArith {
+    Add,
+    Sub,
+    Mul,
+}
+
+impl CheckedArith {
+    fn new(sym: Symbol) -> Option<Self> {
+        let res = match sym {
+            sym::checked_add => Self::Add,
+            sym::checked_sub => Self::Sub,
+            sym::checked_mul => Self::Mul,
+            _ => return None,
+        };
+        Some(res)
+    }
+
+    fn as_saturating(self) -> &'static str {
+        match self {
+            Self::Add => "saturating_add",
+            Self::Sub => "saturating_sub",
+            Self::Mul => "saturating_mul",
+        }
+    }
+}
+
 #[derive(PartialEq, Eq)]
 enum MinMax {
     Min,
     Max,
 }
 
-fn is_min_or_max(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<MinMax> {
+fn is_min_or_max(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<MinMax> {
     // `T::max_value()` `T::min_value()` inherent methods
     if let hir::ExprKind::Call(func, []) = &expr.kind
         && let hir::ExprKind::Path(hir::QPath::TypeRelative(_, segment)) = &func.kind
@@ -105,7 +178,7 @@ fn is_min_or_max(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<MinMax> {
         (0, if bits == 128 { !0 } else { (1 << bits) - 1 })
     };
 
-    let check_lit = |expr: &hir::Expr<'_>, check_min: bool| {
+    let check_lit = |expr: &Expr<'_>, check_min: bool| {
         if let hir::ExprKind::Lit(lit) = &expr.kind
             && let ast::LitKind::Int(value, _) = lit.node
         {
@@ -140,7 +213,7 @@ enum Sign {
     Neg,
 }
 
-fn lit_sign(expr: &hir::Expr<'_>) -> Option<Sign> {
+fn lit_sign(expr: &Expr<'_>) -> Option<Sign> {
     if let hir::ExprKind::Unary(hir::UnOp::Neg, inner) = &expr.kind {
         if let hir::ExprKind::Lit(..) = &inner.kind {
             return Some(Sign::Neg);

clippy_lints/src/methods/mod.rs

@@ -5488,14 +5488,7 @@ impl Methods {
                 (sym::unwrap_or, [u_arg]) => {
                     match method_call(recv) {
                         Some((arith @ (sym::checked_add | sym::checked_sub | sym::checked_mul), lhs, [rhs], _, _)) => {
-                            manual_saturating_arithmetic::check(
-                                cx,
-                                expr,
-                                lhs,
-                                rhs,
-                                u_arg,
-                                &arith.as_str()[const { "checked_".len() }..],
-                            );
+                            manual_saturating_arithmetic::check_unwrap_or(cx, expr, lhs, rhs, u_arg, arith);
                         },
                         Some((sym::map, m_recv, [m_arg], span, _)) => {
                             option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, self.msrv);
@@ -5509,6 +5502,9 @@ impl Methods {
                 },
                 (sym::unwrap_or_default, []) => {
                     match method_call(recv) {
+                        Some((sym::checked_sub, lhs, [rhs], _, _)) => {
+                            manual_saturating_arithmetic::check_unwrap_or_default(cx, expr, lhs, rhs);
+                        },
                         Some((sym::map, m_recv, [arg], span, _)) => {
                             manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv);
                         },

tests/ui/manual_saturating_arithmetic.fixed

@@ -58,3 +58,13 @@ fn main() {
     let _ = 1i8.checked_sub(1).unwrap_or(127); // ok
     let _ = 1i8.checked_sub(-1).unwrap_or(-128); // ok
 }
+
+fn issue15655() {
+    let _ = 5u32.saturating_sub(1u32); //~ manual_saturating_arithmetic
+    let _ = 5u32.checked_add(1u32).unwrap_or_default(); // ok
+    let _ = 5u32.checked_mul(1u32).unwrap_or_default(); // ok
+
+    let _ = 5i32.checked_sub(1i32).unwrap_or_default(); // ok
+    let _ = 5i32.checked_add(1i32).unwrap_or_default(); // ok
+    let _ = 5i32.checked_mul(1i32).unwrap_or_default(); // ok
+}

tests/ui/manual_saturating_arithmetic.rs

@@ -73,3 +73,13 @@ fn main() {
     let _ = 1i8.checked_sub(1).unwrap_or(127); // ok
     let _ = 1i8.checked_sub(-1).unwrap_or(-128); // ok
 }
+
+fn issue15655() {
+    let _ = 5u32.checked_sub(1u32).unwrap_or_default(); //~ manual_saturating_arithmetic
+    let _ = 5u32.checked_add(1u32).unwrap_or_default(); // ok
+    let _ = 5u32.checked_mul(1u32).unwrap_or_default(); // ok
+
+    let _ = 5i32.checked_sub(1i32).unwrap_or_default(); // ok
+    let _ = 5i32.checked_add(1i32).unwrap_or_default(); // ok
+    let _ = 5i32.checked_mul(1i32).unwrap_or_default(); // ok
+}

tests/ui/manual_saturating_arithmetic.stderr

@@ -165,5 +165,11 @@ LL | |         .checked_sub(-1)
 LL | |         .unwrap_or(170_141_183_460_469_231_731_687_303_715_884_105_727);
    | |_______________________________________________________________________^ help: consider using `saturating_sub`: `1i128.saturating_sub(-1)`
 
-error: aborting due to 24 previous errors
+error: manual saturating arithmetic
+  --> tests/ui/manual_saturating_arithmetic.rs:78:13
+   |
+LL |     let _ = 5u32.checked_sub(1u32).unwrap_or_default();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `saturating_sub`: `5u32.saturating_sub(1u32)`
+
+error: aborting due to 25 previous errors