|
| 1 | +use if_chain::if_chain; |
| 2 | +use rustc_errors::Applicability; |
| 3 | +use rustc_hir::{ |
| 4 | + AnonConst, Block, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Node, PatKind, StmtKind, TraitFn, |
| 5 | + TraitItem, TraitItemKind, |
| 6 | +}; |
| 7 | +use rustc_lint::{LateContext, LateLintPass, LintContext}; |
| 8 | +use rustc_middle::lint::in_external_macro; |
| 9 | +use rustc_middle::mir::{Body, Rvalue, Statement, StatementKind}; |
| 10 | +use rustc_session::{declare_lint_pass, declare_tool_lint}; |
| 11 | +use rustc_span::source_map::Span; |
| 12 | + |
| 13 | +use crate::utils::{get_enclosing_block, in_macro, match_qpath, snippet_opt, span_lint_and_then}; |
| 14 | + |
| 15 | +declare_clippy_lint! { |
| 16 | + /// **What it does:** Checks for `let`-bindings, which are subsequently |
| 17 | + /// returned. |
| 18 | + /// |
| 19 | + /// **Why is this bad?** It is just extraneous code. Remove it to make your code |
| 20 | + /// more rusty. |
| 21 | + /// |
| 22 | + /// **Known problems:** None. |
| 23 | + /// |
| 24 | + /// **Example:** |
| 25 | + /// ```rust |
| 26 | + /// fn foo() -> String { |
| 27 | + /// let x = String::new(); |
| 28 | + /// x |
| 29 | + /// } |
| 30 | + /// ``` |
| 31 | + /// instead, use |
| 32 | + /// ``` |
| 33 | + /// fn foo() -> String { |
| 34 | + /// String::new() |
| 35 | + /// } |
| 36 | + /// ``` |
| 37 | + pub LET_AND_RETURN, |
| 38 | + style, |
| 39 | + "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block" |
| 40 | +} |
| 41 | + |
| 42 | +declare_lint_pass!(LetReturn => [LET_AND_RETURN]); |
| 43 | + |
| 44 | +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn { |
| 45 | + fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx Block<'_>) { |
| 46 | + // we need both a let-binding stmt and an expr |
| 47 | + if_chain! { |
| 48 | + if let Some(retexpr) = block.expr; |
| 49 | + if let Some(stmt) = block.stmts.iter().last(); |
| 50 | + if let StmtKind::Local(local) = &stmt.kind; |
| 51 | + if local.ty.is_none(); |
| 52 | + if local.attrs.is_empty(); |
| 53 | + if let Some(initexpr) = &local.init; |
| 54 | + if let PatKind::Binding(.., ident, _) = local.pat.kind; |
| 55 | + if let ExprKind::Path(qpath) = &retexpr.kind; |
| 56 | + if match_qpath(qpath, &[&*ident.name.as_str()]); |
| 57 | + if !in_external_macro(cx.sess(), initexpr.span); |
| 58 | + if !in_external_macro(cx.sess(), retexpr.span); |
| 59 | + if !in_external_macro(cx.sess(), local.span); |
| 60 | + if !in_macro(local.span); |
| 61 | + if !last_statement_borrows_locals(cx, block); |
| 62 | + then { |
| 63 | + span_lint_and_then( |
| 64 | + cx, |
| 65 | + LET_AND_RETURN, |
| 66 | + retexpr.span, |
| 67 | + "returning the result of a `let` binding from a block", |
| 68 | + |err| { |
| 69 | + err.span_label(local.span, "unnecessary `let` binding"); |
| 70 | + |
| 71 | + if let Some(snippet) = snippet_opt(cx, initexpr.span) { |
| 72 | + err.multipart_suggestion( |
| 73 | + "return the expression directly", |
| 74 | + vec![ |
| 75 | + (local.span, String::new()), |
| 76 | + (retexpr.span, snippet), |
| 77 | + ], |
| 78 | + Applicability::MachineApplicable, |
| 79 | + ); |
| 80 | + } else { |
| 81 | + err.span_help(initexpr.span, "this expression can be directly returned"); |
| 82 | + } |
| 83 | + }, |
| 84 | + ); |
| 85 | + } |
| 86 | + } |
| 87 | + } |
| 88 | +} |
| 89 | + |
| 90 | +// Check if we can suggest turning the last statement into a block tail expression without hitting |
| 91 | +// "does not live long enough" errors. |
| 92 | +fn last_statement_borrows_locals(cx: &LateContext<'_, '_>, block: &'_ Block<'_>) -> bool { |
| 93 | + // Search for the enclosing fn-like node to retrieve the MIR for. |
| 94 | + fn enclosing_node(cx: &LateContext<'_, '_>, hir_id: HirId) -> Option<HirId> { |
| 95 | + for (hir_id, node) in cx.tcx.hir().parent_iter(hir_id) { |
| 96 | + match node { |
| 97 | + Node::Expr(Expr { |
| 98 | + kind: ExprKind::Closure(..), |
| 99 | + .. |
| 100 | + }) |
| 101 | + | Node::Item(Item { |
| 102 | + kind: ItemKind::Fn(..) | ItemKind::Static(..) | ItemKind::Const(..), |
| 103 | + .. |
| 104 | + }) |
| 105 | + | Node::ImplItem(ImplItem { |
| 106 | + kind: ImplItemKind::Fn(..) | ImplItemKind::Const(..), |
| 107 | + .. |
| 108 | + }) |
| 109 | + | Node::TraitItem(TraitItem { |
| 110 | + kind: TraitItemKind::Fn(.., TraitFn::Provided(_)) | TraitItemKind::Const(.., Some(_)), |
| 111 | + .. |
| 112 | + }) |
| 113 | + | Node::AnonConst(AnonConst { .. }) => { |
| 114 | + return Some(hir_id); |
| 115 | + }, |
| 116 | + _ => {}, |
| 117 | + } |
| 118 | + } |
| 119 | + |
| 120 | + None |
| 121 | + } |
| 122 | + |
| 123 | + // Find the span of the outmost block where locals can't be borrowed. |
| 124 | + fn scope_filter(cx: &LateContext<'_, '_>, block: &Block<'_>) -> Span { |
| 125 | + fn is_parent_tail_expr(hir_id: HirId, parent: &Expr<'_>) -> bool { |
| 126 | + matches!(parent.kind, ExprKind::Block(Block { hir_id: tail_id, .. }, _) if *tail_id == hir_id) |
| 127 | + } |
| 128 | + |
| 129 | + let mut outer_block = block; |
| 130 | + |
| 131 | + while let Some(parent_block) = get_enclosing_block(cx, outer_block.hir_id) { |
| 132 | + match parent_block.expr { |
| 133 | + Some(tail_expr) if is_parent_tail_expr(outer_block.hir_id, tail_expr) => {}, |
| 134 | + _ => break, |
| 135 | + } |
| 136 | + |
| 137 | + outer_block = parent_block; |
| 138 | + } |
| 139 | + |
| 140 | + outer_block.span |
| 141 | + } |
| 142 | + |
| 143 | + // Search for `_2 = &_1` where _2 is a temporary and _1 is a local inside the relevant span. |
| 144 | + fn is_relevant_assign(body: &Body<'_>, statement: &Statement<'_>, span: Span) -> bool { |
| 145 | + if let StatementKind::Assign(box (assigned_place, Rvalue::Ref(_, _, borrowed_place))) = statement.kind { |
| 146 | + let assigned = &body.local_decls[assigned_place.local]; |
| 147 | + let borrowed = &body.local_decls[borrowed_place.local]; |
| 148 | + |
| 149 | + !assigned.is_user_variable() && borrowed.is_user_variable() && span.contains(borrowed.source_info.span) |
| 150 | + } else { |
| 151 | + false |
| 152 | + } |
| 153 | + } |
| 154 | + |
| 155 | + if let Some(last_stmt) = block.stmts.iter().last() { |
| 156 | + if let Some(node_hir_id) = enclosing_node(cx, block.hir_id) { |
| 157 | + let def_id = cx.tcx.hir().local_def_id(node_hir_id); |
| 158 | + let body = cx.tcx.optimized_mir(def_id.to_def_id()); |
| 159 | + let span = scope_filter(cx, block); |
| 160 | + |
| 161 | + return body.basic_blocks().iter().any(|bbdata| { |
| 162 | + bbdata |
| 163 | + .statements |
| 164 | + .iter() |
| 165 | + .any(|stmt| last_stmt.span.contains(stmt.source_info.span) && is_relevant_assign(body, stmt, span)) |
| 166 | + }); |
| 167 | + } |
| 168 | + } |
| 169 | + |
| 170 | + false |
| 171 | +} |
0 commit comments