[WIP] Working on some changes

teofr · teofr · commit 8596bc068bc0 · 2025-10-16T00:14:26.000+01:00
When checking if a function is an identity function, try to use existing
functionalities.

Some cases are no longer covered, but I believe that's ok and there were
some bugs.
diff --git a/clippy_lints/src/methods/unnecessary_result_map_or_else.rs b/clippy_lints/src/methods/unnecessary_result_map_or_else.rs
@@ -1,15 +1,14 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::peel_blocks;
+use clippy_utils::is_expr_identity_function;
 use clippy_utils::res::MaybeDef;
 use clippy_utils::source::snippet;
 use rustc_errors::Applicability;
-use rustc_hir as hir;
-use rustc_hir::{Closure, Expr, ExprKind, HirId, QPath};
+use rustc_hir::Expr;
+
 use rustc_lint::LateContext;
 use rustc_span::symbol::sym;
 
 use super::UNNECESSARY_RESULT_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 `Result::map_or_else` value";
@@ -26,22 +25,6 @@ fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &E
     );
 }
 
-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 hir::def::Res::Local(hir_id) = path.res
-        && expected_hir_id == hir_id
-    {
-        emit_lint(cx, expr, recv, def_arg);
-    }
-}
-
 /// lint use of `_.map_or_else(|err| err, |n| n)` for `Result`s.
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
@@ -51,30 +34,7 @@ pub(super) fn check<'tcx>(
     map_arg: &'tcx Expr<'_>,
 ) {
     // lint if the caller of `map_or_else()` is a `Result`
-    if cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Result)
-        && let ExprKind::Closure(&Closure { body, .. }) = map_arg.kind
-        && let body = cx.tcx.hir_body(body)
-        && 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);
-                }
-            },
-            _ => {},
-        }
+    if cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Result) && is_expr_identity_function(cx, map_arg) {
+        emit_lint(cx, expr, recv, def_arg);
     }
 }
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
@@ -1785,6 +1785,8 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
         return false;
     };
 
+    let mut param_pat = param.pat;
+
     let mut expr = func.value;
     loop {
         match expr.kind {
@@ -1813,7 +1815,25 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
                     return false;
                 }
             },
-            _ => return is_expr_identity_of_pat(cx, param.pat, expr, true),
+            ExprKind::Block(
+                &Block {
+                    stmts, expr: Some(e), ..
+                },
+                _,
+            ) => {
+                for stmt in stmts {
+                    if let StmtKind::Let(local) = stmt.kind
+                        && let Some(init) = local.init
+                        && is_expr_identity_of_pat(cx, param_pat, init, true)
+                    {
+                        param_pat = local.pat;
+                    } else {
+                        return false;
+                    }
+                }
+                expr = e;
+            },
+            _ => return is_expr_identity_of_pat(cx, param_pat, expr, true),
         }
     }
 }
diff --git a/tests/ui/unnecessary_result_map_or_else.fixed b/tests/ui/unnecessary_result_map_or_else.fixed
@@ -11,12 +11,6 @@ fn main() {
     x.unwrap_or_else(|err: ()| err);
     //~^ unnecessary_result_map_or_else
 
-    // Auto-deref.
-    let y = String::new();
-    let x: Result<&String, &String> = Ok(&y);
-    let y: &str = x.unwrap_or_else(|err| err);
-    //~^ unnecessary_result_map_or_else
-
     // Temporary variable.
     let x: Result<(), ()> = Ok(());
     x.unwrap_or_else(|err| err);
@@ -61,4 +55,8 @@ fn main() {
             y
         },
     );
+
+    let x: Result<((), ()), ((), ())> = Ok(((), ()));
+    x.unwrap_or_else(|err| err);
+    //~^ unnecessary_result_map_or_else
 }
diff --git a/tests/ui/unnecessary_result_map_or_else.rs b/tests/ui/unnecessary_result_map_or_else.rs
@@ -11,12 +11,6 @@ fn main() {
     x.map_or_else(|err: ()| err, |n: ()| n);
     //~^ unnecessary_result_map_or_else
 
-    // Auto-deref.
-    let y = String::new();
-    let x: Result<&String, &String> = Ok(&y);
-    let y: &str = x.map_or_else(|err| err, |n| n);
-    //~^ unnecessary_result_map_or_else
-
     // Temporary variable.
     let x: Result<(), ()> = Ok(());
     x.map_or_else(
@@ -69,4 +63,8 @@ fn main() {
             y
         },
     );
+
+    let x: Result<((), ()), ((), ())> = Ok(((), ()));
+    x.map_or_else(|err| err, |(a, b)| (a, b));
+    //~^ unnecessary_result_map_or_else
 }
diff --git a/tests/ui/unnecessary_result_map_or_else.stderr b/tests/ui/unnecessary_result_map_or_else.stderr
@@ -14,13 +14,7 @@ LL |     x.map_or_else(|err: ()| err, |n: ()| n);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err: ()| err)`
 
 error: unused "map closure" when calling `Result::map_or_else` value
-  --> tests/ui/unnecessary_result_map_or_else.rs:17:19
-   |
-LL |     let y: &str = x.map_or_else(|err| err, |n| n);
-   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)`
-
-error: unused "map closure" when calling `Result::map_or_else` value
-  --> tests/ui/unnecessary_result_map_or_else.rs:22:5
+  --> tests/ui/unnecessary_result_map_or_else.rs:16:5
    |
 LL | /     x.map_or_else(
 LL | |
@@ -31,5 +25,11 @@ LL | |         },
 LL | |     );
    | |_____^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)`
 
+error: unused "map closure" when calling `Result::map_or_else` value
+  --> tests/ui/unnecessary_result_map_or_else.rs:68:5
+   |
+LL |     x.map_or_else(|err| err, |(a, b)| (a, b));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)`
+
 error: aborting due to 4 previous errors
 
