Skip to content

Commit bec2183

Browse files
samueltardieusamueltardieu
committed
Add collapse-let-chains option to collapsible_if lint
Since `collapsible_if` is an early lint, it is not possible to automatically check whether the `let_chains` unstable rustc feature is enabled or not, as features are not available in the `EarlyContext` object. For this reason, the `collapse-let-chains` needs to be enabled explicitly. This can be reexamined later when the `let_chains` feature is stabilized.
1 parent 6509de3 commit bec2183

File tree

10 files changed

+167
-15
lines changed

10 files changed

+167
-15
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6350,6 +6350,7 @@ Released 2018-09-13
63506350
[`check-inconsistent-struct-field-initializers`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-inconsistent-struct-field-initializers
63516351
[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
63526352
[`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold
6353+
[`collapse-let-chains`]: https://doc.rust-lang.org/clippy/lint_configuration.html#collapse-let-chains
63536354
[`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros
63546355
[`disallowed-methods`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods
63556356
[`disallowed-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-names

‎book/src/lint_configuration.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,17 @@ The maximum cognitive complexity a function can have
475475
* [`cognitive_complexity`](https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity)
476476

477477

478+
## `collapse-let-chains`
479+
Whether `if let` chains should be collapsed. This requires the use of the unstable
480+
`let_chains` rustc feature.
481+
482+
**Default Value:** `false`
483+
484+
---
485+
**Affected lints:**
486+
* [`collapsible_if`](https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if)
487+
488+
478489
## `disallowed-macros`
479490
The list of disallowed macros, written as fully qualified paths.
480491

‎clippy_config/src/conf.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,10 @@ define_Conf! {
500500
/// The maximum cognitive complexity a function can have
501501
#[lints(cognitive_complexity)]
502502
cognitive_complexity_threshold: u64 = 25,
503+
/// Whether `if let` chains should be collapsed. This requires the use of the unstable
504+
/// `let_chains` rustc feature.
505+
#[lints(collapsible_if)]
506+
collapse_let_chains: bool = false,
503507
/// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY.
504508
///
505509
/// Use the Cognitive Complexity lint instead.

‎clippy_lints/src/collapsible_if.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,24 @@ declare_clippy_lint! {
7979
}
8080

8181
pub struct CollapsibleIf {
82+
collapse_let_chains: bool,
8283
lint_commented_code: bool,
8384
}
8485

8586
impl CollapsibleIf {
8687
pub fn new(conf: &'static Conf) -> Self {
8788
Self {
89+
collapse_let_chains: conf.collapse_let_chains,
8890
lint_commented_code: conf.lint_commented_code,
8991
}
9092
}
9193

94+
/// Prevent triggering on `if c { if let a = b { .. } }` unless the
95+
/// `collapse_let_chains` config option is set.
96+
fn is_collapsible(&self, cond: &ast::Expr) -> bool {
97+
self.collapse_let_chains || !matches!(cond.kind, ast::ExprKind::Let(..))
98+
}
99+
92100
fn check_collapsible_else_if(cx: &EarlyContext<'_>, then_span: Span, else_: &ast::Expr) {
93101
if let ast::ExprKind::Block(ref block, _) = else_.kind
94102
&& !block_starts_with_comment(cx, block)
@@ -127,8 +135,7 @@ impl CollapsibleIf {
127135
if let Some(inner) = expr_block(then)
128136
&& inner.attrs.is_empty()
129137
&& let ast::ExprKind::If(check_inner, _, None) = &inner.kind
130-
// Prevent triggering on `if c { if let a = b { .. } }`.
131-
&& !matches!(check_inner.kind, ast::ExprKind::Let(..))
138+
&& self.is_collapsible(check_inner)
132139
&& let ctxt = expr.span.ctxt()
133140
&& inner.span.ctxt() == ctxt
134141
&& let contains_comment = span_contains_comment(cx.sess().source_map(), check.span.to(check_inner.span))
@@ -175,8 +182,7 @@ impl EarlyLintPass for CollapsibleIf {
175182
{
176183
if let Some(else_) = else_ {
177184
Self::check_collapsible_else_if(cx, then.span, else_);
178-
} else if !matches!(cond.kind, ast::ExprKind::Let(..)) {
179-
// Prevent triggering on `if c { if let a = b { .. } }`.
185+
} else if self.is_collapsible(cond) {
180186
self.check_collapsible_if_if(cx, expr, cond, then);
181187
}
182188
}

‎tests/ui-toml/collapsible_if/clippy.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
collapse-let-chains = true
12
lint-commented-code = true

‎tests/ui-toml/collapsible_if/collapsible_if.fixed‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![feature(let_chains)]
12
#![allow(clippy::eq_op, clippy::nonminimal_bool)]
23

34
#[rustfmt::skip]
@@ -31,4 +32,28 @@ fn main() {
3132
println!("Hello world!");
3233
}
3334
//~^^^^^ collapsible_if
35+
36+
if true
37+
&& true {
38+
println!("No comment, linted");
39+
}
40+
//~^^^^^ collapsible_if
41+
42+
if let Some(a) = Some(3)
43+
&& let Some(b) = Some(4) {
44+
let _ = a + b;
45+
}
46+
//~^^^^^ collapsible_if
47+
48+
if let Some(a) = Some(3)
49+
&& a + 1 == 4 {
50+
let _ = a;
51+
}
52+
//~^^^^^ collapsible_if
53+
54+
if Some(3) == Some(4).map(|x| x - 1)
55+
&& let Some(b) = Some(4) {
56+
let _ = b;
57+
}
58+
//~^^^^^ collapsible_if
3459
}

‎tests/ui-toml/collapsible_if/collapsible_if.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![feature(let_chains)]
12
#![allow(clippy::eq_op, clippy::nonminimal_bool)]
23

34
#[rustfmt::skip]
@@ -35,4 +36,32 @@ fn main() {
3536
}
3637
}
3738
//~^^^^^ collapsible_if
39+
40+
if true {
41+
if true {
42+
println!("No comment, linted");
43+
}
44+
}
45+
//~^^^^^ collapsible_if
46+
47+
if let Some(a) = Some(3) {
48+
if let Some(b) = Some(4) {
49+
let _ = a + b;
50+
}
51+
}
52+
//~^^^^^ collapsible_if
53+
54+
if let Some(a) = Some(3) {
55+
if a + 1 == 4 {
56+
let _ = a;
57+
}
58+
}
59+
//~^^^^^ collapsible_if
60+
61+
if Some(3) == Some(4).map(|x| x - 1) {
62+
if let Some(b) = Some(4) {
63+
let _ = b;
64+
}
65+
}
66+
//~^^^^^ collapsible_if
3867
}

‎tests/ui-toml/collapsible_if/collapsible_if.stderr‎

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` statement can be collapsed
2-
--> tests/ui-toml/collapsible_if/collapsible_if.rs:8:5
2+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:9:5
33
|
44
LL | / if x == "hello" {
55
LL | | // Comment must be kept
@@ -21,7 +21,7 @@ LL ~ }
2121
|
2222

2323
error: this `if` statement can be collapsed
24-
--> tests/ui-toml/collapsible_if/collapsible_if.rs:17:5
24+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:18:5
2525
|
2626
LL | / if x == "hello" { // Inner comment
2727
LL | | if y == "world" {
@@ -39,7 +39,7 @@ LL ~ }
3939
|
4040

4141
error: this `if` statement can be collapsed
42-
--> tests/ui-toml/collapsible_if/collapsible_if.rs:24:5
42+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:25:5
4343
|
4444
LL | / if x == "hello" {
4545
LL | | /* Inner comment */
@@ -59,7 +59,7 @@ LL ~ }
5959
|
6060

6161
error: this `if` statement can be collapsed
62-
--> tests/ui-toml/collapsible_if/collapsible_if.rs:32:5
62+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:33:5
6363
|
6464
LL | / if x == "hello" { /* Inner comment */
6565
LL | | if y == "world" {
@@ -76,5 +76,77 @@ LL | println!("Hello world!");
7676
LL ~ }
7777
|
7878

79-
error: aborting due to 4 previous errors
79+
error: this `if` statement can be collapsed
80+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:40:5
81+
|
82+
LL | / if true {
83+
LL | | if true {
84+
LL | | println!("No comment, linted");
85+
LL | | }
86+
LL | | }
87+
| |______^
88+
|
89+
help: collapse nested if block
90+
|
91+
LL ~ if true
92+
LL ~ && true {
93+
LL | println!("No comment, linted");
94+
LL ~ }
95+
|
96+
97+
error: this `if` statement can be collapsed
98+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:47:5
99+
|
100+
LL | / if let Some(a) = Some(3) {
101+
LL | | if let Some(b) = Some(4) {
102+
LL | | let _ = a + b;
103+
LL | | }
104+
LL | | }
105+
| |_____^
106+
|
107+
help: collapse nested if block
108+
|
109+
LL ~ if let Some(a) = Some(3)
110+
LL ~ && let Some(b) = Some(4) {
111+
LL | let _ = a + b;
112+
LL ~ }
113+
|
114+
115+
error: this `if` statement can be collapsed
116+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:54:5
117+
|
118+
LL | / if let Some(a) = Some(3) {
119+
LL | | if a + 1 == 4 {
120+
LL | | let _ = a;
121+
LL | | }
122+
LL | | }
123+
| |_____^
124+
|
125+
help: collapse nested if block
126+
|
127+
LL ~ if let Some(a) = Some(3)
128+
LL ~ && a + 1 == 4 {
129+
LL | let _ = a;
130+
LL ~ }
131+
|
132+
133+
error: this `if` statement can be collapsed
134+
--> tests/ui-toml/collapsible_if/collapsible_if.rs:61:5
135+
|
136+
LL | / if Some(3) == Some(4).map(|x| x - 1) {
137+
LL | | if let Some(b) = Some(4) {
138+
LL | | let _ = b;
139+
LL | | }
140+
LL | | }
141+
| |_____^
142+
|
143+
help: collapse nested if block
144+
|
145+
LL ~ if Some(3) == Some(4).map(|x| x - 1)
146+
LL ~ && let Some(b) = Some(4) {
147+
LL | let _ = b;
148+
LL ~ }
149+
|
150+
151+
error: aborting due to 8 previous errors
80152

‎tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
3434
check-inconsistent-struct-field-initializers
3535
check-private-items
3636
cognitive-complexity-threshold
37+
collapse-let-chains
3738
disallowed-macros
3839
disallowed-methods
3940
disallowed-names
@@ -126,6 +127,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
126127
check-inconsistent-struct-field-initializers
127128
check-private-items
128129
cognitive-complexity-threshold
130+
collapse-let-chains
129131
disallowed-macros
130132
disallowed-methods
131133
disallowed-names
@@ -218,6 +220,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
218220
check-inconsistent-struct-field-initializers
219221
check-private-items
220222
cognitive-complexity-threshold
223+
collapse-let-chains
221224
disallowed-macros
222225
disallowed-methods
223226
disallowed-names

‎tests/ui/auxiliary/proc_macros.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,12 @@ fn write_with_span(s: Span, mut input: IntoIter, out: &mut TokenStream) -> Resul
131131
pub fn make_it_big(input: TokenStream) -> TokenStream {
132132
let mut expr_repeat = syn::parse_macro_input!(input as syn::ExprRepeat);
133133
let len_span = expr_repeat.len.span();
134-
if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len {
135-
if let syn::Lit::Int(lit_int) = &expr_lit.lit {
136-
let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter");
137-
let new_val = orig_val.saturating_mul(10);
138-
expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val);
139-
}
134+
if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len
135+
&& let syn::Lit::Int(lit_int) = &expr_lit.lit
136+
{
137+
let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter");
138+
let new_val = orig_val.saturating_mul(10);
139+
expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val);
140140
}
141141
quote::quote!(#expr_repeat).into()
142142
}

0 commit comments

Comments
 (0)