Lint test case false positives corrected

Author: benacq

clippy_lints/src/manual_checked_sub.rs

@@ -33,7 +33,7 @@ declare_clippy_lint! {
     #[clippy::version = "1.86.0"]
     pub MANUAL_CHECKED_SUB,
     complexity,
-    "default lint description"
+    "Checks for manual re-implementations of checked subtraction."
 }
 
 pub struct ManualCheckedSub {
@@ -59,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualCheckedSub {
 
         let applicability = Applicability::MachineApplicable;
 
-        if let ExprKind::If(drop_temp_expr, body_block, _) = expr.kind
+        if let ExprKind::If(drop_temp_expr, body_block, else_block) = expr.kind
             && let ExprKind::DropTemps(condition_expr) = drop_temp_expr.kind
             && let ExprKind::Binary(op, lhs, rhs) = condition_expr.kind
             && is_unsigned_int(cx, &lhs)
@@ -69,6 +69,9 @@ impl<'tcx> LateLintPass<'tcx> for ManualCheckedSub {
                 SubExprVisitor {
                     cx,
                     applicability,
+                    if_expr: expr,
+                    if_body_block: body_block,
+                    else_block,
                     condition_lhs: &lhs,
                     condition_rhs: &rhs,
                 }
@@ -82,6 +85,9 @@ impl<'tcx> LateLintPass<'tcx> for ManualCheckedSub {
                     SubExprVisitor {
                         cx,
                         applicability,
+                        if_expr: expr,
+                        if_body_block: body_block,
+                        else_block,
                         condition_lhs: &lhs,
                         condition_rhs: &rhs,
                     }
@@ -94,6 +100,9 @@ impl<'tcx> LateLintPass<'tcx> for ManualCheckedSub {
 
 struct SubExprVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
+    if_expr: &'tcx Expr<'tcx>,
+    if_body_block: &'tcx Expr<'tcx>,
+    else_block: Option<&'tcx Expr<'tcx>>,
     applicability: Applicability,
     condition_lhs: &'tcx Expr<'tcx>,
     condition_rhs: &'tcx Expr<'tcx>,
@@ -127,21 +136,43 @@ impl<'tcx> Visitor<'tcx> for SubExprVisitor<'_, 'tcx> {
 
 impl<'a, 'tcx> SubExprVisitor<'a, 'tcx> {
     fn build_suggession(&mut self, expr: &Expr<'tcx>) {
+        // if let Some(else_expr) = self.else_block {
+        //     println!("ELSE BLOCK in suggestion:::: {:#?}", else_expr);
+        // }
+
+        let body_snippet = snippet_with_applicability(self.cx, self.if_body_block.span, "..", &mut self.applicability);
+
+        let binary_expr_snippet = snippet_with_applicability(self.cx, expr.span, "..", &mut self.applicability);
+        let updated_usage_context_snippet = body_snippet.as_ref().replace(binary_expr_snippet.as_ref(), "result");
+        // let else_snippet = snippet_with_applicability(self.cx, self.if_else_block.span, "..", &mut
+        // self.applicability);
+
         let lhs_snippet = snippet_with_applicability(self.cx, self.condition_lhs.span, "..", &mut self.applicability);
         let rhs_snippet = snippet_with_applicability(self.cx, self.condition_rhs.span, "..", &mut self.applicability);
 
         let lhs_needs_parens = matches!(self.condition_lhs.kind, ExprKind::Cast(..));
 
-        let suggestion = if lhs_needs_parens {
-            format!("({}).checked_sub({})", lhs_snippet, rhs_snippet)
+        let mut suggestion = if lhs_needs_parens {
+            format!(
+                "if let Some(result) = ({}).checked_sub({}) {}",
+                lhs_snippet, rhs_snippet, updated_usage_context_snippet
+            )
         } else {
-            format!("{}.checked_sub({})", lhs_snippet, rhs_snippet)
+            format!(
+                "if let Some(result) = {}.checked_sub({})  {}",
+                lhs_snippet, rhs_snippet, updated_usage_context_snippet
+            )
         };
 
+        if let Some(else_expr) = self.else_block {
+            let else_snippet = snippet_with_applicability(self.cx, else_expr.span, "..", &mut self.applicability);
+            suggestion.push_str(&format!(" else {}", else_snippet));
+        }
+
         span_lint_and_sugg(
             self.cx,
             MANUAL_CHECKED_SUB,
-            expr.span,
+            self.if_expr.span,
             "manual re-implementation of checked subtraction",
             "consider using `.checked_sub()`",
             suggestion,
@@ -212,7 +243,7 @@ fn is_referencing_same_variable<'tcx>(expr1: &'tcx Expr<'_>, expr2: &'tcx Expr<'
 
 fn get_resolved_path_id<'tcx>(expr: &'tcx Expr<'_>) -> Option<Res> {
     if let ExprKind::Path(QPath::Resolved(_, path)) = &expr.kind {
-        path.segments.first().map(|segment| segment.res)
+        path.segments.first().and_then(|segment| Some(segment.res))
     } else {
         None
     }

tests/ui/implicit_saturating_sub.rs

@@ -1,4 +1,9 @@
-#![allow(unused_assignments, unused_mut, clippy::assign_op_pattern)]
+#![allow(
+    unused_assignments,
+    unused_mut,
+    clippy::assign_op_pattern,
+    clippy::manual_checked_sub
+)]
 #![warn(clippy::implicit_saturating_sub)]
 
 use std::cmp::PartialEq;

tests/ui/manual_checked_sub.fixed

@@ -1,102 +1,141 @@
 #![allow(clippy::collapsible_if)]
 #![warn(clippy::manual_checked_sub)]
 
-//Positive test, lint
 fn positive_tests() {
     let a: u32 = 10;
     let b: u32 = 5;
+    let c = 3;
 
-    // Case 1: a - b inside an if a >= b
-    if a >= b {
-        let _ = a.checked_sub(b);
+    // Basic subtraction inside an if
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        let c = result;
+    }
+
+    // Basic subtraction inside an if with an else block
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        let c = result;
+    } else {
+        let c = a + b;
     }
 
-    // Case 2: a - 1 inside an if a > 0
-    if a > 0 {
-        let _ = a.checked_sub(0);
+    // Decrementing inside an if condition
+    if let Some(result) = a.checked_sub(0)  {
+        //~^ manual_checked_sub
+        let _ = result;
     }
 
-    // Case 3: Both suffixed, should lint
-    if 10u32 >= 5u32 {
-        let _ = 10u32.checked_sub(5u32);
+    // Variable subtraction used in a function call
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        some_function(result);
+    }
+
+    // Subtracting two suffixed literals
+    if let Some(result) = 10u32.checked_sub(5u32)  {
+        //~^ manual_checked_sub
+        let _ = result;
     }
 
     let x: i32 = 10;
     let y: i32 = 5;
 
-    // Case 4: casted variables inside an if, should lint
-    if x as u32 >= y as u32 {
-        let _ = (x as u32).checked_sub(y as u32);
+    // Casted variables inside an if
+    if let Some(result) = (x as u32).checked_sub(y as u32) {
+        //~^ manual_checked_sub
+        let _ = result;
+    }
+
+    // Casted variable and literal inside an if
+    if let Some(result) = (x as u32).checked_sub(5u32) {
+        //~^ manual_checked_sub
+        let _ = result;
+    }
+
+    // Casting subtraction result
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        let _ = result as u64;
+    }
+
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        macro_rules! subtract {
+            () => {
+                result
+            };
+        }
+        let _ = subtract!();
+    }
+
+    struct Example {
+        value: u32,
+    }
+
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        let _ = Example { value: result };
     }
 
-    // Case 5: casted variable and literal inside an if, should lint
-    if x as u32 >= 5u32 {
-        let _ = (x as u32).checked_sub(5u32);
+    // Subtraction inside a nested if, should lint
+    if let Some(result) = a.checked_sub(b)  {
+        //~^ manual_checked_sub
+        if c > 0 {
+            let _ = result;
+        }
     }
 }
 
-// Negative tests, no lint
+fn some_function(a: u32) {}
+
 fn negative_tests() {
     let a: u32 = 10;
     let b: u32 = 5;
 
-    // Case 1: Uses checked_sub() correctly, no lint
+    // Uses `.checked_sub()`, should not trigger
     let _ = a.checked_sub(b);
 
-    // Case 2: Works with signed integers (i32), no lint
+    // Works with signed integers (i32), should not trigger
     let x: i32 = 10;
     let y: i32 = 5;
+
     if x >= y {
         let _ = x - y;
     }
 
-    // Case 3: If condition doesn't match, no lint
+    // If condition does not match subtraction variables
     if a == b {
         let _ = a - b;
     }
 
-    // Case 4: a - b inside an if a > b
+    // a - b inside an if a > b (shouldn't lint)
     if a > b {
         let _ = a - b;
     }
 
-    // Case 5: Unsuffixed literals, no lint
+    // Unsuffixed literals (shouldn't lint)
     if 10 >= 5 {
         let _ = 10 - 5;
     }
 
-    // Case 6: Suffixed lhs, unsuffixed rhs, no lint
+    // Suffixed LHS, unsuffixed RHS (shouldn't lint)
     if 10u32 >= 5 {
         let _ = 10u32 - 5;
     }
 
-    // Case 7: Unsuffixed lhs, suffixed rhs, no lint
+    //  Unsuffixed LHS, suffixed RHS (shouldn't lint)
     if 10 >= 5u32 {
         let _ = 10 - 5u32;
     }
-}
-
-fn edge_cases() {
-    let a: u32 = 10;
-    let b: u32 = 5;
-    let c = 3;
 
-    // Edge Case 1: Subtraction inside a nested if
+    // Using `.wrapping_sub()`, should not trigger
     if a >= b {
-        if c > 0 {
-            let _ = a.checked_sub(b);
-        }
-    }
-
-    if a > b {
-        if a - b > c {
-            let _ = a - b; // This subtraction is safe because `a > b` is checked
-        }
+        let _ = a.wrapping_sub(b);
     }
 }
 
 fn main() {
     positive_tests();
     negative_tests();
-    edge_cases();
 }