From 075548e9e98013c1ac5e04ae36052b64eab6a5b5 Mon Sep 17 00:00:00 2001 From: Linshu Yang Date: Sat, 13 Dec 2025 18:51:30 +0000 Subject: [PATCH 1/3] fix: `unchecked_time_subtraction` FN on `Ops::sub` method call --- clippy_lints/src/time_subtraction.rs | 55 ++++++++++------------ tests/ui/unchecked_time_subtraction.fixed | 10 ++++ tests/ui/unchecked_time_subtraction.rs | 10 ++++ tests/ui/unchecked_time_subtraction.stderr | 14 +++++- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/clippy_lints/src/time_subtraction.rs b/clippy_lints/src/time_subtraction.rs index e0fdca97dbee..b4a07db9aeda 100644 --- a/clippy_lints/src/time_subtraction.rs +++ b/clippy_lints/src/time_subtraction.rs @@ -8,7 +8,6 @@ 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,41 @@ pub fn new(conf: &'static Conf) -> Self { 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) + && let Some(sugg) = Sugg::hir_opt(cx, rhs) + { + print_manual_instant_elapsed_sugg(cx, expr, sugg); } - // 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); + } } } diff --git a/tests/ui/unchecked_time_subtraction.fixed b/tests/ui/unchecked_time_subtraction.fixed index 2f923fef4c25..a75de6c9717c 100644 --- a/tests/ui/unchecked_time_subtraction.fixed +++ b/tests/ui/unchecked_time_subtraction.fixed @@ -35,3 +35,13 @@ 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 +} diff --git a/tests/ui/unchecked_time_subtraction.rs b/tests/ui/unchecked_time_subtraction.rs index cf727f62aafa..dd40d8aa14c4 100644 --- a/tests/ui/unchecked_time_subtraction.rs +++ b/tests/ui/unchecked_time_subtraction.rs @@ -35,3 +35,13 @@ 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 +} diff --git a/tests/ui/unchecked_time_subtraction.stderr b/tests/ui/unchecked_time_subtraction.stderr index c129497447fc..046024d11f75 100644 --- a/tests/ui/unchecked_time_subtraction.stderr +++ b/tests/ui/unchecked_time_subtraction.stderr @@ -49,5 +49,17 @@ 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: aborting due to 10 previous errors From e95ceddbc6aea8bda568bad7793939749558a992 Mon Sep 17 00:00:00 2001 From: Linshu Yang Date: Sat, 13 Dec 2025 19:06:44 +0000 Subject: [PATCH 2/3] fix: `unchecked_time_subtraction` wrongly unmangled macros --- clippy_lints/src/time_subtraction.rs | 5 +++-- tests/ui/unchecked_time_subtraction.fixed | 15 +++++++++++++++ tests/ui/unchecked_time_subtraction.rs | 15 +++++++++++++++ tests/ui/unchecked_time_subtraction.stderr | 14 +++++++++++++- 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/time_subtraction.rs b/clippy_lints/src/time_subtraction.rs index b4a07db9aeda..b3ad13d8488d 100644 --- a/clippy_lints/src/time_subtraction.rs +++ b/clippy_lints/src/time_subtraction.rs @@ -178,8 +178,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, "", &mut applicability); - let right_sugg = Sugg::hir_with_applicability(cx, right_expr, "", &mut applicability); + let left_sugg = Sugg::hir_with_context(cx, left_expr, expr.span.ctxt(), "", &mut applicability); + let right_sugg = + Sugg::hir_with_context(cx, right_expr, expr.span.ctxt(), "", &mut applicability); diag.span_suggestion( expr.span, diff --git a/tests/ui/unchecked_time_subtraction.fixed b/tests/ui/unchecked_time_subtraction.fixed index a75de6c9717c..830b737f18e7 100644 --- a/tests/ui/unchecked_time_subtraction.fixed +++ b/tests/ui/unchecked_time_subtraction.fixed @@ -45,3 +45,18 @@ fn issue16230() { 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 index dd40d8aa14c4..e41860157c41 100644 --- a/tests/ui/unchecked_time_subtraction.rs +++ b/tests/ui/unchecked_time_subtraction.rs @@ -45,3 +45,18 @@ fn issue16230() { 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 index 046024d11f75..fa4bd1db81ae 100644 --- a/tests/ui/unchecked_time_subtraction.stderr +++ b/tests/ui/unchecked_time_subtraction.stderr @@ -61,5 +61,17 @@ error: unchecked subtraction of a `Duration` LL | let _ = Duration::ZERO - Duration::MAX; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Duration::ZERO.checked_sub(Duration::MAX).unwrap()` -error: aborting due to 10 previous errors +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 From e0a74c33352706d6c43fe8a14694241f5833b1eb Mon Sep 17 00:00:00 2001 From: Linshu Yang Date: Sat, 13 Dec 2025 20:57:44 +0000 Subject: [PATCH 3/3] fix: `manual_instant_elapsed` wrongly unmangled macros --- clippy_lints/src/time_subtraction.rs | 13 ++++++------- tests/ui/manual_instant_elapsed.fixed | 16 ++++++++++++++++ tests/ui/manual_instant_elapsed.rs | 16 ++++++++++++++++ tests/ui/manual_instant_elapsed.stderr | 14 +++++++++++++- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/time_subtraction.rs b/clippy_lints/src/time_subtraction.rs index b3ad13d8488d..92bce998c32f 100644 --- a/clippy_lints/src/time_subtraction.rs +++ b/clippy_lints/src/time_subtraction.rs @@ -96,11 +96,8 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ 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) - && let Some(sugg) = Sugg::hir_opt(cx, rhs) - { - print_manual_instant_elapsed_sugg(cx, expr, sugg); + if is_instant_now_call(cx, lhs) && rhs_ty.is_diag_item(cx, sym::Instant) { + print_manual_instant_elapsed_sugg(cx, expr, rhs); } // instant - duration else if rhs_ty.is_diag_item(cx, sym::Duration) @@ -150,7 +147,9 @@ 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(), "", &mut applicability); span_lint_and_sugg( cx, MANUAL_INSTANT_ELAPSED, @@ -158,7 +157,7 @@ fn print_manual_instant_elapsed_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, sugg "manual implementation of `Instant::elapsed`", "try", format!("{}.elapsed()", sugg.maybe_paren()), - Applicability::MachineApplicable, + applicability, ); } diff --git a/tests/ui/manual_instant_elapsed.fixed b/tests/ui/manual_instant_elapsed.fixed index a04c601e08c1..2fa5702f8a04 100644 --- 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 index 7c67f6acf85d..e7a0e6499e74 100644 --- 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 index e84f3126f707..e42ac5739a29 100644 --- 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