Skip to content

Commit 990bbdc

Browse files
borsbors
committed
Auto merge of rust-lang#10656 - Centri3:master, r=xFrednet
Add configuration for `semicolon_block` lints Does exactly what it says on the tin, suggests moving a block's final semicolon inside if it's multiline and outside if it's singleline. I don't really like how this is implemented so I'm not too sure if this is ready yet. Alas, it might be ok. --- fixes rust-lang#10654 changelog: Enhancement: [`semicolon_inside_block`]: Added `semicolon-inside-block-ignore-singleline` as a new config value. [rust-lang#10656](rust-lang/rust-clippy#10656) changelog: Enhancement: [`semicolon_outside_block`]: Added `semicolon-outside-block-ignore-multiline` as a new config value. [rust-lang#10656](rust-lang/rust-clippy#10656) <!-- changelog_checked -->
2 parents a7335cb + 8c8cf40 commit 990bbdc

File tree

15 files changed

+744
-44
lines changed

15 files changed

+744
-44
lines changed

‎book/src/lint_configuration.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ Please use that command to update the file and do not edit it by hand.
1313
| [msrv](#msrv) | `None` |
1414
| [cognitive-complexity-threshold](#cognitive-complexity-threshold) | `25` |
1515
| [disallowed-names](#disallowed-names) | `["foo", "baz", "quux"]` |
16+
| [semicolon-inside-block-ignore-singleline](#semicolon-inside-block-ignore-singleline) | `false` |
17+
| [semicolon-outside-block-ignore-multiline](#semicolon-outside-block-ignore-multiline) | `false` |
1618
| [doc-valid-idents](#doc-valid-idents) | `["KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", "ECMAScript", "GPLv2", "GPLv3", "GitHub", "GitLab", "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", "NaN", "NaNs", "OAuth", "GraphQL", "OCaml", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenDNS", "WebGL", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase"]` |
1719
| [too-many-arguments-threshold](#too-many-arguments-threshold) | `7` |
1820
| [type-complexity-threshold](#type-complexity-threshold) | `250` |
@@ -203,6 +205,22 @@ default configuration of Clippy. By default, any configuration will replace the
203205
* [disallowed_names](https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names)
204206

205207

208+
### semicolon-inside-block-ignore-singleline
209+
Whether to lint only if it's multiline.
210+
211+
**Default Value:** `false` (`bool`)
212+
213+
* [semicolon_inside_block](https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block)
214+
215+
216+
### semicolon-outside-block-ignore-multiline
217+
Whether to lint only if it's singleline.
218+
219+
**Default Value:** `false` (`bool`)
220+
221+
* [semicolon_outside_block](https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block)
222+
223+
206224
### doc-valid-idents
207225
The list of words this lint should not consider as identifiers needing ticks. The value
208226
`".."` can be used as part of the list to indicate, that the configured values should be appended to the

‎clippy_lints/src/lib.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
933933
store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr));
934934
store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow));
935935
store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv())));
936-
store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock));
936+
let semicolon_inside_block_ignore_singleline = conf.semicolon_inside_block_ignore_singleline;
937+
let semicolon_outside_block_ignore_multiline = conf.semicolon_outside_block_ignore_multiline;
938+
store.register_late_pass(move |_| {
939+
Box::new(semicolon_block::SemicolonBlock::new(
940+
semicolon_inside_block_ignore_singleline,
941+
semicolon_outside_block_ignore_multiline,
942+
))
943+
});
937944
store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck));
938945
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
939946
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));

‎clippy_lints/src/semicolon_block.rs‎

Lines changed: 82 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and
22
use rustc_errors::Applicability;
33
use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
44
use rustc_lint::{LateContext, LateLintPass, LintContext};
5-
use rustc_session::{declare_lint_pass, declare_tool_lint};
5+
use rustc_session::{declare_tool_lint, impl_lint_pass};
66
use rustc_span::Span;
77

