-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
let_and_return: avoid "does not live long enough" errors
- Loading branch information
Showing
6 changed files
with
267 additions
and
84 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| use if_chain::if_chain; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{ | ||
| AnonConst, Block, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Node, PatKind, StmtKind, TraitFn, | ||
| TraitItem, TraitItemKind, | ||
| }; | ||
| use rustc_lint::{LateContext, LateLintPass, LintContext}; | ||
| use rustc_middle::lint::in_external_macro; | ||
| use rustc_middle::mir::{Body, Rvalue, Statement, StatementKind}; | ||
| use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
| use rustc_span::source_map::Span; | ||
|
|
||
| use crate::utils::{get_enclosing_block, in_macro, match_qpath, snippet_opt, span_lint_and_then}; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** Checks for `let`-bindings, which are subsequently | ||
| /// returned. | ||
| /// | ||
| /// **Why is this bad?** It is just extraneous code. Remove it to make your code | ||
| /// more rusty. | ||
| /// | ||
| /// **Known problems:** None. | ||
| /// | ||
| /// **Example:** | ||
| /// ```rust | ||
| /// fn foo() -> String { | ||
| /// let x = String::new(); | ||
| /// x | ||
| /// } | ||
| /// ``` | ||
| /// instead, use | ||
| /// ``` | ||
| /// fn foo() -> String { | ||
| /// String::new() | ||
| /// } | ||
| /// ``` | ||
| pub LET_AND_RETURN, | ||
| style, | ||
| "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block" | ||
| } | ||
|
|
||
| declare_lint_pass!(LetReturn => [LET_AND_RETURN]); | ||
|
|
||
| impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn { | ||
| fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx Block<'_>) { | ||
| // we need both a let-binding stmt and an expr | ||
| if_chain! { | ||
| if let Some(retexpr) = block.expr; | ||
| if let Some(stmt) = block.stmts.iter().last(); | ||
| if let StmtKind::Local(local) = &stmt.kind; | ||
| if local.ty.is_none(); | ||
| if local.attrs.is_empty(); | ||
| if let Some(initexpr) = &local.init; | ||
| if let PatKind::Binding(.., ident, _) = local.pat.kind; | ||
| if let ExprKind::Path(qpath) = &retexpr.kind; | ||
| if match_qpath(qpath, &[&*ident.name.as_str()]); | ||
| if !in_external_macro(cx.sess(), initexpr.span); | ||
| if !in_external_macro(cx.sess(), retexpr.span); | ||
| if !in_external_macro(cx.sess(), local.span); | ||
| if !in_macro(local.span); | ||
| if !last_statement_borrows_locals(cx, block); | ||
| then { | ||
| span_lint_and_then( | ||
| cx, | ||
| LET_AND_RETURN, | ||
| retexpr.span, | ||
| "returning the result of a `let` binding from a block", | ||
| |err| { | ||
| err.span_label(local.span, "unnecessary `let` binding"); | ||
|
|
||
| if let Some(snippet) = snippet_opt(cx, initexpr.span) { | ||
| err.multipart_suggestion( | ||
| "return the expression directly", | ||
| vec![ | ||
| (local.span, String::new()), | ||
| (retexpr.span, snippet), | ||
| ], | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } else { | ||
| err.span_help(initexpr.span, "this expression can be directly returned"); | ||
| } | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check if we can suggest turning the last statement into a block tail expression without hitting | ||
| // "does not live long enough" errors. | ||
| fn last_statement_borrows_locals(cx: &LateContext<'_, '_>, block: &'_ Block<'_>) -> bool { | ||
| // Search for the enclosing fn-like node to retrieve the MIR for. | ||
| fn enclosing_node(cx: &LateContext<'_, '_>, hir_id: HirId) -> Option<HirId> { | ||
| for (hir_id, node) in cx.tcx.hir().parent_iter(hir_id) { | ||
| match node { | ||
| Node::Expr(Expr { | ||
| kind: ExprKind::Closure(..), | ||
| .. | ||
| }) | ||
| | Node::Item(Item { | ||
| kind: ItemKind::Fn(..) | ItemKind::Static(..) | ItemKind::Const(..), | ||
| .. | ||
| }) | ||
| | Node::ImplItem(ImplItem { | ||
| kind: ImplItemKind::Fn(..) | ImplItemKind::Const(..), | ||
| .. | ||
| }) | ||
| | Node::TraitItem(TraitItem { | ||
| kind: TraitItemKind::Fn(.., TraitFn::Provided(_)) | TraitItemKind::Const(.., Some(_)), | ||
| .. | ||
| }) | ||
| | Node::AnonConst(AnonConst { .. }) => { | ||
| return Some(hir_id); | ||
| }, | ||
| _ => {}, | ||
| } | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| // Find the span of the outmost block where locals can't be borrowed. | ||
| fn scope_filter(cx: &LateContext<'_, '_>, block: &Block<'_>) -> Span { | ||
| fn is_parent_tail_expr(hir_id: HirId, parent: &Expr<'_>) -> bool { | ||
| matches!(parent.kind, ExprKind::Block(Block { hir_id: tail_id, .. }, _) if *tail_id == hir_id) | ||
| } | ||
|
|
||
| let mut outer_block = block; | ||
|
|
||
| while let Some(parent_block) = get_enclosing_block(cx, outer_block.hir_id) { | ||
| match parent_block.expr { | ||
| Some(tail_expr) if is_parent_tail_expr(outer_block.hir_id, tail_expr) => {}, | ||
| _ => break, | ||
| } | ||
|
|
||
| outer_block = parent_block; | ||
| } | ||
|
|
||
| outer_block.span | ||
| } | ||
|
|
||
| // Search for `_2 = &_1` where _2 is a temporary and _1 is a local inside the relevant span. | ||
| fn is_relevant_assign(body: &Body<'_>, statement: &Statement<'_>, span: Span) -> bool { | ||
| if let StatementKind::Assign(box (assigned_place, Rvalue::Ref(_, _, borrowed_place))) = statement.kind { | ||
| let assigned = &body.local_decls[assigned_place.local]; | ||
| let borrowed = &body.local_decls[borrowed_place.local]; | ||
|
|
||
| !assigned.is_user_variable() && borrowed.is_user_variable() && span.contains(borrowed.source_info.span) | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| if let Some(last_stmt) = block.stmts.iter().last() { | ||
| if let Some(node_hir_id) = enclosing_node(cx, block.hir_id) { | ||
| let def_id = cx.tcx.hir().local_def_id(node_hir_id); | ||
| let body = cx.tcx.optimized_mir(def_id.to_def_id()); | ||
| let span = scope_filter(cx, block); | ||
|
|
||
| return body.basic_blocks().iter().any(|bbdata| { | ||
| bbdata | ||
| .statements | ||
| .iter() | ||
| .any(|stmt| last_stmt.span.contains(stmt.source_info.span) && is_relevant_assign(body, stmt, span)) | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| false | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.