Fix unchecked_time_subtraction FN on Ops::sub method call (#16233)

Jarcho · web-flow · commit 9a089b63a0cf · 2025-12-13T22:19:32.000Z
Closes #16230 Closes #16234 Closes #16236 changelog: [`unchecked_time_subtraction`] fix FN on `Ops::sub` method call
diff --git a/clippy_lints/src/time_subtraction.rs b/clippy_lints/src/time_subtraction.rs
@@ -8,7 +8,6 @@ use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::Ty;
 use rustc_session::impl_lint_pass;
-use rustc_span::source_map::Spanned;
 use rustc_span::sym;
 
 declare_clippy_lint! {
@@ -84,43 +83,38 @@ impl_lint_pass!(UncheckedTimeSubtraction => [MANUAL_INSTANT_ELAPSED, UNCHECKED_T
 
 impl LateLintPass<'_> for UncheckedTimeSubtraction {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
-        if let ExprKind::Binary(
-            Spanned {
-                node: BinOpKind::Sub, ..
+        let (lhs, rhs) = match expr.kind {
+            ExprKind::Binary(op, lhs, rhs) if matches!(op.node, BinOpKind::Sub,) => (lhs, rhs),
+            ExprKind::MethodCall(fn_name, lhs, [rhs], _) if cx.ty_based_def(expr).is_diag_item(cx, sym::sub) => {
+                (lhs, rhs)
             },
-            lhs,
-            rhs,
-        ) = expr.kind
-        {
-            let typeck = cx.typeck_results();
-            let lhs_ty = typeck.expr_ty(lhs);
-            let rhs_ty = typeck.expr_ty(rhs);
+            _ => return,
+        };
+        let typeck = cx.typeck_results();
+        let lhs_ty = typeck.expr_ty(lhs);
+        let rhs_ty = typeck.expr_ty(rhs);
 
-            if lhs_ty.is_diag_item(cx, sym::Instant) {
-                // Instant::now() - instant
-                if is_instant_now_call(cx, lhs)
-                    && rhs_ty.is_diag_item(cx, sym::Instant)
-                    && let Some(sugg) = Sugg::hir_opt(cx, rhs)
-                {
-                    print_manual_instant_elapsed_sugg(cx, expr, sugg);
-                }
-                // instant - duration
-                else if rhs_ty.is_diag_item(cx, sym::Duration)
-                    && !expr.span.from_expansion()
-                    && self.msrv.meets(cx, msrvs::TRY_FROM)
-                {
-                    print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
-                }
+        if lhs_ty.is_diag_item(cx, sym::Instant) {
+            // Instant::now() - instant
+            if is_instant_now_call(cx, lhs) && rhs_ty.is_diag_item(cx, sym::Instant) {
+                print_manual_instant_elapsed_sugg(cx, expr, rhs);
             }
-            // duration - duration
-            else if lhs_ty.is_diag_item(cx, sym::Duration)
-                && rhs_ty.is_diag_item(cx, sym::Duration)
+            // instant - duration
+            else if rhs_ty.is_diag_item(cx, sym::Duration)
                 && !expr.span.from_expansion()
                 && self.msrv.meets(cx, msrvs::TRY_FROM)
             {
                 print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
             }
         }
+        // duration - duration
+        else if lhs_ty.is_diag_item(cx, sym::Duration)
+            && rhs_ty.is_diag_item(cx, sym::Duration)
+            && !expr.span.from_expansion()
+            && self.msrv.meets(cx, msrvs::TRY_FROM)
+        {
+            print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
+        }
     }
 }
 
@@ -153,15 +147,17 @@ fn is_time_type(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
     ty.is_diag_item(cx, sym::Duration) || ty.is_diag_item(cx, sym::Instant)
 }
 
-fn print_manual_instant_elapsed_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, sugg: Sugg<'_>) {
+fn print_manual_instant_elapsed_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, rhs: &Expr<'_>) {
+    let mut applicability = Applicability::MachineApplicable;
+    let sugg = Sugg::hir_with_context(cx, rhs, expr.span.ctxt(), "<instant>", &mut applicability);
     span_lint_and_sugg(
         cx,
         MANUAL_INSTANT_ELAPSED,
         expr.span,
         "manual implementation of `Instant::elapsed`",
         "try",
         format!("{}.elapsed()", sugg.maybe_paren()),
-        Applicability::MachineApplicable,
+        applicability,
     );
 }
 
@@ -181,8 +177,9 @@ fn print_unchecked_duration_subtraction_sugg(
             // avoid suggestions
             if !is_chained_time_subtraction(cx, left_expr) {
                 let mut applicability = Applicability::MachineApplicable;
-                let left_sugg = Sugg::hir_with_applicability(cx, left_expr, "<left>", &mut applicability);
-                let right_sugg = Sugg::hir_with_applicability(cx, right_expr, "<right>", &mut applicability);
+                let left_sugg = Sugg::hir_with_context(cx, left_expr, expr.span.ctxt(), "<left>", &mut applicability);
+                let right_sugg =
+                    Sugg::hir_with_context(cx, right_expr, expr.span.ctxt(), "<right>", &mut applicability);
 
                 diag.span_suggestion(
                     expr.span,
diff --git a/tests/ui/manual_instant_elapsed.fixed b/tests/ui/manual_instant_elapsed.fixed
@@ -28,3 +28,19 @@ fn main() {
     //
     //~^^ manual_instant_elapsed
 }
+
+fn issue16236() {
+    use std::ops::Sub as _;
+    macro_rules! deref {
+        ($e:expr) => {
+            *$e
+        };
+    }
+
+    let start = &Instant::now();
+    let _ = deref!(start).elapsed();
+    //~^ manual_instant_elapsed
+
+    deref!(start).elapsed();
+    //~^ manual_instant_elapsed
+}
diff --git a/tests/ui/manual_instant_elapsed.rs b/tests/ui/manual_instant_elapsed.rs
@@ -28,3 +28,19 @@ fn main() {
     //
     //~^^ manual_instant_elapsed
 }
+
+fn issue16236() {
+    use std::ops::Sub as _;
+    macro_rules! deref {
+        ($e:expr) => {
+            *$e
+        };
+    }
+
+    let start = &Instant::now();
+    let _ = Instant::now().sub(deref!(start));
+    //~^ manual_instant_elapsed
+
+    Instant::now() - deref!(start);
+    //~^ manual_instant_elapsed
+}
diff --git a/tests/ui/manual_instant_elapsed.stderr b/tests/ui/manual_instant_elapsed.stderr
@@ -13,5 +13,17 @@ error: manual implementation of `Instant::elapsed`
 LL |     Instant::now() - *ref_to_instant; // to ensure parens are added correctly
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(*ref_to_instant).elapsed()`
 
-error: aborting due to 2 previous errors
+error: manual implementation of `Instant::elapsed`
+  --> tests/ui/manual_instant_elapsed.rs:41:13
+   |
+LL |     let _ = Instant::now().sub(deref!(start));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `deref!(start).elapsed()`
+
+error: manual implementation of `Instant::elapsed`
+  --> tests/ui/manual_instant_elapsed.rs:44:5
+   |
+LL |     Instant::now() - deref!(start);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `deref!(start).elapsed()`
+
+error: aborting due to 4 previous errors
 
diff --git a/tests/ui/unchecked_time_subtraction.fixed b/tests/ui/unchecked_time_subtraction.fixed
@@ -35,3 +35,28 @@ fn main() {
     let _ = (2 * dur1).checked_sub(dur2).unwrap();
     //~^ unchecked_time_subtraction
 }
+
+fn issue16230() {
+    use std::ops::Sub as _;
+
+    Duration::ZERO.checked_sub(Duration::MAX).unwrap();
+    //~^ unchecked_time_subtraction
+
+    let _ = Duration::ZERO.checked_sub(Duration::MAX).unwrap();
+    //~^ unchecked_time_subtraction
+}
+
+fn issue16234() {
+    use std::ops::Sub as _;
+
+    macro_rules! duration {
+        ($secs:expr) => {
+            Duration::from_secs($secs)
+        };
+    }
+
+    duration!(0).checked_sub(duration!(1)).unwrap();
+    //~^ unchecked_time_subtraction
+    let _ = duration!(0).checked_sub(duration!(1)).unwrap();
+    //~^ unchecked_time_subtraction
+}
diff --git a/tests/ui/unchecked_time_subtraction.rs b/tests/ui/unchecked_time_subtraction.rs
@@ -35,3 +35,28 @@ fn main() {
     let _ = 2 * dur1 - dur2;
     //~^ unchecked_time_subtraction
 }
+
+fn issue16230() {
+    use std::ops::Sub as _;
+
+    Duration::ZERO.sub(Duration::MAX);
+    //~^ unchecked_time_subtraction
+
+    let _ = Duration::ZERO - Duration::MAX;
+    //~^ unchecked_time_subtraction
+}
+
+fn issue16234() {
+    use std::ops::Sub as _;
+
+    macro_rules! duration {
+        ($secs:expr) => {
+            Duration::from_secs($secs)
+        };
+    }
+
+    duration!(0).sub(duration!(1));
+    //~^ unchecked_time_subtraction
+    let _ = duration!(0) - duration!(1);
+    //~^ unchecked_time_subtraction
+}
diff --git a/tests/ui/unchecked_time_subtraction.stderr b/tests/ui/unchecked_time_subtraction.stderr
@@ -49,5 +49,29 @@ error: unchecked subtraction of a `Duration`
 LL |     let _ = 2 * dur1 - dur2;
    |             ^^^^^^^^^^^^^^^ help: try: `(2 * dur1).checked_sub(dur2).unwrap()`
 
-error: aborting due to 8 previous errors
+error: unchecked subtraction of a `Duration`
+  --> tests/ui/unchecked_time_subtraction.rs:42:5
+   |
+LL |     Duration::ZERO.sub(Duration::MAX);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Duration::ZERO.checked_sub(Duration::MAX).unwrap()`
+
+error: unchecked subtraction of a `Duration`
+  --> tests/ui/unchecked_time_subtraction.rs:45:13
+   |
+LL |     let _ = Duration::ZERO - Duration::MAX;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Duration::ZERO.checked_sub(Duration::MAX).unwrap()`
+
+error: unchecked subtraction of a `Duration`
+  --> tests/ui/unchecked_time_subtraction.rs:58:5
+   |
+LL |     duration!(0).sub(duration!(1));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `duration!(0).checked_sub(duration!(1)).unwrap()`
+
+error: unchecked subtraction of a `Duration`
+  --> tests/ui/unchecked_time_subtraction.rs:60:13
+   |
+LL |     let _ = duration!(0) - duration!(1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `duration!(0).checked_sub(duration!(1)).unwrap()`
+
+error: aborting due to 12 previous errors
 
