src/tools/clippy/clippy_lints/src/manual_let_else.rs RUST 456 lines View on github.com → Search inside
1use crate::question_mark::{QUESTION_MARK, QuestionMark};2use clippy_config::types::MatchLintBehaviour;3use clippy_utils::diagnostics::span_lint_and_then;4use clippy_utils::higher::IfLetOrMatch;5use clippy_utils::res::{MaybeDef, MaybeQPath};6use clippy_utils::source::snippet_with_context;7use clippy_utils::{is_lint_allowed, is_never_expr, is_wild, msrvs, pat_and_expr_can_be_question_mark, peel_blocks};8use rustc_ast::BindingMode;9use rustc_data_structures::fx::FxHashMap;10use rustc_errors::Applicability;11use rustc_hir::def::{CtorOf, DefKind, Res};12use rustc_hir::{13    Arm, BlockCheckMode, Expr, ExprKind, MatchSource, Pat, PatExpr, PatExprKind, PatKind, QPath, Stmt, StmtKind,14};15use rustc_lint::{LateContext, LintContext};16use rustc_span::Span;17use rustc_span::symbol::{Symbol, sym};18use std::slice;1920declare_clippy_lint! {21    /// ### What it does22    ///23    /// Warn of cases where `let...else` could be used24    ///25    /// ### Why is this bad?26    ///27    /// `let...else` provides a standard construct for this pattern28    /// that people can easily recognize. It's also more compact.29    ///30    /// ### Example31    ///32    /// ```no_run33    /// # let w = Some(0);34    /// let v = if let Some(v) = w { v } else { return };35    /// ```36    ///37    /// Could be written:38    ///39    /// ```no_run40    /// # fn main () {41    /// # let w = Some(0);42    /// let Some(v) = w else { return };43    /// # }44    /// ```45    #[clippy::version = "1.67.0"]46    pub MANUAL_LET_ELSE,47    pedantic,48    "manual implementation of a let...else statement"49}5051impl<'tcx> QuestionMark {52    pub(crate) fn check_manual_let_else(&self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) {53        if let StmtKind::Let(local) = stmt.kind54            && let Some(init) = local.init55            && local.els.is_none()56            && local.ty.is_none()57            && init.span.eq_ctxt(stmt.span)58            && let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)59            && !stmt.span.in_external_macro(cx.sess().source_map())60            && self.msrv.meets(cx, msrvs::LET_ELSE)61        {62            match if_let_or_match {63                IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else, ..) => {64                    if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then)65                        && let Some(if_else) = if_else66                        && is_never_expr(cx, if_else).is_some()67                        && let qm_allowed = is_lint_allowed(cx, QUESTION_MARK, stmt.hir_id)68                        && (qm_allowed || pat_and_expr_can_be_question_mark(cx, let_pat, if_else).is_none())69                    {70                        emit_manual_let_else(cx, stmt.span, if_let_expr, &ident_map, let_pat, if_else);71                    }72                },73                IfLetOrMatch::Match(match_expr, arms, source) => {74                    if self.matches_behaviour == MatchLintBehaviour::Never {75                        return;76                    }77                    if source != MatchSource::Normal {78                        return;79                    }80                    // Any other number than two arms doesn't (necessarily)81                    // have a trivial mapping to let else.82                    if arms.len() != 2 {83                        return;84                    }85                    // Guards don't give us an easy mapping either86                    if arms.iter().any(|arm| arm.guard.is_some()) {87                        return;88                    }89                    let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes;90                    let diverging_arm_opt = arms.iter().enumerate().find(|(_, arm)| {91                        is_never_expr(cx, arm.body).is_some() && pat_allowed_for_else(cx, arm.pat, check_types)92                    });93                    let Some((idx, diverging_arm)) = diverging_arm_opt else {94                        return;95                    };9697                    let pat_arm = &arms[1 - idx];98                    // If the non-diverging arm is the first one, its pattern can be reused in a let/else statement.99                    // However, if it arrives in second position, its pattern may cover some cases already covered100                    // by the diverging one.101                    if idx == 0 && !is_arms_disjointed(cx, diverging_arm, pat_arm) {102                        return;103                    }104105                    let Some(ident_map) = expr_simple_identity_map(local.pat, pat_arm.pat, pat_arm.body) else {106                        return;107                    };108109                    emit_manual_let_else(cx, stmt.span, match_expr, &ident_map, pat_arm.pat, diverging_arm.body);110                },111            }112        }113    }114}115116/// Checks if the patterns of the arms are disjointed. Currently, we only support patterns of simple117/// enum variants without nested patterns or bindings.118///119/// TODO: Support more complex patterns.120fn is_arms_disjointed(cx: &LateContext<'_>, arm1: &Arm<'_>, arm2: &Arm<'_>) -> bool {121    if arm1.guard.is_some() || arm2.guard.is_some() {122        return false;123    }124125    if !is_enum_variant(cx, arm1.pat) || !is_enum_variant(cx, arm2.pat) {126        return false;127    }128129    true130}131132/// Returns `true` if the given pattern is a variant of an enum.133pub fn is_enum_variant(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {134    let path = match pat.kind {135        PatKind::Struct(ref qpath, fields, _)136            if fields137                .iter()138                .all(|field| is_wild(field.pat) || matches!(field.pat.kind, PatKind::Binding(..))) =>139        {140            (qpath, pat.hir_id)141        },142        PatKind::TupleStruct(ref qpath, pats, _)143            if pats144                .iter()145                .all(|pat| is_wild(pat) || matches!(pat.kind, PatKind::Binding(..))) =>146        {147            (qpath, pat.hir_id)148        },149        PatKind::Expr(e) if let Some((qpath, id)) = e.opt_qpath() => (qpath, id),150        _ => return false,151    };152    let res = path.res(cx);153    matches!(154        res,155        Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(CtorOf::Variant, _), _)156    )157}158159fn emit_manual_let_else(160    cx: &LateContext<'_>,161    span: Span,162    expr: &Expr<'_>,163    ident_map: &FxHashMap<Symbol, (&Pat<'_>, BindingMode)>,164    pat: &Pat<'_>,165    else_body: &Expr<'_>,166) {167    span_lint_and_then(168        cx,169        MANUAL_LET_ELSE,170        span,171        "this could be rewritten as `let...else`",172        |diag| {173            // This is far from perfect, for example there needs to be:174            // * renamings of the bindings for many `PatKind`s like slices, etc.175            // * limitations in the existing replacement algorithms176            // * unused binding collision detection with existing ones177            // for this to be machine applicable.178            let mut app = Applicability::HasPlaceholders;179            let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app);180            let (sn_else, else_is_mac_call) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app);181182            let else_bl = if let ExprKind::Block(block, None) = else_body.kind183                && matches!(block.rules, BlockCheckMode::DefaultBlock)184                && !else_is_mac_call185            {186                sn_else.into_owned()187            } else {188                format!("{{ {sn_else} }}")189            };190            let sn_bl = replace_in_pattern(cx, span, ident_map, pat, &mut app, true);191            let sugg = if sn_expr.ends_with('}') {192                // let-else statement expressions are not allowed to end with `}`193                // https://rust-lang.github.io/rfcs/3137-let-else.html#let-pattern--if--else--else-194                format!("let {sn_bl} = ({sn_expr}) else {else_bl};")195            } else {196                format!("let {sn_bl} = {sn_expr} else {else_bl};")197            };198            diag.span_suggestion(span, "consider writing", sugg, app);199        },200    );201}202203/// Replaces the locals in the pattern204///205/// For this example:206///207/// ```ignore208/// let (a, FooBar { b, c }) = if let Bar { Some(a_i), b_i } = ex { (a_i, b_i) } else { return };209/// ```210///211/// We have:212///213/// ```ignore214/// pat: Bar { Some(a_i), b_i }215/// ident_map: (a_i) -> (a), (b_i) -> (FooBar { b, c })216/// ```217///218/// We return:219///220/// ```ignore221/// Bar { Some(a), b_i: FooBar { b, c } }222/// ```223fn replace_in_pattern(224    cx: &LateContext<'_>,225    span: Span,226    ident_map: &FxHashMap<Symbol, (&Pat<'_>, BindingMode)>,227    pat: &Pat<'_>,228    app: &mut Applicability,229    top_level: bool,230) -> String {231    // We put a labeled block here so that we can implement the fallback in this function.232    // As the function has multiple call sites, implementing the fallback via an Option<T>233    // return type and unwrap_or_else would cause repetition. Similarly, the function also234    // invokes the fall back multiple times.235    'a: {236        // If the ident map is empty, there is no replacement to do.237        // The code following this if assumes a non-empty ident_map.238        if ident_map.is_empty() {239            break 'a;240        }241242        match pat.kind {243            PatKind::Binding(ann, _id, binding_name, opt_subpt) => {244                match (ident_map.get(&binding_name.name), opt_subpt) {245                    (Some((pat_to_put, binding_mode)), opt_subpt) => {246                        let sn_pfx = binding_mode.prefix_str();247                        let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app);248                        if let Some(subpt) = opt_subpt {249                            let subpt = replace_in_pattern(cx, span, ident_map, subpt, app, false);250                            return format!("{sn_pfx}{sn_ptp} @ {subpt}");251                        }252                        return format!("{sn_pfx}{sn_ptp}");253                    },254                    (None, Some(subpt)) => {255                        let subpt = replace_in_pattern(cx, span, ident_map, subpt, app, false);256                        // scanning for a value that matches is not sensitive to order257                        #[expect(rustc::potential_query_instability)]258                        if ident_map.values().any(|(other_pat, _)| {259                            if let PatKind::Binding(_, _, other_name, _) = other_pat.kind {260                                other_name == binding_name261                            } else {262                                false263                            }264                        }) {265                            // this name is shadowed, and, therefore, not usable266                            return subpt;267                        }268                        let binding_pfx = ann.prefix_str();269                        return format!("{binding_pfx}{binding_name} @ {subpt}");270                    },271                    (None, None) => break 'a,272                }273            },274            PatKind::Or(pats) => {275                let patterns = pats276                    .iter()277                    .map(|pat| replace_in_pattern(cx, span, ident_map, pat, app, false))278                    .collect::<Vec<_>>();279                let or_pat = patterns.join(" | ");280                if top_level {281                    return format!("({or_pat})");282                }283                return or_pat;284            },285            PatKind::Struct(path, fields, dot_dot) => {286                let fields = fields287                    .iter()288                    .map(|fld| {289                        if let PatKind::Binding(_, _, name, None) = fld.pat.kind290                            && let Some((pat_to_put, binding_mode)) = ident_map.get(&name.name)291                        {292                            let sn_pfx = binding_mode.prefix_str();293                            let (sn_fld_name, _) = snippet_with_context(cx, fld.ident.span, span.ctxt(), "", app);294                            let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app);295                            // TODO: this is a bit of a hack, but it does its job. Ideally, we'd check if pat_to_put is296                            // a PatKind::Binding but that is also hard to get right.297                            if sn_fld_name == sn_ptp {298                                // Field init shorthand299                                return format!("{sn_pfx}{sn_fld_name}");300                            }301                            return format!("{sn_fld_name}: {sn_pfx}{sn_ptp}");302                        }303                        let (sn_fld, _) = snippet_with_context(cx, fld.span, span.ctxt(), "", app);304                        sn_fld.into_owned()305                    })306                    .collect::<Vec<_>>();307                let fields_string = fields.join(", ");308309                let dot_dot_str = if dot_dot.is_some() { ", .." } else { "" };310                let (sn_pth, _) = snippet_with_context(cx, path.span(), span.ctxt(), "", app);311                return format!("{sn_pth} {{ {fields_string}{dot_dot_str} }}");312            },313            // Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`.314            PatKind::TupleStruct(ref w, args, dot_dot_pos) => {315                let mut args = args316                    .iter()317                    .map(|pat| replace_in_pattern(cx, span, ident_map, pat, app, false))318                    .collect::<Vec<_>>();319                if let Some(pos) = dot_dot_pos.as_opt_usize() {320                    args.insert(pos, "..".to_owned());321                }322                let args = args.join(", ");323                let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default();324                return format!("{sn_wrapper}({args})");325            },326            PatKind::Tuple(args, dot_dot_pos) => {327                let mut args = args328                    .iter()329                    .map(|pat| replace_in_pattern(cx, span, ident_map, pat, app, false))330                    .collect::<Vec<_>>();331                if let Some(pos) = dot_dot_pos.as_opt_usize() {332                    args.insert(pos, "..".to_owned());333                }334                let args = args.join(", ");335                return format!("({args})");336            },337            _ => {},338        }339    }340    let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", app);341    sn_pat.into_owned()342}343344fn pat_allowed_for_else(cx: &LateContext<'_>, pat: &'_ Pat<'_>, check_types: bool) -> bool {345    // Check whether the pattern contains any bindings, as the346    // binding might potentially be used in the body.347    // TODO: only look for *used* bindings.348    let mut has_bindings = false;349    pat.each_binding_or_first(&mut |_, _, _, _| has_bindings = true);350    if has_bindings {351        return false;352    }353354    // If we shouldn't check the types, exit early.355    if !check_types {356        return true;357    }358359    // Check whether any possibly "unknown" patterns are included,360    // because users might not know which values some enum has.361    // Well-known enums are excepted, as we assume people know them.362    // We do a deep check, to be able to disallow Err(En::Foo(_))363    // for usage of the En::Foo variant, as we disallow En::Foo(_),364    // but we allow Err(_).365    let typeck_results = cx.typeck_results();366    let mut has_disallowed = false;367    pat.walk_always(|pat| {368        // Only do the check if the type is "spelled out" in the pattern369        if !matches!(370            pat.kind,371            PatKind::Struct(..)372                | PatKind::TupleStruct(..)373                | PatKind::Expr(PatExpr {374                    kind: PatExprKind::Path(..),375                    ..376                },)377        ) {378            return;379        }380        let ty = typeck_results.pat_ty(pat);381        // Option and Result are allowed, everything else isn't.382        if !matches!(ty.opt_diag_name(cx), Some(sym::Option | sym::Result)) {383            has_disallowed = true;384        }385    });386    !has_disallowed387}388389/// Checks if the passed block is a simple identity referring to bindings created by the pattern,390/// and if yes, returns a mapping between the relevant sub-pattern and the identifier it corresponds391/// to.392///393/// We support patterns with multiple bindings and tuples, e.g.:394///395/// ```ignore396/// let (foo_o, bar_o) = if let (Some(foo), bar) = g() { (foo, bar) } else { ... }397/// ```398///399/// The expected params would be:400///401/// ```ignore402/// local_pat: (foo_o, bar_o)403/// let_pat: (Some(foo), bar)404/// expr: (foo, bar)405/// ```406///407/// We build internal `sub_pats` so that it looks like `[foo_o, bar_o]` and `paths` so that it looks408/// like `[foo, bar]`. Then we turn that into `FxHashMap [(foo) -> (foo_o), (bar) -> (bar_o)]` which409/// we return.410fn expr_simple_identity_map<'a, 'hir>(411    local_pat: &'a Pat<'hir>,412    let_pat: &'_ Pat<'hir>,413    expr: &'_ Expr<'hir>,414) -> Option<FxHashMap<Symbol, (&'a Pat<'hir>, BindingMode)>> {415    let peeled = peel_blocks(expr);416    let (sub_pats, paths) = match (local_pat.kind, peeled.kind) {417        (PatKind::Tuple(pats, _), ExprKind::Tup(exprs)) | (PatKind::Slice(pats, ..), ExprKind::Array(exprs)) => {418            (pats, exprs)419        },420        (_, ExprKind::Path(_)) => (slice::from_ref(local_pat), slice::from_ref(peeled)),421        _ => return None,422    };423424    // There is some length mismatch, which indicates usage of .. in the patterns above e.g.:425    // let (a, ..) = if let [a, b, _c] = ex { (a, b) } else { ... };426    // We bail in these cases as they should be rare.427    if paths.len() != sub_pats.len() {428        return None;429    }430431    let mut pat_bindings = FxHashMap::default();432    let_pat.each_binding_or_first(&mut |binding_mode, _hir_id, _sp, ident| {433        pat_bindings.insert(ident, binding_mode);434    });435    if pat_bindings.len() < paths.len() {436        // This rebinds some bindings from the outer scope, or it repeats some copy-able bindings multiple437        // times. We don't support these cases so we bail here. E.g.:438        // let foo = 0;439        // let (new_foo, bar, bar_copied) = if let Some(bar) = Some(0) { (foo, bar, bar) } else { .. };440        return None;441    }442    let mut ident_map = FxHashMap::default();443    for (sub_pat, path) in sub_pats.iter().zip(paths.iter()) {444        if let ExprKind::Path(QPath::Resolved(_ty, path)) = path.kind445            && let [path_seg] = path.segments446            && let ident = path_seg.ident447            && let Some(let_binding_mode) = pat_bindings.remove(&ident)448        {449            ident_map.insert(ident.name, (sub_pat, let_binding_mode));450        } else {451            return None;452        }453    }454    Some(ident_map)455}

Code quality findings 3

Warning: Direct indexing (e.g., `vec[i]`, `slice[i]`) panics on out-of-bounds access. Prefer using `.get(index)` or `.get_mut(index)` which return Option<&T>/Option<&mut T>.
warning correctness unchecked-indexing
let pat_arm = &arms[1 - idx];
Warning: Direct indexing (e.g., `vec[i]`, `slice[i]`) panics on out-of-bounds access. Prefer using `.get(index)` or `.get_mut(index)` which return Option<&T>/Option<&mut T>.
warning correctness unchecked-indexing
/// like `[foo, bar]`. Then we turn that into `FxHashMap [(foo) -> (foo_o), (bar) -> (bar_o)]` which
Info: Ensure 'match' statements are exhaustive. If matching on enums, consider adding a wildcard arm `_ => {}` only if necessary and intentional, as it suppresses warnings about unhandled variants.
info correctness match-wildcard
let (sub_pats, paths) = match (local_pat.kind, peeled.kind) {

Get this view in your editor

Same data, no extra tab — call code_get_file + code_get_findings over MCP from Claude/Cursor/Copilot.