diff --git a/clippy_lints/src/significant_drop_tightening.rs b/clippy_lints/src/significant_drop_tightening.rs index c4604fb1558d..8e6c20bdba5c 100644 --- a/clippy_lints/src/significant_drop_tightening.rs +++ b/clippy_lints/src/significant_drop_tightening.rs @@ -87,39 +87,47 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> { first_bind_ident.span, "temporary with significant `Drop` can be early dropped", |diag| { - match apa.counter { - 0 | 1 => {}, - 2 => { - let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)); - let init_method = snippet(cx, apa.first_method_span, ".."); - let usage_method = snippet(cx, apa.last_method_span, ".."); - let stmt = if let Some(last_bind_ident) = apa.last_bind_ident { - format!( - "\n{indent}let {} = {init_method}.{usage_method};", - snippet(cx, last_bind_ident.span, ".."), - ) - } else { - format!("\n{indent}{init_method}.{usage_method};") - }; - - diag.multipart_suggestion_verbose( - "merge the temporary construction with its single usage", - vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())], - Applicability::MaybeIncorrect, - ); - }, - _ => { - diag.span_suggestion( - apa.last_stmt_span.shrink_to_hi(), - "drop the temporary after the end of its last usage", - format!( - "\n{}drop({});", - " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)), - first_bind_ident - ), - Applicability::MaybeIncorrect, - ); - }, + if !apa.last_stmt_span.is_dummy() { + match apa.counter { + 0 | 1 => unreachable!("checked above"), + 2 => + { + #[expect(clippy::if_not_else, reason = "for symmetry with the outer check")] + if !apa.last_method_span.is_dummy() { + let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)); + let init_method = snippet(cx, apa.first_method_span, ".."); + let usage_method = snippet(cx, apa.last_method_span, ".."); + let stmt = if let Some(last_bind_ident) = apa.last_bind_ident { + format!( + "\n{indent}let {} = {init_method}.{usage_method};", + snippet(cx, last_bind_ident.span, ".."), + ) + } else { + format!("\n{indent}{init_method}.{usage_method};") + }; + diag.multipart_suggestion_verbose( + "merge the temporary construction with its single usage", + vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())], + Applicability::MaybeIncorrect, + ); + } else { + diag.help("merge the temporary construction with its single usage"); + diag.span_note(apa.last_stmt_span, "single usage here"); + } + }, + _ => { + diag.span_suggestion( + apa.last_stmt_span.shrink_to_hi(), + "drop the temporary after the end of its last usage", + format!( + "\n{}drop({});", + " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)), + first_bind_ident + ), + Applicability::MaybeIncorrect, + ); + }, + } } diag.note("this might lead to unnecessary resource contention"); diag.span_label( @@ -276,13 +284,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> { if let hir::StmtKind::Let(local) = self.ap.curr_stmt.kind && let hir::PatKind::Binding(_, hir_id, ident, _) = local.pat.kind && !self.ap.apas.contains_key(&hir_id) - && { - if let Some(local_hir_id) = expr.res_local_id() { - local_hir_id == hir_id - } else { - true - } - } + && expr.res_local_id().is_none_or(|local_hir_id| local_hir_id == hir_id) { let mut apa = AuxParamsAttr { first_block_hir_id: self.ap.curr_block_hir_id, @@ -418,7 +420,8 @@ fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> { } fn has_drop(cx: &LateContext<'_>, expr: &hir::Expr<'_>, first_bind_ident: Option) -> bool { - if let hir::ExprKind::Call(fun, [first_arg]) = expr.kind + if let Some(first_bind_ident) = first_bind_ident + && let hir::ExprKind::Call(fun, [first_arg]) = expr.kind && let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind && let Res::Def(DefKind::Fn, did) = fun_path.res && cx.tcx.is_diagnostic_item(sym::mem_drop, did) @@ -426,7 +429,6 @@ fn has_drop(cx: &LateContext<'_>, expr: &hir::Expr<'_>, first_bind_ident: Option let has_ident = |local_expr: &hir::Expr<'_>| { if let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &local_expr.kind && let [first_arg_ps, ..] = arg_path.segments - && let Some(first_bind_ident) = first_bind_ident && first_arg_ps.ident == first_bind_ident { true @@ -448,7 +450,5 @@ fn has_drop(cx: &LateContext<'_>, expr: &hir::Expr<'_>, first_bind_ident: Option fn is_inexpensive_expr(expr: &hir::Expr<'_>) -> bool { let actual = peel_hir_expr_unary(expr).0; - let is_path = matches!(actual.kind, hir::ExprKind::Path(_)); - let is_lit = matches!(actual.kind, hir::ExprKind::Lit(_)); - is_path || is_lit + matches!(actual.kind, hir::ExprKind::Path(_) | hir::ExprKind::Lit(_)) } diff --git a/tests/ui/significant_drop_tightening_unfixable.rs b/tests/ui/significant_drop_tightening_unfixable.rs new file mode 100644 index 000000000000..28fa3cc84d6f --- /dev/null +++ b/tests/ui/significant_drop_tightening_unfixable.rs @@ -0,0 +1,46 @@ +//@no-rustfix +#![warn(clippy::significant_drop_tightening)] + +mod issue_15574 { + use std::io::{BufRead, Read, stdin}; + use std::process; + + // NOTE: this requires `no_rustfix` for multiple reasons: + // + // There should be two suggestions, one to merge the line: + // ``` + // let stdin = stdin.lock(); + // ``` + // into: + // ``` + // let mut stdin = stdin.take(40); + // ``` + // and one to merge the latter into the `if`. + // + // This causes the following problems: + // - the first suggestion lacks the `mut` before `stdin`, which doesn't compile + // - the second suggestion isn't a suggestion but a help message, so the warning isn't gone after + // rustfix + // - when the second help becomes a suggestion, it will overlap with the first one + fn main() { + //Let's read from stdin + println!("Hello, what's your name?"); + let stdin = stdin().lock(); + //~^ significant_drop_tightening + let mut buffer = String::with_capacity(10); + //Here we lock stdin and block to 10 bytes + // Our string is now then only 10 bytes. + //Even if it overflows like expected, it will reallocate. + let mut stdin = stdin.take(40); + //~^ significant_drop_tightening + if stdin.read_line(&mut buffer).is_err() { + eprintln!("An error has occured while reading."); + return; + } //Now we print the result, our data is safe + println!("Our string has a capacity of {}", buffer.capacity()); + println!("Hello {}!", buffer); + //The string is freed automatically. + } +} + +fn main() {} diff --git a/tests/ui/significant_drop_tightening_unfixable.stderr b/tests/ui/significant_drop_tightening_unfixable.stderr new file mode 100644 index 000000000000..53b2590617ae --- /dev/null +++ b/tests/ui/significant_drop_tightening_unfixable.stderr @@ -0,0 +1,54 @@ +error: temporary with significant `Drop` can be early dropped + --> tests/ui/significant_drop_tightening_unfixable.rs:28:13 + | +LL | fn main() { + | _______________- +LL | | //Let's read from stdin +LL | | println!("Hello, what's your name?"); +LL | | let stdin = stdin().lock(); + | | ^^^^^ +... | +LL | | } + | |_____- temporary `stdin` is currently being dropped at the end of its contained scope + | + = note: this might lead to unnecessary resource contention + = note: `-D clippy::significant-drop-tightening` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::significant_drop_tightening)]` +help: merge the temporary construction with its single usage + | +LL ~ +LL + let stdin = stdin().lock().take(40); +LL | +... +LL | //Even if it overflows like expected, it will reallocate. +LL ~ + | + +error: temporary with significant `Drop` can be early dropped + --> tests/ui/significant_drop_tightening_unfixable.rs:34:17 + | +LL | fn main() { + | _______________- +LL | | //Let's read from stdin +LL | | println!("Hello, what's your name?"); +LL | | let stdin = stdin().lock(); +... | +LL | | let mut stdin = stdin.take(40); + | | ^^^^^ +... | +LL | | } + | |_____- temporary `stdin` is currently being dropped at the end of its contained scope + | + = help: merge the temporary construction with its single usage +note: single usage here + --> tests/ui/significant_drop_tightening_unfixable.rs:36:9 + | +LL | / if stdin.read_line(&mut buffer).is_err() { +LL | | eprintln!("An error has occured while reading."); +LL | | return; +LL | | } //Now we print the result, our data is safe + | |_________^ + = note: this might lead to unnecessary resource contention + +error: aborting due to 2 previous errors +