From c3bd902c4a7b8ec6163d60900068df9f11cd7630 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 09:27:07 +0200 Subject: [PATCH 1/9] misc: match on `StmtKind` directly --- clippy_lints/src/semicolon_block.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index f6c128d4c529..6007ab3a77b8 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -152,11 +152,7 @@ impl LateLintPass<'_> for SemicolonBlock { else { return; }; - let &Stmt { - kind: StmtKind::Semi(expr), - .. - } = stmt - else { + let StmtKind::Semi(expr) = stmt.kind else { return; }; self.semicolon_outside_block(cx, block, expr); From c19853e397d7513ff6a6b21c982636854f199ed8 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 09:29:44 +0200 Subject: [PATCH 2/9] misc: use a let-chain --- clippy_lints/src/semicolon_block.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index 6007ab3a77b8..3e367276fdea 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -144,18 +144,15 @@ impl LateLintPass<'_> for SemicolonBlock { kind: ExprKind::Block(block, _), .. }) if !block.span.from_expansion() && stmt.span.contains(block.span) => { - let Block { + if let Block { expr: None, stmts: [.., stmt], .. } = block - else { - return; - }; - let StmtKind::Semi(expr) = stmt.kind else { - return; - }; - self.semicolon_outside_block(cx, block, expr); + && let StmtKind::Semi(expr) = stmt.kind + { + self.semicolon_outside_block(cx, block, expr); + } }, StmtKind::Semi(Expr { kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _), From 3c2b14370f7796e8dae98f2dc4422065bcba390a Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 09:30:11 +0200 Subject: [PATCH 3/9] misc: break up match for a nicer let-chain --- clippy_lints/src/semicolon_block.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index 3e367276fdea..b8a2d7e99a35 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -144,11 +144,8 @@ impl LateLintPass<'_> for SemicolonBlock { kind: ExprKind::Block(block, _), .. }) if !block.span.from_expansion() && stmt.span.contains(block.span) => { - if let Block { - expr: None, - stmts: [.., stmt], - .. - } = block + if let None = block.expr + && let [.., stmt] = block.stmts && let StmtKind::Semi(expr) = stmt.kind { self.semicolon_outside_block(cx, block, expr); From e406f8fd7932347fdfee867a729769871b159161 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 09:31:23 +0200 Subject: [PATCH 4/9] misc: shorten `get_line` --- clippy_lints/src/semicolon_block.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index b8a2d7e99a35..0bbcc041e152 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -163,9 +163,5 @@ impl LateLintPass<'_> for SemicolonBlock { } fn get_line(cx: &LateContext<'_>, span: Span) -> Option { - 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) } From 528afcb6b6b1c9cb20daca064bbaada703b8b546 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 10:14:25 +0200 Subject: [PATCH 5/9] misc: move `expr` check down in the inside case as well for consistency --- clippy_lints/src/semicolon_block.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index 0bbcc041e152..f4c4912deea3 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -152,10 +152,12 @@ impl LateLintPass<'_> for SemicolonBlock { } }, 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, block, tail, stmt.span); + } }, _ => (), } From 04f4b5b1b8910d0b748a2e8359b2f3dc37db9bc0 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 12:00:55 +0200 Subject: [PATCH 6/9] misc: use `shrink_to_hi` --- clippy_lints/src/semicolon_block.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index f4c4912deea3..18ff68a9bb0c 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -102,7 +102,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 From c8c6eff351ae3b97a1a19fdb041326cfa686c80c Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 10:50:04 +0200 Subject: [PATCH 7/9] add failing tests --- tests/ui/semicolon_inside_block.fixed | 19 +++++++++- tests/ui/semicolon_inside_block.rs | 19 +++++++++- tests/ui/semicolon_inside_block.stderr | 50 +++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/tests/ui/semicolon_inside_block.fixed b/tests/ui/semicolon_inside_block.fixed index 7eb53e733ad5..f253a3b8b929 100644 --- a/tests/ui/semicolon_inside_block.fixed +++ b/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,19 @@ fn main() { unit_fn_block() } + +fn issue15380() { + #[rustfmt::skip] + ( {0;0;} + //~^ semicolon_inside_block + + ({ + //~^ semicolon_inside_block + 0; + 0; + } + + #[rustfmt::skip] + (({0;} + //~^ semicolon_inside_block +} diff --git a/tests/ui/semicolon_inside_block.rs b/tests/ui/semicolon_inside_block.rs index 9fa5b117194d..6110a17b5dfd 100644 --- a/tests/ui/semicolon_inside_block.rs +++ b/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,19 @@ fn main() { unit_fn_block() } + +fn issue15380() { + #[rustfmt::skip] + ( {0;0}); + //~^ semicolon_inside_block + + ({ + //~^ semicolon_inside_block + 0; + 0 + }); + + #[rustfmt::skip] + (({0})) ; + //~^ semicolon_inside_block +} diff --git a/tests/ui/semicolon_inside_block.stderr b/tests/ui/semicolon_inside_block.stderr index 23433f4e7ef9..7c3b575f9a3e 100644 --- a/tests/ui/semicolon_inside_block.stderr +++ b/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:93:5 + | +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:96:5 + | +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:103:5 + | +LL | (({0})) ; + | ^^^^^^^^^^^^ + | +help: put the `;` here + | +LL - (({0})) ; +LL + (({0;} + | + +error: aborting due to 7 previous errors From d8c73db4f3328c102f57e1ce40d2dbbcfd2f2aea Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 12:49:02 +0200 Subject: [PATCH 8/9] make tests uglier because of further issues with this lint --- tests/ui/semicolon_inside_block.fixed | 66 +++++++++++++++++++++---- tests/ui/semicolon_inside_block.rs | 68 +++++++++++++++++++++----- tests/ui/semicolon_inside_block.stderr | 36 +++++++------- 3 files changed, 131 insertions(+), 39 deletions(-) diff --git a/tests/ui/semicolon_inside_block.fixed b/tests/ui/semicolon_inside_block.fixed index f253a3b8b929..e19c5339cdea 100644 --- a/tests/ui/semicolon_inside_block.fixed +++ b/tests/ui/semicolon_inside_block.fixed @@ -88,18 +88,64 @@ fn main() { unit_fn_block() } -fn issue15380() { +// 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] - ( {0;0;} - //~^ semicolon_inside_block - - ({ + fn first() { + ( {0;0;} //~^ semicolon_inside_block - 0; - 0; } - #[rustfmt::skip] - (({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 + } } diff --git a/tests/ui/semicolon_inside_block.rs b/tests/ui/semicolon_inside_block.rs index 6110a17b5dfd..ea1329c701ec 100644 --- a/tests/ui/semicolon_inside_block.rs +++ b/tests/ui/semicolon_inside_block.rs @@ -88,18 +88,64 @@ fn main() { unit_fn_block() } -fn issue15380() { +// 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] - ( {0;0}); - //~^ semicolon_inside_block - - ({ + fn first() { + ( {0;0}); //~^ semicolon_inside_block - 0; - 0 - }); + } - #[rustfmt::skip] - (({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 + } } diff --git a/tests/ui/semicolon_inside_block.stderr b/tests/ui/semicolon_inside_block.stderr index 7c3b575f9a3e..3b4dd8aed348 100644 --- a/tests/ui/semicolon_inside_block.stderr +++ b/tests/ui/semicolon_inside_block.stderr @@ -53,43 +53,43 @@ LL + { m!(()); } | error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:93:5 + --> tests/ui/semicolon_inside_block.rs:134:9 | -LL | ( {0;0}); - | ^^^^^^^^^ +LL | ( {0;0}); + | ^^^^^^^^^ | help: put the `;` here | -LL - ( {0;0}); -LL + ( {0;0;} +LL - ( {0;0}); +LL + ( {0;0;} | error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:96:5 + --> tests/ui/semicolon_inside_block.rs:139:9 | -LL | / ({ +LL | / ({ LL | | -LL | | 0; -LL | | 0 -LL | | }); - | |_______^ +LL | | 0; +LL | | 0 +LL | | }); + | |___________^ | help: put the `;` here | -LL ~ 0; -LL ~ } +LL ~ 0; +LL ~ } | error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:103:5 + --> tests/ui/semicolon_inside_block.rs:148:9 | -LL | (({0})) ; - | ^^^^^^^^^^^^ +LL | (({ 0 })) ; + | ^^^^^^^^^^^^^^^^ | help: put the `;` here | -LL - (({0})) ; -LL + (({0;} +LL - (({ 0 })) ; +LL + (({ 0; } | error: aborting due to 7 previous errors From 7e935198f2fe447987e220a9f330efe9488aae87 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 2 Aug 2025 11:58:36 +0200 Subject: [PATCH 9/9] fix --- clippy_lints/src/semicolon_block.rs | 12 ++++++++---- tests/ui/semicolon_inside_block.fixed | 6 +++--- tests/ui/semicolon_inside_block.stderr | 6 +++--- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index 18ff68a9bb0c..87a6ad774e13 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/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; @@ -156,7 +160,7 @@ impl LateLintPass<'_> for SemicolonBlock { .. }) if !block.span.from_expansion() => { if let Some(tail) = block.expr { - self.semicolon_inside_block(cx, block, tail, stmt.span); + self.semicolon_inside_block(cx, tail, stmt.span); } }, _ => (), diff --git a/tests/ui/semicolon_inside_block.fixed b/tests/ui/semicolon_inside_block.fixed index e19c5339cdea..1b839c4384b4 100644 --- a/tests/ui/semicolon_inside_block.fixed +++ b/tests/ui/semicolon_inside_block.fixed @@ -131,7 +131,7 @@ mod issue15380 { // But that wouldn't compile because `[rustfmt::skip]` is now placed on an expr #[rustfmt::skip] fn first() { - ( {0;0;} + ( {0;0;}) //~^ semicolon_inside_block } @@ -140,12 +140,12 @@ mod issue15380 { //~^ semicolon_inside_block 0; 0; - } + }) } #[rustfmt::skip] // TODO: apply onto the block itself, see explanation in `first` fn third() { - (({ 0; } + (({ 0; })) //~^ semicolon_inside_block } } diff --git a/tests/ui/semicolon_inside_block.stderr b/tests/ui/semicolon_inside_block.stderr index 3b4dd8aed348..30b48a219864 100644 --- a/tests/ui/semicolon_inside_block.stderr +++ b/tests/ui/semicolon_inside_block.stderr @@ -61,7 +61,7 @@ LL | ( {0;0}); help: put the `;` here | LL - ( {0;0}); -LL + ( {0;0;} +LL + ( {0;0;}) | error: consider moving the `;` inside the block for consistent formatting @@ -77,7 +77,7 @@ LL | | }); help: put the `;` here | LL ~ 0; -LL ~ } +LL ~ }) | error: consider moving the `;` inside the block for consistent formatting @@ -89,7 +89,7 @@ LL | (({ 0 })) ; help: put the `;` here | LL - (({ 0 })) ; -LL + (({ 0; } +LL + (({ 0; })) | error: aborting due to 7 previous errors