fix: useless_let_if_seq FN when if is in the last expr of block

Author: profetia

clippy_lints/src/let_if_seq.rs

@@ -59,80 +59,91 @@ declare_lint_pass!(LetIfSeq => [USELESS_LET_IF_SEQ]);
 impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
     fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
         for [stmt, next] in block.stmts.array_windows::<2>() {
-            if let hir::StmtKind::Let(local) = stmt.kind
-                && let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind
-                && let hir::StmtKind::Expr(if_) = next.kind
-                && let hir::ExprKind::If(cond, then, else_) = if_.kind
-                && !is_local_used(cx, cond, canonical_id)
-                && let hir::ExprKind::Block(then, _) = then.kind
-                && let Some(value) = check_assign(cx, canonical_id, then)
-                && !is_local_used(cx, value, canonical_id)
-            {
-                let span = stmt.span.to(if_.span);
+            if let hir::StmtKind::Expr(if_) = next.kind {
+                check_block_inner(cx, stmt, if_);
+            }
+        }
 
-                let has_interior_mutability = !cx
-                    .typeck_results()
-                    .node_type(canonical_id)
-                    .is_freeze(cx.tcx, cx.typing_env());
-                if has_interior_mutability {
-                    return;
-                }
+        if let Some(expr) = block.expr
+            && let Some(stmt) = block.stmts.last()
+        {
+            check_block_inner(cx, stmt, expr);
+        }
+    }
+}
+
+fn check_block_inner<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>, if_: &'tcx hir::Expr<'tcx>) {
+    if let hir::StmtKind::Let(local) = stmt.kind
+        && let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind
+        && let hir::ExprKind::If(cond, then, else_) = if_.kind
+        && !is_local_used(cx, cond, canonical_id)
+        && let hir::ExprKind::Block(then, _) = then.kind
+        && let Some(value) = check_assign(cx, canonical_id, then)
+        && !is_local_used(cx, value, canonical_id)
+    {
+        let span = stmt.span.to(if_.span);
 
-                let (default_multi_stmts, default) = if let Some(else_) = else_ {
-                    if let hir::ExprKind::Block(else_, _) = else_.kind {
-                        if let Some(default) = check_assign(cx, canonical_id, else_) {
-                            (else_.stmts.len() > 1, default)
-                        } else if let Some(default) = local.init {
-                            (true, default)
-                        } else {
-                            continue;
-                        }
-                    } else {
-                        continue;
-                    }
+        let has_interior_mutability = !cx
+            .typeck_results()
+            .node_type(canonical_id)
+            .is_freeze(cx.tcx, cx.typing_env());
+        if has_interior_mutability {
+            return;
+        }
+
+        let (default_multi_stmts, default) = if let Some(else_) = else_ {
+            if let hir::ExprKind::Block(else_, _) = else_.kind {
+                if let Some(default) = check_assign(cx, canonical_id, else_) {
+                    (else_.stmts.len() > 1, default)
                 } else if let Some(default) = local.init {
-                    (false, default)
+                    (true, default)
                 } else {
-                    continue;
-                };
+                    return;
+                }
+            } else {
+                return;
+            }
+        } else if let Some(default) = local.init {
+            (false, default)
+        } else {
+            return;
+        };
 
-                let mutability = match mode {
-                    BindingMode(_, Mutability::Mut) => "<mut> ",
-                    _ => "",
-                };
+        let mutability = match mode {
+            BindingMode(_, Mutability::Mut) => "<mut> ",
+            _ => "",
+        };
 
-                // FIXME: this should not suggest `mut` if we can detect that the variable is not
-                // use mutably after the `if`
+        // FIXME: this should not suggest `mut` if we can detect that the variable is not
+        // use mutably after the `if`
 
-                let sug = format!(
-                    "let {mutability}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};",
-                    name=ident.name,
-                    cond=snippet(cx, cond.span, "_"),
-                    then=if then.stmts.len() > 1 { " ..;" } else { "" },
-                    else=if default_multi_stmts { " ..;" } else { "" },
-                    value=snippet(cx, value.span, "<value>"),
-                    default=snippet(cx, default.span, "<default>"),
-                );
-                span_lint_hir_and_then(
-                    cx,
-                    USELESS_LET_IF_SEQ,
-                    local.hir_id,
+        let sug = format!(
+            "let {mutability}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};",
+            name=ident.name,
+            cond=snippet(cx, cond.span, "_"),
+            then=if then.stmts.len() > 1 { " ..;" } else { "" },
+            else=if default_multi_stmts { " ..;" } else { "" },
+            value=snippet(cx, value.span, "<value>"),
+            default=snippet(cx, default.span, "<default>"),
+        );
+        span_lint_hir_and_then(
+            cx,
+            USELESS_LET_IF_SEQ,
+            local.hir_id,
+            span,
+            "`if _ { .. } else { .. }` is an expression",
+            |diag| {
+                diag.span_suggestion(
                     span,
-                    "`if _ { .. } else { .. }` is an expression",
-                    |diag| {
-                        diag.span_suggestion(
-                            span,
-                            "it is more idiomatic to write",
-                            sug,
-                            Applicability::HasPlaceholders,
-                        );
-                        if !mutability.is_empty() {
-                            diag.note("you might not need `mut` at all");
-                        }
-                    },
+                    "it is more idiomatic to write",
+                    sug,
+                    Applicability::HasPlaceholders,
                 );
-            }
-        }
+                if !mutability.is_empty() {
+                    diag.note("you might not need `mut` at all");
+                }
+            },
+        );
     }
 }
 

tests/ui/let_if_seq.rs

@@ -139,3 +139,13 @@ fn main() {
     }
     println!("{}", val.get());
 }
+
+fn issue16062(bar: fn() -> bool) {
+    let foo;
+    //~^ useless_let_if_seq
+    if bar() {
+        foo = 42;
+    } else {
+        foo = 0;
+    }
+}

tests/ui/let_if_seq.stderr

@@ -52,5 +52,17 @@ LL | |     }
    |
    = note: you might not need `mut` at all
 
-error: aborting due to 4 previous errors
+error: `if _ { .. } else { .. }` is an expression
+  --> tests/ui/let_if_seq.rs:144:5
+   |
+LL | /     let foo;
+LL | |
+LL | |     if bar() {
+LL | |         foo = 42;
+LL | |     } else {
+LL | |         foo = 0;
+LL | |     }
+   | |_____^ help: it is more idiomatic to write: `let foo = if bar() { 42 } else { 0 };`
+
+error: aborting due to 5 previous errors