Added new lint for map_or(..., identity)

Author: teofr

clippy_lints/src/declared_lints.rs

@@ -421,6 +421,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::MAP_ERR_IGNORE_INFO,
     crate::methods::MAP_FLATTEN_INFO,
     crate::methods::MAP_IDENTITY_INFO,
+    crate::methods::MAP_OR_IDENTITY_INFO,
     crate::methods::MAP_UNWRAP_OR_INFO,
     crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES_INFO,
     crate::methods::MUT_MUTEX_LOCK_INFO,

clippy_lints/src/methods/map_or_identity.rs

@@ -0,0 +1,42 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_expr_identity_function;
+use clippy_utils::res::MaybeDef;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+
+use rustc_lint::LateContext;
+use rustc_span::Symbol;
+use rustc_span::symbol::sym;
+
+use super::MAP_OR_IDENTITY;
+
+/// Emit a lint for an unnecessary `map_or` over `Option` or `Result`
+///
+/// Note: `symbol` can only be `sym::Option` or `sym::Result`
+fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, symbol: Symbol) {
+    assert!(symbol == sym::Option || symbol == sym::Result);
+    let msg = format!("unused \"map closure\" when calling `{symbol}::map_or` 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,
+        MAP_OR_IDENTITY,
+        expr.span,
+        msg,
+        "consider using `unwrap_or`",
+        format!("{self_snippet}.unwrap_or({err_snippet})"),
+        applicability,
+    );
+}
+
+/// lint use of `_.map_or(err, |n| n)` for `Result`s and `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()` is a `Result` or an `Option`
+    if let Some(x @ (sym::Result | sym::Option)) = cx.typeck_results().expr_ty(recv).opt_diag_name(cx)
+        && is_expr_identity_function(cx, map_arg)
+    {
+        emit_lint(cx, expr, recv, def_arg, x);
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -73,6 +73,7 @@ mod map_collect_result_unit;
 mod map_err_ignore;
 mod map_flatten;
 mod map_identity;
+mod map_or_identity;
 mod map_unwrap_or;
 mod map_with_unused_argument_over_ranges;
 mod mut_mutex_lock;
@@ -4714,6 +4715,29 @@ declare_clippy_lint! {
     "filtering `std::io::Lines` with `filter_map()`, `flat_map()`, or `flatten()` might cause an infinite loop"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks the usage of `.map_or(...)` with an identity function for `Option` and `Result` types.
+    ///
+    /// ### Why is this bad?
+    /// This can be written more concisely by using `unwrap_or()`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let opt = Some(1);
+    /// opt.map_or(42, |v| v);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let opt = Some(1);
+    /// opt.unwrap_or(42);
+    /// ```
+    #[clippy::version = "1.93.0"]
+    pub MAP_OR_IDENTITY,
+    suspicious,
+    "using an identity function when mapping with `.map_or(|err| ..., |x| x)`"
+}
+
 #[expect(clippy::struct_excessive_bools)]
 pub struct Methods {
     avoid_breaking_exported_api: bool,
@@ -4897,6 +4921,7 @@ impl_lint_pass!(Methods => [
     REDUNDANT_ITER_CLONED,
     UNNECESSARY_OPTION_MAP_OR_ELSE,
     LINES_FILTER_MAP_OK,
+    MAP_OR_IDENTITY,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -5368,6 +5393,7 @@ impl Methods {
                     option_map_or_none::check(cx, expr, recv, def, map);
                     manual_ok_or::check(cx, expr, recv, def, map);
                     unnecessary_map_or::check(cx, expr, recv, def, map, span, self.msrv);
+                    map_or_identity::check(cx, expr, recv, def, map);
                 },
                 (sym::map_or_else, [def, map]) => {
                     result_map_or_else_none::check(cx, expr, recv, def, map);

tests/ui/manual_ok_or.fixed

@@ -4,6 +4,7 @@
 #![allow(clippy::redundant_closure)]
 #![allow(dead_code)]
 #![allow(unused_must_use)]
+#![allow(clippy::map_or_identity)]
 
 fn main() {
     // basic case

tests/ui/manual_ok_or.rs

@@ -4,6 +4,7 @@
 #![allow(clippy::redundant_closure)]
 #![allow(dead_code)]
 #![allow(unused_must_use)]
+#![allow(clippy::map_or_identity)]
 
 fn main() {
     // basic case

tests/ui/manual_ok_or.stderr

@@ -1,5 +1,5 @@
 error: this pattern reimplements `Option::ok_or`
-  --> tests/ui/manual_ok_or.rs:11:5
+  --> tests/ui/manual_ok_or.rs:12:5
    |
 LL |     foo.map_or(Err("error"), |v| Ok(v));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
@@ -8,19 +8,19 @@ LL |     foo.map_or(Err("error"), |v| Ok(v));
    = help: to override `-D warnings` add `#[allow(clippy::manual_ok_or)]`
 
 error: this pattern reimplements `Option::ok_or`
-  --> tests/ui/manual_ok_or.rs:15:5
+  --> tests/ui/manual_ok_or.rs:16:5
    |
 LL |     foo.map_or(Err("error"), Ok);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
 
 error: this pattern reimplements `Option::ok_or`
-  --> tests/ui/manual_ok_or.rs:19:5
+  --> tests/ui/manual_ok_or.rs:20:5
    |
 LL |     None::<i32>.map_or(Err("error"), |v| Ok(v));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `None::<i32>.ok_or("error")`
 
 error: this pattern reimplements `Option::ok_or`
-  --> tests/ui/manual_ok_or.rs:24:5
+  --> tests/ui/manual_ok_or.rs:25:5
    |
 LL | /     foo.map_or(Err::<i32, &str>(
 LL | |

tests/ui/map_or_identity.fixed

@@ -0,0 +1,17 @@
+#![warn(clippy::map_or_identity)]
+
+mod issue15801 {
+
+    fn foo(opt: Option<i32>, default: i32) -> i32 {
+        opt.unwrap_or(default)
+        //~^ map_or_identity
+    }
+
+    fn bar(res: Result<i32, &str>, default: i32) -> i32 {
+        res.unwrap_or(default)
+        //~^ map_or_identity
+    }
+}
+fn main() {
+    // test code goes here
+}

tests/ui/map_or_identity.rs

@@ -0,0 +1,17 @@
+#![warn(clippy::map_or_identity)]
+
+mod issue15801 {
+
+    fn foo(opt: Option<i32>, default: i32) -> i32 {
+        opt.map_or(default, |o| o)
+        //~^ map_or_identity
+    }
+
+    fn bar(res: Result<i32, &str>, default: i32) -> i32 {
+        res.map_or(default, |o| o)
+        //~^ map_or_identity
+    }
+}
+fn main() {
+    // test code goes here
+}

tests/ui/map_or_identity.stderr

@@ -0,0 +1,17 @@
+error: unused "map closure" when calling `Option::map_or` value
+  --> tests/ui/map_or_identity.rs:6:9
+   |
+LL |         opt.map_or(default, |o| o)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or`: `opt.unwrap_or(default)`
+   |
+   = note: `-D clippy::map-or-identity` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::map_or_identity)]`
+
+error: unused "map closure" when calling `Result::map_or` value
+  --> tests/ui/map_or_identity.rs:11:9
+   |
+LL |         res.map_or(default, |o| o)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or`: `res.unwrap_or(default)`
+
+error: aborting due to 2 previous errors
+

tests/ui/option_if_let_else.fixed

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