diff --git a/CHANGELOG.md b/CHANGELOG.md index 559b560dde4b..a0a7780f4c27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4884,6 +4884,7 @@ Released 2018-09-13 [`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 +[`semicolon_outside_block_if_singleline`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block_if_singleline [`separated_literal_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#separated_literal_suffix [`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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index f24dab627809..70379b6caf11 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -546,6 +546,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO, crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO, crate::semicolon_block::SEMICOLON_OUTSIDE_BLOCK_INFO, + crate::semicolon_block::SEMICOLON_OUTSIDE_BLOCK_IF_SINGLELINE_INFO, crate::semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED_INFO, crate::serde_api::SERDE_API_MISUSE_INFO, crate::shadow::SHADOW_REUSE_INFO, diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index 34a3e5ddf4f6..d791100b9b9a 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -64,7 +64,48 @@ declare_clippy_lint! { restriction, "add a semicolon outside the block" } -declare_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK]); +declare_clippy_lint! { + /// ### What it does + /// + /// Suggests moving the semicolon from a block's final expression outside of + /// the block if it's singleline, and inside the block if it's multiline. + /// + /// ### Why is this bad? + /// + /// Some may prefer if the semicolon is outside if a block is only one + /// expression, as this allows rustfmt to make it singleline. In the case that + /// it isn't, it should be inside. + /// Take a look at both `semicolon_inside_block` and `semicolon_outside_block` for alternatives. + /// + /// ### Example + /// + /// ```rust + /// # fn f(_: u32) {} + /// # let x = 0; + /// unsafe { f(x); } + /// + /// unsafe { + /// let x = 1; + /// f(x) + /// }; + /// ``` + /// Use instead: + /// ```rust + /// # fn f(_: u32) {} + /// # let x = 0; + /// unsafe { f(x) }; + /// + /// unsafe { + /// let x = 1; + /// f(x); + /// } + /// ``` + #[clippy::version = "1.68.0"] + pub SEMICOLON_OUTSIDE_BLOCK_IF_SINGLELINE, + restriction, + "add a semicolon inside the block if it's singleline, otherwise outside" +} +declare_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK_IF_SINGLELINE]); impl LateLintPass<'_> for SemicolonBlock { fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { @@ -98,6 +139,8 @@ fn semicolon_inside_block(cx: &LateContext<'_>, block: &Block<'_>, tail: &Expr<' let insert_span = tail.span.source_callsite().shrink_to_hi(); let remove_span = semi_span.with_lo(block.span.hi()); + check_semicolon_outside_block_if_singleline(cx, block, remove_span, insert_span, true, "inside"); + span_lint_and_then( cx, SEMICOLON_INSIDE_BLOCK, @@ -120,6 +163,8 @@ fn semicolon_outside_block(cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_ex let semi_span = cx.sess().source_map().stmt_span(semi_span, block.span); let remove_span = semi_span.with_lo(tail_stmt_expr.span.source_callsite().hi()); + check_semicolon_outside_block_if_singleline(cx, block, remove_span, insert_span, false, "outside"); + span_lint_and_then( cx, SEMICOLON_OUTSIDE_BLOCK, @@ -135,3 +180,48 @@ fn semicolon_outside_block(cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_ex }, ); } + +fn check_semicolon_outside_block_if_singleline( + cx: &LateContext<'_>, + block: &Block<'_>, + remove_span: Span, + insert_span: Span, + inequality: bool, + ty: &str, +) { + let remove_line = cx + .sess() + .source_map() + .lookup_line(remove_span.lo()) + .expect("failed to get `remove_span`'s line") + .line; + let insert_line = cx + .sess() + .source_map() + .lookup_line(insert_span.lo()) + .expect("failed to get `insert_span`'s line") + .line; + + let eq = if inequality { + remove_line != insert_line + } else { + remove_line == insert_line + }; + + if eq { + span_lint_and_then( + cx, + SEMICOLON_OUTSIDE_BLOCK_IF_SINGLELINE, + block.span, + &format!("consider moving the `;` {ty} the block for consistent formatting"), + |diag| { + multispan_sugg_with_applicability( + diag, + "put the `;` here", + Applicability::MachineApplicable, + [(remove_span, String::new()), (insert_span, ";".to_owned())], + ); + }, + ); + } +} diff --git a/tests/ui/semicolon_outside_block_if_singleline.fixed b/tests/ui/semicolon_outside_block_if_singleline.fixed new file mode 100644 index 000000000000..592f9c49ed77 --- /dev/null +++ b/tests/ui/semicolon_outside_block_if_singleline.fixed @@ -0,0 +1,85 @@ +// run-rustfix +#![allow( + unused, + clippy::unused_unit, + clippy::unnecessary_operation, + clippy::no_effect, + clippy::single_element_loop +)] +#![warn(clippy::semicolon_outside_block_if_singleline)] + +macro_rules! m { + (()) => { + () + }; + (0) => {{ + 0 + };}; + (1) => {{ + 1; + }}; + (2) => {{ + 2; + }}; +} + +fn unit_fn_block() { + () +} + +#[rustfmt::skip] +fn main() { + { unit_fn_block() } + unsafe { unit_fn_block() } + + { + unit_fn_block() + } + + { unit_fn_block() }; + unsafe { unit_fn_block() }; + + { unit_fn_block() }; + unsafe { unit_fn_block() }; + + { unit_fn_block(); }; + unsafe { unit_fn_block(); }; + + { + unit_fn_block(); + unit_fn_block(); + } + { + unit_fn_block(); + unit_fn_block(); + } + { + unit_fn_block(); + unit_fn_block(); + }; + + { m!(()) }; + { m!(()) }; + { m!(()); }; + m!(0); + m!(1); + m!(2); + + for _ in [()] { + unit_fn_block(); + } + for _ in [()] { + unit_fn_block() + } + + let _d = || { + unit_fn_block(); + }; + let _d = || { + unit_fn_block() + }; + + { unit_fn_block(); }; + + unit_fn_block() +} \ No newline at end of file diff --git a/tests/ui/semicolon_outside_block_if_singleline.rs b/tests/ui/semicolon_outside_block_if_singleline.rs new file mode 100644 index 000000000000..21dd61445a5b --- /dev/null +++ b/tests/ui/semicolon_outside_block_if_singleline.rs @@ -0,0 +1,85 @@ +// run-rustfix +#![allow( + unused, + clippy::unused_unit, + clippy::unnecessary_operation, + clippy::no_effect, + clippy::single_element_loop +)] +#![warn(clippy::semicolon_outside_block_if_singleline)] + +macro_rules! m { + (()) => { + () + }; + (0) => {{ + 0 + };}; + (1) => {{ + 1; + }}; + (2) => {{ + 2; + }}; +} + +fn unit_fn_block() { + () +} + +#[rustfmt::skip] +fn main() { + { unit_fn_block() } + unsafe { unit_fn_block() } + + { + unit_fn_block() + } + + { unit_fn_block() }; + unsafe { unit_fn_block() }; + + { unit_fn_block(); } + unsafe { unit_fn_block(); } + + { unit_fn_block(); }; + unsafe { unit_fn_block(); }; + + { + unit_fn_block(); + unit_fn_block() + }; + { + unit_fn_block(); + unit_fn_block(); + } + { + unit_fn_block(); + unit_fn_block(); + }; + + { m!(()) }; + { m!(()); } + { m!(()); }; + m!(0); + m!(1); + m!(2); + + for _ in [()] { + unit_fn_block(); + } + for _ in [()] { + unit_fn_block() + } + + let _d = || { + unit_fn_block(); + }; + let _d = || { + unit_fn_block() + }; + + { unit_fn_block(); }; + + unit_fn_block() +} \ No newline at end of file diff --git a/tests/ui/semicolon_outside_block_if_singleline.stderr b/tests/ui/semicolon_outside_block_if_singleline.stderr new file mode 100644 index 000000000000..dda083f2be3e --- /dev/null +++ b/tests/ui/semicolon_outside_block_if_singleline.stderr @@ -0,0 +1,54 @@ +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block_if_singleline.rs:42:5 + | +LL | { unit_fn_block(); } + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::semicolon-outside-block-if-singleline` implied by `-D warnings` +help: put the `;` here + | +LL - { unit_fn_block(); } +LL + { unit_fn_block() }; + | + +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block_if_singleline.rs:43:5 + | +LL | unsafe { unit_fn_block(); } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: put the `;` here + | +LL - unsafe { unit_fn_block(); } +LL + unsafe { unit_fn_block() }; + | + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_outside_block_if_singleline.rs:48:5 + | +LL | / { +LL | | unit_fn_block(); +LL | | unit_fn_block() +LL | | }; + | |_____^ + | +help: put the `;` here + | +LL ~ unit_fn_block(); +LL ~ } + | + +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block_if_singleline.rs:62:5 + | +LL | { m!(()); } + | ^^^^^^^^^^^ + | +help: put the `;` here + | +LL - { m!(()); } +LL + { m!(()) }; + | + +error: aborting due to 4 previous errors +