Auto merge of #12440 - GuillaumeGomez:add-match_option_and_default, r…

…=llogiq

Add new `manual_unwrap_or_default` lint

This adds a new lint checking if a `match` or a `if let` can be replaced with `unwrap_or_default`.

----

changelog: Add new [`manual_unwrap_or_default`] lint

CHANGELOG.md

@@ -5377,6 +5377,7 @@ Released 2018-09-13
 [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
 [`manual_try_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold
 [`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
+[`manual_unwrap_or_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
 [`manual_while_let_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_while_let_some
 [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
 [`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone

clippy_lints/src/declared_lints.rs

@@ -310,6 +310,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
     crate::manual_string_new::MANUAL_STRING_NEW_INFO,
     crate::manual_strip::MANUAL_STRIP_INFO,
+    crate::manual_unwrap_or_default::MANUAL_UNWRAP_OR_DEFAULT_INFO,
     crate::map_unit_fn::OPTION_MAP_UNIT_FN_INFO,
     crate::map_unit_fn::RESULT_MAP_UNIT_FN_INFO,
     crate::match_result_ok::MATCH_RESULT_OK_INFO,

clippy_lints/src/lib.rs

@@ -211,6 +211,7 @@ mod manual_retain;
 mod manual_slice_size_calculation;
 mod manual_string_new;
 mod manual_strip;
+mod manual_unwrap_or_default;
 mod map_unit_fn;
 mod match_result_ok;
 mod matches;
@@ -1122,6 +1123,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
     store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
     store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
+    store.register_late_pass(|_| Box::new(manual_unwrap_or_default::ManualUnwrapOrDefault));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/manual_unwrap_or_default.rs

@@ -0,0 +1,181 @@
+use rustc_errors::Applicability;
+use rustc_hir::def::Res;
+use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, MatchSource, Pat, PatKind, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_default_equivalent;
+use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::implements_trait;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks if a `match` or `if let` expression can be simplified using
+    /// `.unwrap_or_default()`.
+    ///
+    /// ### Why is this bad?
+    /// It can be done in one call with `.unwrap_or_default()`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let x: Option<String> = Some(String::new());
+    /// let y: String = match x {
+    ///     Some(v) => v,
+    ///     None => String::new(),
+    /// };
+    ///
+    /// let x: Option<Vec<String>> = Some(Vec::new());
+    /// let y: Vec<String> = if let Some(v) = x {
+    ///     v
+    /// } else {
+    ///     Vec::new()
+    /// };
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let x: Option<String> = Some(String::new());
+    /// let y: String = x.unwrap_or_default();
+    ///
+    /// let x: Option<Vec<String>> = Some(Vec::new());
+    /// let y: Vec<String> = x.unwrap_or_default();
+    /// ```
+    #[clippy::version = "1.78.0"]
+    pub MANUAL_UNWRAP_OR_DEFAULT,
+    suspicious,
+    "check if a `match` or `if let` can be simplified with `unwrap_or_default`"
+}
+
+declare_lint_pass!(ManualUnwrapOrDefault => [MANUAL_UNWRAP_OR_DEFAULT]);
+
+fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> {
+    if let PatKind::TupleStruct(QPath::Resolved(_, path), &[pat], _) = pat.kind
+        && let Some(def_id) = path.res.opt_def_id()
+        // Since it comes from a pattern binding, we need to get the parent to actually match
+        // against it.
+        && let Some(def_id) = cx.tcx.opt_parent(def_id)
+        && cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id)
+    {
+        let mut bindings = Vec::new();
+        pat.each_binding(|_, id, _, _| bindings.push(id));
+        if let &[id] = bindings.as_slice() {
+            Some(id)
+        } else {
+            None
+        }
+    } else {
+        None
+    }
+}
+
+fn get_none<'tcx>(cx: &LateContext<'tcx>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    if let PatKind::Path(QPath::Resolved(_, path)) = arm.pat.kind
+        && let Some(def_id) = path.res.opt_def_id()
+        // Since it comes from a pattern binding, we need to get the parent to actually match
+        // against it.
+        && let Some(def_id) = cx.tcx.opt_parent(def_id)
+        && cx.tcx.lang_items().get(LangItem::OptionNone) == Some(def_id)
+    {
+        Some(arm.body)
+    } else if let PatKind::Wild = arm.pat.kind {
+        // We consider that the `Some` check will filter it out if it's not right.
+        Some(arm.body)
+    } else {
+        None
+    }
+}
+
+fn get_some_and_none_bodies<'tcx>(
+    cx: &LateContext<'tcx>,
+    arm1: &'tcx Arm<'tcx>,
+    arm2: &'tcx Arm<'tcx>,
+) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
+    if let Some(binding_id) = get_some(cx, arm1.pat)
+        && let Some(body_none) = get_none(cx, arm2)
+    {
+        Some(((arm1.body, binding_id), body_none))
+    } else if let Some(binding_id) = get_some(cx, arm2.pat)
+        && let Some(body_none) = get_none(cx, arm1)
+    {
+        Some(((arm2.body, binding_id), body_none))
+    } else {
+        None
+    }
+}
+
+fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
+    let ExprKind::Match(match_expr, [arm1, arm2], MatchSource::Normal | MatchSource::ForLoopDesugar) = expr.kind else {
+        return false;
+    };
+    // We don't want conditions on the arms to simplify things.
+    if arm1.guard.is_none()
+        && arm2.guard.is_none()
+        // We check that the returned type implements the `Default` trait.
+        && let match_ty = cx.typeck_results().expr_ty(expr)
+        && let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
+        && implements_trait(cx, match_ty, default_trait_id, &[])
+        // We now get the bodies for both the `Some` and `None` arms.
+        && let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2)
+        // We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
+        && let ExprKind::Path(QPath::Resolved(_, path)) = body_some.peel_blocks().kind
+        && let Res::Local(local_id) = path.res
+        && local_id == binding_id
+        // We now check the `None` arm is calling a method equivalent to `Default::default`.
+        && let body_none = body_none.peel_blocks()
+        && is_default_equivalent(cx, body_none)
+        && let Some(match_expr_snippet) = snippet_opt(cx, match_expr.span)
+    {
+        span_lint_and_sugg(
+            cx,
+            MANUAL_UNWRAP_OR_DEFAULT,
+            expr.span,
+            "match can be simplified with `.unwrap_or_default()`",
+            "replace it with",
+            format!("{match_expr_snippet}.unwrap_or_default()"),
+            Applicability::MachineApplicable,
+        );
+    }
+    true
+}
+
+fn handle_if_let<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+    if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind
+        && let ExprKind::Let(let_) = cond.kind
+        && let ExprKind::Block(_, _) = else_expr.kind
+        // We check that the returned type implements the `Default` trait.
+        && let match_ty = cx.typeck_results().expr_ty(expr)
+        && let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
+        && implements_trait(cx, match_ty, default_trait_id, &[])
+        && let Some(binding_id) = get_some(cx, let_.pat)
+        // We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
+        && let ExprKind::Path(QPath::Resolved(_, path)) = if_block.peel_blocks().kind
+        && let Res::Local(local_id) = path.res
+        && local_id == binding_id
+        // We now check the `None` arm is calling a method equivalent to `Default::default`.
+        && let body_else = else_expr.peel_blocks()
+        && is_default_equivalent(cx, body_else)
+        && let Some(if_let_expr_snippet) = snippet_opt(cx, let_.init.span)
+    {
+        span_lint_and_sugg(
+            cx,
+            MANUAL_UNWRAP_OR_DEFAULT,
+            expr.span,
+            "if let can be simplified with `.unwrap_or_default()`",
+            "replace it with",
+            format!("{if_let_expr_snippet}.unwrap_or_default()"),
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOrDefault {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if expr.span.from_expansion() {
+            return;
+        }
+        if !handle_match(cx, expr) {
+            handle_if_let(cx, expr);
+        }
+    }
+}

tests/ui/manual_let_else.rs

@@ -8,7 +8,8 @@
     clippy::never_loop,
     clippy::needless_if,
     clippy::diverging_sub_expression,
-    clippy::single_match
+    clippy::single_match,
+    clippy::manual_unwrap_or_default
 )]
 #![warn(clippy::manual_let_else)]
 //@no-rustfix