Skip to content

Commit d4cd91c

Browse files
borsbors
committed
Auto merge of #9826 - Veykril:semi-blocks, r=Alexendoo
Add semicolon-outside/inside-block lints changelog: Add `semicolon_outside_block` and `semicolon_inside_block` lints Fixes #7322 An earlier attempt at this can be found here #7564. This PR still implements two separate lints but I am open to merging them into a single one that's configurable.
2 parents 39f0719 + 20ec2ce commit d4cd91c

10 files changed

+591
-0
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -4353,6 +4353,8 @@ Released 2018-09-13
43534353
[`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors
43544354
[`self_named_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_module_files
43554355
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
4356+
[`semicolon_inside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block
4357+
[`semicolon_outside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block
43564358
[`separated_literal_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#separated_literal_suffix
43574359
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
43584360
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse

‎clippy_lints/src/declared_lints.rs

+2
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
525525
crate::returns::NEEDLESS_RETURN_INFO,
526526
crate::same_name_method::SAME_NAME_METHOD_INFO,
527527
crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO,
528+
crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO,
529+
crate::semicolon_block::SEMICOLON_OUTSIDE_BLOCK_INFO,
528530
crate::semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED_INFO,
529531
crate::serde_api::SERDE_API_MISUSE_INFO,
530532
crate::shadow::SHADOW_REUSE_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ mod return_self_not_must_use;
256256
mod returns;
257257
mod same_name_method;
258258
mod self_named_constructors;
259+
mod semicolon_block;
259260
mod semicolon_if_nothing_returned;
260261
mod serde_api;
261262
mod shadow;
@@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
900901
store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr));
901902
store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow));
902903
store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv())));
904+
store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock));
903905
// add lints here, do not remove this comment, it's used in `new_lint`
904906
}
905907

‎clippy_lints/src/semicolon_block.rs

+137
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then};
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
4+
use rustc_lint::{LateContext, LateLintPass, LintContext};
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
use rustc_span::Span;
7+
8+
declare_clippy_lint! {
9+
/// ### What it does
10+
///
11+
/// Suggests moving the semicolon after a block to the inside of the block, after its last
12+
/// expression.
13+
///
14+
/// ### Why is this bad?
15+
///
16+
/// For consistency it's best to have the semicolon inside/outside the block. Either way is fine
17+
/// and this lint suggests inside the block.
18+
/// Take a look at `semicolon_outside_block` for the other alternative.
19+
///
20+
/// ### Example
21+
///
22+
/// ```rust
23+
/// # fn f(_: u32) {}
24+
/// # let x = 0;
25+
/// unsafe { f(x) };
26+
/// ```
27+
/// Use instead:
28+
/// ```rust
29+
/// # fn f(_: u32) {}
30+
/// # let x = 0;
31+
/// unsafe { f(x); }
32+
/// ```
33+
#[clippy::version = "1.66.0"]
34+
pub SEMICOLON_INSIDE_BLOCK,
35+
restriction,
36+
"add a semicolon inside the block"
37+
}
38+
declare_clippy_lint! {
39+
/// ### What it does
40+
///
41+
/// Suggests moving the semicolon from a block's final expression outside of the block.
42+
///
43+
/// ### Why is this bad?
44+
///
45+
/// For consistency it's best to have the semicolon inside/outside the block. Either way is fine
46+
/// and this lint suggests outside the block.
47+
/// Take a look at `semicolon_inside_block` for the other alternative.
48+
///
49+
/// ### Example
50+
///
51+
/// ```rust
52+
/// # fn f(_: u32) {}
53+
/// # let x = 0;
54+
/// unsafe { f(x); }
55+
/// ```
56+
/// Use instead:
57+
/// ```rust
58+
/// # fn f(_: u32) {}
59+
/// # let x = 0;
60+
/// unsafe { f(x) };
61+
/// ```
62+
#[clippy::version = "1.66.0"]
63+
pub SEMICOLON_OUTSIDE_BLOCK,
64+
restriction,
65+
"add a semicolon outside the block"
66+
}
67+
declare_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK]);
68+
69+
impl LateLintPass<'_> for SemicolonBlock {
70+
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
71+
match stmt.kind {
72+
StmtKind::Expr(Expr {
73+
kind: ExprKind::Block(block, _),
74+
..
75+
}) if !block.span.from_expansion() => {
76+
let Block {
77+
expr: None,
78+
stmts: [.., stmt],
79+
..
80+
} = block else { return };
81+
let &Stmt {
82+
kind: StmtKind::Semi(expr),
83+
span,
84+
..
85+
} = stmt else { return };
86+
semicolon_outside_block(cx, block, expr, span);
87+
},
88+
StmtKind::Semi(Expr {
89+
kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _),
90+
..
91+
}) if !block.span.from_expansion() => semicolon_inside_block(cx, block, tail, stmt.span),
92+
_ => (),
93+
}
94+
}
95+
}
96+
97+
fn semicolon_inside_block(cx: &LateContext<'_>, block: &Block<'_>, tail: &Expr<'_>, semi_span: Span) {
98+
let insert_span = tail.span.source_callsite().shrink_to_hi();
99+
let remove_span = semi_span.with_lo(block.span.hi());
100+
101+
span_lint_and_then(
102+
cx,
103+
SEMICOLON_INSIDE_BLOCK,
104+
semi_span,
105+
"consider moving the `;` inside the block for consistent formatting",
106+
|diag| {
107+
multispan_sugg_with_applicability(
108+
diag,
109+
"put the `;` here",
110+
Applicability::MachineApplicable,
111+
[(remove_span, String::new()), (insert_span, ";".to_owned())],
112+
);
113+
},
114+
);
115+
}
116+
117+
fn semicolon_outside_block(cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_expr: &Expr<'_>, semi_span: Span) {
118+
let insert_span = block.span.with_lo(block.span.hi());
119+
// account for macro calls
120+
let semi_span = cx.sess().source_map().stmt_span(semi_span, block.span);
121+
let remove_span = semi_span.with_lo(tail_stmt_expr.span.source_callsite().hi());
122+
123+
span_lint_and_then(
124+
cx,
125+
SEMICOLON_OUTSIDE_BLOCK,
126+
block.span,
127+
"consider moving the `;` outside the block for consistent formatting",
128+
|diag| {
129+
multispan_sugg_with_applicability(
130+
diag,
131+
"put the `;` here",
132+
Applicability::MachineApplicable,
133+
[(remove_span, String::new()), (insert_span, ";".to_owned())],
134+
);
135+
},
136+
);
137+
}

