Skip to content

Commit 8c80b65

Browse files
committed
Auto merge of #4123 - Centril:rustup-let-chains-ast, r=Manishearth
Fix fallout from rust-lang/rust PR 60861 Fixes incoming breakage for unlanded rust-lang/rust#60861. Tests are passing locally; the Rust PR now needs to land first. @Manishearth also says we'll want to split out to a `collapsible_if_let` once we have let-chains working in Rust nightly or something.
2 parents c5d1ecd + 46a0e54 commit 8c80b65

8 files changed

+73
-40
lines changed

clippy_lints/src/collapsible_if.rs

+23-26
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,14 @@ impl EarlyLintPass for CollapsibleIf {
8484
}
8585

8686
fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
87-
match expr.node {
88-
ast::ExprKind::If(ref check, ref then, ref else_) => {
89-
if let Some(ref else_) = *else_ {
90-
check_collapsible_maybe_if_let(cx, else_);
91-
} else {
92-
check_collapsible_no_if_let(cx, expr, check, then);
93-
}
94-
},
95-
ast::ExprKind::IfLet(_, _, _, Some(ref else_)) => {
87+
if let ast::ExprKind::If(check, then, else_) = &expr.node {
88+
if let Some(else_) = else_ {
9689
check_collapsible_maybe_if_let(cx, else_);
97-
},
98-
_ => (),
90+
} else if let ast::ExprKind::Let(..) = check.node {
91+
// Prevent triggering on `if let a = b { if c { .. } }`.
92+
} else {
93+
check_collapsible_no_if_let(cx, expr, check, then);
94+
}
9995
}
10096
}
10197

