Skip to content

Commit

Permalink
Auto merge of #13214 - apoisternex:issue12907, r=Centri3
Browse files Browse the repository at this point in the history
Fix [`needless_return`] false negative

Fixes #12907

changelog: Fix [`needless_return`] false negative when returned expression borrows a value.
  • Loading branch information
bors committed Sep 7, 2024
2 parents d9c4523 + 66283bf commit 41dc86d
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 7 deletions.
22 changes: 18 additions & 4 deletions clippy_lints/src/returns.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::source::{snippet_with_context, SpanRangeExt};
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,
Expand Down Expand Up @@ -389,10 +389,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;
}
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/needless_return.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,8 @@ fn conjunctive_blocks() -> String {
({ "a".to_string() } + "b" + { "c" })
}

fn issue12907() -> String {
"".split("").next().unwrap().to_string()
}

fn main() {}
4 changes: 4 additions & 0 deletions tests/ui/needless_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
14 changes: 13 additions & 1 deletion tests/ui/needless_return.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 41dc86d

Please sign in to comment.