Add collapse-let-chains option to collapsible_if lint

CHANGELOG.md

@@ -6350,6 +6350,7 @@ Released 2018-09-13
 [`check-inconsistent-struct-field-initializers`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-inconsistent-struct-field-initializers
 [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
 [`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold
+[`collapse-let-chains`]: https://doc.rust-lang.org/clippy/lint_configuration.html#collapse-let-chains
 [`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros
 [`disallowed-methods`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods
 [`disallowed-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-names

book/src/lint_configuration.md

@@ -475,6 +475,17 @@ The maximum cognitive complexity a function can have
 * [`cognitive_complexity`](https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity)
 
 
+## `collapse-let-chains`
+Whether `if let` chains should be collapsed. This requires the use of the unstable
+`let_chains` rustc feature.
+
+**Default Value:** `false`
+
+---
+**Affected lints:**
+* [`collapsible_if`](https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if)
+
+
 ## `disallowed-macros`
 The list of disallowed macros, written as fully qualified paths.
 

clippy_config/src/conf.rs

@@ -500,6 +500,10 @@ define_Conf! {
     /// The maximum cognitive complexity a function can have
     #[lints(cognitive_complexity)]
     cognitive_complexity_threshold: u64 = 25,
+    /// Whether `if let` chains should be collapsed. This requires the use of the unstable
+    /// `let_chains` rustc feature.
+    #[lints(collapsible_if)]
+    collapse_let_chains: bool = false,
     /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY.
     ///
     /// Use the Cognitive Complexity lint instead.

clippy_lints/src/collapsible_if.rs

@@ -79,16 +79,24 @@ declare_clippy_lint! {
 }
 
 pub struct CollapsibleIf {
+    collapse_let_chains: bool,
     lint_commented_code: bool,
 }
 
 impl CollapsibleIf {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
+            collapse_let_chains: conf.collapse_let_chains,
             lint_commented_code: conf.lint_commented_code,
         }
     }
 
+    /// Prevent triggering on `if c { if let a = b { .. } }` unless the
+    /// `collapse_let_chains` config option is set.
+    fn is_collapsible(&self, cond: &ast::Expr) -> bool {
+        self.collapse_let_chains || !matches!(cond.kind, ast::ExprKind::Let(..))
+    }
+
     fn check_collapsible_else_if(cx: &EarlyContext<'_>, then_span: Span, else_: &ast::Expr) {
         if let ast::ExprKind::Block(ref block, _) = else_.kind
             && !block_starts_with_comment(cx, block)
@@ -127,8 +135,7 @@ impl CollapsibleIf {
         if let Some(inner) = expr_block(then)
             && inner.attrs.is_empty()
             && let ast::ExprKind::If(check_inner, _, None) = &inner.kind
-            // Prevent triggering on `if c { if let a = b { .. } }`.
-            && !matches!(check_inner.kind, ast::ExprKind::Let(..))
+            && self.is_collapsible(check_inner)
             && let ctxt = expr.span.ctxt()
             && inner.span.ctxt() == ctxt
             && let contains_comment = span_contains_comment(cx.sess().source_map(), check.span.to(check_inner.span))
@@ -175,8 +182,7 @@ impl EarlyLintPass for CollapsibleIf {
         {
             if let Some(else_) = else_ {
                 Self::check_collapsible_else_if(cx, then.span, else_);
-            } else if !matches!(cond.kind, ast::ExprKind::Let(..)) {
-                // Prevent triggering on `if c { if let a = b { .. } }`.
+            } else if self.is_collapsible(cond) {
                 self.check_collapsible_if_if(cx, expr, cond, then);
             }
         }

tests/ui-toml/collapsible_if/clippy.toml

@@ -1 +1,2 @@
+collapse-let-chains = true
 lint-commented-code = true

tests/ui-toml/collapsible_if/collapsible_if.fixed

@@ -1,3 +1,4 @@
+#![feature(let_chains)]
 #![allow(clippy::eq_op, clippy::nonminimal_bool)]
 
 #[rustfmt::skip]
@@ -31,4 +32,28 @@ fn main() {
             println!("Hello world!");
         }
     //~^^^^^ collapsible_if
+
+    if true
+         && true {
+             println!("No comment, linted");
+         }
+     //~^^^^^ collapsible_if
+
+    if let Some(a) = Some(3)
+        && let Some(b) = Some(4) {
+            let _ = a + b;
+        }
+    //~^^^^^ collapsible_if
+
+    if let Some(a) = Some(3)
+        && a + 1 == 4 {
+            let _ = a;
+        }
+    //~^^^^^ collapsible_if
+
+    if Some(3) == Some(4).map(|x| x - 1)
+        && let Some(b) = Some(4) {
+            let _ = b;
+        }
+    //~^^^^^ collapsible_if
 }

tests/ui-toml/collapsible_if/collapsible_if.rs

@@ -1,3 +1,4 @@
+#![feature(let_chains)]
 #![allow(clippy::eq_op, clippy::nonminimal_bool)]
 
 #[rustfmt::skip]
@@ -35,4 +36,32 @@ fn main() {
         }
     }
     //~^^^^^ collapsible_if
+
+    if true {
+         if true {
+             println!("No comment, linted");
+         }
+     }
+     //~^^^^^ collapsible_if
+
+    if let Some(a) = Some(3) {
+        if let Some(b) = Some(4) {
+            let _ = a + b;
+        }
+    }
+    //~^^^^^ collapsible_if
+
+    if let Some(a) = Some(3) {
+        if a + 1 == 4 {
+            let _ = a;
+        }
+    }
+    //~^^^^^ collapsible_if
+
+    if Some(3) == Some(4).map(|x| x - 1) {
+        if let Some(b) = Some(4) {
+            let _ = b;
+        }
+    }
+    //~^^^^^ collapsible_if
 }

tests/ui-toml/collapsible_if/collapsible_if.stderr

@@ -1,5 +1,5 @@
 error: this `if` statement can be collapsed
-  --> tests/ui-toml/collapsible_if/collapsible_if.rs:8:5
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:9:5
    |
 LL | /     if x == "hello" {
 LL | |         // Comment must be kept
@@ -21,7 +21,7 @@ LL ~         }
    |
 
 error: this `if` statement can be collapsed
-  --> tests/ui-toml/collapsible_if/collapsible_if.rs:17:5
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:18:5
    |
 LL | /     if x == "hello" { // Inner comment
 LL | |         if y == "world" {
@@ -39,7 +39,7 @@ LL ~         }
    |
 
 error: this `if` statement can be collapsed
-  --> tests/ui-toml/collapsible_if/collapsible_if.rs:24:5
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:25:5
    |
 LL | /     if x == "hello" {
 LL | |         /* Inner comment */
@@ -59,7 +59,7 @@ LL ~         }
    |
 
 error: this `if` statement can be collapsed
-  --> tests/ui-toml/collapsible_if/collapsible_if.rs:32:5
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:33:5
    |
 LL | /     if x == "hello" { /* Inner comment */
 LL | |         if y == "world" {
@@ -76,5 +76,77 @@ LL |             println!("Hello world!");
 LL ~         }
    |
 
-error: aborting due to 4 previous errors
+error: this `if` statement can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:40:5
+   |
+LL | /     if true {
+LL | |          if true {
+LL | |              println!("No comment, linted");
+LL | |          }
+LL | |      }
+   | |______^
+   |
+help: collapse nested if block
+   |
+LL ~     if true
+LL ~          && true {
+LL |              println!("No comment, linted");
+LL ~          }
+   |
+
+error: this `if` statement can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:47:5
+   |
+LL | /     if let Some(a) = Some(3) {
+LL | |         if let Some(b) = Some(4) {
+LL | |             let _ = a + b;
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     if let Some(a) = Some(3)
+LL ~         && let Some(b) = Some(4) {
+LL |             let _ = a + b;
+LL ~         }
+   |
+
+error: this `if` statement can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:54:5
+   |
+LL | /     if let Some(a) = Some(3) {
+LL | |         if a + 1 == 4 {
+LL | |             let _ = a;
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     if let Some(a) = Some(3)
+LL ~         && a + 1 == 4 {
+LL |             let _ = a;
+LL ~         }
+   |
+
+error: this `if` statement can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:61:5
+   |
+LL | /     if Some(3) == Some(4).map(|x| x - 1) {
+LL | |         if let Some(b) = Some(4) {
+LL | |             let _ = b;
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     if Some(3) == Some(4).map(|x| x - 1)
+LL ~         && let Some(b) = Some(4) {
+LL |             let _ = b;
+LL ~         }
+   |
+
+error: aborting due to 8 previous errors
 

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -34,6 +34,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            check-inconsistent-struct-field-initializers
            check-private-items
            cognitive-complexity-threshold
+           collapse-let-chains
            disallowed-macros
            disallowed-methods
            disallowed-names
@@ -126,6 +127,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            check-inconsistent-struct-field-initializers
            check-private-items
            cognitive-complexity-threshold
+           collapse-let-chains
            disallowed-macros
            disallowed-methods
            disallowed-names
@@ -218,6 +220,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            check-inconsistent-struct-field-initializers
            check-private-items
            cognitive-complexity-threshold
+           collapse-let-chains
            disallowed-macros
            disallowed-methods
            disallowed-names

tests/ui/auxiliary/proc_macros.rs

@@ -131,12 +131,12 @@ fn write_with_span(s: Span, mut input: IntoIter, out: &mut TokenStream) -> Resul
 pub fn make_it_big(input: TokenStream) -> TokenStream {
     let mut expr_repeat = syn::parse_macro_input!(input as syn::ExprRepeat);
     let len_span = expr_repeat.len.span();
-    if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len {
-        if let syn::Lit::Int(lit_int) = &expr_lit.lit {
-            let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter");
-            let new_val = orig_val.saturating_mul(10);
-            expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val);
-        }
+    if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len
+        && let syn::Lit::Int(lit_int) = &expr_lit.lit
+    {
+        let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter");
+        let new_val = orig_val.saturating_mul(10);
+        expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val);
     }
     quote::quote!(#expr_repeat).into()
 }