Skip to content

Commit 4a87fd6

Browse files
ada4aada4a
committed
make AuxParamsAttr.last_method_span optional
1 parent 2e0d4c5 commit 4a87fd6

File tree

3 files changed

+143
-39
lines changed

3 files changed

+143
-39
lines changed

‎clippy_lints/src/significant_drop_tightening.rs‎

Lines changed: 45 additions & 39 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,46 +80,52 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
8080
continue;
8181
}
8282
let first_bind_ident = apa.first_bind_ident.unwrap();
83+
let emit_sugg = |diag: &mut Diag<'_, _>| match apa.counter {
84+
0 | 1 => {},
85+
2 => {
86+
if let Some(last_method_span) = apa.last_method_span {
87+
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
88+
let init_method = snippet(cx, apa.first_method_span, "..");
89+
let usage_method = snippet(cx, last_method_span, "..");
90+
let stmt = if let Some(last_bind_ident) = apa.last_bind_ident {
91+
format!(
92+
"\n{indent}let {} = {init_method}.{usage_method};",
93+
snippet(cx, last_bind_ident.span, ".."),
94+
)
95+
} else {
96+
format!("\n{indent}{init_method}.{usage_method};")
97+
};
98+
99+
diag.multipart_suggestion_verbose(
100+
"merge the temporary construction with its single usage",
101+
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
102+
Applicability::MaybeIncorrect,
103+
);
104+
} else {
105+
diag.help("merge the temporary construction with its single usage");
106+
diag.span_note(apa.last_stmt_span, "single usage here");
107+
}
108+
},
109+
_ => {
110+
diag.span_suggestion(
111+
apa.last_stmt_span.shrink_to_hi(),
112+
"drop the temporary after the end of its last usage",
113+
format!(
114+
"\n{}drop({});",
115+
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
116+
first_bind_ident
117+
),
118+
Applicability::MaybeIncorrect,
119+
);
120+
},
121+
};
83122
span_lint_and_then(
84123
cx,
85124
SIGNIFICANT_DROP_TIGHTENING,
86125
first_bind_ident.span,
87126
"temporary with significant `Drop` can be early dropped",
88127
|diag| {
89-
match apa.counter {
90-
0 | 1 => {},
91-
2 => {
92-
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
93-
let init_method = snippet(cx, apa.first_method_span, "..");
94-
let usage_method = snippet(cx, apa.last_method_span, "..");
95-
let stmt = if let Some(last_bind_ident) = apa.last_bind_ident {
96-
format!(
97-
"\n{indent}let {} = {init_method}.{usage_method};",
98-
snippet(cx, last_bind_ident.span, ".."),
99-
)
100-
} else {
101-
format!("\n{indent}{init_method}.{usage_method};")
102-
};
103-
104-
diag.multipart_suggestion_verbose(
105-
"merge the temporary construction with its single usage",
106-
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
107-
Applicability::MaybeIncorrect,
108-
);
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-
},
122-
}
128+
emit_sugg(diag);
123129
diag.note("this might lead to unnecessary resource contention");
124130
diag.span_label(
125131
apa.first_block_span,
@@ -309,7 +315,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
309315
if let Some(local_init) = local.init
310316
&& let hir::ExprKind::MethodCall(_, _, _, span) = local_init.kind
311317
{
312-
apa.last_method_span = span;
318+
apa.last_method_span = Some(span);
313319
}
314320
},
315321
hir::StmtKind::Semi(semi_expr) => {
@@ -319,7 +325,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
319325
return;
320326
}
321327
if let hir::ExprKind::MethodCall(_, _, _, span) = semi_expr.kind {
322-
apa.last_method_span = span;
328+
apa.last_method_span = Some(span);
323329
}
324330
},
325331
_ => {},
@@ -380,7 +386,7 @@ struct AuxParamsAttr {
380386
/// marked with `#[has_significant_drop]`.
381387
last_bind_ident: Option<Ident>,
382388
/// Similar to `last_bind_span` but encompasses the right-hand method call.
383-
last_method_span: Span,
389+
last_method_span: Option<Span>,
384390
/// Similar to `last_bind_span` but encompasses the whole contained statement.
385391
last_stmt_span: Span,
386392
}
@@ -396,7 +402,7 @@ impl Default for AuxParamsAttr {
396402
first_method_span: DUMMY_SP,
397403
first_stmt_span: DUMMY_SP,
398404
last_bind_ident: None,
399-
last_method_span: DUMMY_SP,
405+
last_method_span: None,
400406
last_stmt_span: DUMMY_SP,
401407
}
402408
}

‎tests/ui/significant_drop_tightening_unfixable.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#![warn(clippy::significant_drop_tightening)]
2+
3+
// we split the reproducer in two, because otherwise the two suggestions overlap,
4+
// causing rustfix to complain
5+
mod issue_15574 {
6+
use std::io::{BufRead, Read, stdin};
7+
use std::process;
8+
9+
fn v1() {
10+
//Let's read from stdin
11+
println!("Hello, what's your name?");
12+
let mut stdin = stdin().lock().take(40);
13+
//~^ significant_drop_tightening
14+
let mut buffer = String::with_capacity(10);
15+
//Here we lock stdin and block to 10 bytes
16+
// Our string is now then only 10 bytes.
17+
//Even if it overflows like expected, it will reallocate.
18+
if stdin.read_line(&mut buffer).is_err() {
19+
eprintln!("An error has occured while reading.");
20+
return;
21+
} //Now we print the result, our data is safe
22+
println!("Our string has a capacity of {}", buffer.capacity());
23+
println!("Hello {}!", buffer);
24+
//The string is freed automatically.
25+
}
26+
27+
fn v2() {
28+
//Let's read from stdin
29+
println!("Hello, what's your name?");
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().lock().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+
}

‎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:10:17
3+
|
4+
LL | fn v1() {
5+
| _____________-
6+
LL | | //Let's read from stdin
7+
LL | | println!("Hello, what's your name?");
8+
LL | | let mut stdin = stdin().lock().take(40);
9+
| | ^^^^^
10+
... |
11+
LL | | }
12+
| |_____- temporary `stdin` is currently being dropped at the end of its contained scope
13+
|
14+
= help: merge the temporary construction with its single usage
15+
note: single usage here
16+
--> tests/ui/significant_drop_tightening_unfixable.rs:16:9
17+
|
18+
LL | / if stdin.read_line(&mut buffer).is_err() {
19+
LL | | eprintln!("An error has occured while reading.");
20+
LL | | return;
21+
LL | | } //Now we print the result, our data is safe
22+
| |_________^
23+
= note: this might lead to unnecessary resource contention
24+
= note: `-D clippy::significant-drop-tightening` implied by `-D warnings`
25+
= help: to override `-D warnings` add `#[allow(clippy::significant_drop_tightening)]`
26+
27+
error: temporary with significant `Drop` can be early dropped
28+
--> tests/ui/significant_drop_tightening_unfixable.rs:32:17
29+
|
30+
LL | fn v2() {
31+
| _____________-
32+
LL | | //Let's read from stdin
33+
LL | | println!("Hello, what's your name?");
34+
LL | | let mut buffer = String::with_capacity(10);
35+
... |
36+
LL | | let mut stdin = stdin().lock().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:34: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)