diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index a41f89a6a..34bf97448 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -1,10 +1,9 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_macro_callsite; -use clippy_utils::{get_parent_expr_for_hir, get_parent_node}; +use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then}; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, Node, Stmt, StmtKind}; +use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -65,69 +64,73 @@ declare_clippy_lint! { declare_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK]); impl LateLintPass<'_> for SemicolonBlock { - fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) { - semicolon_inside_block(cx, block); - semicolon_outside_block(cx, block); - } -} - -fn semicolon_inside_block(cx: &LateContext<'_>, block: &Block<'_>) { - if !block.span.from_expansion() - && let Some(tail) = block.expr - && let Some(block_expr @ Expr { kind: ExprKind::Block(_, _), ..}) = get_parent_expr_for_hir(cx, block.hir_id) - && let Some(Node::Stmt(Stmt { kind: StmtKind::Semi(_), span, .. })) = get_parent_node(cx.tcx, block_expr.hir_id) - { - let expr_snip = snippet_with_macro_callsite(cx, tail.span, ".."); - - let mut suggestion: String = snippet_with_macro_callsite(cx, block.span, "..").to_string(); - - if let Some((expr_offset, _)) = suggestion.rmatch_indices(&*expr_snip).next() { - suggestion.insert(expr_offset + expr_snip.len(), ';'); - } else { - return; + fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { + match stmt.kind { + StmtKind::Expr(Expr { + kind: + ExprKind::Block( + block @ Block { + expr: None, + stmts: + &[ + .., + Stmt { + kind: StmtKind::Semi(expr), + span, + .. + }, + ], + .. + }, + _, + ), + .. + }) if !block.span.from_expansion() => semicolon_outside_block(cx, block, expr, span), + StmtKind::Semi(Expr { + kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _), + .. + }) if !block.span.from_expansion() => semicolon_inside_block(cx, block, tail, stmt.span), + _ => (), } - - span_lint_and_sugg( - cx, - SEMICOLON_INSIDE_BLOCK, - *span, - "consider moving the `;` inside the block for consistent formatting", - "put the `;` here", - suggestion, - Applicability::MaybeIncorrect, - ); } } -fn semicolon_outside_block(cx: &LateContext<'_>, block: &Block<'_>) { - if !block.span.from_expansion() - && block.expr.is_none() - && let [.., Stmt { kind: StmtKind::Semi(expr), .. }] = block.stmts - && let Some(block_expr @ Expr { kind: ExprKind::Block(_, _), ..}) = get_parent_expr_for_hir(cx,block.hir_id) - && let Some(Node::Stmt(Stmt { kind: StmtKind::Expr(_), .. })) = get_parent_node(cx.tcx, block_expr.hir_id) - { - let expr_snip = snippet_with_macro_callsite(cx, expr.span, ".."); +fn semicolon_inside_block(cx: &LateContext<'_>, block: &Block<'_>, tail: &Expr<'_>, semi_span: Span) { + let insert_span = tail.span.with_lo(tail.span.hi()); + let remove_span = semi_span.with_lo(block.span.hi()); - let mut suggestion: String = snippet_with_macro_callsite(cx, block.span, "..").to_string(); - - if let Some((expr_offset, _)) = suggestion.rmatch_indices(&*expr_snip).next() - && let Some(semi_offset) = suggestion[expr_offset + expr_snip.len()..].find(';') - { - suggestion.remove(expr_offset + expr_snip.len() + semi_offset); - } else { - return; - } - - suggestion.push(';'); - - span_lint_and_sugg( - cx, - SEMICOLON_OUTSIDE_BLOCK, - block.span, - "consider moving the `;` outside the block for consistent formatting", - "put the `;` outside the block", - suggestion, - Applicability::MaybeIncorrect, - ); - } + span_lint_and_then( + cx, + SEMICOLON_INSIDE_BLOCK, + semi_span, + "consider moving the `;` inside the block for consistent formatting", + |diag| { + multispan_sugg_with_applicability( + diag, + "put the `;` here", + Applicability::MachineApplicable, + [(remove_span, String::new()), (insert_span, ";".to_owned())], + ); + }, + ); +} + +fn semicolon_outside_block(cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_expr: &Expr<'_>, semi_span: Span) { + let insert_span = block.span.with_lo(block.span.hi()); + let remove_span = semi_span.with_lo(tail_stmt_expr.span.source_callsite().hi()); + + span_lint_and_then( + cx, + SEMICOLON_OUTSIDE_BLOCK, + block.span, + "consider moving the `;` outside the block for consistent formatting", + |diag| { + multispan_sugg_with_applicability( + diag, + "put the `;` here", + Applicability::MachineApplicable, + [(remove_span, String::new()), (insert_span, ";".to_owned())], + ); + }, + ); } diff --git a/tests/ui/semicolon_inside_block.fixed b/tests/ui/semicolon_inside_block.fixed index 4cd112dd5..d2cd8b218 100644 --- a/tests/ui/semicolon_inside_block.fixed +++ b/tests/ui/semicolon_inside_block.fixed @@ -10,7 +10,7 @@ macro_rules! m { (()) => { - () + (); }; (0) => {{ 0 @@ -58,7 +58,7 @@ fn main() { unit_fn_block(); }; - { m!(()); } + { m!(()) } { m!(()); } { m!(()); }; m!(0); diff --git a/tests/ui/semicolon_inside_block.stderr b/tests/ui/semicolon_inside_block.stderr index febe74b49..99f3b65be 100644 --- a/tests/ui/semicolon_inside_block.stderr +++ b/tests/ui/semicolon_inside_block.stderr @@ -2,15 +2,26 @@ error: consider moving the `;` inside the block for consistent formatting --> $DIR/semicolon_inside_block.rs:39:5 | LL | { unit_fn_block() }; - | ^^^^^^^^^^^^^^^^^^^^ help: put the `;` here: `{ unit_fn_block(); }` + | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::semicolon-inside-block` implied by `-D warnings` +help: put the `;` here + | +LL - { unit_fn_block() }; +LL + { unit_fn_block(); } + | error: consider moving the `;` inside the block for consistent formatting --> $DIR/semicolon_inside_block.rs:40:5 | LL | unsafe { unit_fn_block() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: put the `;` here: `unsafe { unit_fn_block(); }` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: put the `;` here + | +LL - unsafe { unit_fn_block() }; +LL + unsafe { unit_fn_block(); } + | error: consider moving the `;` inside the block for consistent formatting --> $DIR/semicolon_inside_block.rs:48:5 @@ -23,17 +34,24 @@ LL | | }; | help: put the `;` here | -LL ~ { -LL + unit_fn_block(); -LL + unit_fn_block(); -LL + } +LL ~ unit_fn_block(); +LL ~ } | error: consider moving the `;` inside the block for consistent formatting --> $DIR/semicolon_inside_block.rs:61:5 | LL | { m!(()) }; - | ^^^^^^^^^^^ help: put the `;` here: `{ m!(()); }` + | ^^^^^^^^^^^ + | +help: put the `;` here + | +LL ~ (); +LL | }; + ... +LL | +LL ~ { m!(()) } + | error: aborting due to 4 previous errors diff --git a/tests/ui/semicolon_outside_block.fixed b/tests/ui/semicolon_outside_block.fixed index 5bc18faaa..35eacfe6d 100644 --- a/tests/ui/semicolon_outside_block.fixed +++ b/tests/ui/semicolon_outside_block.fixed @@ -21,6 +21,9 @@ macro_rules! m { (2) => {{ 2; }}; + (stmt) => { + stmt; + }; } fn unit_fn_block() { @@ -39,8 +42,8 @@ fn main() { { unit_fn_block() }; unsafe { unit_fn_block() }; - { unit_fn_block() }; - unsafe { unit_fn_block() }; + { unit_fn_block(); } + unsafe { unit_fn_block(); } { unit_fn_block(); }; unsafe { unit_fn_block(); }; @@ -51,19 +54,20 @@ fn main() { }; { unit_fn_block(); - unit_fn_block() - }; + unit_fn_block(); + } { unit_fn_block(); unit_fn_block(); }; { m!(()) }; - { m!(()) }; + { m!(()); } { m!(()); }; m!(0); m!(1); m!(2); + { m!(stmt) }; for _ in [()] { unit_fn_block(); diff --git a/tests/ui/semicolon_outside_block.stderr b/tests/ui/semicolon_outside_block.stderr index fddf51208..c2151f7af 100644 --- a/tests/ui/semicolon_outside_block.stderr +++ b/tests/ui/semicolon_outside_block.stderr @@ -2,15 +2,26 @@ error: consider moving the `;` outside the block for consistent formatting --> $DIR/semicolon_outside_block.rs:42:5 | LL | { unit_fn_block(); } - | ^^^^^^^^^^^^^^^^^^^^ help: put the `;` outside the block: `{ unit_fn_block() };` + | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::semicolon-outside-block` implied by `-D warnings` +help: put the `;` here + | +LL - { unit_fn_block(); } +LL + { unit_fn_block() }; + | error: consider moving the `;` outside the block for consistent formatting --> $DIR/semicolon_outside_block.rs:43:5 | LL | unsafe { unit_fn_block(); } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: put the `;` outside the block: `unsafe { unit_fn_block() };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: put the `;` here + | +LL - unsafe { unit_fn_block(); } +LL + unsafe { unit_fn_block() }; + | error: consider moving the `;` outside the block for consistent formatting --> $DIR/semicolon_outside_block.rs:52:5 @@ -21,19 +32,23 @@ LL | | unit_fn_block(); LL | | } | |_____^ | -help: put the `;` outside the block +help: put the `;` here | -LL ~ { -LL + unit_fn_block(); -LL + unit_fn_block() -LL + }; +LL ~ unit_fn_block() +LL ~ }; | error: consider moving the `;` outside the block for consistent formatting --> $DIR/semicolon_outside_block.rs:62:5 | LL | { m!(()); } - | ^^^^^^^^^^^ help: put the `;` outside the block: `{ m!(()) };` + | ^^^^^^^^^^^ + | +help: put the `;` here + | +LL - () +LL + (); }; + | error: aborting due to 4 previous errors