From f1ce81faad6dee864646fd7cc086c5517e255a0e Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Tue, 27 Jul 2021 14:00:06 +0200 Subject: [PATCH 1/8] Added semicolon_inside_block lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 4 ++ clippy_lints/src/semicolon_inside_block.rs | 75 ++++++++++++++++++++++ tests/ui/semicolon_inside_block.rs | 48 ++++++++++++++ tests/ui/semicolon_inside_block.stderr | 58 +++++++++++++++++ 5 files changed, 186 insertions(+) create mode 100644 clippy_lints/src/semicolon_inside_block.rs create mode 100644 tests/ui/semicolon_inside_block.rs create mode 100644 tests/ui/semicolon_inside_block.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b89170073be..5648d675725d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2904,6 +2904,7 @@ Released 2018-09-13 [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors [`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned +[`semicolon_inside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse [`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse [`shadow_same`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_same diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 18600498e1c4..d0c944df6ff1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -331,6 +331,7 @@ mod returns; mod self_assignment; mod self_named_constructors; mod semicolon_if_nothing_returned; +mod semicolon_inside_block; mod serde_api; mod shadow; mod single_component_path_imports; @@ -903,6 +904,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: self_assignment::SELF_ASSIGNMENT, self_named_constructors::SELF_NAMED_CONSTRUCTORS, semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED, + semicolon_inside_block::SEMICOLON_INSIDE_BLOCK, serde_api::SERDE_API_MISUSE, shadow::SHADOW_REUSE, shadow::SHADOW_SAME, @@ -1133,6 +1135,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(redundant_else::REDUNDANT_ELSE), LintId::of(ref_option_ref::REF_OPTION_REF), LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED), + LintId::of(semicolon_inside_block::SEMICOLON_INSIDE_BLOCK), LintId::of(shadow::SHADOW_UNRELATED), LintId::of(strings::STRING_ADD_ASSIGN), LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS), @@ -2088,6 +2091,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box manual_ok_or::ManualOkOr); store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs); store.register_late_pass(|| box semicolon_if_nothing_returned::SemicolonIfNothingReturned); + store.register_late_pass(|| box semicolon_inside_block::SemicolonInsideBlock); store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync); let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::>(); store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods)); diff --git a/clippy_lints/src/semicolon_inside_block.rs b/clippy_lints/src/semicolon_inside_block.rs new file mode 100644 index 000000000000..0fe9d022d9de --- /dev/null +++ b/clippy_lints/src/semicolon_inside_block.rs @@ -0,0 +1,75 @@ +use crate::rustc_lint::LintContext; +use clippy_utils::diagnostics::span_lint_and_sugg; +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::{Block, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** For () returning expressions, check that the semicolon is inside the block. + /// + /// **Why is this bad?** For consistency it's best to have the semicolon inside/outside the block. Either way is fine and this lint suggests inside the block. + /// Take a look at `semicolon_outside_block` for the other alternative. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// unsafe { f(x) }; + /// ``` + /// Use instead: + /// ```rust + /// unsafe { f(x); } + /// ``` + pub SEMICOLON_INSIDE_BLOCK, + pedantic, + "add a semicolon inside the block" +} + +declare_lint_pass!(SemicolonInsideBlock => [SEMICOLON_INSIDE_BLOCK]); + +impl LateLintPass<'_> for SemicolonInsideBlock { + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) { + if_chain! { + if !in_macro(block.span); + if let Some(expr) = block.expr; + let t_expr = cx.typeck_results().expr_ty(expr); + if t_expr.is_unit(); + if let snippet = snippet_with_macro_callsite(cx, expr.span, "}"); + if !snippet.ends_with("};") && !snippet.ends_with('}'); + then { + // filter out the desugared `for` loop + if let ExprKind::DropTemps(..) = &expr.kind { + return; + } + + let expr_snip = snippet_with_macro_callsite(cx, expr.span, ".."); + + // check for the right suggestion and span, differs if the block spans + // multiple lines + let (suggestion, span) = if cx.sess().source_map().is_multiline(block.span) { + (format!("{};", expr_snip), expr.span.source_callsite()) + } else { + let block_with_pot_sem = cx.sess().source_map().span_extend_to_next_char(block.span, '\n', false); + let block_snip = snippet_with_macro_callsite(cx, block.span, ".."); + + (block_snip.replace(expr_snip.as_ref(), &format!("{};", expr_snip)), block_with_pot_sem) + }; + + span_lint_and_sugg( + cx, + SEMICOLON_INSIDE_BLOCK, + span, + "consider moving the `;` inside the block for consistent formatting", + "put the `;` here", + suggestion, + Applicability::MaybeIncorrect, + ); + } + } + } +} diff --git a/tests/ui/semicolon_inside_block.rs b/tests/ui/semicolon_inside_block.rs new file mode 100644 index 000000000000..0888cd625a25 --- /dev/null +++ b/tests/ui/semicolon_inside_block.rs @@ -0,0 +1,48 @@ +#![warn(clippy::semicolon_inside_block)] + +unsafe fn f(arg: u32) {} + +fn main() { + let x = 32; + + unsafe { f(x) }; +} + +fn get_unit() {} + +fn fooooo() { + unsafe { f(32) } +} + +fn moin() { + println!("Hello") +} + +fn hello() { + get_unit() +} + +fn basic101(x: i32) { + let y: i32; + y = x + 1 +} + +#[rustfmt::skip] +fn closure_error() { + let _d = || { + hello() + }; +} + +fn my_own_block() { + let x: i32; + { + let y = 42; + x = y + 1; + basic101(x) + } + assert_eq!(43, 43) +} + +#[rustfmt::skip] +fn one_line_block() { println!("Foo") } diff --git a/tests/ui/semicolon_inside_block.stderr b/tests/ui/semicolon_inside_block.stderr new file mode 100644 index 000000000000..0e900ba79b1b --- /dev/null +++ b/tests/ui/semicolon_inside_block.stderr @@ -0,0 +1,58 @@ +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:8:5 + | +LL | unsafe { f(x) }; + | ^^^^^^^^^^^^^^^^ help: put the `;` here: `unsafe { f(x); }` + | + = note: `-D clippy::semicolon-inside-block` implied by `-D warnings` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:14:5 + | +LL | unsafe { f(32) } + | ^^^^^^^^^^^^^^^^ help: put the `;` here: `unsafe { f(32); }` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:18:5 + | +LL | println!("Hello") + | ^^^^^^^^^^^^^^^^^ help: put the `;` here: `println!("Hello");` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:22:5 + | +LL | get_unit() + | ^^^^^^^^^^ help: put the `;` here: `get_unit();` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:27:5 + | +LL | y = x + 1 + | ^^^^^^^^^ help: put the `;` here: `y = x + 1;` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:33:9 + | +LL | hello() + | ^^^^^^^ help: put the `;` here: `hello();` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:44:5 + | +LL | assert_eq!(43, 43) + | ^^^^^^^^^^^^^^^^^^ help: put the `;` here: `assert_eq!(43, 43);` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:42:9 + | +LL | basic101(x) + | ^^^^^^^^^^^ help: put the `;` here: `basic101(x);` + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:48:21 + | +LL | fn one_line_block() { println!("Foo") } + | ^^^^^^^^^^^^^^^^^^^ help: put the `;` here: `{ println!("Foo"); }` + +error: aborting due to 9 previous errors + From 8a47d2090ac51a978bb7d9cdb349e36de2099886 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Fri, 13 Aug 2021 10:25:20 +0200 Subject: [PATCH 2/8] Added semicolon_outside_block lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 4 + clippy_lints/src/semicolon_outside_block.rs | 102 ++++++++++++++++++++ tests/ui/semicolon_outside_block.rs | 63 ++++++++++++ tests/ui/semicolon_outside_block.stderr | 82 ++++++++++++++++ 5 files changed, 252 insertions(+) create mode 100644 clippy_lints/src/semicolon_outside_block.rs create mode 100644 tests/ui/semicolon_outside_block.rs create mode 100644 tests/ui/semicolon_outside_block.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 5648d675725d..3f81eacf357f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2905,6 +2905,7 @@ Released 2018-09-13 [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors [`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned [`semicolon_inside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block +[`semicolon_outside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse [`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse [`shadow_same`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_same diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d0c944df6ff1..c3f53db53be8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -332,6 +332,7 @@ mod self_assignment; mod self_named_constructors; mod semicolon_if_nothing_returned; mod semicolon_inside_block; +mod semicolon_outside_block; mod serde_api; mod shadow; mod single_component_path_imports; @@ -905,6 +906,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: self_named_constructors::SELF_NAMED_CONSTRUCTORS, semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED, semicolon_inside_block::SEMICOLON_INSIDE_BLOCK, + semicolon_outside_block::SEMICOLON_OUTSIDE_BLOCK, serde_api::SERDE_API_MISUSE, shadow::SHADOW_REUSE, shadow::SHADOW_SAME, @@ -1136,6 +1138,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(ref_option_ref::REF_OPTION_REF), LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED), LintId::of(semicolon_inside_block::SEMICOLON_INSIDE_BLOCK), + LintId::of(semicolon_outside_block::SEMICOLON_OUTSIDE_BLOCK), LintId::of(shadow::SHADOW_UNRELATED), LintId::of(strings::STRING_ADD_ASSIGN), LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS), @@ -2092,6 +2095,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs); store.register_late_pass(|| box semicolon_if_nothing_returned::SemicolonIfNothingReturned); store.register_late_pass(|| box semicolon_inside_block::SemicolonInsideBlock); + store.register_late_pass(|| box semicolon_outside_block::SemicolonOutsideBlock); store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync); let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::>(); store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods)); diff --git a/clippy_lints/src/semicolon_outside_block.rs b/clippy_lints/src/semicolon_outside_block.rs new file mode 100644 index 000000000000..6e9166c2fb6d --- /dev/null +++ b/clippy_lints/src/semicolon_outside_block.rs @@ -0,0 +1,102 @@ +use crate::rustc_lint::LintContext; +use clippy_utils::diagnostics::span_lint_and_sugg; +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::ExprKind; +use rustc_hir::{Block, StmtKind, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::BytePos; +use rustc_span::Span; +use std::ops::Add; + +declare_clippy_lint! { + /// **What it does:** For () returning expressions, check that the semicolon is outside the block. + /// + /// **Why is this bad?** For consistency it's best to have the semicolon inside/outside the block. Either way is fine and this lint suggests outside the block. + /// Take a look at `semicolon_inside_block` for the other alternative. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// unsafe { f(x); } + /// ``` + /// Use instead: + /// ```rust + /// unsafe { f(x) }; + /// ``` + pub SEMICOLON_OUTSIDE_BLOCK, + pedantic, + "add a semicolon outside the block" +} + +declare_lint_pass!(SemicolonOutsideBlock => [SEMICOLON_OUTSIDE_BLOCK]); + +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(); + + // make sure that the block does not belong to a function + let parent_item_id = cx.tcx.hir().get_parent_item(block.hir_id); + let parent_item = cx.tcx.hir().expect_item(parent_item_id); + if let ItemKind::Fn(_, _, body_id) = parent_item.kind; + let item_body = cx.tcx.hir().body(body_id); + if let ExprKind::Block(fn_block, _) = item_body.value.kind; + if fn_block.hir_id != block.hir_id; + then { + // filter out other blocks and the desugared for loop + if let ExprKind::Block(..) | ExprKind::DropTemps(..) = expr.kind { return } + + // 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(); + + // 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, + ); + } + } + } +} + +/// Takes a span and extzends it until after a semicolon in the last line of the span. +fn expand_span_to_semicolon<'tcx>(cx: &LateContext<'tcx>, expr_span: Span) -> Span { + let expr_span_with_sem = cx.sess().source_map().span_extend_to_next_char(expr_span, ';', false); + expr_span_with_sem.with_hi(expr_span_with_sem.hi().add(BytePos(1))) +} diff --git a/tests/ui/semicolon_outside_block.rs b/tests/ui/semicolon_outside_block.rs new file mode 100644 index 000000000000..96fdff1392c5 --- /dev/null +++ b/tests/ui/semicolon_outside_block.rs @@ -0,0 +1,63 @@ +#![warn(clippy::semicolon_outside_block)] + +unsafe fn f(arg: u32) {} + +#[rustfmt::skip] +fn main() { + let x = 32; + + unsafe { f(x); } +} + +fn foo() { + let x = 32; + + unsafe { + f(x); + } +} + +fn bar() { + let x = 32; + + unsafe { + let _this = 1; + let _is = 2; + let _a = 3; + let _long = 4; + let _list = 5; + let _of = 6; + let _variables = 7; + f(x); + }; +} + +fn get_unit() {} + +fn moin() { + { + let _u = get_unit(); + println!("Hello"); + } +} + +#[rustfmt::skip] +fn closure_error() { + let _d = || { + get_unit(); + }; +} + +fn my_own_block() { + let x: i32; + { + let y = 42; + x = y + 1; + get_unit(); + } + assert_eq!(43, 43) +} + +fn just_get_unit() { + get_unit(); +} diff --git a/tests/ui/semicolon_outside_block.stderr b/tests/ui/semicolon_outside_block.stderr new file mode 100644 index 000000000000..3523d3d3e74b --- /dev/null +++ b/tests/ui/semicolon_outside_block.stderr @@ -0,0 +1,82 @@ +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:9:5 + | +LL | unsafe { f(x); } + | ^^^^^^^^^^^^^^^^ help: put the `;` outside the block: `unsafe { f(x) };` + | + = note: `-D clippy::semicolon-outside-block` implied by `-D warnings` + +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:15:5 + | +LL | / unsafe { +LL | | f(x); +LL | | } + | |_____^ + | +help: put the `;` outside the block + | +LL | unsafe { +LL | f(x) +LL | }; + | + +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:23:5 + | +LL | / unsafe { +LL | | let _this = 1; +LL | | let _is = 2; +LL | | let _a = 3; +... | +LL | | f(x); +LL | | }; + | |______^ + | +help: put the `;` outside the block + | +LL | unsafe { +LL | let _this = 1; +LL | let _is = 2; +LL | let _a = 3; +LL | let _long = 4; +LL | let _list = 5; + ... + +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:46:17 + | +LL | let _d = || { + | _________________^ +LL | | get_unit(); +LL | | }; + | |______^ + | +help: put the `;` outside the block + | +LL | let _d = || { +LL | get_unit() +LL | }; + | + +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:53:5 + | +LL | / { +LL | | let y = 42; +LL | | x = y + 1; +LL | | get_unit(); +LL | | } + | |_____^ + | +help: put the `;` outside the block + | +LL | { +LL | let y = 42; +LL | x = y + 1; +LL | get_unit() +LL | }; + | + +error: aborting due to 5 previous errors + From 595feab95eb5e94d2e863fe8be82dbff3a5986d1 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Fri, 13 Aug 2021 11:16:55 +0200 Subject: [PATCH 3/8] Updated semicolon_outside_block lint to fix a bug --- clippy_lints/src/semicolon_outside_block.rs | 23 ++++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/semicolon_outside_block.rs b/clippy_lints/src/semicolon_outside_block.rs index 6e9166c2fb6d..b800e44aaa4d 100644 --- a/clippy_lints/src/semicolon_outside_block.rs +++ b/clippy_lints/src/semicolon_outside_block.rs @@ -5,7 +5,7 @@ use clippy_utils::source::snippet_with_macro_callsite; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::ExprKind; -use rustc_hir::{Block, StmtKind, ItemKind}; +use rustc_hir::{Block, BodyOwnerKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::BytePos; @@ -45,15 +45,18 @@ impl LateLintPass<'_> for SemicolonOutsideBlock { if let StmtKind::Semi(expr) = last.kind; let t_expr = cx.typeck_results().expr_ty(expr); if t_expr.is_unit(); - - // make sure that the block does not belong to a function - let parent_item_id = cx.tcx.hir().get_parent_item(block.hir_id); - let parent_item = cx.tcx.hir().expect_item(parent_item_id); - if let ItemKind::Fn(_, _, body_id) = parent_item.kind; - let item_body = cx.tcx.hir().body(body_id); - if let ExprKind::Block(fn_block, _) = item_body.value.kind; - if fn_block.hir_id != block.hir_id; then { + // make sure that the block does not belong to a function + for (hir_id, _) in cx.tcx.hir().parent_iter(block.hir_id) { + if_chain! { + if let Some(body_id) = cx.tcx.hir().maybe_body_owned_by(hir_id); + if let BodyOwnerKind::Fn = cx.tcx.hir().body_owner_kind(hir_id); + let item_body = cx.tcx.hir().body(body_id); + if let ExprKind::Block(fn_block, _) = item_body.value.kind; + if fn_block.hir_id == block.hir_id; + then { return } + } + } // filter out other blocks and the desugared for loop if let ExprKind::Block(..) | ExprKind::DropTemps(..) = expr.kind { return } @@ -95,7 +98,7 @@ impl LateLintPass<'_> for SemicolonOutsideBlock { } } -/// Takes a span and extzends it until after a semicolon in the last line of the span. +/// Takes a span and extends it until after a semicolon in the last line of the span. fn expand_span_to_semicolon<'tcx>(cx: &LateContext<'tcx>, expr_span: Span) -> Span { let expr_span_with_sem = cx.sess().source_map().span_extend_to_next_char(expr_span, ';', false); expr_span_with_sem.with_hi(expr_span_with_sem.hi().add(BytePos(1))) From 28075237b5094f0f0633fd127b5d4beb1cec40c0 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Fri, 13 Aug 2021 11:38:55 +0200 Subject: [PATCH 4/8] Fixed error with `if` blocks --- clippy_lints/src/semicolon_outside_block.rs | 21 ++++++++++++++------- tests/ui/semicolon_outside_block.rs | 8 ++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/semicolon_outside_block.rs b/clippy_lints/src/semicolon_outside_block.rs index b800e44aaa4d..eebd923871d0 100644 --- a/clippy_lints/src/semicolon_outside_block.rs +++ b/clippy_lints/src/semicolon_outside_block.rs @@ -48,13 +48,20 @@ impl LateLintPass<'_> for SemicolonOutsideBlock { then { // make sure that the block does not belong to a function for (hir_id, _) in cx.tcx.hir().parent_iter(block.hir_id) { - if_chain! { - if let Some(body_id) = cx.tcx.hir().maybe_body_owned_by(hir_id); - if let BodyOwnerKind::Fn = cx.tcx.hir().body_owner_kind(hir_id); - let item_body = cx.tcx.hir().body(body_id); - if let ExprKind::Block(fn_block, _) = item_body.value.kind; - if fn_block.hir_id == block.hir_id; - then { return } + if let Some(body_id) = cx.tcx.hir().maybe_body_owned_by(hir_id) { + if let BodyOwnerKind::Fn = cx.tcx.hir().body_owner_kind(hir_id) { + let item_body = cx.tcx.hir().body(body_id); + if let ExprKind::Block(fn_block, _) = item_body.value.kind { + if let Some(pot_if) = fn_block.expr { + if let ExprKind::If(..) = pot_if.kind { + return; + } + } + if fn_block.hir_id == block.hir_id { + return + } + } + } } } // filter out other blocks and the desugared for loop diff --git a/tests/ui/semicolon_outside_block.rs b/tests/ui/semicolon_outside_block.rs index 96fdff1392c5..35deea1528b3 100644 --- a/tests/ui/semicolon_outside_block.rs +++ b/tests/ui/semicolon_outside_block.rs @@ -61,3 +61,11 @@ fn my_own_block() { fn just_get_unit() { get_unit(); } + +fn test_if() { + if 1 > 2 { + get_unit(); + } else { + println!("everything alright!"); + } +} From bbc3c2158a6c5e7c2e078054400b8bacd88647c3 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Fri, 13 Aug 2021 11:55:43 +0200 Subject: [PATCH 5/8] Fixed error with `for` and `match` blocks --- clippy_lints/src/semicolon_outside_block.rs | 31 +++++++++++++++++---- tests/ui/semicolon_outside_block.rs | 18 ++++++++++++ tests/ui/semicolon_outside_block.stderr | 10 +++---- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/semicolon_outside_block.rs b/clippy_lints/src/semicolon_outside_block.rs index eebd923871d0..8a71a871a21c 100644 --- a/clippy_lints/src/semicolon_outside_block.rs +++ b/clippy_lints/src/semicolon_outside_block.rs @@ -49,23 +49,44 @@ impl LateLintPass<'_> for SemicolonOutsideBlock { // make sure that the block does not belong to a function 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 let BodyOwnerKind::Fn = cx.tcx.hir().body_owner_kind(hir_id) { + 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 { - if let Some(pot_if) = fn_block.expr { - if let ExprKind::If(..) = pot_if.kind { + for stmt in fn_block.stmts { + if let StmtKind::Expr(pot_ille_expr) = stmt.kind { + if let ExprKind::If(..) | + ExprKind::Loop(..) | + ExprKind::DropTemps(..) | + ExprKind::Match(..) = pot_ille_expr.kind { + return + } + } + } + + if let Some(last_expr) = fn_block.expr { + if let ExprKind::If(..) | + ExprKind::Loop(..) | + ExprKind::DropTemps(..) | + ExprKind::Match(..) = last_expr.kind { return; } } - if fn_block.hir_id == block.hir_id { + + if fn_block.hir_id == block.hir_id && !matches!(cx.tcx.hir().body_owner_kind(hir_id), BodyOwnerKind::Closure) { return } } } } } + + // filter out other blocks and the desugared for loop - if let ExprKind::Block(..) | ExprKind::DropTemps(..) = expr.kind { return } + if let ExprKind::Block(..) | + ExprKind::DropTemps(..) | + ExprKind::If(..) | + ExprKind::Loop(..) | + ExprKind::Match(..) = expr.kind { return } // 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); diff --git a/tests/ui/semicolon_outside_block.rs b/tests/ui/semicolon_outside_block.rs index 35deea1528b3..131034e70cd0 100644 --- a/tests/ui/semicolon_outside_block.rs +++ b/tests/ui/semicolon_outside_block.rs @@ -1,4 +1,5 @@ #![warn(clippy::semicolon_outside_block)] +#![allow(clippy::single_match)] unsafe fn f(arg: u32) {} @@ -69,3 +70,20 @@ fn test_if() { println!("everything alright!"); } } + +fn test_for() { + for project in &[ + "clippy_workspace_tests", + "clippy_workspace_tests/src", + "clippy_workspace_tests/subcrate", + "clippy_workspace_tests/subcrate/src", + "clippy_dev", + "clippy_lints", + "clippy_utils", + "rustc_tools_util", + ] { + get_unit(); + } + + get_unit(); +} diff --git a/tests/ui/semicolon_outside_block.stderr b/tests/ui/semicolon_outside_block.stderr index 3523d3d3e74b..13db8f216aeb 100644 --- a/tests/ui/semicolon_outside_block.stderr +++ b/tests/ui/semicolon_outside_block.stderr @@ -1,5 +1,5 @@ error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:9:5 + --> $DIR/semicolon_outside_block.rs:10:5 | LL | unsafe { f(x); } | ^^^^^^^^^^^^^^^^ help: put the `;` outside the block: `unsafe { f(x) };` @@ -7,7 +7,7 @@ LL | unsafe { f(x); } = note: `-D clippy::semicolon-outside-block` implied by `-D warnings` error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:15:5 + --> $DIR/semicolon_outside_block.rs:16:5 | LL | / unsafe { LL | | f(x); @@ -22,7 +22,7 @@ LL | }; | error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:23:5 + --> $DIR/semicolon_outside_block.rs:24:5 | LL | / unsafe { LL | | let _this = 1; @@ -44,7 +44,7 @@ LL | let _list = 5; ... error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:46:17 + --> $DIR/semicolon_outside_block.rs:47:17 | LL | let _d = || { | _________________^ @@ -60,7 +60,7 @@ LL | }; | error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:53:5 + --> $DIR/semicolon_outside_block.rs:54:5 | LL | / { LL | | let y = 42; From 27fe0475ae6a21c2f97a938369e5fa05b693d9fa Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Fri, 13 Aug 2021 14:09:01 +0200 Subject: [PATCH 6/8] Added .fixed test file --- clippy_lints/src/semicolon_outside_block.rs | 42 +++++----- tests/ui/semicolon_outside_block.fixed | 85 +++++++++++++++++++++ tests/ui/semicolon_outside_block.rs | 18 ++--- tests/ui/semicolon_outside_block.stderr | 10 +-- 4 files changed, 119 insertions(+), 36 deletions(-) create mode 100644 tests/ui/semicolon_outside_block.fixed diff --git a/clippy_lints/src/semicolon_outside_block.rs b/clippy_lints/src/semicolon_outside_block.rs index 8a71a871a21c..aff3714a78ea 100644 --- a/clippy_lints/src/semicolon_outside_block.rs +++ b/clippy_lints/src/semicolon_outside_block.rs @@ -36,6 +36,18 @@ 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! { @@ -46,33 +58,28 @@ impl LateLintPass<'_> for SemicolonOutsideBlock { 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 + // 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 { - if let ExprKind::If(..) | - ExprKind::Loop(..) | - ExprKind::DropTemps(..) | - ExprKind::Match(..) = pot_ille_expr.kind { - return - } + check_expr_return!(pot_ille_expr.kind); } } + //.. the potential last statement .. if let Some(last_expr) = fn_block.expr { - if let ExprKind::If(..) | - ExprKind::Loop(..) | - ExprKind::DropTemps(..) | - ExprKind::Match(..) = last_expr.kind { - return; - } + check_expr_return!(last_expr.kind); } - if fn_block.hir_id == block.hir_id && !matches!(cx.tcx.hir().body_owner_kind(hir_id), BodyOwnerKind::Closure) { + // .. 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 } } @@ -80,13 +87,8 @@ impl LateLintPass<'_> for SemicolonOutsideBlock { } } - // filter out other blocks and the desugared for loop - if let ExprKind::Block(..) | - ExprKind::DropTemps(..) | - ExprKind::If(..) | - ExprKind::Loop(..) | - ExprKind::Match(..) = expr.kind { return } + 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); diff --git a/tests/ui/semicolon_outside_block.fixed b/tests/ui/semicolon_outside_block.fixed new file mode 100644 index 000000000000..469ccfac92ab --- /dev/null +++ b/tests/ui/semicolon_outside_block.fixed @@ -0,0 +1,85 @@ +// run-rustfix +#![warn(clippy::semicolon_outside_block)] +#![allow(dead_code)] + +unsafe fn f(_arg: u32) {} + +#[rustfmt::skip] +fn main() { + let x = 32; + + unsafe { f(x) }; +} + +fn foo() { + let x = 32; + + unsafe { + f(x) + }; +} + +fn bar() { + let x = 32; + + unsafe { + let _this = 1; + let _is = 2; + let _a = 3; + let _long = 4; + let _list = 5; + let _of = 6; + let _variables = 7; + f(x) + }; +} + +fn get_unit() {} + +#[rustfmt::skip] +fn closure_error() { + let _d = || { + get_unit() + }; +} + +fn my_own_block() { + let x: i32; + { + let y = 42; + x = y + 1; + get_unit() + }; + assert_eq!(x, 43) +} + +// This is all ok + +fn just_get_unit() { + get_unit(); +} + +fn test_if() { + if 1 > 2 { + get_unit(); + } else { + println!("everything alright!"); + } +} + +fn test_for() { + for _project in &[ + "clippy_workspace_tests", + "clippy_workspace_tests/src", + "clippy_workspace_tests/subcrate", + "clippy_workspace_tests/subcrate/src", + "clippy_dev", + "clippy_lints", + "clippy_utils", + "rustc_tools_util", + ] { + get_unit(); + } + + get_unit(); +} diff --git a/tests/ui/semicolon_outside_block.rs b/tests/ui/semicolon_outside_block.rs index 131034e70cd0..e3f4b7a40e6b 100644 --- a/tests/ui/semicolon_outside_block.rs +++ b/tests/ui/semicolon_outside_block.rs @@ -1,7 +1,8 @@ +// run-rustfix #![warn(clippy::semicolon_outside_block)] -#![allow(clippy::single_match)] +#![allow(dead_code)] -unsafe fn f(arg: u32) {} +unsafe fn f(_arg: u32) {} #[rustfmt::skip] fn main() { @@ -35,13 +36,6 @@ fn bar() { fn get_unit() {} -fn moin() { - { - let _u = get_unit(); - println!("Hello"); - } -} - #[rustfmt::skip] fn closure_error() { let _d = || { @@ -56,9 +50,11 @@ fn my_own_block() { x = y + 1; get_unit(); } - assert_eq!(43, 43) + assert_eq!(x, 43) } +// This is all ok + fn just_get_unit() { get_unit(); } @@ -72,7 +68,7 @@ fn test_if() { } fn test_for() { - for project in &[ + for _project in &[ "clippy_workspace_tests", "clippy_workspace_tests/src", "clippy_workspace_tests/subcrate", diff --git a/tests/ui/semicolon_outside_block.stderr b/tests/ui/semicolon_outside_block.stderr index 13db8f216aeb..d74e73b4deb0 100644 --- a/tests/ui/semicolon_outside_block.stderr +++ b/tests/ui/semicolon_outside_block.stderr @@ -1,5 +1,5 @@ error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:10:5 + --> $DIR/semicolon_outside_block.rs:11:5 | LL | unsafe { f(x); } | ^^^^^^^^^^^^^^^^ help: put the `;` outside the block: `unsafe { f(x) };` @@ -7,7 +7,7 @@ LL | unsafe { f(x); } = note: `-D clippy::semicolon-outside-block` implied by `-D warnings` error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:16:5 + --> $DIR/semicolon_outside_block.rs:17:5 | LL | / unsafe { LL | | f(x); @@ -22,7 +22,7 @@ LL | }; | error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:24:5 + --> $DIR/semicolon_outside_block.rs:25:5 | LL | / unsafe { LL | | let _this = 1; @@ -44,7 +44,7 @@ LL | let _list = 5; ... error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:47:17 + --> $DIR/semicolon_outside_block.rs:41:17 | LL | let _d = || { | _________________^ @@ -60,7 +60,7 @@ LL | }; | error: consider moving the `;` outside the block for consistent formatting - --> $DIR/semicolon_outside_block.rs:54:5 + --> $DIR/semicolon_outside_block.rs:48:5 | LL | / { LL | | let y = 42; From ccbb20050b6d85f9d66f75014bcfa87b4535de83 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Fri, 13 Aug 2021 15:59:08 +0200 Subject: [PATCH 7/8] Fixed .stderr files --- tests/ui/semicolon_outside_block.stderr | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/ui/semicolon_outside_block.stderr b/tests/ui/semicolon_outside_block.stderr index d74e73b4deb0..ceabd0da0d2e 100644 --- a/tests/ui/semicolon_outside_block.stderr +++ b/tests/ui/semicolon_outside_block.stderr @@ -16,9 +16,9 @@ LL | | } | help: put the `;` outside the block | -LL | unsafe { -LL | f(x) -LL | }; +LL ~ unsafe { +LL + f(x) +LL + }; | error: consider moving the `;` outside the block for consistent formatting @@ -35,12 +35,12 @@ LL | | }; | help: put the `;` outside the block | -LL | unsafe { -LL | let _this = 1; -LL | let _is = 2; -LL | let _a = 3; -LL | let _long = 4; -LL | let _list = 5; +LL ~ unsafe { +LL + let _this = 1; +LL + let _is = 2; +LL + let _a = 3; +LL + let _long = 4; +LL + let _list = 5; ... error: consider moving the `;` outside the block for consistent formatting @@ -54,9 +54,9 @@ LL | | }; | help: put the `;` outside the block | -LL | let _d = || { -LL | get_unit() -LL | }; +LL ~ let _d = || { +LL + get_unit() +LL + }; | error: consider moving the `;` outside the block for consistent formatting @@ -71,11 +71,11 @@ LL | | } | help: put the `;` outside the block | -LL | { -LL | let y = 42; -LL | x = y + 1; -LL | get_unit() -LL | }; +LL ~ { +LL + let y = 42; +LL + x = y + 1; +LL + get_unit() +LL + }; | error: aborting due to 5 previous errors From ed4a438ad2dd4c84f151b180fec6cd3af339de92 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Sun, 22 Aug 2021 19:45:38 +0200 Subject: [PATCH 8/8] 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..5ec9c7fdc667 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, + ); } } }