Auto merge of #13542 - blyxyas:fix-implicit_saturating_sub, r=y21

[`implicit_saturating_sub`] Fix suggestion with a less volatile approach

Related to #13533, such and obvious mistake got pass my watch, quite embarassing :/

Revert #13533 and implement a more robust solution.

Revert "Fix span issue on `implicit_saturating_sub`
This reverts commit 140a1275f24ab951ffb0daee568385049de153d5.

changelog: [`lint_name`]: Fix suggestion for `if {} else if {} else {}` cases

r? `@y21`

clippy_lints/src/implicit_saturating_sub.rs

@@ -107,7 +107,9 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
         }) = higher::If::hir(expr)
             && let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind
         {
-            check_manual_check(cx, cond_op, cond_left, cond_right, if_block, else_block, &self.msrv);
+            check_manual_check(
+                cx, expr, cond_op, cond_left, cond_right, if_block, else_block, &self.msrv,
+            );
         }
     }
 
@@ -117,6 +119,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
 #[allow(clippy::too_many_arguments)]
 fn check_manual_check<'tcx>(
     cx: &LateContext<'tcx>,
+    expr: &Expr<'tcx>,
     condition: &BinOp,
     left_hand: &Expr<'tcx>,
     right_hand: &Expr<'tcx>,
@@ -127,12 +130,40 @@ fn check_manual_check<'tcx>(
     let ty = cx.typeck_results().expr_ty(left_hand);
     if ty.is_numeric() && !ty.is_signed() {
         match condition.node {
-            BinOpKind::Gt | BinOpKind::Ge => {
+            BinOpKind::Gt | BinOpKind::Ge => check_gt(
-                check_gt(cx, condition.span, left_hand, right_hand, if_block, else_block, msrv);
+                cx,
-            },
+                condition.span,
-            BinOpKind::Lt | BinOpKind::Le => {
+                expr.span,
-                check_gt(cx, condition.span, right_hand, left_hand, if_block, else_block, msrv);
+                left_hand,
-            },
+                right_hand,
+                if_block,
+                else_block,
+                msrv,
+                matches!(
+                    clippy_utils::get_parent_expr(cx, expr),
+                    Some(Expr {
+                        kind: ExprKind::If(..),
+                        ..
+                    })
+                ),
+            ),
+            BinOpKind::Lt | BinOpKind::Le => check_gt(
+                cx,
+                condition.span,
+                expr.span,
+                right_hand,
+                left_hand,
+                if_block,
+                else_block,
+                msrv,
+                matches!(
+                    clippy_utils::get_parent_expr(cx, expr),
+                    Some(Expr {
+                        kind: ExprKind::If(..),
+                        ..
+                    })
+                ),
+            ),
             _ => {},
         }
     }
@@ -142,16 +173,28 @@ fn check_manual_check<'tcx>(
 fn check_gt(
     cx: &LateContext<'_>,
     condition_span: Span,
+    expr_span: Span,
     big_var: &Expr<'_>,
     little_var: &Expr<'_>,
     if_block: &Expr<'_>,
     else_block: &Expr<'_>,
     msrv: &Msrv,
+    is_composited: bool,
 ) {
     if let Some(big_var) = Var::new(big_var)
         && let Some(little_var) = Var::new(little_var)
     {
-        check_subtraction(cx, condition_span, big_var, little_var, if_block, else_block, msrv);
+        check_subtraction(
+            cx,
+            condition_span,
+            expr_span,
+            big_var,
+            little_var,
+            if_block,
+            else_block,
+            msrv,
+            is_composited,
+        );
     }
 }
 
