Add collapse_let_chains option to collapsible_if lint

Since `collapsible_if` is an early lint, it is not possible to
automatically check whether the `let_chains` unstable rustc feature is
enabled or not, as features are not available in the `EarlyContext`
object.

Author: samueltardieu

Diff for: .github/workflows/lintcheck.yml

@@ -66,7 +66,7 @@ jobs:
 
     - name: Run lintcheck
       if: steps.cache-json.outputs.cache-hit != 'true'
-      run: ./target/debug/lintcheck --format json --all-lints --crates-toml ./lintcheck/ci_crates.toml
+      run: env CLIPPY_CONF_DIR="$PWD/lintcheck/ci-config" ./target/debug/lintcheck --format json --all-lints --crates-toml ./lintcheck/ci_crates.toml
 
     - name: Upload base JSON
       uses: actions/upload-artifact@v4
@@ -97,7 +97,7 @@ jobs:
       run: cargo build --manifest-path=lintcheck/Cargo.toml
 
     - name: Run lintcheck
-      run: ./target/debug/lintcheck --format json --all-lints --crates-toml ./lintcheck/ci_crates.toml
+      run: env CLIPPY_CONF_DIR="$PWD/lintcheck/ci-config" ./target/debug/lintcheck --format json --all-lints --crates-toml ./lintcheck/ci_crates.toml
 
     - name: Upload head JSON
       uses: actions/upload-artifact@v4

Diff for: CHANGELOG.md