‎tests/ui/semicolon_inside_block.fixed

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// run-rustfix
2+
#![allow(
3+
unused,
4+
clippy::unused_unit,
5+
clippy::unnecessary_operation,
6+
clippy::no_effect,
7+
clippy::single_element_loop
8+
)]
9+
#![warn(clippy::semicolon_inside_block)]
10+
11+
macro_rules! m {
12+
(()) => {
13+
()
14+
};
15+
(0) => {{
16+
0
17+
};};
18+
(1) => {{
19+
1;
20+
}};
21+
(2) => {{
22+
2;
23+
}};
24+
}
25+
26+
fn unit_fn_block() {
27+
()
28+
}
29+
30+
#[rustfmt::skip]
31+
fn main() {
32+
{ unit_fn_block() }
33+
unsafe { unit_fn_block() }
34+
35+
{
36+
unit_fn_block()
37+
}
38+
39+
{ unit_fn_block(); }
40+
unsafe { unit_fn_block(); }
41+
42+
{ unit_fn_block(); }
43+
unsafe { unit_fn_block(); }
44+
45+
{ unit_fn_block(); };
46+
unsafe { unit_fn_block(); };
47+
48+
{
49+
unit_fn_block();
50+
unit_fn_block();
51+
}
52+
{
53+
unit_fn_block();
54+
unit_fn_block();
55+
}
56+
{
57+
unit_fn_block();
58+
unit_fn_block();
59+
};
60+
61+
{ m!(()); }
62+
{ m!(()); }
63+
{ m!(()); };
64+
m!(0);
65+
m!(1);
66+
m!(2);
67+
68+
for _ in [()] {
69+
unit_fn_block();
70+
}
71+
for _ in [()] {
72+
unit_fn_block()
73+
}
74+
75+
let _d = || {
76+
unit_fn_block();
77+
};
78+
let _d = || {
79+
unit_fn_block()
80+
};
81+
82+
{ unit_fn_block(); };
83+
84+
unit_fn_block()
85+
}

