Skip to content

Commit f296de4

Browse files
committed
Implement Duration - Duration
Renames InstantSubtraction to UncheckedTimeSubtraction
1 parent b0566b9 commit f296de4

8 files changed

+201
-27
lines changed

clippy_lints/src/declared_lints.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
226226
crate::inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY_INFO,
227227
crate::init_numbered_fields::INIT_NUMBERED_FIELDS_INFO,
228228
crate::inline_fn_without_body::INLINE_FN_WITHOUT_BODY_INFO,
229-
crate::time_subtraction::MANUAL_INSTANT_ELAPSED_INFO,
230-
crate::time_subtraction::UNCHECKED_TIME_SUBTRACTION_INFO,
231229
crate::int_plus_one::INT_PLUS_ONE_INFO,
232230
crate::integer_division_remainder_used::INTEGER_DIVISION_REMAINDER_USED_INFO,
233231
crate::invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS_INFO,
@@ -703,6 +701,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
703701
crate::tabs_in_doc_comments::TABS_IN_DOC_COMMENTS_INFO,
704702
crate::temporary_assignment::TEMPORARY_ASSIGNMENT_INFO,
705703
crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
704+
crate::time_subtraction::MANUAL_INSTANT_ELAPSED_INFO,
705+
crate::time_subtraction::UNCHECKED_TIME_SUBTRACTION_INFO,
706706
crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
707707
crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO,
708708
crate::toplevel_ref_arg::TOPLEVEL_REF_ARG_INFO,

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
718718
store.register_late_pass(move |_| Box::new(manual_rotate::ManualRotate));
719719
store.register_late_pass(move |_| Box::new(operators::Operators::new(conf)));
720720
store.register_late_pass(move |_| Box::new(std_instead_of_core::StdReexports::new(conf)));
721-
store.register_late_pass(move |_| Box::new(time_subtraction::InstantSubtraction::new(conf)));
721+
store.register_late_pass(move |_| Box::new(time_subtraction::UncheckedTimeSubtraction::new(conf)));
722722
store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));
723723
store.register_late_pass(move |_| Box::new(manual_abs_diff::ManualAbsDiff::new(conf)));
724724
store.register_late_pass(move |_| Box::new(manual_clamp::ManualClamp::new(conf)));

clippy_lints/src/time_subtraction.rs