@@ -6299,6 +6299,7 @@ Released 2018-09-13
 [`cargo-ignore-publish`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cargo-ignore-publish
 [`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

Diff for: book/src/lint_configuration.md

@@ -437,6 +437,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.
 

Diff for: clippy_config/src/conf.rs

@@ -469,6 +469,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.
@@ -538,11 +542,11 @@ define_Conf! {
     /// The maximum size of the `Err`-variant in a `Result` returned from a function
     #[lints(result_large_err)]
     large_error_threshold: u64 = 128,
-     /// Whether collapsible `if` chains are linted if they contain comments inside the parts
+    /// Whether collapsible `if` chains are linted if they contain comments inside the parts
     /// that would be collapsed.
     #[lints(collapsible_if)]
     lint_commented_code: bool = true,
-   /// Whether to suggest reordering constructor fields when initializers are present.
+    /// Whether to suggest reordering constructor fields when initializers are present.
     ///
     /// Warnings produced by this configuration aren't necessarily fixed by just reordering the fields. Even if the
     /// suggested code would compile, it can change semantics if the initializer expressions have side effects. The

Diff for: clippy_lints/src/collapsible_if.rs

@@ -78,16 +78,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)
@@ -126,8 +134,7 @@ impl CollapsibleIf {
         if let Some(inner) = expr_block(then)
             && inner.attrs.is_empty()
             && let ast::ExprKind::If(ref check_inner, ref content, 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))
@@ -171,8 +178,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);
             }
         }

Diff for: clippy_lints/src/escape.rs

@@ -94,11 +94,12 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal {
             // find `self` ty for this trait if relevant
             if let ItemKind::Trait(_, _, _, _, items) = item.kind {
                 for trait_item in items {
-                    if trait_item.id.owner_id.def_id == fn_def_id
+                    if trait_item.id.owner_id.def_id == fn_def_id {
                         // be sure we have `self` parameter in this function
-                        && trait_item.kind == (AssocItemKind::Fn { has_self: true })
-                    {
-                        trait_self_ty = Some(TraitRef::identity(cx.tcx, trait_item.id.owner_id.to_def_id()).self_ty());
+                        if trait_item.kind == (AssocItemKind::Fn { has_self: true }) {
+                            trait_self_ty =
+                                Some(TraitRef::identity(cx.tcx, trait_item.id.owner_id.to_def_id()).self_ty());
+                        }
                     }
                 }
             }

Diff for: clippy_lints/src/item_name_repetitions.rs

@@ -282,21 +282,22 @@ fn check_struct_name_repetition(cx: &LateContext<'_>, item: &Item<'_>, fields: &
                         "field name starts with the struct's name",
                     );
                 }
-                if field_words.len() > item_name_words.len()
+                if field_words.len() > item_name_words.len() {
                     // lint only if the end is not covered by the start
-                    && field_words
+                    if field_words
                         .iter()
                         .rev()
                         .zip(item_name_words.iter().rev())
                         .all(|(a, b)| a == b)
-                {
-                    span_lint_hir(
-                        cx,
-                        STRUCT_FIELD_NAMES,
-                        field.hir_id,
-                        field.span,
-                        "field name ends with the struct's name",
-                    );
+                    {
+                        span_lint_hir(
+                            cx,
+                            STRUCT_FIELD_NAMES,
+                            field.hir_id,
+                            field.span,
+                            "field name ends with the struct's name",
+                        );
+                    }
                 }
             }
         }

Diff for: clippy_lints/src/mem_replace.rs

@@ -305,13 +305,14 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace {
             && let ExprKind::Path(ref func_qpath) = func.kind
             && let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id()
             && cx.tcx.is_diagnostic_item(sym::mem_replace, def_id)
-
+        {
             // Check that second argument is `Option::None`
-            && !check_replace_option_with_none(cx, src, dest, expr.span)
+            if !check_replace_option_with_none(cx, src, dest, expr.span)
                 && !check_replace_option_with_some(cx, src, dest, expr.span, &self.msrv)
                 && !check_replace_with_default(cx, src, dest, expr, &self.msrv)
-        {
-            check_replace_with_uninit(cx, src, dest, expr.span);
+            {
+                check_replace_with_uninit(cx, src, dest, expr.span);
+            }
         }
     }
     extract_msrv_attr!(LateContext);

Diff for: clippy_lints/src/methods/map_clone.rs

@@ -114,17 +114,19 @@ fn handle_path(
 ) {
     if let Some(path_def_id) = cx.qpath_res(qpath, arg.hir_id).opt_def_id()
         && cx.tcx.lang_items().get(LangItem::CloneFn) == Some(path_def_id)
+    {
         // The `copied` and `cloned` methods are only available on `&T` and `&mut T` in `Option`
         // and `Result`.
-        && let ty::Adt(_, args) = cx.typeck_results().expr_ty(recv).kind()
+        if let ty::Adt(_, args) = cx.typeck_results().expr_ty(recv).kind()
             && let args = args.as_slice()
             && let Some(ty) = args.iter().find_map(|generic_arg| generic_arg.as_type())
             && let ty::Ref(_, ty, Mutability::Not) = ty.kind()
             && let ty::FnDef(_, lst) = cx.typeck_results().expr_ty(arg).kind()
             && lst.iter().all(|l| l.as_type() == Some(*ty))
             && !should_call_clone_as_function(cx, *ty)
-    {
-        lint_path(cx, e.span, recv.span, is_copy(cx, ty.peel_refs()));
+        {
+            lint_path(cx, e.span, recv.span, is_copy(cx, ty.peel_refs()));
+        }
     }
 }
 

Diff for: clippy_lints/src/methods/seek_from_current.rs

@@ -38,11 +38,13 @@ fn arg_is_seek_from_current<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>)
         && let ExprKind::Path(ref path) = f.kind
         && let Some(ctor_call_id) = cx.qpath_res(path, f.hir_id).opt_def_id()
         && is_enum_variant_ctor(cx, sym::SeekFrom, sym!(Current), ctor_call_id)
+    {
         // check if argument of `SeekFrom::Current` is `0`
-        && let ExprKind::Lit(lit) = arg.kind
+        if let ExprKind::Lit(lit) = arg.kind
             && let LitKind::Int(Pu128(0), LitIntType::Unsuffixed) = lit.node
-    {
-        return true;
+        {
+            return true;
+        }
     }
 
     false

Diff for: clippy_lints/src/mixed_read_write_in_expression.rs

@@ -327,22 +327,22 @@ impl<'tcx> Visitor<'tcx> for ReadVisitor<'_, 'tcx> {
             return;
         }
 
-        if path_to_local_id(expr, self.var)
+        if path_to_local_id(expr, self.var) {
             // Check that this is a read, not a write.
-            && !is_in_assignment_position(self.cx, expr)
-        {
-            span_lint_and_then(
-                self.cx,
-                MIXED_READ_WRITE_IN_EXPRESSION,
-                expr.span,
-                format!("unsequenced read of `{}`", self.cx.tcx.hir().name(self.var)),
-                |diag| {
-                    diag.span_note(
-                        self.write_expr.span,
-                        "whether read occurs before this write depends on evaluation order",
-                    );
-                },
-            );
+            if !is_in_assignment_position(self.cx, expr) {
+                span_lint_and_then(
+                    self.cx,
+                    MIXED_READ_WRITE_IN_EXPRESSION,
+                    expr.span,
+                    format!("unsequenced read of `{}`", self.cx.tcx.hir().name(self.var)),
+                    |diag| {
+                        diag.span_note(
+                            self.write_expr.span,
+                            "whether read occurs before this write depends on evaluation order",
+                        );
+                    },
+                );
+            }
         }
         match expr.kind {
             // We're about to descend a closure. Since we don't know when (or

Diff for: lintcheck/ci-config/clippy.toml

@@ -0,0 +1,4 @@
+# Configuration applied when running lintcheck from the CI
+#
+# The CI will set the `CLIPPY_CONF_DIR` environment variable
+# to `$PWD/lintcheck/ci-config`.

Diff for: tests/ui-toml/collapsible_if/clippy.toml

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

Diff for: tests/ui-toml/collapsible_if/collapsible_if.fixed

@@ -1,3 +1,4 @@
+#![feature(let_chains)]
 #![allow(clippy::eq_op, clippy::nonminimal_bool)]
 
 #[rustfmt::skip]
@@ -14,4 +15,19 @@ fn main() {
             println!("Hello world!");
         }
     }
+
+    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
 }

Diff for: tests/ui-toml/collapsible_if/collapsible_if.rs

@@ -1,3 +1,4 @@
+#![feature(let_chains)]
 #![allow(clippy::eq_op, clippy::nonminimal_bool)]
 
 #[rustfmt::skip]
@@ -16,4 +17,25 @@ fn main() {
             println!("Hello world!");
         }
     }
+
+    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
 }

Diff for: 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:6:5
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:7:5
    |
 LL | /     if true {
 LL | |         if true {
@@ -17,5 +17,56 @@ LL +         println!("No comment, linted");
 LL +     }
    |
 
-error: aborting due to 1 previous error
+error: this `if` statement can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:21: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) && 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:28: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) && a + 1 == 4 {
+LL +         let _ = a;
+LL +     }
+   |
+
+error: this `if` statement can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_if.rs:35: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) && let Some(b) = Some(4) {
+LL +         let _ = b;
+LL +     }
+   |
+
+error: aborting due to 4 previous errors