‎tests/ui/semicolon_inside_block.rs

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// run-rustfix
2+
#![allow(
3+
unused,
4+
clippy::unused_unit,
5+
clippy::unnecessary_operation,
6+
clippy::no_effect,
7+
clippy::single_element_loop
8+
)]
9+
#![warn(clippy::semicolon_inside_block)]
10+
11+
macro_rules! m {
12+
(()) => {
13+
()
14+
};
15+
(0) => {{
16+
0
17+
};};
18+
(1) => {{
19+
1;
20+
}};
21+
(2) => {{
22+
2;
23+
}};
24+
}
25+
26+
fn unit_fn_block() {
27+
()
28+
}
29+
30+
#[rustfmt::skip]
31+
fn main() {
32+
{ unit_fn_block() }
33+
unsafe { unit_fn_block() }
34+
35+
{
36+
unit_fn_block()
37+
}
38+
39+
{ unit_fn_block() };
40+
unsafe { unit_fn_block() };
41+
42+
{ unit_fn_block(); }
43+
unsafe { unit_fn_block(); }
44+
45+
{ unit_fn_block(); };
46+
unsafe { unit_fn_block(); };
47+
48+
{
49+
unit_fn_block();
50+
unit_fn_block()
51+
};
52+
{
53+
unit_fn_block();
54+
unit_fn_block();
55+
}
56+
{
57+
unit_fn_block();
58+
unit_fn_block();
59+
};
60+
61+
{ m!(()) };
62+
{ m!(()); }
63+
{ m!(()); };
64+
m!(0);
65+
m!(1);
66+
m!(2);
67+
68+
for _ in [()] {
69+
unit_fn_block();
70+
}
71+
for _ in [()] {
72+
unit_fn_block()
73+
}
74+
75+
let _d = || {
76+
unit_fn_block();
77+
};
78+
let _d = || {
79+
unit_fn_block()
80+
};
81+
82+
{ unit_fn_block(); };
83+
84+
unit_fn_block()
85+
}

‎tests/ui/semicolon_inside_block.stderr

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
error: consider moving the `;` inside the block for consistent formatting
2+
--> $DIR/semicolon_inside_block.rs:39:5
3+
|
4+
LL | { unit_fn_block() };
5+
| ^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::semicolon-inside-block` implied by `-D warnings`
8+
help: put the `;` here
9+
|
10+
LL - { unit_fn_block() };
11+
LL + { unit_fn_block(); }
12+
|
13+
14+
error: consider moving the `;` inside the block for consistent formatting
15+
--> $DIR/semicolon_inside_block.rs:40:5
16+
|
17+
LL | unsafe { unit_fn_block() };
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
|
20+
help: put the `;` here
21+
|
22+
LL - unsafe { unit_fn_block() };
23+
LL + unsafe { unit_fn_block(); }
24+
|
25+
26+
error: consider moving the `;` inside the block for consistent formatting
27+
--> $DIR/semicolon_inside_block.rs:48:5
28+
|
29+
LL | / {
30+
LL | | unit_fn_block();
31+
LL | | unit_fn_block()
32+
LL | | };
33+
| |______^
34+
|
35+
help: put the `;` here
36+
|
37+
LL ~ unit_fn_block();
38+
LL ~ }
39+
|
40+
41+
error: consider moving the `;` inside the block for consistent formatting
42+
--> $DIR/semicolon_inside_block.rs:61:5
43+
|
44+
LL | { m!(()) };
45+
| ^^^^^^^^^^^
46+
|
47+
help: put the `;` here
48+
|
49+
LL - { m!(()) };
50+
LL + { m!(()); }
51+
|
52+
53+
error: aborting due to 4 previous errors
54+

0 commit comments

Comments
 (0)