Add lint to tell about let else pattern

CHANGELOG.md

@@ -3997,6 +3997,7 @@ Released 2018-09-13
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
 [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
+[`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
 [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
 [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive

clippy_lints/src/declared_lints.rs

@@ -251,6 +251,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::manual_bits::MANUAL_BITS_INFO,
     crate::manual_clamp::MANUAL_CLAMP_INFO,
     crate::manual_instant_elapsed::MANUAL_INSTANT_ELAPSED_INFO,
+    crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
     crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
     crate::manual_retain::MANUAL_RETAIN_INFO,

clippy_lints/src/dereference.rs

@@ -274,9 +274,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
         }
 
         let typeck = cx.typeck_results();
-        let (kind, sub_expr) = if let Some(x) = try_parse_ref_op(cx.tcx, typeck, expr) {
-            x
-        } else {
+        let Some((kind, sub_expr)) = try_parse_ref_op(cx.tcx, typeck, expr) else {
             // The whole chain of reference operations has been seen
             if let Some((state, data)) = self.state.take() {
                 report(cx, expr, state, data);

clippy_lints/src/lib.rs

@@ -170,6 +170,7 @@ mod manual_async_fn;
 mod manual_bits;
 mod manual_clamp;
 mod manual_instant_elapsed;
+mod manual_let_else;
 mod manual_non_exhaustive;
 mod manual_rem_euclid;
 mod manual_retain;
@@ -603,6 +604,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         ))
     });
     store.register_late_pass(move |_| Box::new(matches::Matches::new(msrv)));
+    let matches_for_let_else = conf.matches_for_let_else;
+    store.register_late_pass(move |_| Box::new(manual_let_else::ManualLetElse::new(msrv, matches_for_let_else)));
     store.register_early_pass(move || Box::new(manual_non_exhaustive::ManualNonExhaustiveStruct::new(msrv)));
     store.register_late_pass(move |_| Box::new(manual_non_exhaustive::ManualNonExhaustiveEnum::new(msrv)));
     store.register_late_pass(move |_| Box::new(manual_strip::ManualStrip::new(msrv)));

clippy_lints/src/manual_let_else.rs

@@ -0,0 +1,297 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::higher::IfLetOrMatch;
+use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::visitors::{for_each_expr, Descend};
+use clippy_utils::{meets_msrv, msrvs, peel_blocks};
+use if_chain::if_chain;
+use rustc_data_structures::fx::FxHashSet;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, Stmt, StmtKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_semver::RustcVersion;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::symbol::sym;
+use rustc_span::Span;
+use serde::Deserialize;
+use std::ops::ControlFlow;
+
+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
+    ///
+    /// ```rust
+    /// # let w = Some(0);
+    /// let v = if let Some(v) = w { v } else { return };
+    /// ```
+    ///
+    /// Could be written:
+    ///
+    /// ```rust
+    /// # #![feature(let_else)]
+    /// # 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"
+}
+
+pub struct ManualLetElse {
+    msrv: Option<RustcVersion>,
+    matches_behaviour: MatchLintBehaviour,
+}
+
+impl ManualLetElse {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>, matches_behaviour: MatchLintBehaviour) -> Self {
+        Self {
+            msrv,
+            matches_behaviour,
+        }
+    }
+}
+
+impl_lint_pass!(ManualLetElse => [MANUAL_LET_ELSE]);
+
+impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
+    fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) {
+        let if_let_or_match = if_chain! {
+            if meets_msrv(self.msrv, msrvs::LET_ELSE);
+            if !in_external_macro(cx.sess(), stmt.span);
+            if let StmtKind::Local(local) = stmt.kind;
+            if let Some(init) = local.init;
+            if local.els.is_none();
+            if local.ty.is_none();
+            if init.span.ctxt() == stmt.span.ctxt();
+            if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init);
+            then {
+                if_let_or_match
+            } else {
+                return;
+            }
+        };
+
+        match if_let_or_match {
+            IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! {
+                if expr_is_simple_identity(let_pat, if_then);
+                if let Some(if_else) = if_else;
+                if expr_diverges(cx, if_else);
+                then {
+                    emit_manual_let_else(cx, stmt.span, if_let_expr, 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 (neccessarily)
+                // 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)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types));
+                let Some((idx, diverging_arm)) = diverging_arm_opt else { return; };
+                let pat_arm = &arms[1 - idx];
+                if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) {
+                    return;
+                }
+
+                emit_manual_let_else(cx, stmt.span, match_expr, pat_arm.pat, diverging_arm.body);
+            },
+        }
+    }
+
+    extract_msrv_attr!(LateContext);
+}
+
+fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, 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:
+            // * mut additions for the bindings
+            // * renamings of the bindings
+            // * unused binding collision detection with existing ones
+            // * putting patterns with at the top level | inside ()
+            // for this to be machine applicable.
+            let app = Applicability::HasPlaceholders;
+
+            if let Some(sn_pat) = snippet_opt(cx, pat.span) &&
+                let Some(sn_expr) = snippet_opt(cx, expr.span) &&
+                let Some(sn_else) = snippet_opt(cx, else_body.span)
+            {
+                let else_bl = if matches!(else_body.kind, ExprKind::Block(..)) {
+                    sn_else
+                } else {
+                    format!("{{ {sn_else} }}")
+                };
+                let sugg = format!("let {sn_pat} = {sn_expr} else {else_bl};");
+                diag.span_suggestion(span, "consider writing", sugg, app);
+            }
+        },
+    );
+}
+
+fn expr_diverges(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
+    fn is_never(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
+        if let Some(ty) = cx.typeck_results().expr_ty_opt(expr) {
+            return ty.is_never();
+        }
+        false
+    }
+    // We can't just call is_never on expr and be done, because the type system
+    // sometimes coerces the ! type to something different before we can get
+    // our hands on it. So instead, we do a manual search. We do fall back to
+    // is_never in some places when there is no better alternative.
+    for_each_expr(expr, |ex| {
+        match ex.kind {
+            ExprKind::Continue(_) | ExprKind::Break(_, _) | ExprKind::Ret(_) => ControlFlow::Break(()),
+            ExprKind::Call(call, _) => {
+                if is_never(cx, ex) || is_never(cx, call) {
+                    return ControlFlow::Break(());
+                }
+                ControlFlow::Continue(Descend::Yes)
+            },
+            ExprKind::MethodCall(..) => {
+                if is_never(cx, ex) {
+                    return ControlFlow::Break(());
+                }
+                ControlFlow::Continue(Descend::Yes)
+            },
+            ExprKind::If(if_expr, if_then, if_else) => {
+                let else_diverges = if_else.map_or(false, |ex| expr_diverges(cx, ex));
+                let diverges = expr_diverges(cx, if_expr) || (else_diverges && expr_diverges(cx, if_then));
+                if diverges {
+                    return ControlFlow::Break(());
+                }
+                ControlFlow::Continue(Descend::No)
+            },
+            ExprKind::Match(match_expr, match_arms, _) => {
+                let diverges = expr_diverges(cx, match_expr)
+                    || match_arms.iter().all(|arm| {
+                        let guard_diverges = arm.guard.as_ref().map_or(false, |g| expr_diverges(cx, g.body()));
+                        guard_diverges || expr_diverges(cx, arm.body)
+                    });
+                if diverges {
+                    return ControlFlow::Break(());
+                }
+                ControlFlow::Continue(Descend::No)
+            },
+
+            // Don't continue into loops or labeled blocks, as they are breakable,
+            // and we'd have to start checking labels.
+            ExprKind::Block(_, Some(_)) | ExprKind::Loop(..) => ControlFlow::Continue(Descend::No),
+
+            // Default: descend
+            _ => ControlFlow::Continue(Descend::Yes),
+        }
+    })
+    .is_some()
+}
+
+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
+fn expr_is_simple_identity(pat: &'_ Pat<'_>, expr: &'_ Expr<'_>) -> bool {
+    // We support patterns with multiple bindings and tuples, like:
+    //   let ... = if let (Some(foo), bar) = g() { (foo, bar) } else { ... }
+    let peeled = peel_blocks(expr);
+    let paths = match peeled.kind {
+        ExprKind::Tup(exprs) | ExprKind::Array(exprs) => exprs,
+        ExprKind::Path(_) => std::slice::from_ref(peeled),
+        _ => return false,
+    };
+    let mut pat_bindings = FxHashSet::default();
+    pat.each_binding_or_first(&mut |_ann, _hir_id, _sp, ident| {
+        pat_bindings.insert(ident);
+    });
+    if pat_bindings.len() < paths.len() {
+        return false;
+    }
+    for path in paths {
+        if_chain! {
+            if let ExprKind::Path(QPath::Resolved(_ty, path)) = path.kind;
+            if let [path_seg] = path.segments;
+            then {
+                if !pat_bindings.remove(&path_seg.ident) {
+                    return false;
+                }
+            } else {
+                return false;
+            }
+        }
+    }
+    true
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Deserialize)]
+pub enum MatchLintBehaviour {
+    AllTypes,
+    WellKnownTypes,
+    Never,
+}

