From af35dcd33012c131bf2d3675bb40591b740c7728 Mon Sep 17 00:00:00 2001 From: apoisternex Date: Sat, 3 Aug 2024 21:24:51 -0300 Subject: [PATCH 1/2] Fix [`needless_return`] false negative when returned expression borrows a value Fixes #12907 changelog: Fix [`needless_return`] false negative when returned expression borrows a value --- clippy_lints/src/returns.rs | 22 ++++++++++++++++++---- tests/ui/needless_return.fixed | 4 ++++ tests/ui/needless_return.rs | 4 ++++ tests/ui/needless_return.stderr | 14 +++++++++++++- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 13016cdad..034c44a97 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; use clippy_utils::source::{snippet_opt, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; -use clippy_utils::visitors::{for_each_expr, Descend}; +use clippy_utils::visitors::{for_each_expr, for_each_unconsumed_temporary, Descend}; use clippy_utils::{ binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor, path_res, path_to_local_id, span_contains_cfg, span_find_starting_semi, @@ -384,10 +384,24 @@ fn check_final_expr<'tcx>( } }; - let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner)); - if borrows { - return; + if let Some(inner) = inner { + if for_each_unconsumed_temporary(cx, inner, |temporary_ty| { + if temporary_ty.has_significant_drop(cx.tcx, cx.param_env) + && temporary_ty + .walk() + .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(re) if !re.is_static())) + { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) + } + }) + .is_break() + { + return; + } } + if ret_span.from_expansion() { return; } diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index fc4129e1d..c5c570690 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -355,4 +355,8 @@ fn conjunctive_blocks() -> String { ({ "a".to_string() } + "b" + { "c" }) } +fn issue12907() -> String { + "".split("").next().unwrap().to_string() +} + fn main() {} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 61c7a0200..738611391 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -365,4 +365,8 @@ fn conjunctive_blocks() -> String { return { "a".to_string() } + "b" + { "c" }; } +fn issue12907() -> String { + return "".split("").next().unwrap().to_string(); +} + fn main() {} diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index ea9c230ea..da0fa220d 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -665,5 +665,17 @@ LL - return { "a".to_string() } + "b" + { "c" }; LL + ({ "a".to_string() } + "b" + { "c" }) | -error: aborting due to 53 previous errors +error: unneeded `return` statement + --> tests/ui/needless_return.rs:369:5 + | +LL | return "".split("").next().unwrap().to_string(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove `return` + | +LL - return "".split("").next().unwrap().to_string(); +LL + "".split("").next().unwrap().to_string() + | + +error: aborting due to 54 previous errors From 66283bff5fde5f310f3dd373d22cd4f892b3d312 Mon Sep 17 00:00:00 2001 From: apoisternex Date: Sun, 4 Aug 2024 10:45:42 -0300 Subject: [PATCH 2/2] apply [`needless_return`] suggestion --- clippy_lints/src/useless_conversion.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 2f7d54e73..1890b3fdd 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -217,12 +217,12 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { }, ExprKind::MethodCall(.., args, _) => { cx.typeck_results().type_dependent_def_id(parent.hir_id).map(|did| { - return ( + ( did, args, cx.typeck_results().node_args(parent.hir_id), MethodOrFunction::Method, - ); + ) }) }, _ => None,