diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 00d7bf3b5..12c838736 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -84,18 +84,14 @@ impl EarlyLintPass for CollapsibleIf { } fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) { - match expr.node { - ast::ExprKind::If(ref check, ref then, ref else_) => { - if let Some(ref else_) = *else_ { - check_collapsible_maybe_if_let(cx, else_); - } else { - check_collapsible_no_if_let(cx, expr, check, then); - } - }, - ast::ExprKind::IfLet(_, _, _, Some(ref else_)) => { + if let ast::ExprKind::If(check, then, else_) = &expr.node { + if let Some(else_) = else_ { check_collapsible_maybe_if_let(cx, else_); - }, - _ => (), + } else if let ast::ExprKind::Let(..) = check.node { + // Prevent triggering on `if let a = b { if c { .. } }`. + } else { + check_collapsible_no_if_let(cx, expr, check, then); + } } } @@ -113,22 +109,18 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) { if !block_starts_with_comment(cx, block); if let Some(else_) = expr_block(block); if !in_macro_or_desugar(else_.span); + if let ast::ExprKind::If(..) = else_.node; then { - match else_.node { - ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - COLLAPSIBLE_IF, - block.span, - "this `else { if .. }` block can be collapsed", - "try", - snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(), - applicability, - ); - } - _ => (), - } + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + COLLAPSIBLE_IF, + block.span, + "this `else { if .. }` block can be collapsed", + "try", + snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(), + applicability, + ); } } } @@ -139,6 +131,11 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: & if let Some(inner) = expr_block(then); if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node; then { + if let ast::ExprKind::Let(..) = check_inner.node { + // Prevent triggering on `if c { if let a = b { .. } }`. + return; + } + if expr.span.ctxt() != inner.span.ctxt() { return; } diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 5916a0659..1d2e50a78 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -248,9 +248,7 @@ fn is_block(expr: &ast::Expr) -> bool { /// Match `if` or `if let` expressions and return the `then` and `else` block. fn unsugar_if(expr: &ast::Expr) -> Option<(&P, &Option>)> { match expr.node { - ast::ExprKind::If(_, ref then, ref else_) | ast::ExprKind::IfLet(_, _, ref then, ref else_) => { - Some((then, else_)) - }, + ast::ExprKind::If(_, ref then, ref else_) => Some((then, else_)), _ => None, } } diff --git a/clippy_lints/src/identity_conversion.rs b/clippy_lints/src/identity_conversion.rs index 50ba2b90e..baf7791b7 100644 --- a/clippy_lints/src/identity_conversion.rs +++ b/clippy_lints/src/identity_conversion.rs @@ -50,8 +50,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { }; if let ExprKind::Call(_, ref args) = e.node { self.try_desugar_arm.push(args[0].hir_id); - } else { - return; } }, diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index 883515815..97175f869 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -209,12 +209,11 @@ fn with_loop_block(expr: &ast::Expr, mut func: F) where F: FnMut(&ast::Block, Option<&ast::Label>), { - match expr.node { - ast::ExprKind::While(_, ref loop_block, ref label) - | ast::ExprKind::WhileLet(_, _, ref loop_block, ref label) - | ast::ExprKind::ForLoop(_, _, ref loop_block, ref label) - | ast::ExprKind::Loop(ref loop_block, ref label) => func(loop_block, label.as_ref()), - _ => {}, + if let ast::ExprKind::While(_, loop_block, label) + | ast::ExprKind::ForLoop(_, _, loop_block, label) + | ast::ExprKind::Loop(loop_block, label) = &expr.node + { + func(loop_block, label.as_ref()); } } diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 70286dc95..a9643a740 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -135,7 +135,7 @@ impl<'a> Sugg<'a> { | ast::ExprKind::Box(..) | ast::ExprKind::Closure(..) | ast::ExprKind::If(..) - | ast::ExprKind::IfLet(..) + | ast::ExprKind::Let(..) | ast::ExprKind::Unary(..) | ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet), ast::ExprKind::Async(..) @@ -162,7 +162,6 @@ impl<'a> Sugg<'a> { | ast::ExprKind::Tup(..) | ast::ExprKind::Array(..) | ast::ExprKind::While(..) - | ast::ExprKind::WhileLet(..) | ast::ExprKind::Await(..) | ast::ExprKind::Err => Sugg::NonParen(snippet), ast::ExprKind::Range(.., RangeLimits::HalfOpen) => Sugg::BinOp(AssocOp::DotDot, snippet), diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index 5474837d8..3427d88ff 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -37,7 +37,7 @@ pub fn is_potentially_mutated<'a, 'tcx>(variable: &'tcx Path, expr: &'tcx Expr, if let Res::Local(id) = variable.res { mutated_variables(expr, cx).map_or(true, |mutated| mutated.contains(&id)) } else { - return true; + true } } diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index 69570c515..810fd2e69 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -172,4 +172,25 @@ else { println!("Hello world!"); } } + + // Test behavior wrt. `let_chains`. + // None of the cases below should be collapsed. + fn truth() -> bool { true } + + // Prefix: + if let 0 = 1 { + if truth() {} + } + + // Suffix: + if truth() { + if let 0 = 1 {} + } + + // Midfix: + if truth() { + if let 0 = 1 { + if truth() {} + } + } } diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index 5dac42a3d..7b322acae 100644 --- a/tests/ui/collapsible_if.rs +++ b/tests/ui/collapsible_if.rs @@ -200,4 +200,25 @@ fn main() { println!("Hello world!"); } } + + // Test behavior wrt. `let_chains`. + // None of the cases below should be collapsed. + fn truth() -> bool { true } + + // Prefix: + if let 0 = 1 { + if truth() {} + } + + // Suffix: + if truth() { + if let 0 = 1 {} + } + + // Midfix: + if truth() { + if let 0 = 1 { + if truth() {} + } + } }