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;