@@ -113,22 +109,18 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
113109
if !block_starts_with_comment(cx, block);
114110
if let Some(else_) = expr_block(block);
115111
if !in_macro_or_desugar(else_.span);
112+
if let ast::ExprKind::If(..) = else_.node;
116113
then {
117-
match else_.node {
118-
ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => {
119-
let mut applicability = Applicability::MachineApplicable;
120-
span_lint_and_sugg(
121-
cx,
122-
COLLAPSIBLE_IF,
123-
block.span,
124-
"this `else { if .. }` block can be collapsed",
125-
"try",
126-
snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(),
127-
applicability,
128-
);
129-
}
130-
_ => (),
131-
}
114+
let mut applicability = Applicability::MachineApplicable;
115+
span_lint_and_sugg(
116+
cx,
117+
COLLAPSIBLE_IF,
118+
block.span,
119+
"this `else { if .. }` block can be collapsed",
120+
"try",
121+
snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(),
122+
applicability,
123+
);
132124
}
133125
}
134126
}
@@ -139,6 +131,11 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &
139131
if let Some(inner) = expr_block(then);
140132
if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node;
141133
then {
134+
if let ast::ExprKind::Let(..) = check_inner.node {
135+
// Prevent triggering on `if c { if let a = b { .. } }`.
136+
return;
137+
}
138+
142139
if expr.span.ctxt() != inner.span.ctxt() {
143140
return;
144141
}

clippy_lints/src/formatting.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,7 @@ fn is_block(expr: &ast::Expr) -> bool {
248248
/// Match `if` or `if let` expressions and return the `then` and `else` block.
249249
fn unsugar_if(expr: &ast::Expr) -> Option<(&P<ast::Block>, &Option<P<ast::Expr>>)> {
250250
match expr.node {
251-
ast::ExprKind::If(_, ref then, ref else_) | ast::ExprKind::IfLet(_, _, ref then, ref else_) => {
252-
Some((then, else_))
253-
},
251+
ast::ExprKind::If(_, ref then, ref else_) => Some((then, else_)),
254252
_ => None,
255253
}
256254
}

clippy_lints/src/identity_conversion.rs

-2
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion {
5050
};
5151
if let ExprKind::Call(_, ref args) = e.node {
5252
self.try_desugar_arm.push(args[0].hir_id);
53-
} else {
54-
return;
5553
}
5654
},
5755

clippy_lints/src/needless_continue.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,11 @@ fn with_loop_block<F>(expr: &ast::Expr, mut func: F)
209209
where
210210
F: FnMut(&ast::Block, Option<&ast::Label>),
211211
{
212-
match expr.node {
213-
ast::ExprKind::While(_, ref loop_block, ref label)
214-
| ast::ExprKind::WhileLet(_, _, ref loop_block, ref label)
215-
| ast::ExprKind::ForLoop(_, _, ref loop_block, ref label)
216-
| ast::ExprKind::Loop(ref loop_block, ref label) => func(loop_block, label.as_ref()),
217-
_ => {},
212+
if let ast::ExprKind::While(_, loop_block, label)
213+
| ast::ExprKind::ForLoop(_, _, loop_block, label)
214+
| ast::ExprKind::Loop(loop_block, label) = &expr.node
215+
{
216+
func(loop_block, label.as_ref());
218217
}
219218
}
220219

clippy_lints/src/utils/sugg.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ impl<'a> Sugg<'a> {
135135
| ast::ExprKind::Box(..)
136136
| ast::ExprKind::Closure(..)
137137
| ast::ExprKind::If(..)
138-
| ast::ExprKind::IfLet(..)
138+
| ast::ExprKind::Let(..)
139139
| ast::ExprKind::Unary(..)
140140
| ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet),
141141
ast::ExprKind::Async(..)
@@ -162,7 +162,6 @@ impl<'a> Sugg<'a> {
162162
| ast::ExprKind::Tup(..)
163163
| ast::ExprKind::Array(..)
164164
| ast::ExprKind::While(..)
165-
| ast::ExprKind::WhileLet(..)
166165
| ast::ExprKind::Await(..)
167166
| ast::ExprKind::Err => Sugg::NonParen(snippet),
168167
ast::ExprKind::Range(.., RangeLimits::HalfOpen) => Sugg::BinOp(AssocOp::DotDot, snippet),

clippy_lints/src/utils/usage.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ pub fn is_potentially_mutated<'a, 'tcx>(variable: &'tcx Path, expr: &'tcx Expr,
3737
if let Res::Local(id) = variable.res {
3838
mutated_variables(expr, cx).map_or(true, |mutated| mutated.contains(&id))
3939
} else {
40-
return true;
40+
true
4141
}
4242
}
4343

tests/ui/collapsible_if.fixed

+21
Original file line numberDiff line numberDiff line change
@@ -172,4 +172,25 @@ else {
172172
println!("Hello world!");
173173
}
174174
}
175+
176+
// Test behavior wrt. `let_chains`.
177+
// None of the cases below should be collapsed.
178+
fn truth() -> bool { true }
179+
180+
// Prefix:
181+
if let 0 = 1 {
182+
if truth() {}
183+
}
184+
185+
// Suffix:
186+
if truth() {
187+
if let 0 = 1 {}
188+
}
189+
190+
// Midfix:
191+
if truth() {
192+
if let 0 = 1 {
193+
if truth() {}
194+
}
195+
}
175196
}

tests/ui/collapsible_if.rs

+21
Original file line numberDiff line numberDiff line change
@@ -200,4 +200,25 @@ fn main() {
200200
println!("Hello world!");
201201
}
202202
}
203+
204+
// Test behavior wrt. `let_chains`.
205+
// None of the cases below should be collapsed.
206+
fn truth() -> bool { true }
207+
208+
// Prefix:
209+
if let 0 = 1 {
210+
if truth() {}
211+
}
212+
213+
// Suffix:
214+
if truth() {
215+
if let 0 = 1 {}
216+
}
217+
218+
// Midfix:
219+
if truth() {
220+
if let 0 = 1 {
221+
if truth() {}
222+
}
223+
}
203224
}

0 commit comments

Comments
 (0)