Add semicolon inside/outside block lints

CHANGELOG.md

@@ -2904,6 +2904,8 @@ 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
+[`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

clippy_lints/src/lib.rs

@@ -331,6 +331,8 @@ mod returns;
 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;
@@ -903,6 +905,8 @@ 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,
+        semicolon_outside_block::SEMICOLON_OUTSIDE_BLOCK,
         serde_api::SERDE_API_MISUSE,
         shadow::SHADOW_REUSE,
         shadow::SHADOW_SAME,
@@ -1133,6 +1137,8 @@ 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(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),
@@ -2088,6 +2094,8 @@ 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 semicolon_outside_block::SemicolonOutsideBlock);
     store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
     let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::<FxHashSet<_>>();
     store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods));

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,
+            );
+         }
+        }
+    }
+}

clippy_lints/src/semicolon_outside_block.rs

@@ -0,0 +1,160 @@
+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::intravisit::FnKind;
+use rustc_hir::ExprKind;
+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;
+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_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);
+            }
+
+            // 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),
+                    _ => (),
+                }
+            }
+
+            // 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);
+            }
+        }
+    }
+}
+
+/// 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);
+    }
+
+    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();
+
+            // 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 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)))
+}

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") }

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
+