Skip to content

Commit cd70152

Browse files
samueltardieusamueltardieu
committed
Make collapsible_if recognize the let_chains feature
Until `if let` chains are stabilized, we do not collapse them together or with other `if` expressions unless the `let_chains` feature is enabled. This is the case for example in Clippy sources.
1 parent 9df5177 commit cd70152

File tree

9 files changed

+254
-13
lines changed

9 files changed

+254
-13
lines changed

‎clippy_lints/src/collapsible_if.rs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use rustc_ast::BinOpKind;
55
use rustc_errors::Applicability;
66
use rustc_hir::{Block, Expr, ExprKind, StmtKind};
77
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::ty::TyCtxt;
89
use rustc_session::impl_lint_pass;
910
use rustc_span::Span;
1011

@@ -77,12 +78,14 @@ declare_clippy_lint! {
7778
}
7879

7980
pub struct CollapsibleIf {
81+
let_chains_enabled: bool,
8082
lint_commented_code: bool,
8183
}
8284

8385
impl CollapsibleIf {
84-
pub fn new(conf: &'static Conf) -> Self {
86+
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
8587
Self {
88+
let_chains_enabled: tcx.features().let_chains(),
8689
lint_commented_code: conf.lint_commented_code,
8790
}
8891
}
@@ -124,8 +127,7 @@ impl CollapsibleIf {
124127
if let Some(inner) = expr_block(then)
125128
&& cx.tcx.hir_attrs(inner.hir_id).is_empty()
126129
&& let ExprKind::If(check_inner, _, None) = &inner.kind
127-
// Prevent triggering on `if c { if let a = b { .. } }`.
128-
&& !matches!(check_inner.kind, ExprKind::Let(..))
130+
&& self.eligible_condition(check_inner)
129131
&& let ctxt = expr.span.ctxt()
130132
&& inner.span.ctxt() == ctxt
131133
&& (self.lint_commented_code || !block_starts_with_comment(cx, then))
@@ -160,6 +162,10 @@ impl CollapsibleIf {
160162
);
161163
}
162164
}
165+
166+
pub fn eligible_condition(&self, cond: &Expr<'_>) -> bool {
167+
self.let_chains_enabled || !matches!(cond.kind, ExprKind::Let(..))
168+
}
163169
}
164170

165171
impl_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF, COLLAPSIBLE_ELSE_IF]);
@@ -174,11 +180,10 @@ impl LateLintPass<'_> for CollapsibleIf {
174180
{
175181
Self::check_collapsible_else_if(cx, then.span, else_);
176182
} else if else_.is_none()
177-
&& !matches!(cond.kind, ExprKind::Let(..))
183+
&& self.eligible_condition(cond)
178184
&& let ExprKind::Block(then, None) = then.kind
179185
{
180-
// Prevent triggering on `if c { if let a = b { .. } }`.
181-
self.check_collapsible_if_if(cx, expr, cond, block!(then));
186+
self.check_collapsible_if_if(cx, expr, cond, then);
182187
}
183188
}
184189
}