@@ -173,11 +216,13 @@ impl Var {
 fn check_subtraction(
     cx: &LateContext<'_>,
     condition_span: Span,
+    expr_span: Span,
     big_var: Var,
     little_var: Var,
     if_block: &Expr<'_>,
     else_block: &Expr<'_>,
     msrv: &Msrv,
+    is_composited: bool,
 ) {
     let if_block = peel_blocks(if_block);
     let else_block = peel_blocks(else_block);
@@ -189,7 +234,17 @@ fn check_subtraction(
         }
         // If the subtraction is done in the `else` block, then we need to also revert the two
         // variables as it means that the check was reverted too.
-        check_subtraction(cx, condition_span, little_var, big_var, else_block, if_block, msrv);
+        check_subtraction(
+            cx,
+            condition_span,
+            expr_span,
+            little_var,
+            big_var,
+            else_block,
+            if_block,
+            msrv,
+            is_composited,
+        );
         return;
     }
     if is_integer_literal(else_block, 0)
@@ -205,13 +260,18 @@ fn check_subtraction(
                 && let Some(little_var_snippet) = snippet_opt(cx, little_var.span)
                 && (!is_in_const_context(cx) || msrv.meets(msrvs::SATURATING_SUB_CONST))
             {
+                let sugg = format!(
+                    "{}{big_var_snippet}.saturating_sub({little_var_snippet}){}",
+                    if is_composited { "{ " } else { "" },
+                    if is_composited { " }" } else { "" }
+                );
                 span_lint_and_sugg(
                     cx,
                     IMPLICIT_SATURATING_SUB,
-                    else_block.span,
+                    expr_span,
                     "manual arithmetic check found",
                     "replace it with",
-                    format!("{big_var_snippet}.saturating_sub({little_var_snippet})"),
+                    sugg,
                     Applicability::MachineApplicable,
                 );
             }

tests/ui/implicit_saturating_sub.fixed

@@ -223,13 +223,8 @@ fn main() {
     };
 }
 
-// https://github.com/rust-lang/rust-clippy/issues/13524
 fn regression_13524(a: usize, b: usize, c: bool) -> usize {
     if c {
         123
-    } else if a >= b {
+    } else { b.saturating_sub(a) }
-        b.saturating_sub(a)
-    } else {
-        b - a
-    }
 }

tests/ui/implicit_saturating_sub.rs

@@ -269,7 +269,6 @@ fn main() {
     };
 }
 
-// https://github.com/rust-lang/rust-clippy/issues/13524
 fn regression_13524(a: usize, b: usize, c: bool) -> usize {
     if c {
         123

tests/ui/implicit_saturating_sub.stderr

@@ -186,10 +186,15 @@ LL | |     }
    | |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
 
 error: manual arithmetic check found
-  --> tests/ui/implicit_saturating_sub.rs:277:9
+  --> tests/ui/implicit_saturating_sub.rs:275:12
    |
-LL |         0
+LL |       } else if a >= b {
-   |         ^ help: replace it with: `b.saturating_sub(a)`
+   |  ____________^
+LL | |         0
+LL | |     } else {
+LL | |         b - a
+LL | |     }
+   | |_____^ help: replace it with: `{ b.saturating_sub(a) }`
 
 error: aborting due to 24 previous errors
 

tests/ui/manual_arithmetic_check.fixed

@@ -6,14 +6,14 @@ fn main() {
     let b = 13u32;
     let c = 8u32;
 
-    let result = if a > b { a - b } else { a.saturating_sub(b) };
+    let result = a.saturating_sub(b);
     //~^ ERROR: manual arithmetic check found
-    let result = if b < a { a - b } else { a.saturating_sub(b) };
+    let result = a.saturating_sub(b);
     //~^ ERROR: manual arithmetic check found
 
-    let result = if a < b { a.saturating_sub(b) } else { a - b };
+    let result = a.saturating_sub(b);
     //~^ ERROR: manual arithmetic check found
-    let result = if b > a { a.saturating_sub(b) } else { a - b };
+    let result = a.saturating_sub(b);
     //~^ ERROR: manual arithmetic check found
 
     // Should not warn!

tests/ui/manual_arithmetic_check.stderr

@@ -1,29 +1,29 @@
 error: manual arithmetic check found
-  --> tests/ui/manual_arithmetic_check.rs:9:44
+  --> tests/ui/manual_arithmetic_check.rs:9:18
    |
 LL |     let result = if a > b { a - b } else { 0 };
-   |                                            ^ help: replace it with: `a.saturating_sub(b)`
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
    |
    = note: `-D clippy::implicit-saturating-sub` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::implicit_saturating_sub)]`
 
 error: manual arithmetic check found
-  --> tests/ui/manual_arithmetic_check.rs:11:44
+  --> tests/ui/manual_arithmetic_check.rs:11:18
    |
 LL |     let result = if b < a { a - b } else { 0 };
-   |                                            ^ help: replace it with: `a.saturating_sub(b)`
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
 
 error: manual arithmetic check found
-  --> tests/ui/manual_arithmetic_check.rs:14:29
+  --> tests/ui/manual_arithmetic_check.rs:14:18
    |
 LL |     let result = if a < b { 0 } else { a - b };
-   |                             ^ help: replace it with: `a.saturating_sub(b)`
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
 
 error: manual arithmetic check found
-  --> tests/ui/manual_arithmetic_check.rs:16:29
+  --> tests/ui/manual_arithmetic_check.rs:16:18
    |
 LL |     let result = if b > a { 0 } else { a - b };
-   |                             ^ help: replace it with: `a.saturating_sub(b)`
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
 
 error: aborting due to 4 previous errors