Skip to content

Add semicolon-outside/inside-block lints #9826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
137 changes: 137 additions & 0 deletions clippy_lints/src/semicolon_block.rs
Original file line number Diff line number Diff line change
@@ -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())],
);
},
);
}
85 changes: 85 additions & 0 deletions tests/ui/semicolon_inside_block.fixed
Original file line number Diff line number Diff line change
@@ -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()
}
85 changes: 85 additions & 0 deletions tests/ui/semicolon_inside_block.rs
Original file line number Diff line number Diff line change
@@ -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()
}
54 changes: 54 additions & 0 deletions tests/ui/semicolon_inside_block.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading