diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b6b12c623af..dac4a54b11fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4353,6 +4353,8 @@ Released 2018-09-13 [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors [`self_named_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_module_files [`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 [`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 eb3210946f11..76b4a2ccf5e1 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -525,6 +525,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::returns::NEEDLESS_RETURN_INFO, crate::same_name_method::SAME_NAME_METHOD_INFO, crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO, + crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO, + crate::semicolon_block::SEMICOLON_OUTSIDE_BLOCK_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/lib.rs b/clippy_lints/src/lib.rs index 17dbd983b6c0..cbca317a1463 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -257,6 +257,7 @@ mod return_self_not_must_use; mod returns; mod same_name_method; mod self_named_constructors; +mod semicolon_block; mod semicolon_if_nothing_returned; mod serde_api; mod shadow; @@ -884,6 +885,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr)); store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow)); store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv()))); + store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs new file mode 100644 index 000000000000..8f1d1490e1f0 --- /dev/null +++ b/clippy_lints/src/semicolon_block.rs @@ -0,0 +1,137 @@ +use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then}; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// + /// Suggests moving the semicolon after a block to the inside of the block, after its last + /// expression. + /// + /// ### 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. + /// + /// ### Example + /// + /// ```rust + /// # fn f(_: u32) {} + /// # let x = 0; + /// unsafe { f(x) }; + /// ``` + /// Use instead: + /// ```rust + /// # fn f(_: u32) {} + /// # let x = 0; + /// unsafe { f(x); } + /// ``` + #[clippy::version = "1.66.0"] + pub SEMICOLON_INSIDE_BLOCK, + restriction, + "add a semicolon inside the block" +} +declare_clippy_lint! { + /// ### What it does + /// + /// Suggests moving the semicolon from a block's final expression outside of 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. + /// + /// ### Example + /// + /// ```rust + /// # fn f(_: u32) {} + /// # let x = 0; + /// unsafe { f(x); } + /// ``` + /// Use instead: + /// ```rust + /// # fn f(_: u32) {} + /// # let x = 0; + /// unsafe { f(x) }; + /// ``` + #[clippy::version = "1.66.0"] + pub SEMICOLON_OUTSIDE_BLOCK, + restriction, + "add a semicolon outside the block" +} +declare_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK]); + +impl LateLintPass<'_> for SemicolonBlock { + fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { + match stmt.kind { + StmtKind::Expr(Expr { + kind: ExprKind::Block(block, _), + .. + }) if !block.span.from_expansion() => { + let Block { + expr: None, + stmts: [.., stmt], + .. + } = block else { return }; + let &Stmt { + kind: StmtKind::Semi(expr), + span, + .. + } = stmt else { return }; + semicolon_outside_block(cx, block, expr, span); + }, + StmtKind::Semi(Expr { + kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _), + .. + }) if !block.span.from_expansion() => semicolon_inside_block(cx, block, tail, stmt.span), + _ => (), + } + } +} + +fn semicolon_inside_block(cx: &LateContext<'_>, block: &Block<'_>, 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()); + + span_lint_and_then( + cx, + SEMICOLON_INSIDE_BLOCK, + semi_span, + "consider moving the `;` inside the block for consistent formatting", + |diag| { + multispan_sugg_with_applicability( + diag, + "put the `;` here", + Applicability::MachineApplicable, + [(remove_span, String::new()), (insert_span, ";".to_owned())], + ); + }, + ); +} + +fn semicolon_outside_block(cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_expr: &Expr<'_>, semi_span: Span) { + let insert_span = block.span.with_lo(block.span.hi()); + // account for macro calls + 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()); + + span_lint_and_then( + cx, + SEMICOLON_OUTSIDE_BLOCK, + block.span, + "consider moving the `;` outside 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_inside_block.fixed b/tests/ui/semicolon_inside_block.fixed new file mode 100644 index 000000000000..42e97e1ca358 --- /dev/null +++ b/tests/ui/semicolon_inside_block.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_inside_block)] + +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() +} diff --git a/tests/ui/semicolon_inside_block.rs b/tests/ui/semicolon_inside_block.rs new file mode 100644 index 000000000000..f40848f702e1 --- /dev/null +++ b/tests/ui/semicolon_inside_block.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_inside_block)] + +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() +} diff --git a/tests/ui/semicolon_inside_block.stderr b/tests/ui/semicolon_inside_block.stderr new file mode 100644 index 000000000000..48d3690e2bde --- /dev/null +++ b/tests/ui/semicolon_inside_block.stderr @@ -0,0 +1,54 @@ +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:39:5 + | +LL | { unit_fn_block() }; + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::semicolon-inside-block` implied by `-D warnings` +help: put the `;` here + | +LL - { unit_fn_block() }; +LL + { unit_fn_block(); } + | + +error: consider moving the `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:40: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_inside_block.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 `;` inside the block for consistent formatting + --> $DIR/semicolon_inside_block.rs:61:5 + | +LL | { m!(()) }; + | ^^^^^^^^^^^ + | +help: put the `;` here + | +LL - { m!(()) }; +LL + { m!(()); } + | + +error: aborting due to 4 previous errors + diff --git a/tests/ui/semicolon_outside_block.fixed b/tests/ui/semicolon_outside_block.fixed new file mode 100644 index 000000000000..091eaa7518e9 --- /dev/null +++ b/tests/ui/semicolon_outside_block.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)] + +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() +} diff --git a/tests/ui/semicolon_outside_block.rs b/tests/ui/semicolon_outside_block.rs new file mode 100644 index 000000000000..7ce46431fac9 --- /dev/null +++ b/tests/ui/semicolon_outside_block.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)] + +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() +} diff --git a/tests/ui/semicolon_outside_block.stderr b/tests/ui/semicolon_outside_block.stderr new file mode 100644 index 000000000000..dcc102e60994 --- /dev/null +++ b/tests/ui/semicolon_outside_block.stderr @@ -0,0 +1,54 @@ +error: consider moving the `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:42:5 + | +LL | { unit_fn_block(); } + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::semicolon-outside-block` 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.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 `;` outside the block for consistent formatting + --> $DIR/semicolon_outside_block.rs:52: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.rs:62:5 + | +LL | { m!(()); } + | ^^^^^^^^^^^ + | +help: put the `;` here + | +LL - { m!(()); } +LL + { m!(()) }; + | + +error: aborting due to 4 previous errors +