fix [`redundant_closure`] suggesting incorrect code with `F: Fn()`

4 files changed, 54 insertions, 9 deletions

diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index 306a4a9e55c..df30351c155 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/clippy_lints/src/eta_reduction.rs
@@ -123,7 +123,8 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
                     ExprKind::Path(QPath::Resolved(..) | QPath::TypeRelative(..))
                 ) =>
             {
-                let callee_ty = typeck.expr_ty(callee).peel_refs();
+                let callee_ty_raw = typeck.expr_ty(callee);
+                let callee_ty = callee_ty_raw.peel_refs();
                 if matches!(type_diagnostic_name(cx, callee_ty), Some(sym::Arc | sym::Rc))
                     || !check_inputs(typeck, body.params, None, args)
                 {
@@ -170,15 +171,25 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
                 {
                     span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
                         if let Some(mut snippet) = snippet_opt(cx, callee.span) {
-                            if let Ok((ClosureKind::FnMut, _)) = cx.tcx.infer_ctxt().build().type_implements_fn_trait(
-                                cx.param_env,
-                                Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
-                                ty::PredicatePolarity::Positive,
-                            ) && path_to_local(callee).map_or(false, |l| {
+                            if path_to_local(callee).map_or(false, |l| {
+                                // FIXME: Do we really need this `local_used_in` check?
+                                // Isn't it checking something like... `callee(callee)`?
+                                // If somehow this check is needed, add some test for it,
+                                // 'cuz currently nothing changes after deleting this check.
                                 local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr)
                             }) {
-                                // Mutable closure is used after current expr; we cannot consume it.
-                                snippet = format!("&mut {snippet}");
+                                match cx.tcx.infer_ctxt().build().type_implements_fn_trait(
+                                    cx.param_env,
+                                    Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
+                                    ty::PredicatePolarity::Positive,
+                                ) {
+                                    // Mutable closure is used after current expr; we cannot consume it.
+                                    Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"),
+                                    Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => {
+                                        snippet = format!("&{snippet}");
+                                    },
+                                    _ => (),
+                                }
                             }
                             diag.span_suggestion(
                                 expr.span,
diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed
index da28ec2e653..7126f279945 100644
--- a/tests/ui/eta.fixed
+++ b/tests/ui/eta.fixed
@@ -471,3 +471,14 @@ mod issue_10854 {
         }
     }
 }
+
+mod issue_12853 {
+    fn f_by_value<F: Fn(u32)>(f: F) {
+        let x = Box::new(|| None.map(&f));
+        x();
+    }
+    fn f_by_ref<F: Fn(u32)>(f: &F) {
+        let x = Box::new(|| None.map(f));
+        x();
+    }
+}
diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs
index f924100f8f4..0787abf5f3e 100644
--- a/tests/ui/eta.rs
+++ b/tests/ui/eta.rs
@@ -471,3 +471,14 @@ mod issue_10854 {
         }
     }
 }
+
+mod issue_12853 {
+    fn f_by_value<F: Fn(u32)>(f: F) {
+        let x = Box::new(|| None.map(|x| f(x)));
+        x();
+    }
+    fn f_by_ref<F: Fn(u32)>(f: &F) {
+        let x = Box::new(|| None.map(|x| f(x)));
+        x();
+    }
+}
diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr
index d9a8768a682..c757601042f 100644
--- a/tests/ui/eta.stderr
+++ b/tests/ui/eta.stderr
@@ -190,5 +190,17 @@ error: redundant closure
 LL |                     test.map(|t| t.method())
    |                              ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `crate::issue_10854::d::Test::method`
 
-error: aborting due to 31 previous errors
+error: redundant closure
+  --> tests/ui/eta.rs:477:38
+   |
+LL |         let x = Box::new(|| None.map(|x| f(x)));
+   |                                      ^^^^^^^^ help: replace the closure with the function itself: `&f`
+
+error: redundant closure
+  --> tests/ui/eta.rs:481:38
+   |
+LL |         let x = Box::new(|| None.map(|x| f(x)));
+   |                                      ^^^^^^^^ help: replace the closure with the function itself: `f`
+
+error: aborting due to 33 previous errors