From ef1ee716663575e52ff6c3af1b7f0060b15d7455 Mon Sep 17 00:00:00 2001 From: linshuy2 Date: Fri, 6 Feb 2026 22:30:39 +0000 Subject: [PATCH] Enhance `unchecked_time_subtraction` to better handle `Duration` literals --- clippy_lints/src/time_subtraction.rs | 72 ++++++++++++++++++- clippy_utils/src/sym.rs | 3 + tests/ui/unchecked_time_subtraction.fixed | 16 +++-- tests/ui/unchecked_time_subtraction.rs | 14 +++- tests/ui/unchecked_time_subtraction.stderr | 24 +++---- .../unchecked_time_subtraction_unfixable.rs | 9 +++ ...nchecked_time_subtraction_unfixable.stderr | 26 ++++++- 7 files changed, 138 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/time_subtraction.rs b/clippy_lints/src/time_subtraction.rs index c241935d6c31..457a8f48f7e2 100644 --- a/clippy_lints/src/time_subtraction.rs +++ b/clippy_lints/src/time_subtraction.rs @@ -1,14 +1,18 @@ +use std::time::Duration; + use clippy_config::Conf; -use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::consts::{ConstEvalCtxt, Constant}; +use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::res::{MaybeDef, MaybeTypeckRes}; use clippy_utils::sugg::Sugg; +use clippy_utils::sym; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::Ty; use rustc_session::impl_lint_pass; -use rustc_span::sym; +use rustc_span::SyntaxContext; declare_clippy_lint! { /// ### What it does @@ -111,6 +115,27 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { && !expr.span.from_expansion() && self.msrv.meets(cx, msrvs::TRY_FROM) { + let const_eval = ConstEvalCtxt::new(cx); + let ctxt = expr.span.ctxt(); + if let Some(lhs) = const_eval_duration(&const_eval, lhs, ctxt) + && let Some(rhs) = const_eval_duration(&const_eval, rhs, ctxt) + { + if lhs >= rhs { + // If the duration subtraction can be proven to not underflow, then we don't lint + return; + } + + span_lint_and_note( + cx, + UNCHECKED_TIME_SUBTRACTION, + expr.span, + "unchecked subtraction of two `Duration` that will underflow", + None, + "if this is intentional, consider allowing the lint", + ); + return; + } + print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr); } } @@ -189,3 +214,44 @@ fn print_unchecked_duration_subtraction_sugg( }, ); } + +fn const_eval_duration(const_eval: &ConstEvalCtxt<'_>, expr: &Expr<'_>, ctxt: SyntaxContext) -> Option { + if let ExprKind::Call(func, args) = expr.kind + && let ExprKind::Path(QPath::TypeRelative(_, func_name)) = func.kind + { + macro_rules! try_parse_duration { + (($( $name:ident : $var:ident ( $ty:ty ) ),+ $(,)?) -> $ctor:ident ( $($args:tt)* )) => {{ + let [$( $name ),+] = args else { return None }; + $( + let Some(Constant::$var(v)) = const_eval.eval_local($name, ctxt) else { return None }; + let $name = <$ty>::try_from(v).ok()?; + )+ + Some(Duration::$ctor($($args)*)) + }}; + } + + return match func_name.ident.name { + sym::new => try_parse_duration! { (secs: Int(u64), nanos: Int(u32)) -> new(secs, nanos) }, + sym::from_nanos => try_parse_duration! { (nanos: Int(u64)) -> from_nanos(nanos) }, + sym::from_nanos_u128 => try_parse_duration! { (nanos: Int(u128)) -> from_nanos_u128(nanos) }, + sym::from_micros => try_parse_duration! { (micros: Int(u64)) -> from_micros(micros) }, + sym::from_millis => try_parse_duration! { (millis: Int(u64)) -> from_millis(millis) }, + sym::from_secs => try_parse_duration! { (secs: Int(u64)) -> from_secs(secs) }, + sym::from_secs_f32 => try_parse_duration! { (secs: F32(f32)) -> from_secs_f32(secs) }, + sym::from_secs_f64 => try_parse_duration! { (secs: F64(f64)) -> from_secs_f64(secs) }, + sym::from_mins => try_parse_duration! { (mins: Int(u64)) -> from_mins(mins) }, + sym::from_hours => { + try_parse_duration! { (hours: Int(u64)) -> from_hours(hours) } + }, + sym::from_days => { + try_parse_duration! { (days: Int(u64)) -> from_hours(days * 24) } + }, + sym::from_weeks => { + try_parse_duration! { (weeks: Int(u64)) -> from_hours(weeks * 24 * 7) } + }, + _ => None, + }; + } + + None +} diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index bd99525a86fb..9c274efa72be 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -173,11 +173,14 @@ macro_rules! generate { from_millis, from_mins, from_nanos, + from_nanos_u128, from_ne_bytes, from_ptr, from_raw, from_raw_parts, from_secs, + from_secs_f32, + from_secs_f64, from_str_radix, from_weeks, fs, diff --git a/tests/ui/unchecked_time_subtraction.fixed b/tests/ui/unchecked_time_subtraction.fixed index 830b737f18e7..5fe51dd5ba7c 100644 --- a/tests/ui/unchecked_time_subtraction.fixed +++ b/tests/ui/unchecked_time_subtraction.fixed @@ -25,8 +25,7 @@ fn main() { let _ = dur1.checked_sub(dur2).unwrap(); //~^ unchecked_time_subtraction - let _ = Duration::from_secs(10).checked_sub(Duration::from_secs(5)).unwrap(); - //~^ unchecked_time_subtraction + let _ = Duration::from_secs(10) - Duration::from_secs(5); let _ = second.checked_sub(dur1).unwrap(); //~^ unchecked_time_subtraction @@ -55,8 +54,17 @@ fn issue16234() { }; } - duration!(0).checked_sub(duration!(1)).unwrap(); + let d = duration!(0); + d.checked_sub(duration!(1)).unwrap(); //~^ unchecked_time_subtraction - let _ = duration!(0).checked_sub(duration!(1)).unwrap(); + let _ = d.checked_sub(duration!(1)).unwrap(); //~^ unchecked_time_subtraction } + +fn issue16499() { + let _ = Duration::from_millis(2) - Duration::from_millis(1); + let _ = Duration::new(10000, 0) - Duration::from_secs(1); + let _ = Duration::from_nanos_u128(1000) - Duration::from_nanos(100); + let _ = Duration::from_secs_f32(1.5) - Duration::from_secs_f64(0.5); + let _ = Duration::from_mins(70) - Duration::from_hours(1); +} diff --git a/tests/ui/unchecked_time_subtraction.rs b/tests/ui/unchecked_time_subtraction.rs index e41860157c41..466d96a9a606 100644 --- a/tests/ui/unchecked_time_subtraction.rs +++ b/tests/ui/unchecked_time_subtraction.rs @@ -26,7 +26,6 @@ fn main() { //~^ unchecked_time_subtraction let _ = Duration::from_secs(10) - Duration::from_secs(5); - //~^ unchecked_time_subtraction let _ = second - dur1; //~^ unchecked_time_subtraction @@ -55,8 +54,17 @@ macro_rules! duration { }; } - duration!(0).sub(duration!(1)); + let d = duration!(0); + d.sub(duration!(1)); //~^ unchecked_time_subtraction - let _ = duration!(0) - duration!(1); + let _ = d - duration!(1); //~^ unchecked_time_subtraction } + +fn issue16499() { + let _ = Duration::from_millis(2) - Duration::from_millis(1); + let _ = Duration::new(10000, 0) - Duration::from_secs(1); + let _ = Duration::from_nanos_u128(1000) - Duration::from_nanos(100); + let _ = Duration::from_secs_f32(1.5) - Duration::from_secs_f64(0.5); + let _ = Duration::from_mins(70) - Duration::from_hours(1); +} diff --git a/tests/ui/unchecked_time_subtraction.stderr b/tests/ui/unchecked_time_subtraction.stderr index fa4bd1db81ae..b71e2c474f2e 100644 --- a/tests/ui/unchecked_time_subtraction.stderr +++ b/tests/ui/unchecked_time_subtraction.stderr @@ -32,31 +32,25 @@ LL | let _ = dur1 - dur2; | ^^^^^^^^^^^ help: try: `dur1.checked_sub(dur2).unwrap()` error: unchecked subtraction of a `Duration` - --> tests/ui/unchecked_time_subtraction.rs:28:13 - | -LL | let _ = Duration::from_secs(10) - Duration::from_secs(5); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Duration::from_secs(10).checked_sub(Duration::from_secs(5)).unwrap()` - -error: unchecked subtraction of a `Duration` - --> tests/ui/unchecked_time_subtraction.rs:31:13 + --> tests/ui/unchecked_time_subtraction.rs:30:13 | LL | let _ = second - dur1; | ^^^^^^^^^^^^^ help: try: `second.checked_sub(dur1).unwrap()` error: unchecked subtraction of a `Duration` - --> tests/ui/unchecked_time_subtraction.rs:35:13 + --> tests/ui/unchecked_time_subtraction.rs:34:13 | LL | let _ = 2 * dur1 - dur2; | ^^^^^^^^^^^^^^^ help: try: `(2 * dur1).checked_sub(dur2).unwrap()` error: unchecked subtraction of a `Duration` - --> tests/ui/unchecked_time_subtraction.rs:42:5 + --> tests/ui/unchecked_time_subtraction.rs:41: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 + --> tests/ui/unchecked_time_subtraction.rs:44:13 | LL | let _ = Duration::ZERO - Duration::MAX; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Duration::ZERO.checked_sub(Duration::MAX).unwrap()` @@ -64,14 +58,14 @@ LL | let _ = Duration::ZERO - Duration::MAX; 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()` +LL | d.sub(duration!(1)); + | ^^^^^^^^^^^^^^^^^^^ help: try: `d.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()` +LL | let _ = d - duration!(1); + | ^^^^^^^^^^^^^^^^ help: try: `d.checked_sub(duration!(1)).unwrap()` -error: aborting due to 12 previous errors +error: aborting due to 11 previous errors diff --git a/tests/ui/unchecked_time_subtraction_unfixable.rs b/tests/ui/unchecked_time_subtraction_unfixable.rs index 4b6a5ca15620..6c3777eec915 100644 --- a/tests/ui/unchecked_time_subtraction_unfixable.rs +++ b/tests/ui/unchecked_time_subtraction_unfixable.rs @@ -20,3 +20,12 @@ fn main() { //~^ unchecked_time_subtraction //~| unchecked_time_subtraction } + +fn issue16499() { + let _ = Duration::from_millis(1) - Duration::from_millis(2); + //~^ unchecked_time_subtraction + let _ = Duration::from_millis(1) - Duration::from_mins(2); + //~^ unchecked_time_subtraction + let _ = Duration::from_nanos(1) - Duration::from_secs(1); + //~^ unchecked_time_subtraction +} diff --git a/tests/ui/unchecked_time_subtraction_unfixable.stderr b/tests/ui/unchecked_time_subtraction_unfixable.stderr index 017e5b1c7c11..a48676bf5c16 100644 --- a/tests/ui/unchecked_time_subtraction_unfixable.stderr +++ b/tests/ui/unchecked_time_subtraction_unfixable.stderr @@ -25,5 +25,29 @@ error: unchecked subtraction of a `Duration` LL | let _ = instant1 - dur2 - dur3; | ^^^^^^^^^^^^^^^ help: try: `instant1.checked_sub(dur2).unwrap()` -error: aborting due to 4 previous errors +error: unchecked subtraction of two `Duration` that will underflow + --> tests/ui/unchecked_time_subtraction_unfixable.rs:25:13 + | +LL | let _ = Duration::from_millis(1) - Duration::from_millis(2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: if this is intentional, consider allowing the lint + +error: unchecked subtraction of two `Duration` that will underflow + --> tests/ui/unchecked_time_subtraction_unfixable.rs:27:13 + | +LL | let _ = Duration::from_millis(1) - Duration::from_mins(2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: if this is intentional, consider allowing the lint + +error: unchecked subtraction of two `Duration` that will underflow + --> tests/ui/unchecked_time_subtraction_unfixable.rs:29:13 + | +LL | let _ = Duration::from_nanos(1) - Duration::from_secs(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: if this is intentional, consider allowing the lint + +error: aborting due to 7 previous errors