clippy_lints/src/utils/conf.rs

@@ -213,7 +213,7 @@ define_Conf! {
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.
     (avoid_breaking_exported_api: bool = true),
-    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP.
+    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE.
     ///
     /// The minimum rust version that the project supports
     (msrv: Option<String> = None),
@@ -335,6 +335,12 @@ define_Conf! {
     ///
     /// Enables verbose mode. Triggers if there is more than one uppercase char next to each other
     (upper_case_acronyms_aggressive: bool = false),
+    /// Lint: MANUAL_LET_ELSE.
+    ///
+    /// Whether the matches should be considered by the lint, and whether there should
+    /// be filtering for common types.
+    (matches_for_let_else: crate::manual_let_else::MatchLintBehaviour =
+        crate::manual_let_else::MatchLintBehaviour::WellKnownTypes),
     /// Lint: _CARGO_COMMON_METADATA.
     ///
     /// For internal testing only, ignores the current `publish` settings in the Cargo manifest.

clippy_utils/src/consts.rs

@@ -147,9 +147,8 @@ impl Constant {
                 _ => None,
             },
             (&Self::Vec(ref l), &Self::Vec(ref r)) => {
-                let cmp_type = match *cmp_type.kind() {
-                    ty::Array(ty, _) | ty::Slice(ty) => ty,
-                    _ => return None,
+                let (ty::Array(cmp_type, _) | ty::Slice(cmp_type)) = *cmp_type.kind() else {
+                    return None
                 };
                 iter::zip(l, r)
                     .map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri))
@@ -401,10 +400,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
         use self::Constant::{Int, F32, F64};
         match *o {
             Int(value) => {
-                let ity = match *ty.kind() {
-                    ty::Int(ity) => ity,
-                    _ => return None,
-                };
+                let ty::Int(ity) = *ty.kind() else { return None };
                 // sign extend
                 let value = sext(self.lcx.tcx, value, ity);
                 let value = value.checked_neg()?;