Add lint unnecessary_option_map_or_else (#14662)

changelog: [`unnecessary_option_map_or_else`]: Added lint
unnecessary_option_map_or_else. As suggested in the issue description,
the implementation takes as reference the issue
rust-lang/rust-clippy#7328. The tests for lints `option_if_let_else` and
`or_fun_call` needed to be adjusted to comply with new lint.
fixes rust-lang/rust-clippy#14588

Author: llogiq

CHANGELOG.md

@@ -6897,6 +6897,7 @@ Released 2018-09-13
 [`unnecessary_min_or_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_or_max
 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
+[`unnecessary_option_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_option_map_or_else
 [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
 [`unnecessary_result_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_result_map_or_else
 [`unnecessary_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment

clippy_lints/src/declared_lints.rs

@@ -488,6 +488,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
     crate::methods::UNNECESSARY_MAP_OR_INFO,
     crate::methods::UNNECESSARY_MIN_OR_MAX_INFO,
+    crate::methods::UNNECESSARY_OPTION_MAP_OR_ELSE_INFO,
     crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO,
     crate::methods::UNNECESSARY_SORT_BY_INFO,
     crate::methods::UNNECESSARY_TO_OWNED_INFO,

clippy_lints/src/methods/mod.rs

@@ -133,6 +133,7 @@ mod unnecessary_lazy_eval;
 mod unnecessary_literal_unwrap;
 mod unnecessary_map_or;
 mod unnecessary_min_or_max;
+mod unnecessary_option_map_or_else;
 mod unnecessary_result_map_or_else;
 mod unnecessary_sort_by;
 mod unnecessary_to_owned;
@@ -4639,6 +4640,30 @@ declare_clippy_lint! {
     "detects redundant calls to `Iterator::cloned`"
 }
 
+declare_clippy_lint! {
+    /// Checks for usage of `.map_or_else()` "map closure" for `Option` type.
+    ///
+    /// ### Why is this bad?
+    /// This can be written more concisely by using `unwrap_or_else()`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let k = 10;
+    /// let x: Option<u32> = Some(4);
+    /// let y = x.map_or_else(|| 2 * k, |n| n);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let k = 10;
+    /// let x: Option<u32> = Some(4);
+    /// let y = x.unwrap_or_else(|| 2 * k);
+    /// ```
+    #[clippy::version = "1.88.0"]
+    pub UNNECESSARY_OPTION_MAP_OR_ELSE,
+    suspicious,
+    "making no use of the \"map closure\" when calling `.map_or_else(|| 2 * k, |n| n)`"
+}
+
 #[expect(clippy::struct_excessive_bools)]
 pub struct Methods {
     avoid_breaking_exported_api: bool,
@@ -4820,6 +4845,7 @@ impl_lint_pass!(Methods => [
     SWAP_WITH_TEMPORARY,
     IP_CONSTANT,
     REDUNDANT_ITER_CLONED,
+    UNNECESSARY_OPTION_MAP_OR_ELSE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -5275,6 +5301,7 @@ impl Methods {
                 },
                 (sym::map_or_else, [def, map]) => {
                     result_map_or_else_none::check(cx, expr, recv, def, map);
+                    unnecessary_option_map_or_else::check(cx, expr, recv, def, map);
                     unnecessary_result_map_or_else::check(cx, expr, recv, def, map);
                 },
                 (sym::next, []) => {

clippy_lints/src/methods/unnecessary_option_map_or_else.rs

@@ -0,0 +1,111 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::{expr_or_init, find_binding_init, peel_blocks};
+use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Body, BodyId, Closure, Expr, ExprKind, HirId, QPath};
+use rustc_lint::LateContext;
+use rustc_span::symbol::sym;
+
+use super::UNNECESSARY_OPTION_MAP_OR_ELSE;
+use super::utils::get_last_chain_binding_hir_id;
+
+fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>) {
+    let msg = "unused \"map closure\" when calling `Option::map_or_else` value";
+    let mut applicability = Applicability::MachineApplicable;
+    let self_snippet = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
+    let err_snippet = snippet_with_applicability(cx, def_arg.span, "..", &mut applicability);
+    span_lint_and_sugg(
+        cx,
+        UNNECESSARY_OPTION_MAP_OR_ELSE,
+        expr.span,
+        msg,
+        "consider using `unwrap_or_else`",
+        format!("{self_snippet}.unwrap_or_else({err_snippet})"),
+        Applicability::MachineApplicable,
+    );
+}
+
+fn handle_qpath(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    recv: &Expr<'_>,
+    def_arg: &Expr<'_>,
+    expected_hir_id: HirId,
+    qpath: QPath<'_>,
+) {
+    if let QPath::Resolved(_, path) = qpath
+        && let Res::Local(hir_id) = path.res
+        && expected_hir_id == hir_id
+    {
+        emit_lint(cx, expr, recv, def_arg);
+    }
+}
+
+fn handle_closure(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, body_id: BodyId) {
+    let body = cx.tcx.hir_body(body_id);
+    handle_fn_body(cx, expr, recv, def_arg, body);
+}
+
+fn handle_fn_body(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, body: &Body<'_>) {
+    if let Some(first_param) = body.params.first() {
+        let body_expr = peel_blocks(body.value);
+        match body_expr.kind {
+            ExprKind::Path(qpath) => {
+                handle_qpath(cx, expr, recv, def_arg, first_param.pat.hir_id, qpath);
+            },
+            // If this is a block (that wasn't peeled off), then it means there are statements.
+            ExprKind::Block(block, _) => {
+                if let Some(block_expr) = block.expr
+                        // First we ensure that this is a "binding chain" (each statement is a binding
+                        // of the previous one) and that it is a binding of the closure argument.
+                        && let Some(last_chain_binding_id) =
+                            get_last_chain_binding_hir_id(first_param.pat.hir_id, block.stmts)
+                        && let ExprKind::Path(qpath) = block_expr.kind
+                {
+                    handle_qpath(cx, expr, recv, def_arg, last_chain_binding_id, qpath);
+                }
+            },
+            _ => {},
+        }
+    }
+}
+
+/// lint use of `_.map_or_else(|err| err, |n| n)` for `Option`s.
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, map_arg: &Expr<'_>) {
+    // lint if the caller of `map_or_else()` is an `Option`
+    if !is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option) {
+        return;
+    }
+    match map_arg.kind {
+        // If the second argument is a closure, we can check its body.
+        ExprKind::Closure(&Closure { body, .. }) => {
+            handle_closure(cx, expr, recv, def_arg, body);
+        },
+        ExprKind::Path(qpath) => {
+            let res = cx.qpath_res(&qpath, map_arg.hir_id);
+            match res {
+                // Case 1: Local variable (could be a closure)
+                Res::Local(hir_id) => {
+                    if let Some(init_expr) = find_binding_init(cx, hir_id) {
+                        let origin = expr_or_init(cx, init_expr);
+                        if let ExprKind::Closure(&Closure { body, .. }) = origin.kind {
+                            handle_closure(cx, expr, recv, def_arg, body);
+                        }
+                    }
+                },
+                // Case 2: Function definition
+                Res::Def(DefKind::Fn, def_id) => {
+                    if let Some(local_def_id) = def_id.as_local()
+                        && let Some(body) = cx.tcx.hir_maybe_body_owned_by(local_def_id)
+                    {
+                        handle_fn_body(cx, expr, recv, def_arg, body);
+                    }
+                },
+                _ => (),
+            }
+        },
+        _ => (),
+    }
+}

tests/ui/option_if_let_else.fixed

@@ -6,7 +6,8 @@
     clippy::redundant_locals,
     clippy::manual_midpoint,
     clippy::manual_unwrap_or_default,
-    clippy::manual_unwrap_or
+    clippy::manual_unwrap_or,
+    clippy::unnecessary_option_map_or_else
 )]
 
 fn bad1(string: Option<&str>) -> (bool, &str) {

tests/ui/option_if_let_else.rs

@@ -6,7 +6,8 @@
     clippy::redundant_locals,
     clippy::manual_midpoint,
     clippy::manual_unwrap_or_default,
-    clippy::manual_unwrap_or
+    clippy::manual_unwrap_or,
+    clippy::unnecessary_option_map_or_else
 )]
 
 fn bad1(string: Option<&str>) -> (bool, &str) {