‎clippy_lints/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
771771
store.register_late_pass(|_| Box::new(redundant_closure_call::RedundantClosureCall));
772772
store.register_early_pass(|| Box::new(unused_unit::UnusedUnit));
773773
store.register_late_pass(|_| Box::new(returns::Return));
774-
store.register_late_pass(move |_| Box::new(collapsible_if::CollapsibleIf::new(conf)));
774+
store.register_late_pass(move |tcx| Box::new(collapsible_if::CollapsibleIf::new(tcx, conf)));
775775
store.register_late_pass(|_| Box::new(items_after_statements::ItemsAfterStatements));
776776
store.register_early_pass(|| Box::new(precedence::Precedence));
777777
store.register_late_pass(|_| Box::new(needless_parens_on_range_literals::NeedlessParensOnRangeLiterals));

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![feature(let_chains)]
2+
#![warn(clippy::collapsible_if)]
3+
4+
fn main() {
5+
if let Some(a) = Some(3)
6+
// with comment
7+
&& let Some(b) = Some(4) {
8+
let _ = a + b;
9+
}
10+
//~^^^^^^ collapsible_if
11+
12+
if let Some(a) = Some(3)
13+
// with comment
14+
&& a + 1 == 4 {
15+
let _ = a;
16+
}
17+
//~^^^^^^ collapsible_if
18+
19+
if Some(3) == Some(4).map(|x| x - 1)
20+
// with comment
21+
&& let Some(b) = Some(4) {
22+
let _ = b;
23+
}
24+
//~^^^^^^ collapsible_if
25+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#![feature(let_chains)]
2+
#![warn(clippy::collapsible_if)]
3+
4+
fn main() {
5+
if let Some(a) = Some(3) {
6+
// with comment
7+
if let Some(b) = Some(4) {
8+
let _ = a + b;
9+
}
10+
}
11+
//~^^^^^^ collapsible_if
12+
13+
if let Some(a) = Some(3) {
14+
// with comment
15+
if a + 1 == 4 {
16+
let _ = a;
17+
}
18+
}
19+
//~^^^^^^ collapsible_if
20+
21+
if Some(3) == Some(4).map(|x| x - 1) {
22+
// with comment
23+
if let Some(b) = Some(4) {
24+
let _ = b;
25+
}
26+
}
27+
//~^^^^^^ collapsible_if
28+
}

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
error: this `if` statement can be collapsed
2+
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:5:5
3+
|
4+
LL | / if let Some(a) = Some(3) {
5+
LL | | // with comment
6+
LL | | if let Some(b) = Some(4) {
7+
LL | | let _ = a + b;
8+
LL | | }
9+
LL | | }
10+
| |_____^
11+
|
12+
= note: `-D clippy::collapsible-if` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::collapsible_if)]`
14+
help: collapse nested if block
15+
|
16+
LL ~ if let Some(a) = Some(3)
17+
LL | // with comment
18+
LL ~ && let Some(b) = Some(4) {
19+
LL | let _ = a + b;
20+
LL ~ }
21+
|
22+
23+
error: this `if` statement can be collapsed
24+
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:13:5
25+
|
26+
LL | / if let Some(a) = Some(3) {
27+
LL | | // with comment
28+
LL | | if a + 1 == 4 {
29+
LL | | let _ = a;
30+
LL | | }
31+
LL | | }
32+
| |_____^
33+
|
34+
help: collapse nested if block
35+
|
36+
LL ~ if let Some(a) = Some(3)
37+
LL | // with comment
38+
LL ~ && a + 1 == 4 {
39+
LL | let _ = a;
40+
LL ~ }
41+
|
42+
43+
error: this `if` statement can be collapsed
44+
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:21:5
45+
|
46+
LL | / if Some(3) == Some(4).map(|x| x - 1) {
47+
LL | | // with comment
48+
LL | | if let Some(b) = Some(4) {
49+
LL | | let _ = b;
50+
LL | | }
51+
LL | | }
52+
| |_____^
53+
|
54+
help: collapse nested if block
55+
|
56+
LL ~ if Some(3) == Some(4).map(|x| x - 1)
57+
LL | // with comment
58+
LL ~ && let Some(b) = Some(4) {
59+
LL | let _ = b;
60+
LL ~ }
61+
|
62+
63+
error: aborting due to 3 previous errors
64+

‎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
}

‎tests/ui/collapsible_if_let_chains.fixed‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#![feature(let_chains)]
2+
#![warn(clippy::collapsible_if)]
3+
4+
fn main() {
5+
if let Some(a) = Some(3) {
6+
// with comment, so do not lint
7+
if let Some(b) = Some(4) {
8+
let _ = a + b;
9+
}
10+
}
11+
12+
if let Some(a) = Some(3)
13+
&& let Some(b) = Some(4) {
14+
let _ = a + b;
15+
}
16+
//~^^^^^ collapsible_if
17+
18+
if let Some(a) = Some(3)
19+
&& a + 1 == 4 {
20+
let _ = a;
21+
}
22+
//~^^^^^ collapsible_if
23+
24+
if Some(3) == Some(4).map(|x| x - 1)
25+
&& let Some(b) = Some(4) {
26+
let _ = b;
27+
}
28+
//~^^^^^ collapsible_if
29+
}

‎tests/ui/collapsible_if_let_chains.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![feature(let_chains)]
2+
#![warn(clippy::collapsible_if)]
3+
4+
fn main() {
5+
if let Some(a) = Some(3) {
6+
// with comment, so do not lint
7+
if let Some(b) = Some(4) {
8+
let _ = a + b;
9+
}
10+
}
11+
12+
if let Some(a) = Some(3) {
13+
if let Some(b) = Some(4) {
14+
let _ = a + b;
15+
}
16+
}
17+
//~^^^^^ collapsible_if
18+
19+
if let Some(a) = Some(3) {
20+
if a + 1 == 4 {
21+
let _ = a;
22+
}
23+
}
24+
//~^^^^^ collapsible_if
25+
26+
if Some(3) == Some(4).map(|x| x - 1) {
27+
if let Some(b) = Some(4) {
28+
let _ = b;
29+
}
30+
}
31+
//~^^^^^ collapsible_if
32+
}

‎tests/ui/collapsible_if_let_chains.stderr‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
error: this `if` statement can be collapsed
2+
--> tests/ui/collapsible_if_let_chains.rs:12:5
3+
|
4+
LL | / if let Some(a) = Some(3) {
5+
LL | | if let Some(b) = Some(4) {
6+
LL | | let _ = a + b;
7+
LL | | }
8+
LL | | }
9+
| |_____^
10+
|
11+
= note: `-D clippy::collapsible-if` implied by `-D warnings`
12+
= help: to override `-D warnings` add `#[allow(clippy::collapsible_if)]`
13+
help: collapse nested if block
14+
|
15+
LL ~ if let Some(a) = Some(3)
16+
LL ~ && let Some(b) = Some(4) {
17+
LL | let _ = a + b;
18+
LL ~ }
19+
|
20+
21+
error: this `if` statement can be collapsed
22+
--> tests/ui/collapsible_if_let_chains.rs:19:5
23+
|
24+
LL | / if let Some(a) = Some(3) {
25+
LL | | if a + 1 == 4 {
26+
LL | | let _ = a;
27+
LL | | }
28+
LL | | }
29+
| |_____^
30+
|
31+
help: collapse nested if block
32+
|
33+
LL ~ if let Some(a) = Some(3)
34+
LL ~ && a + 1 == 4 {
35+
LL | let _ = a;
36+
LL ~ }
37+
|
38+
39+
error: this `if` statement can be collapsed
40+
--> tests/ui/collapsible_if_let_chains.rs:26:5
41+
|
42+
LL | / if Some(3) == Some(4).map(|x| x - 1) {
43+
LL | | if let Some(b) = Some(4) {
44+
LL | | let _ = b;
45+
LL | | }
46+
LL | | }
47+
| |_____^
48+
|
49+
help: collapse nested if block
50+
|
51+
LL ~ if Some(3) == Some(4).map(|x| x - 1)
52+
LL ~ && let Some(b) = Some(4) {
53+
LL | let _ = b;
54+
LL ~ }
55+
|
56+
57+
error: aborting due to 3 previous errors
58+

0 commit comments

Comments
 (0)