From f1c076be658c04b131f1e65f7f655beedd223b60 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Sun, 22 Aug 2021 19:45:38 +0200 Subject: [PATCH] Refactored lint to use `check_fn` --- clippy_lints/src/semicolon_outside_block.rs | 187 +++++++++++--------- 1 file changed, 106 insertions(+), 81 deletions(-) diff --git a/clippy_lints/src/semicolon_outside_block.rs b/clippy_lints/src/semicolon_outside_block.rs index aff3714a78ea..4237dc65b3b8 100644 --- a/clippy_lints/src/semicolon_outside_block.rs +++ b/clippy_lints/src/semicolon_outside_block.rs @@ -4,8 +4,9 @@ use clippy_utils::in_macro; use clippy_utils::source::snippet_with_macro_callsite; use if_chain::if_chain; use rustc_errors::Applicability; +use rustc_hir::intravisit::FnKind; use rustc_hir::ExprKind; -use rustc_hir::{Block, BodyOwnerKind, StmtKind}; +use rustc_hir::{Block, Body, Expr, FnDecl, HirId, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::BytePos; @@ -36,94 +37,118 @@ declare_clippy_lint! { declare_lint_pass!(SemicolonOutsideBlock => [SEMICOLON_OUTSIDE_BLOCK]); -/// Checks if an ExprKind is of an illegal variant (aka blocks that we don't want) -/// to lint on as it's illegal or unnecessary to put a semicolon afterwards. -/// This macro then inserts a `return` statement to return out of the check_block -/// method. -macro_rules! check_expr_return { - ($l:expr) => { - if let ExprKind::If(..) | ExprKind::Loop(..) | ExprKind::DropTemps(..) | ExprKind::Match(..) = $l { - return; - } - }; -} - impl LateLintPass<'_> for SemicolonOutsideBlock { - fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) { - if_chain! { - if !in_macro(block.span); - if block.expr.is_none(); - if let Some(last) = block.stmts.last(); - if let StmtKind::Semi(expr) = last.kind; - let t_expr = cx.typeck_results().expr_ty(expr); - if t_expr.is_unit(); - then { - // make sure that the block does not belong to a function by iterating over the parents - for (hir_id, _) in cx.tcx.hir().parent_iter(block.hir_id) { - if let Some(body_id) = cx.tcx.hir().maybe_body_owned_by(hir_id) { - // if we're in a body, check if it's an fn or a closure - if cx.tcx.hir().body_owner_kind(hir_id).is_fn_or_closure() { - let item_body = cx.tcx.hir().body(body_id); - if let ExprKind::Block(fn_block, _) = item_body.value.kind { - // check for an illegal statement in the list of statements... - for stmt in fn_block.stmts { - if let StmtKind::Expr(pot_ille_expr) = stmt.kind { - check_expr_return!(pot_ille_expr.kind); - } - } - - //.. the potential last statement .. - if let Some(last_expr) = fn_block.expr { - check_expr_return!(last_expr.kind); - } + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + fn_kind: FnKind<'tcx>, + _: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + _: Span, + _: HirId, + ) { + if let ExprKind::Block(block) = body.value.kind { + // also check this block if we're inside a closure + if matches!(fn_kind, FnKind::Closure) { + check_block(cx, block); + } - // .. or if this is the body of an fn - if fn_block.hir_id == block.hir_id && - !matches!(cx.tcx.hir().body_owner_kind(hir_id), BodyOwnerKind::Closure) { - return - } - } - } - } + // iterate over the statements and check if we find a potential + // block to check + for stmt in block.stmts { + match stmt.kind { + StmtKind::Expr(Expr { + kind: ExprKind::Block(bl, ..), + .. + }) + | StmtKind::Semi(Expr { + kind: ExprKind::Block(bl, ..), + .. + }) => check_block(cx, bl), + _ => (), } + } - // filter out other blocks and the desugared for loop - check_expr_return!(expr.kind); - - // make sure we're also having the semicolon at the end of the expression... - let expr_w_sem = expand_span_to_semicolon(cx, expr.span); - let expr_snip = snippet_with_macro_callsite(cx, expr_w_sem, ".."); - let mut expr_sugg = expr_snip.to_string(); - expr_sugg.pop(); + // check if the potential trailing expr is a block and check it if necessary + if let Some(Expr { + kind: ExprKind::Block(bl, ..), + .. + }) = block.expr + { + check_block(cx, bl); + } + } + } +} - // and the block - let block_w_sem = expand_span_to_semicolon(cx, block.span); - let mut block_snip: String = snippet_with_macro_callsite(cx, block_w_sem, "..").to_string(); - if block_snip.ends_with('\n') { - block_snip.pop(); - } +/// Checks for a block if it's a target of this lint and spans a suggestion +/// if applicable. This method also recurses through all other statements that +/// are blocks. +fn check_block(cx: &LateContext<'_>, block: &Block<'_>) { + // check all subblocks + for stmt in block.stmts { + match stmt.kind { + StmtKind::Expr(Expr { + kind: ExprKind::Block(bl, ..), + .. + }) + | StmtKind::Semi(Expr { + kind: ExprKind::Block(bl, ..), + .. + }) => check_block(cx, bl), + _ => (), + } + } + // check the potential trailing expression + if let Some(Expr { + kind: ExprKind::Block(bl, ..), + .. + }) = block.expr + { + check_block(cx, bl); + } - // retrieve the suggestion - let suggestion = if block_snip.ends_with(';') { - block_snip.replace(expr_snip.as_ref(), &format!("{}", expr_sugg.as_str())) - } else { - format!("{};", block_snip.replace(expr_snip.as_ref(), &format!("{}", expr_sugg.as_str()))) - }; + if_chain! { + if !in_macro(block.span); + if block.expr.is_none(); + if let Some(last) = block.stmts.last(); + if let StmtKind::Semi(expr) = last.kind; + let t_expr = cx.typeck_results().expr_ty(expr); + if t_expr.is_unit(); + then { + // make sure we're also having the semicolon at the end of the expression... + let expr_w_sem = expand_span_to_semicolon(cx, expr.span); + let expr_snip = snippet_with_macro_callsite(cx, expr_w_sem, ".."); + let mut expr_sugg = expr_snip.to_string(); + expr_sugg.pop(); - span_lint_and_sugg( - cx, - SEMICOLON_OUTSIDE_BLOCK, - if block_snip.ends_with(';') { - block_w_sem - } else { - block.span - }, - "consider moving the `;` outside the block for consistent formatting", - "put the `;` outside the block", - suggestion, - Applicability::MaybeIncorrect, - ); + // and the block + let block_w_sem = expand_span_to_semicolon(cx, block.span); + let mut block_snip: String = snippet_with_macro_callsite(cx, block_w_sem, "..").to_string(); + if block_snip.ends_with('\n') { + block_snip.pop(); } + + // retrieve the suggestion + let suggestion = if block_snip.ends_with(';') { + block_snip.replace(expr_snip.as_ref(), &format!("{}", expr_sugg.as_str())) + } else { + format!("{};", block_snip.replace(expr_snip.as_ref(), &format!("{}", expr_sugg.as_str()))) + }; + + span_lint_and_sugg( + cx, + SEMICOLON_OUTSIDE_BLOCK, + if block_snip.ends_with(';') { + block_w_sem + } else { + block.span + }, + "consider moving the `;` outside the block for consistent formatting", + "put the `;` outside the block", + suggestion, + Applicability::MaybeIncorrect, + ); } } }