Lines changed: 88 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use clippy_config::Conf;
2-
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
33
use clippy_utils::msrvs::{self, Msrv};
4-
use clippy_utils::source::snippet_with_context;
54
use clippy_utils::sugg::Sugg;
65
use clippy_utils::{is_path_diagnostic_item, ty};
76
use rustc_errors::Applicability;
87
use rustc_hir::{BinOpKind, Expr, ExprKind};
98
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::ty::Ty;
1010
use rustc_session::impl_lint_pass;
1111
use rustc_span::source_map::Spanned;
1212
use rustc_span::sym;
@@ -41,7 +41,7 @@ declare_clippy_lint! {
4141

4242
declare_clippy_lint! {
4343
/// ### What it does
44-
/// Lints subtraction between an `Instant` and a `Duration`.
44+
/// Lints subtraction between an `Instant` and a `Duration`, or between two `Duration` values.
4545
///
4646
/// ### Why is this bad?
4747
/// Unchecked subtraction could cause underflow on certain platforms, leading to
@@ -51,32 +51,38 @@ declare_clippy_lint! {
5151
/// ```no_run
5252
/// # use std::time::{Instant, Duration};
5353
/// let time_passed = Instant::now() - Duration::from_secs(5);
54+
/// let dur1 = Duration::from_secs(3);
55+
/// let dur2 = Duration::from_secs(5);
56+
/// let diff = dur1 - dur2;
5457
/// ```
5558
///
5659
/// Use instead:
5760
/// ```no_run
5861
/// # use std::time::{Instant, Duration};
5962
/// let time_passed = Instant::now().checked_sub(Duration::from_secs(5));
63+
/// let dur1 = Duration::from_secs(3);
64+
/// let dur2 = Duration::from_secs(5);
65+
/// let diff = dur1.checked_sub(dur2);
6066
/// ```
6167
#[clippy::version = "1.67.0"]
6268
pub UNCHECKED_TIME_SUBTRACTION,
6369
pedantic,
64-
"finds unchecked subtraction of a 'Duration' from an 'Instant'"
70+
"finds unchecked subtraction involving 'Duration' or 'Instant'"
6571
}
6672

67-
pub struct InstantSubtraction {
73+
pub struct UncheckedTimeSubtraction {
6874
msrv: Msrv,
6975
}
7076

71-
impl InstantSubtraction {
77+
impl UncheckedTimeSubtraction {
7278
pub fn new(conf: &'static Conf) -> Self {
7379
Self { msrv: conf.msrv }
7480
}
7581
}
7682

77-
impl_lint_pass!(InstantSubtraction => [MANUAL_INSTANT_ELAPSED, UNCHECKED_TIME_SUBTRACTION]);
83+
impl_lint_pass!(UncheckedTimeSubtraction => [MANUAL_INSTANT_ELAPSED, UNCHECKED_TIME_SUBTRACTION]);
7884

79-
impl LateLintPass<'_> for InstantSubtraction {
85+
impl LateLintPass<'_> for UncheckedTimeSubtraction {
8086
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
8187
if let ExprKind::Binary(
8288
Spanned {
@@ -85,21 +91,54 @@ impl LateLintPass<'_> for InstantSubtraction {
8591
lhs,
8692
rhs,
8793
) = expr.kind
88-
&& let typeck = cx.typeck_results()
89-
&& ty::is_type_diagnostic_item(cx, typeck.expr_ty(lhs), sym::Instant)
9094
{
95+
let typeck = cx.typeck_results();
96+
let lhs_ty = typeck.expr_ty(lhs);
9197
let rhs_ty = typeck.expr_ty(rhs);
9298

93-
if is_instant_now_call(cx, lhs)
94-
&& ty::is_type_diagnostic_item(cx, rhs_ty, sym::Instant)
95-
&& let Some(sugg) = Sugg::hir_opt(cx, rhs)
96-
{
97-
print_manual_instant_elapsed_sugg(cx, expr, sugg);
98-
} else if ty::is_type_diagnostic_item(cx, rhs_ty, sym::Duration)
99+
if ty::is_type_diagnostic_item(cx, lhs_ty, sym::Instant) {
100+
// Instant::now() - instant
101+
if is_instant_now_call(cx, lhs)
102+
&& ty::is_type_diagnostic_item(cx, rhs_ty, sym::Instant)
103+
&& let Some(sugg) = Sugg::hir_opt(cx, rhs)
104+
{
105+
print_manual_instant_elapsed_sugg(cx, expr, sugg);
106+
}
107+
// instant - duration
108+
else if ty::is_type_diagnostic_item(cx, rhs_ty, sym::Duration)
109+
&& !expr.span.from_expansion()
110+
&& self.msrv.meets(cx, msrvs::TRY_FROM)
111+
{
112+
// For chained subtraction like (instant - dur1) - dur2, avoid suggestions
113+
if is_chained_time_subtraction(cx, lhs) {
114+
span_lint(
115+
cx,
116+
UNCHECKED_TIME_SUBTRACTION,
117+
expr.span,
118+
"unchecked subtraction of a 'Duration' from an 'Instant'",
119+
);
120+
} else {
121+
// instant - duration
122+
print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
123+
}
124+
}
125+
} else if ty::is_type_diagnostic_item(cx, lhs_ty, sym::Duration)
126+
&& ty::is_type_diagnostic_item(cx, rhs_ty, sym::Duration)
99127
&& !expr.span.from_expansion()
100128
&& self.msrv.meets(cx, msrvs::TRY_FROM)
101129
{
102-
print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
130+
// For chained subtraction like (dur1 - dur2) - dur3, avoid suggestions
131+
if is_chained_time_subtraction(cx, lhs) {
132+
span_lint(
133+
cx,
134+
UNCHECKED_TIME_SUBTRACTION,
135+
expr.span,
136+
"unchecked subtraction between 'Duration' values",
137+
);
138+
} else {
139+
// duration - duration
140+
print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
141+
}
103142
}
104143
}
105144
}
@@ -115,6 +154,25 @@ fn is_instant_now_call(cx: &LateContext<'_>, expr_block: &'_ Expr<'_>) -> bool {
115154
}
116155
}
117156

157+
/// Returns true if this subtraction is part of a chain like `(a - b) - c`
158+
fn is_chained_time_subtraction(cx: &LateContext<'_>, lhs: &Expr<'_>) -> bool {
159+
if let ExprKind::Binary(op, inner_lhs, inner_rhs) = &lhs.kind
160+
&& matches!(op.node, BinOpKind::Sub)
161+
{
162+
let typeck = cx.typeck_results();
163+
let left_ty = typeck.expr_ty(inner_lhs);
164+
let right_ty = typeck.expr_ty(inner_rhs);
165+
is_time_type(cx, left_ty) && is_time_type(cx, right_ty)
166+
} else {
167+
false
168+
}
169+
}
170+
171+
/// Returns true if the type is Duration or Instant
172+
fn is_time_type(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
173+
ty::is_type_diagnostic_item(cx, ty, sym::Duration) || ty::is_type_diagnostic_item(cx, ty, sym::Instant)
174+
}
175+
118176
fn print_manual_instant_elapsed_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, sugg: Sugg<'_>) {
119177
span_lint_and_sugg(
120178
cx,
@@ -133,19 +191,26 @@ fn print_unchecked_duration_subtraction_sugg(
133191
right_expr: &Expr<'_>,
134192
expr: &Expr<'_>,
135193
) {
136-
let mut applicability = Applicability::MachineApplicable;
194+
let typeck = cx.typeck_results();
195+
let left_ty = typeck.expr_ty(left_expr);
137196

138-
let ctxt = expr.span.ctxt();
139-
let left_expr = snippet_with_context(cx, left_expr.span, ctxt, "<instant>", &mut applicability).0;
140-
let right_expr = snippet_with_context(cx, right_expr.span, ctxt, "<duration>", &mut applicability).0;
197+
let lint_msg = if ty::is_type_diagnostic_item(cx, left_ty, sym::Instant) {
198+
"unchecked subtraction of a 'Duration' from an 'Instant'"
199+
} else {
200+
"unchecked subtraction between 'Duration' values"
201+
};
202+
203+
let mut applicability = Applicability::MachineApplicable;
204+
let left_sugg = Sugg::hir_with_applicability(cx, left_expr, "<left>", &mut applicability);
205+
let right_sugg = Sugg::hir_with_applicability(cx, right_expr, "<right>", &mut applicability);
141206

142207
span_lint_and_sugg(
143208
cx,
144209
UNCHECKED_TIME_SUBTRACTION,
145210
expr.span,
146-
"unchecked subtraction of a 'Duration' from an 'Instant'",
211+
lint_msg,
147212
"try",
148-
format!("{left_expr}.checked_sub({right_expr}).unwrap()"),
213+
format!("{}.checked_sub({}).unwrap()", left_sugg.maybe_paren(), right_sugg),
149214
applicability,
150215
);
151216
}

tests/ui/unchecked_time_subtraction.fixed

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,21 @@ fn main() {
1717

1818
let _ = Instant::now().checked_sub(second).unwrap();
1919
//~^ unchecked_time_subtraction
20+
21+
// Duration - Duration cases
22+
let dur1 = Duration::from_secs(5);
23+
let dur2 = Duration::from_secs(3);
24+
25+
let _ = dur1.checked_sub(dur2).unwrap();
26+
//~^ unchecked_time_subtraction
27+
28+
let _ = Duration::from_secs(10).checked_sub(Duration::from_secs(5)).unwrap();
29+
//~^ unchecked_time_subtraction
30+
31+
let _ = second.checked_sub(dur1).unwrap();
32+
//~^ unchecked_time_subtraction
33+
34+
// Duration multiplication and subtraction
35+
let _ = (2 * dur1).checked_sub(dur2).unwrap();
36+
//~^ unchecked_time_subtraction
2037
}

tests/ui/unchecked_time_subtraction.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,21 @@ fn main() {
1717

1818
let _ = Instant::now() - second;
1919
//~^ unchecked_time_subtraction
20+
21+
// Duration - Duration cases
22+
let dur1 = Duration::from_secs(5);
23+
let dur2 = Duration::from_secs(3);
24+
25+
let _ = dur1 - dur2;
26+
//~^ unchecked_time_subtraction
27+
28+
let _ = Duration::from_secs(10) - Duration::from_secs(5);
29+
//~^ unchecked_time_subtraction
30+
31+
let _ = second - dur1;
32+
//~^ unchecked_time_subtraction
33+
34+
// Duration multiplication and subtraction
35+
let _ = 2 * dur1 - dur2;
36+
//~^ unchecked_time_subtraction
2037
}

tests/ui/unchecked_time_subtraction.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,29 @@ error: unchecked subtraction of a 'Duration' from an 'Instant'
2525
LL | let _ = Instant::now() - second;
2626
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Instant::now().checked_sub(second).unwrap()`
2727

28-
error: aborting due to 4 previous errors
28+
error: unchecked subtraction between 'Duration' values
29+
--> tests/ui/unchecked_time_subtraction.rs:25:13
30+
|
31+
LL | let _ = dur1 - dur2;
32+
| ^^^^^^^^^^^ help: try: `dur1.checked_sub(dur2).unwrap()`
33+
34+
error: unchecked subtraction between 'Duration' values
35+
--> tests/ui/unchecked_time_subtraction.rs:28:13
36+
|
37+
LL | let _ = Duration::from_secs(10) - Duration::from_secs(5);
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Duration::from_secs(10).checked_sub(Duration::from_secs(5)).unwrap()`
39+
40+
error: unchecked subtraction between 'Duration' values
41+
--> tests/ui/unchecked_time_subtraction.rs:31:13
42+
|
43+
LL | let _ = second - dur1;
44+
| ^^^^^^^^^^^^^ help: try: `second.checked_sub(dur1).unwrap()`
45+
46+
error: unchecked subtraction between 'Duration' values
47+
--> tests/ui/unchecked_time_subtraction.rs:35:13
48+
|
49+
LL | let _ = 2 * dur1 - dur2;
50+
| ^^^^^^^^^^^^^^^ help: try: `(2 * dur1).checked_sub(dur2).unwrap()`
51+
52+
error: aborting due to 8 previous errors
2953

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#![warn(clippy::unchecked_time_subtraction)]
2+
//@no-rustfix
3+
4+
use std::time::{Duration, Instant};
5+
6+
fn main() {
7+
let dur1 = Duration::from_secs(5);
8+
let dur2 = Duration::from_secs(3);
9+
let dur3 = Duration::from_secs(1);
10+
11+
// Chained Duration subtraction - should lint without suggestion due to complexity
12+
let _ = dur1 - dur2 - dur3;
13+
//~^ unchecked_time_subtraction
14+
//~| unchecked_time_subtraction
15+
16+
// Chained Instant - Duration subtraction - should lint without suggestion due to complexity
17+
let instant1 = Instant::now();
18+
19+
let _ = instant1 - dur2 - dur3;
20+
//~^ unchecked_time_subtraction
21+
//~| unchecked_time_subtraction
22+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
error: unchecked subtraction between 'Duration' values
2+
--> tests/ui/unchecked_time_subtraction_unfixable.rs:12:13
3+
|
4+
LL | let _ = dur1 - dur2 - dur3;
5+
| ^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::unchecked-time-subtraction` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::unchecked_time_subtraction)]`
9+
10+
error: unchecked subtraction between 'Duration' values
11+
--> tests/ui/unchecked_time_subtraction_unfixable.rs:12:13
12+
|
13+
LL | let _ = dur1 - dur2 - dur3;
14+
| ^^^^^^^^^^^ help: try: `dur1.checked_sub(dur2).unwrap()`
15+
16+
error: unchecked subtraction of a 'Duration' from an 'Instant'
17+
--> tests/ui/unchecked_time_subtraction_unfixable.rs:19:13
18+
|
19+
LL | let _ = instant1 - dur2 - dur3;
20+
| ^^^^^^^^^^^^^^^^^^^^^^
21+
22+
error: unchecked subtraction of a 'Duration' from an 'Instant'
23+
--> tests/ui/unchecked_time_subtraction_unfixable.rs:19:13
24+
|
25+
LL | let _ = instant1 - dur2 - dur3;
26+
| ^^^^^^^^^^^^^^^ help: try: `instant1.checked_sub(dur2).unwrap()`
27+
28+
error: aborting due to 4 previous errors
29+

0 commit comments

Comments
 (0)