fix: semicolon_inside_block don't eat closing paren in ({0});

clippy_lints/src/semicolon_block.rs

@@ -1,10 +1,11 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::{IntoSpan, SpanRangeExt};
 use rustc_errors::Applicability;
 use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::impl_lint_pass;
-use rustc_span::Span;
+use rustc_span::{BytePos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -78,9 +79,12 @@ impl SemicolonBlock {
         }
     }
 
-    fn semicolon_inside_block(&self, cx: &LateContext<'_>, block: &Block<'_>, tail: &Expr<'_>, semi_span: Span) {
+    fn semicolon_inside_block(&self, cx: &LateContext<'_>, tail: &Expr<'_>, semi_span: Span) {
         let insert_span = tail.span.source_callsite().shrink_to_hi();
-        let remove_span = semi_span.with_lo(block.span.hi());
+        let remove_span = semi_span
+            .with_lo(semi_span.hi() - BytePos(1)) // the last byte of `semi_span` is the `;`, so shrink to that
+            .with_leading_whitespace(cx)
+            .into_span();
 
         if self.semicolon_inside_block_ignore_singleline && get_line(cx, remove_span) == get_line(cx, insert_span) {
             return;
@@ -102,7 +106,7 @@ impl SemicolonBlock {
     }
 
     fn semicolon_outside_block(&self, cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_expr: &Expr<'_>) {
-        let insert_span = block.span.with_lo(block.span.hi());
+        let insert_span = block.span.shrink_to_hi();
 
         // For macro call semicolon statements (`mac!();`), the statement's span does not actually
         // include the semicolon itself, so use `mac_call_stmt_semi_span`, which finds the semicolon
@@ -144,38 +148,26 @@ impl LateLintPass<'_> for SemicolonBlock {
                 kind: ExprKind::Block(block, _),
                 ..
             }) if !block.span.from_expansion() && stmt.span.contains(block.span) => {
-                let Block {
-                    expr: None,
-                    stmts: [.., stmt],
-                    ..
-                } = block
-                else {
-                    return;
-                };
-                let &Stmt {
-                    kind: StmtKind::Semi(expr),
-                    ..
-                } = stmt
-                else {
-                    return;
-                };
-                self.semicolon_outside_block(cx, block, expr);
+                if let None = block.expr
+                    && let [.., stmt] = block.stmts
+                    && let StmtKind::Semi(expr) = stmt.kind
+                {
+                    self.semicolon_outside_block(cx, block, expr);
+                }
             },
             StmtKind::Semi(Expr {
-                kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _),
+                kind: ExprKind::Block(block, _),
                 ..
             }) if !block.span.from_expansion() => {
-                self.semicolon_inside_block(cx, block, tail, stmt.span);
+                if let Some(tail) = block.expr {
+                    self.semicolon_inside_block(cx, tail, stmt.span);
+                }
             },
             _ => (),
         }
     }
 }
 
 fn get_line(cx: &LateContext<'_>, span: Span) -> Option<usize> {
-    if let Ok(line) = cx.sess().source_map().lookup_line(span.lo()) {
-        return Some(line.line);
-    }
-
-    None
+    cx.sess().source_map().lookup_line(span.lo()).ok().map(|line| line.line)
 }

tests/ui/semicolon_inside_block.fixed

@@ -3,7 +3,8 @@
     clippy::unused_unit,
     clippy::unnecessary_operation,
     clippy::no_effect,
-    clippy::single_element_loop
+    clippy::single_element_loop,
+    clippy::double_parens
 )]
 #![warn(clippy::semicolon_inside_block)]
 
@@ -86,3 +87,65 @@ fn main() {
 
     unit_fn_block()
 }
+
+// TODO: merge the function bodies once https://github.com/rust-lang/rust-clippy/issues/15389 is fixed
+//
+// Right now, if `first` and `second` were to be merged (uitest comments omitted for clarity):
+// ``` fn issue15380() {
+//     ( {0;0});
+//     ({
+//         0;
+//         0
+//     });
+// }
+// ```
+// then the fixed version would look as follows:
+// ```
+// fn issue15380() {
+//     ( {0;0;})
+//     ({
+//         0;
+//         0
+//     });
+// }
+// ```
+// However, that looks like a function call `(f)(x)`, and so we get an error because `{0;0;}` is
+// not a function.
+mod issue15380 {
+    // TODO: apply `[rustfmt::skip]` only to the block once https://github.com/rust-lang/rust-clippy/issues/15388 is fixed
+    //
+    // If we do this right now:
+    // ```
+    // fn issue15380() {
+    //     #[rustfmt::skip]
+    //     ( {0;0});
+    // }
+    // ```
+    // then the fixed version would look as follows:
+    // ```
+    // fn issue15380() {
+    //     #[rustfmt::skip]
+    //     ( {0;0;})
+    // }
+    // ```
+    // But that wouldn't compile because `[rustfmt::skip]` is now placed on an expr
+    #[rustfmt::skip]
+    fn first() {
+        ( {0;0;})
+        //~^ semicolon_inside_block
+    }
+
+    fn second() {
+        ({
+            //~^ semicolon_inside_block
+            0;
+            0;
+        })
+    }
+
+    #[rustfmt::skip] // TODO: apply onto the block itself, see explanation in `first`
+    fn third() {
+        (({ 0; }))
+        //~^ semicolon_inside_block
+    }
+}

