Skip to content

Commit 3e414c4

Browse files
ada4aada4a
committed
fix(significant_drop_tightening): don't suggest with dummy spans
1 parent 2e0d4c5 commit 3e414c4

File tree

3 files changed

+134
-24
lines changed

3 files changed

+134
-24
lines changed

‎clippy_lints/src/significant_drop_tightening.rs‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::{indent_of, snippet};
33
use clippy_utils::{expr_or_init, get_attr, path_to_local, peel_hir_expr_unary, sym};
44
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
5-
use rustc_errors::Applicability;
5+
use rustc_errors::{Applicability, Diag};
66
use rustc_hir::def::{DefKind, Res};
77
use rustc_hir::intravisit::{Visitor, walk_expr};
88
use rustc_hir::{self as hir, HirId};
@@ -80,15 +80,12 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
8080
continue;
8181
}
8282
let first_bind_ident = apa.first_bind_ident.unwrap();
83-
span_lint_and_then(
84-
cx,
85-
SIGNIFICANT_DROP_TIGHTENING,
86-
first_bind_ident.span,
87-
"temporary with significant `Drop` can be early dropped",
88-
|diag| {
89-
match apa.counter {
90-
0 | 1 => {},
91-
2 => {
83+
let emit_sugg = |diag: &mut Diag<'_, _>| match apa.counter {
84+
0 | 1 => {},
85+
2 => {
86+
if !apa.last_stmt_span.is_dummy() {
87+
#[expect(clippy::if_not_else, reason = "for symmetry with the check above")]
88+
if !apa.last_method_span.is_dummy() {
9289
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
9390
let init_method = snippet(cx, apa.first_method_span, "..");
9491
let usage_method = snippet(cx, apa.last_method_span, "..");
@@ -100,26 +97,39 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
10097
} else {
10198
format!("\n{indent}{init_method}.{usage_method};")
10299
};
103-
104100
diag.multipart_suggestion_verbose(
105101
"merge the temporary construction with its single usage",
106102
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
107103
Applicability::MaybeIncorrect,
108104
);
109-
},
110-
_ => {
111-
diag.span_suggestion(
112-
apa.last_stmt_span.shrink_to_hi(),
113-
"drop the temporary after the end of its last usage",
114-
format!(
115-
"\n{}drop({});",
116-
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
117-
first_bind_ident
118-
),
119-
Applicability::MaybeIncorrect,
120-
);
121-
},
105+
} else {
106+
diag.help("merge the temporary construction with its single usage");
107+
diag.span_note(apa.last_stmt_span, "single usage here");
108+
}
109+
}
110+
},
111+
_ => {
112+
if !apa.last_stmt_span.is_dummy() {
113+
diag.span_suggestion(
114+
apa.last_stmt_span.shrink_to_hi(),
115+
"drop the temporary after the end of its last usage",
116+
format!(
117+
"\n{}drop({});",
118+
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
119+
first_bind_ident
120+
),
121+
Applicability::MaybeIncorrect,
122+
);
122123
}
124+
},
125+
};
126+
span_lint_and_then(
127+
cx,
128+
SIGNIFICANT_DROP_TIGHTENING,
129+
first_bind_ident.span,
130+
"temporary with significant `Drop` can be early dropped",
131+
|diag| {
132+
emit_sugg(diag);
123133
diag.note("this might lead to unnecessary resource contention");
124134
diag.span_label(
125135
apa.first_block_span,

‎tests/ui/significant_drop_tightening_unfixable.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
//@no-rustfix
2+
#![warn(clippy::significant_drop_tightening)]
3+
4+
mod issue_15574 {
5+
use std::io::{BufRead, Read, stdin};
6+
use std::process;
7+
8+
// NOTE: this requires `no_rustfix` for multiple reasons:
9+
//
10+
// There should be two suggestions, one to merge the line:
11+
// ```
12+
// let stdin = stdin.lock();
13+
// ```
14+
// into:
15+
// ```
16+
// let mut stdin = stdin.take(40);
17+
// ```
18+
// and one to merge the latter into the `if`.
19+
//
20+
// This causes the following problems:
21+
// - the first suggestion lacks the `mut` before `stdin`, which doesn't compile
22+
// - the second suggestion isn't a suggestion but a help message, so the warning isn't gone after
23+
// rustfix
24+
// - when the second help becomes a suggestion, it will overlap with the first one
25+
fn main() {
26+
//Let's read from stdin
27+
println!("Hello, what's your name?");
28+
let stdin = stdin().lock();
29+
//~^ significant_drop_tightening
30+
let mut buffer = String::with_capacity(10);
31+
//Here we lock stdin and block to 10 bytes
32+
// Our string is now then only 10 bytes.
33+
//Even if it overflows like expected, it will reallocate.
34+
let mut stdin = stdin.take(40);
35+
//~^ significant_drop_tightening
36+
if stdin.read_line(&mut buffer).is_err() {
37+
eprintln!("An error has occured while reading.");
38+
return;
39+
} //Now we print the result, our data is safe
40+
println!("Our string has a capacity of {}", buffer.capacity());
41+
println!("Hello {}!", buffer);
42+
//The string is freed automatically.
43+
}
44+
}
45+
46+
fn main() {}

‎tests/ui/significant_drop_tightening_unfixable.stderr‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
error: temporary with significant `Drop` can be early dropped
2+
--> tests/ui/significant_drop_tightening_unfixable.rs:28:13
3+
|
4+
LL | fn main() {
5+
| _______________-
6+
LL | | //Let's read from stdin
7+
LL | | println!("Hello, what's your name?");
8+
LL | | let stdin = stdin().lock();
9+
| | ^^^^^
10+
... |
11+
LL | | }
12+
| |_____- temporary `stdin` is currently being dropped at the end of its contained scope
13+
|
14+
= note: this might lead to unnecessary resource contention
15+
= note: `-D clippy::significant-drop-tightening` implied by `-D warnings`
16+
= help: to override `-D warnings` add `#[allow(clippy::significant_drop_tightening)]`
17+
help: merge the temporary construction with its single usage
18+
|
19+
LL ~
20+
LL + let stdin = stdin().lock().take(40);
21+
LL |
22+
...
23+
LL | //Even if it overflows like expected, it will reallocate.
24+
LL ~
25+
|
26+
27+
error: temporary with significant `Drop` can be early dropped
28+
--> tests/ui/significant_drop_tightening_unfixable.rs:34:17
29+
|
30+
LL | fn main() {
31+
| _______________-
32+
LL | | //Let's read from stdin
33+
LL | | println!("Hello, what's your name?");
34+
LL | | let stdin = stdin().lock();
35+
... |
36+
LL | | let mut stdin = stdin.take(40);
37+
| | ^^^^^
38+
... |
39+
LL | | }
40+
| |_____- temporary `stdin` is currently being dropped at the end of its contained scope
41+
|
42+
= help: merge the temporary construction with its single usage
43+
note: single usage here
44+
--> tests/ui/significant_drop_tightening_unfixable.rs:36:9
45+
|
46+
LL | / if stdin.read_line(&mut buffer).is_err() {
47+
LL | | eprintln!("An error has occured while reading.");
48+
LL | | return;
49+
LL | | } //Now we print the result, our data is safe
50+
| |_________^
51+
= note: this might lead to unnecessary resource contention
52+
53+
error: aborting due to 2 previous errors
54+

0 commit comments

Comments
 (0)