Skip to content

Commit ea76ac5

Browse files
mcartonmcarton
committed
Make COLLAPSIBLE_IF consider if let 
1 parent f6ba217 commit ea76ac5

File tree

3 files changed

+126
-29
lines changed

3 files changed

+126
-29
lines changed

‎clippy_lints/src/collapsible_if.rs

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,37 +53,64 @@ impl EarlyLintPass for CollapsibleIf {
5353
}
5454
}
5555

56-
fn check_if(cx: &EarlyContext, e: &ast::Expr) {
57-
if let ast::ExprKind::If(ref check, ref then, ref else_) = e.node {
58-
if let Some(ref else_) = *else_ {
59-
if_let_chain! {[
60-
let ast::ExprKind::Block(ref block) = else_.node,
61-
block.stmts.is_empty(),
62-
let Some(ref else_) = block.expr,
63-
let ast::ExprKind::If(_, _, _) = else_.node
64-
], {
56+
fn check_if(cx: &EarlyContext, expr: &ast::Expr) {
57+
match expr.node {
58+
ast::ExprKind::If(ref check, ref then, ref else_) => {
59+
if let Some(ref else_) = *else_ {
60+
check_collapsible_maybe_if_let(cx, else_);
61+
} else {
62+
check_collapsible_no_if_let(cx, expr, check, then);
63+
}
64+
}
65+
ast::ExprKind::IfLet(_, _, _, Some(ref else_)) => {
66+
check_collapsible_maybe_if_let(cx, else_);
67+
}
68+
_ => (),
69+
}
70+
}
71+
72+
fn check_collapsible_maybe_if_let(cx: &EarlyContext, else_: &ast::Expr) {
73+
if_let_chain! {[
74+
let ast::ExprKind::Block(ref block) = else_.node,
75+
block.stmts.is_empty(),
76+
let Some(ref else_) = block.expr,
77+
], {
78+
match else_.node {
79+
ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => {
6580
span_lint_and_then(cx,
6681
COLLAPSIBLE_IF,
6782
block.span,
6883
"this `else { if .. }` block can be collapsed", |db| {
6984
db.span_suggestion(block.span, "try", snippet_block(cx, else_.span, "..").into_owned());
7085
});
71-
}}
72-
} else if let Some(&ast::Expr { node: ast::ExprKind::If(ref check_inner, ref content, None), span: sp, .. }) =
73-
single_stmt_of_block(then) {
74-
if e.span.expn_id != sp.expn_id {
75-
return;
7686
}
77-
span_lint_and_then(cx, COLLAPSIBLE_IF, e.span, "this if statement can be collapsed", |db| {
78-
db.span_suggestion(e.span,
79-
"try",
80-
format!("if {} && {} {}",
81-
check_to_string(cx, check),
82-
check_to_string(cx, check_inner),
83-
snippet_block(cx, content.span, "..")));
84-
});
87+
_ => (),
8588
}
86-
}
89+
}}
90+
}
91+
92+
fn check_collapsible_no_if_let(
93+
cx: &EarlyContext,
94+
expr: &ast::Expr,
95+
check: &ast::Expr,
96+
then: &ast::Block,
97+
) {
98+
if_let_chain! {[
99+
let Some(inner) = single_stmt_of_block(then),
100+
let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node,
101+
], {
102+
if expr.span.expn_id != inner.span.expn_id {
103+
return;
104+
}
105+
span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| {
106+
db.span_suggestion(expr.span,
107+
"try",
108+
format!("if {} && {} {}",
109+
check_to_string(cx, check),
110+
check_to_string(cx, check_inner),
111+
snippet_block(cx, content.span, "..")));
112+
});
113+
}}
87114
}
88115

89116
fn requires_brackets(e: &ast::Expr) -> bool {

‎tests/compile-fail/collapsible_if.rs

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,32 @@ fn main() {
2626
// Collaspe `else { if .. }` to `else if ..`
2727
if x == "hello" {
2828
print!("Hello ");
29-
} else { //~ERROR: this `else { if .. }`
30-
//~| HELP try
31-
//~| SUGGESTION } else if y == "world"
29+
} else {
30+
//~^ ERROR: this `else { if .. }`
31+
//~| HELP try
32+
//~| SUGGESTION } else if y == "world"
3233
if y == "world" {
3334
println!("world!")
3435
}
3536
}
3637

3738
if x == "hello" {
3839
print!("Hello ");
39-
} else { //~ERROR this `else { if .. }`
40-
//~| HELP try
41-
//~| SUGGESTION } else if y == "world"
40+
} else {
41+
//~^ ERROR: this `else { if .. }`
42+
//~| HELP try
43+
//~| SUGGESTION } else if let Some(42)
44+
if let Some(42) = Some(42) {
45+
println!("world!")
46+
}
47+
}
48+
49+
if x == "hello" {
50+
print!("Hello ");
51+
} else {
52+
//~^ ERROR this `else { if .. }`
53+
//~| HELP try
54+
//~| SUGGESTION } else if y == "world"
4255
if y == "world" {
4356
println!("world")
4457
}
@@ -47,6 +60,62 @@ fn main() {
4760
}
4861
}
4962

63+
if x == "hello" {
64+
print!("Hello ");
65+
} else {
66+
//~^ ERROR this `else { if .. }`
67+
//~| HELP try
68+
//~| SUGGESTION } else if let Some(42)
69+
if let Some(42) = Some(42) {
70+
println!("world")
71+
}
72+
else {
73+
println!("!")
74+
}
75+
}
76+
77+
if let Some(42) = Some(42) {
78+
print!("Hello ");
79+
} else {
80+
//~^ ERROR this `else { if .. }`
81+
//~| HELP try
82+
//~| SUGGESTION } else if let Some(42)
83+
if let Some(42) = Some(42) {
84+
println!("world")
85+
}
86+
else {
87+
println!("!")
88+
}
89+
}
90+
91+
if let Some(42) = Some(42) {
92+
print!("Hello ");
93+
} else {
94+
//~^ ERROR this `else { if .. }`
95+
//~| HELP try
96+
//~| SUGGESTION } else if x == "hello"
97+
if x == "hello" {
98+
println!("world")
99+
}
100+
else {
101+
println!("!")
102+
}
103+
}
104+
105+
if let Some(42) = Some(42) {
106+
print!("Hello ");
107+
} else {
108+
//~^ ERROR this `else { if .. }`
109+
//~| HELP try
110+
//~| SUGGESTION } else if let Some(42)
111+
if let Some(42) = Some(42) {
112+
println!("world")
113+
}
114+
else {
115+
println!("!")
116+
}
117+
}
118+
50119
// Works because any if with an else statement cannot be collapsed.
51120
if x == "hello" {
52121
if y == "world" {

‎tests/compile-fail/copies.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#![allow(unused_variables)]
99
#![allow(cyclomatic_complexity)]
1010
#![allow(blacklisted_name)]
11+
#![allow(collapsible_if)]
1112

1213
fn bar<T>(_: T) {}
1314
fn foo() -> bool { unimplemented!() }

0 commit comments

Comments
 (0)