tests/ui/semicolon_inside_block.rs

@@ -3,7 +3,8 @@
     clippy::unused_unit,
     clippy::unnecessary_operation,
     clippy::no_effect,
-    clippy::single_element_loop
+    clippy::single_element_loop,
+    clippy::double_parens
 )]
 #![warn(clippy::semicolon_inside_block)]
 
@@ -86,3 +87,65 @@ fn main() {
 
     unit_fn_block()
 }
+
+// TODO: merge the function bodies once https://github.com/rust-lang/rust-clippy/issues/15389 is fixed
+//
+// Right now, if `first` and `second` were to be merged (uitest comments omitted for clarity):
+// ``` fn issue15380() {
+//     ( {0;0});
+//     ({
+//         0;
+//         0
+//     });
+// }
+// ```
+// then the fixed version would look as follows:
+// ```
+// fn issue15380() {
+//     ( {0;0;})
+//     ({
+//         0;
+//         0
+//     });
+// }
+// ```
+// However, that looks like a function call `(f)(x)`, and so we get an error because `{0;0;}` is
+// not a function.
+mod issue15380 {
+    // TODO: apply `[rustfmt::skip]` only to the block once https://github.com/rust-lang/rust-clippy/issues/15388 is fixed
+    //
+    // If we do this right now:
+    // ```
+    // fn issue15380() {
+    //     #[rustfmt::skip]
+    //     ( {0;0});
+    // }
+    // ```
+    // then the fixed version would look as follows:
+    // ```
+    // fn issue15380() {
+    //     #[rustfmt::skip]
+    //     ( {0;0;})
+    // }
+    // ```
+    // But that wouldn't compile because `[rustfmt::skip]` is now placed on an expr
+    #[rustfmt::skip]
+    fn first() {
+        ( {0;0});
+        //~^ semicolon_inside_block
+    }
+
+    fn second() {
+        ({
+            //~^ semicolon_inside_block
+            0;
+            0
+        });
+    }
+
+    #[rustfmt::skip] // TODO: apply onto the block itself, see explanation in `first`
+    fn third() {
+        (({ 0 }))      ;
+        //~^ semicolon_inside_block
+    }
+}

tests/ui/semicolon_inside_block.stderr

@@ -1,5 +1,5 @@
 error: consider moving the `;` inside the block for consistent formatting
-  --> tests/ui/semicolon_inside_block.rs:38:5
+  --> tests/ui/semicolon_inside_block.rs:39:5
    |
 LL |     { unit_fn_block() };
    |     ^^^^^^^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL +     { unit_fn_block(); }
    |
 
 error: consider moving the `;` inside the block for consistent formatting
-  --> tests/ui/semicolon_inside_block.rs:40:5
+  --> tests/ui/semicolon_inside_block.rs:41:5
    |
 LL |     unsafe { unit_fn_block() };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL +     unsafe { unit_fn_block(); }
    |
 
 error: consider moving the `;` inside the block for consistent formatting
-  --> tests/ui/semicolon_inside_block.rs:49:5
+  --> tests/ui/semicolon_inside_block.rs:50:5
    |
 LL | /     {
 LL | |
@@ -41,7 +41,7 @@ LL ~     }
    |
 
 error: consider moving the `;` inside the block for consistent formatting
-  --> tests/ui/semicolon_inside_block.rs:63:5
+  --> tests/ui/semicolon_inside_block.rs:64:5
    |
 LL |     { m!(()) };
    |     ^^^^^^^^^^^
@@ -52,5 +52,45 @@ LL -     { m!(()) };
 LL +     { m!(()); }
    |
 
-error: aborting due to 4 previous errors
+error: consider moving the `;` inside the block for consistent formatting
+  --> tests/ui/semicolon_inside_block.rs:134:9
+   |
+LL |         ( {0;0});
+   |         ^^^^^^^^^
+   |
+help: put the `;` here
+   |
+LL -         ( {0;0});
+LL +         ( {0;0;})
+   |
+
+error: consider moving the `;` inside the block for consistent formatting
+  --> tests/ui/semicolon_inside_block.rs:139:9
+   |
+LL | /         ({
+LL | |
+LL | |             0;
+LL | |             0
+LL | |         });
+   | |___________^
+   |
+help: put the `;` here
+   |
+LL ~             0;
+LL ~         })
+   |
+
+error: consider moving the `;` inside the block for consistent formatting
+  --> tests/ui/semicolon_inside_block.rs:148:9
+   |
+LL |         (({ 0 }))      ;
+   |         ^^^^^^^^^^^^^^^^
+   |
+help: put the `;` here
+   |
+LL -         (({ 0 }))      ;
+LL +         (({ 0; }))
+   |
+
+error: aborting due to 7 previous errors