use crate::question_mark::{QuestionMark, QUESTION_MARK}; use clippy_config::msrvs; use clippy_config::types::MatchLintBehaviour; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::IfLetOrMatch; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{is_lint_allowed, is_never_expr, pat_and_expr_can_be_question_mark, peel_blocks}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; use std::slice; declare_clippy_lint! { /// ### What it does /// /// Warn of cases where `let...else` could be used /// /// ### Why is this bad? /// /// `let...else` provides a standard construct for this pattern /// that people can easily recognize. It's also more compact. /// /// ### Example /// /// ```no_run /// # let w = Some(0); /// let v = if let Some(v) = w { v } else { return }; /// ``` /// /// Could be written: /// /// ```no_run /// # fn main () { /// # let w = Some(0); /// let Some(v) = w else { return }; /// # } /// ``` #[clippy::version = "1.67.0"] pub MANUAL_LET_ELSE, pedantic, "manual implementation of a let...else statement" } impl<'tcx> QuestionMark { pub(crate) fn check_manual_let_else(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) { if let StmtKind::Let(local) = stmt.kind && let Some(init) = local.init && local.els.is_none() && local.ty.is_none() && init.span.eq_ctxt(stmt.span) && let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) && self.msrv.meets(msrvs::LET_ELSE) && !in_external_macro(cx.sess(), stmt.span) { match if_let_or_match { IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else, ..) => { if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then) && let Some(if_else) = if_else && is_never_expr(cx, if_else).is_some() && let qm_allowed = is_lint_allowed(cx, QUESTION_MARK, stmt.hir_id) && (qm_allowed || pat_and_expr_can_be_question_mark(cx, let_pat, if_else).is_none()) { emit_manual_let_else(cx, stmt.span, if_let_expr, &ident_map, let_pat, if_else); } }, IfLetOrMatch::Match(match_expr, arms, source) => { if self.matches_behaviour == MatchLintBehaviour::Never { return; } if source != MatchSource::Normal { return; } // Any other number than two arms doesn't (necessarily) // have a trivial mapping to let else. if arms.len() != 2 { return; } // Guards don't give us an easy mapping either if arms.iter().any(|arm| arm.guard.is_some()) { return; } let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes; let diverging_arm_opt = arms.iter().enumerate().find(|(_, arm)| { is_never_expr(cx, arm.body).is_some() && pat_allowed_for_else(cx, arm.pat, check_types) }); let Some((idx, diverging_arm)) = diverging_arm_opt else { return; }; // If the non-diverging arm is the first one, its pattern can be reused in a let/else statement. // However, if it arrives in second position, its pattern may cover some cases already covered // by the diverging one. // TODO: accept the non-diverging arm as a second position if patterns are disjointed. if idx == 0 { return; } let pat_arm = &arms[1 - idx]; let Some(ident_map) = expr_simple_identity_map(local.pat, pat_arm.pat, pat_arm.body) else { return; }; emit_manual_let_else(cx, stmt.span, match_expr, &ident_map, pat_arm.pat, diverging_arm.body); }, } }; } } fn emit_manual_let_else( cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, ident_map: &FxHashMap>, pat: &Pat<'_>, else_body: &Expr<'_>, ) { span_lint_and_then( cx, MANUAL_LET_ELSE, span, "this could be rewritten as `let...else`", |diag| { // This is far from perfect, for example there needs to be: // * renamings of the bindings for many `PatKind`s like slices, etc. // * limitations in the existing replacement algorithms // * unused binding collision detection with existing ones // for this to be machine applicable. let mut app = Applicability::HasPlaceholders; let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app); let (sn_else, else_is_mac_call) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app); let else_bl = if matches!(else_body.kind, ExprKind::Block(..)) && !else_is_mac_call { sn_else.into_owned() } else { format!("{{ {sn_else} }}") }; let sn_bl = replace_in_pattern(cx, span, ident_map, pat, &mut app, true); let sugg = format!("let {sn_bl} = {sn_expr} else {else_bl};"); diag.span_suggestion(span, "consider writing", sugg, app); }, ); } /// Replaces the locals in the pattern /// /// For this example: /// /// ```ignore /// let (a, FooBar { b, c }) = if let Bar { Some(a_i), b_i } = ex { (a_i, b_i) } else { return }; /// ``` /// /// We have: /// /// ```ignore /// pat: Bar { Some(a_i), b_i } /// ident_map: (a_i) -> (a), (b_i) -> (FooBar { b, c }) /// ``` /// /// We return: /// /// ```ignore /// Bar { Some(a), b_i: FooBar { b, c } } /// ``` fn replace_in_pattern( cx: &LateContext<'_>, span: Span, ident_map: &FxHashMap>, pat: &Pat<'_>, app: &mut Applicability, top_level: bool, ) -> String { // We put a labeled block here so that we can implement the fallback in this function. // As the function has multiple call sites, implementing the fallback via an Option // return type and unwrap_or_else would cause repetition. Similarly, the function also // invokes the fall back multiple times. 'a: { // If the ident map is empty, there is no replacement to do. // The code following this if assumes a non-empty ident_map. if ident_map.is_empty() { break 'a; } match pat.kind { PatKind::Binding(_ann, _id, binding_name, opt_subpt) => { let Some(pat_to_put) = ident_map.get(&binding_name.name) else { break 'a; }; let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app); if let Some(subpt) = opt_subpt { let subpt = replace_in_pattern(cx, span, ident_map, subpt, app, false); return format!("{sn_ptp} @ {subpt}"); } return sn_ptp.to_string(); }, PatKind::Or(pats) => { let patterns = pats .iter() .map(|pat| replace_in_pattern(cx, span, ident_map, pat, app, false)) .collect::>(); let or_pat = patterns.join(" | "); if top_level { return format!("({or_pat})"); } return or_pat; }, PatKind::Struct(path, fields, has_dot_dot) => { let fields = fields .iter() .map(|fld| { if let PatKind::Binding(_, _, name, None) = fld.pat.kind && let Some(pat_to_put) = ident_map.get(&name.name) { let (sn_fld_name, _) = snippet_with_context(cx, fld.ident.span, span.ctxt(), "", app); let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app); // TODO: this is a bit of a hack, but it does its job. Ideally, we'd check if pat_to_put is // a PatKind::Binding but that is also hard to get right. if sn_fld_name == sn_ptp { // Field init shorthand return format!("{sn_fld_name}"); } return format!("{sn_fld_name}: {sn_ptp}"); } let (sn_fld, _) = snippet_with_context(cx, fld.span, span.ctxt(), "", app); sn_fld.into_owned() }) .collect::>(); let fields_string = fields.join(", "); let dot_dot_str = if has_dot_dot { " .." } else { "" }; let (sn_pth, _) = snippet_with_context(cx, path.span(), span.ctxt(), "", app); return format!("{sn_pth} {{ {fields_string}{dot_dot_str} }}"); }, // Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`. PatKind::TupleStruct(ref w, args, dot_dot_pos) => { let mut args = args .iter() .map(|pat| replace_in_pattern(cx, span, ident_map, pat, app, false)) .collect::>(); if let Some(pos) = dot_dot_pos.as_opt_usize() { args.insert(pos, "..".to_owned()); } let args = args.join(", "); let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default(); return format!("{sn_wrapper}({args})"); }, PatKind::Tuple(args, dot_dot_pos) => { let mut args = args .iter() .map(|pat| replace_in_pattern(cx, span, ident_map, pat, app, false)) .collect::>(); if let Some(pos) = dot_dot_pos.as_opt_usize() { args.insert(pos, "..".to_owned()); } let args = args.join(", "); return format!("({args})"); }, _ => {}, } } let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", app); sn_pat.into_owned() } fn pat_allowed_for_else(cx: &LateContext<'_>, pat: &'_ Pat<'_>, check_types: bool) -> bool { // Check whether the pattern contains any bindings, as the // binding might potentially be used in the body. // TODO: only look for *used* bindings. let mut has_bindings = false; pat.each_binding_or_first(&mut |_, _, _, _| has_bindings = true); if has_bindings { return false; } // If we shouldn't check the types, exit early. if !check_types { return true; } // Check whether any possibly "unknown" patterns are included, // because users might not know which values some enum has. // Well-known enums are excepted, as we assume people know them. // We do a deep check, to be able to disallow Err(En::Foo(_)) // for usage of the En::Foo variant, as we disallow En::Foo(_), // but we allow Err(_). let typeck_results = cx.typeck_results(); let mut has_disallowed = false; pat.walk_always(|pat| { // Only do the check if the type is "spelled out" in the pattern if !matches!( pat.kind, PatKind::Struct(..) | PatKind::TupleStruct(..) | PatKind::Path(..) ) { return; }; let ty = typeck_results.pat_ty(pat); // Option and Result are allowed, everything else isn't. if !(is_type_diagnostic_item(cx, ty, sym::Option) || is_type_diagnostic_item(cx, ty, sym::Result)) { has_disallowed = true; } }); !has_disallowed } /// Checks if the passed block is a simple identity referring to bindings created by the pattern, /// and if yes, returns a mapping between the relevant sub-pattern and the identifier it corresponds /// to. /// /// We support patterns with multiple bindings and tuples, e.g.: /// /// ```ignore /// let (foo_o, bar_o) = if let (Some(foo), bar) = g() { (foo, bar) } else { ... } /// ``` /// /// The expected params would be: /// /// ```ignore /// local_pat: (foo_o, bar_o) /// let_pat: (Some(foo), bar) /// expr: (foo, bar) /// ``` /// /// We build internal `sub_pats` so that it looks like `[foo_o, bar_o]` and `paths` so that it looks /// like `[foo, bar]`. Then we turn that into `FxHashMap [(foo) -> (foo_o), (bar) -> (bar_o)]` which /// we return. fn expr_simple_identity_map<'a, 'hir>( local_pat: &'a Pat<'hir>, let_pat: &'_ Pat<'hir>, expr: &'_ Expr<'hir>, ) -> Option>> { let peeled = peel_blocks(expr); let (sub_pats, paths) = match (local_pat.kind, peeled.kind) { (PatKind::Tuple(pats, _), ExprKind::Tup(exprs)) | (PatKind::Slice(pats, ..), ExprKind::Array(exprs)) => { (pats, exprs) }, (_, ExprKind::Path(_)) => (slice::from_ref(local_pat), slice::from_ref(peeled)), _ => return None, }; // There is some length mismatch, which indicates usage of .. in the patterns above e.g.: // let (a, ..) = if let [a, b, _c] = ex { (a, b) } else { ... }; // We bail in these cases as they should be rare. if paths.len() != sub_pats.len() { return None; } let mut pat_bindings = FxHashSet::default(); let_pat.each_binding_or_first(&mut |_ann, _hir_id, _sp, ident| { pat_bindings.insert(ident); }); if pat_bindings.len() < paths.len() { // This rebinds some bindings from the outer scope, or it repeats some copy-able bindings multiple // times. We don't support these cases so we bail here. E.g.: // let foo = 0; // let (new_foo, bar, bar_copied) = if let Some(bar) = Some(0) { (foo, bar, bar) } else { .. }; return None; } let mut ident_map = FxHashMap::default(); for (sub_pat, path) in sub_pats.iter().zip(paths.iter()) { if let ExprKind::Path(QPath::Resolved(_ty, path)) = path.kind && let [path_seg] = path.segments { let ident = path_seg.ident; if !pat_bindings.remove(&ident) { return None; } ident_map.insert(ident.name, sub_pat); } else { return None; } } Some(ident_map) }