diff --git a/clippy_lints/src/time_subtraction.rs b/clippy_lints/src/time_subtraction.rs index e0fdca97dbee..92bce998c32f 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,38 @@ 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) { + 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,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, @@ -161,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, ); } @@ -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, "", &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/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 diff --git a/tests/ui/unchecked_time_subtraction.fixed b/tests/ui/unchecked_time_subtraction.fixed index 2f923fef4c25..830b737f18e7 100644 --- 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 index cf727f62aafa..e41860157c41 100644 --- 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 index c129497447fc..fa4bd1db81ae 100644 --- 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