88
declare_clippy_lint! {
@@ -64,7 +64,78 @@ declare_clippy_lint! {
6464
restriction,
6565
"add a semicolon outside the block"
6666
}
67-
declare_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK]);
67+
impl_lint_pass!(SemicolonBlock => [SEMICOLON_INSIDE_BLOCK, SEMICOLON_OUTSIDE_BLOCK]);
68+
69+
#[derive(Copy, Clone)]
70+
pub struct SemicolonBlock {
71+
semicolon_inside_block_ignore_singleline: bool,
72+
semicolon_outside_block_ignore_multiline: bool,
73+
}
74+
75+
impl SemicolonBlock {
76+
pub fn new(semicolon_inside_block_ignore_singleline: bool, semicolon_outside_block_ignore_multiline: bool) -> Self {
77+
Self {
78+
semicolon_inside_block_ignore_singleline,
79+
semicolon_outside_block_ignore_multiline,
80+
}
81+
}
82+
83+
fn semicolon_inside_block(self, cx: &LateContext<'_>, block: &Block<'_>, tail: &Expr<'_>, semi_span: Span) {
84+
let insert_span = tail.span.source_callsite().shrink_to_hi();
85+
let remove_span = semi_span.with_lo(block.span.hi());
86+
87+
if self.semicolon_inside_block_ignore_singleline && get_line(cx, remove_span) == get_line(cx, insert_span) {
88+
return;
89+
}
90+
91+
span_lint_and_then(
92+
cx,
93+
SEMICOLON_INSIDE_BLOCK,
94+
semi_span,
95+
"consider moving the `;` inside the block for consistent formatting",
96+
|diag| {
97+
multispan_sugg_with_applicability(
98+
diag,
99+
"put the `;` here",
100+
Applicability::MachineApplicable,
101+
[(remove_span, String::new()), (insert_span, ";".to_owned())],
102+
);
103+
},
104+
);
105+
}
106+
107+
fn semicolon_outside_block(
108+
self,
109+
cx: &LateContext<'_>,
110+
block: &Block<'_>,
111+
tail_stmt_expr: &Expr<'_>,
112+
semi_span: Span,
113+
) {
114+
let insert_span = block.span.with_lo(block.span.hi());
115+
// account for macro calls
116+
let semi_span = cx.sess().source_map().stmt_span(semi_span, block.span);
117+
let remove_span = semi_span.with_lo(tail_stmt_expr.span.source_callsite().hi());
118+
119+
if self.semicolon_outside_block_ignore_multiline && get_line(cx, remove_span) != get_line(cx, insert_span) {
120+
return;
121+
}
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+
}
138+
}
68139

69140
impl LateLintPass<'_> for SemicolonBlock {
70141
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
@@ -83,55 +154,23 @@ impl LateLintPass<'_> for SemicolonBlock {
83154
span,
84155
..
85156
} = stmt else { return };
86-
semicolon_outside_block(cx, block, expr, span);
157+
self.semicolon_outside_block(cx, block, expr, span);
87158
},
88159
StmtKind::Semi(Expr {
89160
kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _),
90161
..
91-
}) if !block.span.from_expansion() => semicolon_inside_block(cx, block, tail, stmt.span),
162+
}) if !block.span.from_expansion() => {
163+
self.semicolon_inside_block(cx, block, tail, stmt.span);
164+
},
92165
_ => (),
93166
}
94167
}
95168
}
96169

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());
170+
fn get_line(cx: &LateContext<'_>, span: Span) -> Option<usize> {
171+
if let Ok(line) = cx.sess().source_map().lookup_line(span.lo()) {
172+
return Some(line.line);
173+
}
122174

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-
);
175+
None
137176
}

‎clippy_lints/src/utils/conf.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,14 @@ define_Conf! {
277277
/// `".."` can be used as part of the list to indicate, that the configured values should be appended to the
278278
/// default configuration of Clippy. By default, any configuration will replace the default value.
279279
(disallowed_names: Vec<String> = super::DEFAULT_DISALLOWED_NAMES.iter().map(ToString::to_string).collect()),
280+
/// Lint: SEMICOLON_INSIDE_BLOCK.
281+
///
282+
/// Whether to lint only if it's multiline.
283+
(semicolon_inside_block_ignore_singleline: bool = false),
284+
/// Lint: SEMICOLON_OUTSIDE_BLOCK.
285+
///
286+
/// Whether to lint only if it's singleline.
287+
(semicolon_outside_block_ignore_multiline: bool = false),
280288
/// Lint: DOC_MARKDOWN.
281289
///
282290
/// The list of words this lint should not consider as identifiers needing ticks. The value

‎tests/ui-toml/semicolon_block/both.fixed‎

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

‎tests/ui-toml/semicolon_block/both.rs‎

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

0 commit comments

Comments
 (0)