From 19c7555f061fc2bb6deda357a204e50ed6a0bb18 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Fri, 20 Feb 2026 00:45:45 -0800 Subject: [PATCH] Fix `duration_suboptimal_units` false positive on small literal values For plain integer literals, suppress the lint when the promoted value would be <= 10, as the conversion is not necessarily more readable and keeping the smaller unit makes quick adjustments easier (e.g. changing `from_millis(1_000)` to `from_millis(1_200)` without also changing the function name). Expressions like `10 * 60` continue to always trigger the lint since they already signal intent to compute a converted value. --- clippy_lints/src/duration_suboptimal_units.rs | 7 ++ tests/ui/duration_suboptimal_units.fixed | 38 +++++--- tests/ui/duration_suboptimal_units.rs | 30 ++++-- tests/ui/duration_suboptimal_units.stderr | 96 +++++++------------ ...duration_suboptimal_units_days_weeks.fixed | 11 ++- .../duration_suboptimal_units_days_weeks.rs | 9 +- ...uration_suboptimal_units_days_weeks.stderr | 36 +++---- 7 files changed, 124 insertions(+), 103 deletions(-) diff --git a/clippy_lints/src/duration_suboptimal_units.rs b/clippy_lints/src/duration_suboptimal_units.rs index 19f44f8e959d..b6d6a900000a 100644 --- a/clippy_lints/src/duration_suboptimal_units.rs +++ b/clippy_lints/src/duration_suboptimal_units.rs @@ -87,6 +87,13 @@ impl LateLintPass<'_> for DurationSuboptimalUnits { // There is no need to promote e.g. 0 seconds to 0 hours && value != 0 && let Some((promoted_constructor, promoted_value)) = self.promote(cx, func_name.ident.name, value) + // For plain integer literals, only lint if the promoted value is large enough. + // Small promoted values (e.g. `from_millis(1_000)` -> `from_secs(1)`) are not + // necessarily more readable, and keeping the smaller unit makes quick adjustments + // easier (e.g. changing 1_000 to 1_200 without also changing the function name). + // For expressions (e.g. `10 * 60`), always lint since the expression already + // signals intent to compute a converted value. + && !(matches!(arg.kind, ExprKind::Lit(_)) && promoted_value <= 10) { span_lint_and_then( cx, diff --git a/tests/ui/duration_suboptimal_units.fixed b/tests/ui/duration_suboptimal_units.fixed index a4eb981ebfa2..965c0d1bbd4a 100644 --- a/tests/ui/duration_suboptimal_units.fixed +++ b/tests/ui/duration_suboptimal_units.fixed @@ -10,7 +10,7 @@ macro_rules! mac { 3600 }; (duration) => { - Duration::from_mins(5) + Duration::from_mins(12) //~^ duration_suboptimal_units }; (arg => $e:expr) => { @@ -23,18 +23,28 @@ fn main() { let dur = Duration::from_secs(42); let dur = Duration::from_hours(3); - let dur = Duration::from_mins(1); + // Literals with small promoted values should not lint (issue #16532) + let dur = Duration::from_secs(60); + let dur = Duration::from_secs(180); + let dur = Duration::from_millis(5_000); + let dur = Duration::from_millis(1_000); + let dur = Duration::from_secs(3_600); + + // Literals with large promoted values should still lint + let dur = Duration::from_mins(11); //~^ duration_suboptimal_units - let dur = Duration::from_mins(3); + let dur = Duration::from_mins(11); //~^ duration_suboptimal_units + + // Expressions should always lint regardless of the promoted value let dur = Duration::from_mins(10); //~^ duration_suboptimal_units let dur = Duration::from_hours(24); //~^ duration_suboptimal_units - let dur = Duration::from_secs(5); - //~^ duration_suboptimal_units let dur = Duration::from_hours(13); //~^ duration_suboptimal_units + let dur = Duration::from_mins(1); + //~^ duration_suboptimal_units // Constants are intentionally not resolved, as we don't want to recommend a literal value over // using constants. @@ -44,22 +54,26 @@ fn main() { const { let dur = Duration::from_secs(0); - let dur = Duration::from_secs(5); - //~^ duration_suboptimal_units - let dur = Duration::from_mins(3); - //~^ duration_suboptimal_units + // Literals with small promoted values should not lint in const blocks either + let dur = Duration::from_millis(5_000); + let dur = Duration::from_secs(180); + + // Expressions should still lint in const blocks let dur = Duration::from_hours(24); //~^ duration_suboptimal_units let dur = Duration::from_secs(SIXTY); } - // Qualified Durations must be kept - std::time::Duration::from_mins(1); + // Qualified Durations with small promoted values should not lint + std::time::Duration::from_secs(60); + + // Qualified Durations with expressions should still lint + std::time::Duration::from_mins(10); //~^ duration_suboptimal_units // We lint in normal macros - assert_eq!(Duration::from_hours(1), Duration::from_mins(6)); + assert_eq!(Duration::from_mins(10), Duration::from_mins(6)); //~^ duration_suboptimal_units // We lint in normal macros (marker is in macro itself) diff --git a/tests/ui/duration_suboptimal_units.rs b/tests/ui/duration_suboptimal_units.rs index e31ca679b5a1..d63b489ed539 100644 --- a/tests/ui/duration_suboptimal_units.rs +++ b/tests/ui/duration_suboptimal_units.rs @@ -10,7 +10,7 @@ macro_rules! mac { 3600 }; (duration) => { - Duration::from_secs(300) + Duration::from_secs(12 * 60) //~^ duration_suboptimal_units }; (arg => $e:expr) => { @@ -23,18 +23,28 @@ fn main() { let dur = Duration::from_secs(42); let dur = Duration::from_hours(3); + // Literals with small promoted values should not lint (issue #16532) let dur = Duration::from_secs(60); - //~^ duration_suboptimal_units let dur = Duration::from_secs(180); + let dur = Duration::from_millis(5_000); + let dur = Duration::from_millis(1_000); + let dur = Duration::from_secs(3_600); + + // Literals with large promoted values should still lint + let dur = Duration::from_secs(660); //~^ duration_suboptimal_units + let dur = Duration::from_millis(660_000); + //~^ duration_suboptimal_units + + // Expressions should always lint regardless of the promoted value let dur = Duration::from_secs(10 * 60); //~^ duration_suboptimal_units let dur = Duration::from_mins(24 * 60); //~^ duration_suboptimal_units - let dur = Duration::from_millis(5_000); - //~^ duration_suboptimal_units let dur = Duration::from_nanos(13 * 60 * 60 * 1_000 * 1_000 * 1_000); //~^ duration_suboptimal_units + let dur = Duration::from_secs(2 * 30); + //~^ duration_suboptimal_units // Constants are intentionally not resolved, as we don't want to recommend a literal value over // using constants. @@ -44,22 +54,26 @@ fn main() { const { let dur = Duration::from_secs(0); + // Literals with small promoted values should not lint in const blocks either let dur = Duration::from_millis(5_000); - //~^ duration_suboptimal_units let dur = Duration::from_secs(180); - //~^ duration_suboptimal_units + + // Expressions should still lint in const blocks let dur = Duration::from_mins(24 * 60); //~^ duration_suboptimal_units let dur = Duration::from_secs(SIXTY); } - // Qualified Durations must be kept + // Qualified Durations with small promoted values should not lint std::time::Duration::from_secs(60); + + // Qualified Durations with expressions should still lint + std::time::Duration::from_secs(10 * 60); //~^ duration_suboptimal_units // We lint in normal macros - assert_eq!(Duration::from_secs(3_600), Duration::from_mins(6)); + assert_eq!(Duration::from_secs(10 * 60), Duration::from_mins(6)); //~^ duration_suboptimal_units // We lint in normal macros (marker is in macro itself) diff --git a/tests/ui/duration_suboptimal_units.stderr b/tests/ui/duration_suboptimal_units.stderr index 97b26f6667eb..6c51512f9e9c 100644 --- a/tests/ui/duration_suboptimal_units.stderr +++ b/tests/ui/duration_suboptimal_units.stderr @@ -1,31 +1,31 @@ error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:26:15 + --> tests/ui/duration_suboptimal_units.rs:34:15 | -LL | let dur = Duration::from_secs(60); - | ^^^^^^^^^^^^^^^^^^^^^^^ +LL | let dur = Duration::from_secs(660); + | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::duration-suboptimal-units` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::duration_suboptimal_units)]` help: try using from_mins | -LL - let dur = Duration::from_secs(60); -LL + let dur = Duration::from_mins(1); +LL - let dur = Duration::from_secs(660); +LL + let dur = Duration::from_mins(11); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:28:15 + --> tests/ui/duration_suboptimal_units.rs:36:15 | -LL | let dur = Duration::from_secs(180); - | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let dur = Duration::from_millis(660_000); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: try using from_mins | -LL - let dur = Duration::from_secs(180); -LL + let dur = Duration::from_mins(3); +LL - let dur = Duration::from_millis(660_000); +LL + let dur = Duration::from_mins(11); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:30:15 + --> tests/ui/duration_suboptimal_units.rs:40:15 | LL | let dur = Duration::from_secs(10 * 60); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -37,7 +37,7 @@ LL + let dur = Duration::from_mins(10); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:32:15 + --> tests/ui/duration_suboptimal_units.rs:42:15 | LL | let dur = Duration::from_mins(24 * 60); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -49,19 +49,7 @@ LL + let dur = Duration::from_hours(24); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:34:15 - | -LL | let dur = Duration::from_millis(5_000); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try using from_secs - | -LL - let dur = Duration::from_millis(5_000); -LL + let dur = Duration::from_secs(5); - | - -error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:36:15 + --> tests/ui/duration_suboptimal_units.rs:44:15 | LL | let dur = Duration::from_nanos(13 * 60 * 60 * 1_000 * 1_000 * 1_000); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -73,31 +61,19 @@ LL + let dur = Duration::from_hours(13); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:47:19 - | -LL | let dur = Duration::from_millis(5_000); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try using from_secs - | -LL - let dur = Duration::from_millis(5_000); -LL + let dur = Duration::from_secs(5); - | - -error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:49:19 + --> tests/ui/duration_suboptimal_units.rs:46:15 | -LL | let dur = Duration::from_secs(180); - | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let dur = Duration::from_secs(2 * 30); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: try using from_mins | -LL - let dur = Duration::from_secs(180); -LL + let dur = Duration::from_mins(3); +LL - let dur = Duration::from_secs(2 * 30); +LL + let dur = Duration::from_mins(1); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:51:19 + --> tests/ui/duration_suboptimal_units.rs:62:19 | LL | let dur = Duration::from_mins(24 * 60); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -109,34 +85,34 @@ LL + let dur = Duration::from_hours(24); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:58:5 + --> tests/ui/duration_suboptimal_units.rs:72:5 | -LL | std::time::Duration::from_secs(60); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | std::time::Duration::from_secs(10 * 60); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: try using from_mins | -LL - std::time::Duration::from_secs(60); -LL + std::time::Duration::from_mins(1); +LL - std::time::Duration::from_secs(10 * 60); +LL + std::time::Duration::from_mins(10); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:62:16 + --> tests/ui/duration_suboptimal_units.rs:76:16 | -LL | assert_eq!(Duration::from_secs(3_600), Duration::from_mins(6)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | assert_eq!(Duration::from_secs(10 * 60), Duration::from_mins(6)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: try using from_hours +help: try using from_mins | -LL - assert_eq!(Duration::from_secs(3_600), Duration::from_mins(6)); -LL + assert_eq!(Duration::from_hours(1), Duration::from_mins(6)); +LL - assert_eq!(Duration::from_secs(10 * 60), Duration::from_mins(6)); +LL + assert_eq!(Duration::from_mins(10), Duration::from_mins(6)); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable --> tests/ui/duration_suboptimal_units.rs:13:9 | -LL | Duration::from_secs(300) - | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | Duration::from_secs(12 * 60) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | let dur = mac!(duration); | -------------- in this macro invocation @@ -144,12 +120,12 @@ LL | let dur = mac!(duration); = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info) help: try using from_mins | -LL - Duration::from_secs(300) -LL + Duration::from_mins(5) +LL - Duration::from_secs(12 * 60) +LL + Duration::from_mins(12) | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units.rs:105:9 + --> tests/ui/duration_suboptimal_units.rs:119:9 | LL | _ = Duration::from_secs(67_768_040_922_076_800); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -160,5 +136,5 @@ LL - _ = Duration::from_secs(67_768_040_922_076_800); LL + _ = Duration::from_hours(18824455811688); | -error: aborting due to 13 previous errors +error: aborting due to 11 previous errors diff --git a/tests/ui/duration_suboptimal_units_days_weeks.fixed b/tests/ui/duration_suboptimal_units_days_weeks.fixed index b0abcbb7bf03..a42103470eed 100644 --- a/tests/ui/duration_suboptimal_units_days_weeks.fixed +++ b/tests/ui/duration_suboptimal_units_days_weeks.fixed @@ -6,12 +6,17 @@ use std::time::Duration; fn main() { - let dur = Duration::from_mins(1); - //~^ duration_suboptimal_units + // Literals with small promoted values should not lint (issue #16532) + let dur = Duration::from_secs(60); + let dur = Duration::from_hours(24); - let dur = Duration::from_days(1); + // Literals with large promoted values should still lint + let dur = Duration::from_days(11); //~^ duration_suboptimal_units + // Expressions should always lint let dur = Duration::from_weeks(13); //~^ duration_suboptimal_units + let dur = Duration::from_days(1); + //~^ duration_suboptimal_units } diff --git a/tests/ui/duration_suboptimal_units_days_weeks.rs b/tests/ui/duration_suboptimal_units_days_weeks.rs index 663476905c0f..dad6e9b1bbe0 100644 --- a/tests/ui/duration_suboptimal_units_days_weeks.rs +++ b/tests/ui/duration_suboptimal_units_days_weeks.rs @@ -6,12 +6,17 @@ use std::time::Duration; fn main() { + // Literals with small promoted values should not lint (issue #16532) let dur = Duration::from_secs(60); - //~^ duration_suboptimal_units - let dur = Duration::from_hours(24); + + // Literals with large promoted values should still lint + let dur = Duration::from_hours(264); //~^ duration_suboptimal_units + // Expressions should always lint let dur = Duration::from_nanos(13 * 7 * 24 * 60 * 60 * 1_000 * 1_000 * 1_000); //~^ duration_suboptimal_units + let dur = Duration::from_hours(2 * 12); + //~^ duration_suboptimal_units } diff --git a/tests/ui/duration_suboptimal_units_days_weeks.stderr b/tests/ui/duration_suboptimal_units_days_weeks.stderr index 98325358bfa6..42fddaf67e67 100644 --- a/tests/ui/duration_suboptimal_units_days_weeks.stderr +++ b/tests/ui/duration_suboptimal_units_days_weeks.stderr @@ -1,31 +1,19 @@ error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units_days_weeks.rs:9:15 + --> tests/ui/duration_suboptimal_units_days_weeks.rs:14:15 | -LL | let dur = Duration::from_secs(60); - | ^^^^^^^^^^^^^^^^^^^^^^^ +LL | let dur = Duration::from_hours(264); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::duration-suboptimal-units` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::duration_suboptimal_units)]` -help: try using from_mins - | -LL - let dur = Duration::from_secs(60); -LL + let dur = Duration::from_mins(1); - | - -error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units_days_weeks.rs:12:15 - | -LL | let dur = Duration::from_hours(24); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | help: try using from_days | -LL - let dur = Duration::from_hours(24); -LL + let dur = Duration::from_days(1); +LL - let dur = Duration::from_hours(264); +LL + let dur = Duration::from_days(11); | error: constructing a `Duration` using a smaller unit when a larger unit would be more readable - --> tests/ui/duration_suboptimal_units_days_weeks.rs:15:15 + --> tests/ui/duration_suboptimal_units_days_weeks.rs:18:15 | LL | let dur = Duration::from_nanos(13 * 7 * 24 * 60 * 60 * 1_000 * 1_000 * 1_000); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -36,5 +24,17 @@ LL - let dur = Duration::from_nanos(13 * 7 * 24 * 60 * 60 * 1_000 * 1_000 * LL + let dur = Duration::from_weeks(13); | +error: constructing a `Duration` using a smaller unit when a larger unit would be more readable + --> tests/ui/duration_suboptimal_units_days_weeks.rs:20:15 + | +LL | let dur = Duration::from_hours(2 * 12); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try using from_days + | +LL - let dur = Duration::from_hours(2 * 12); +LL + let dur = Duration::from_days(1); + | + error: aborting due to 3 previous errors