From 416da1264c48aa38c6ae8c6f6aefffa49ae3b21f Mon Sep 17 00:00:00 2001
From: Evan Typanski <evantypanski@gmail.com>
Date: Thu, 29 Sep 2022 19:31:32 -0400
Subject: [PATCH] [`redundant_closure`] Add `&mut` to more cases

---
 clippy_lints/src/eta_reduction.rs | 14 +++++---------
 tests/ui/eta.fixed                | 11 +++++++++++
 tests/ui/eta.rs                   | 11 +++++++++++
 tests/ui/eta.stderr               | 14 +++++++++++++-
 4 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index 710fceceae5..a5c6512a5e7 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/clippy_lints/src/eta_reduction.rs
@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::higher::VecArgs;
 use clippy_utils::source::snippet_opt;
-use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
 use clippy_utils::usage::local_used_after_expr;
 use clippy_utils::{higher, is_adjusted, path_to_local, path_to_local_id};
 use if_chain::if_chain;
@@ -11,7 +11,7 @@ use rustc_hir::{Closure, Expr, ExprKind, Param, PatKind, Unsafety};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
 use rustc_middle::ty::binding::BindingMode;
-use rustc_middle::ty::{self, ClosureKind, Ty, TypeVisitable};
+use rustc_middle::ty::{self, Ty, TypeVisitable};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::symbol::sym;
 
@@ -122,15 +122,11 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
             then {
                 span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
                     if let Some(mut snippet) = snippet_opt(cx, callee.span) {
-                        if_chain! {
-                            if let ty::Closure(_, substs) = callee_ty.peel_refs().kind();
-                            if substs.as_closure().kind() == ClosureKind::FnMut;
-                            if path_to_local(callee).map_or(false, |l| local_used_after_expr(cx, l, expr));
-
-                            then {
+                        if let Some(fn_mut_id) = cx.tcx.lang_items().fn_mut_trait() &&
+                            implements_trait(cx, callee_ty.peel_refs(), fn_mut_id, &[]) &&
+                            path_to_local(callee).map_or(false, |l| local_used_after_expr(cx, l, expr)) {
                                 // Mutable closure is used after current expr; we cannot consume it.
                                 snippet = format!("&mut {snippet}");
-                            }
                         }
                         diag.span_suggestion(
                             expr.span,
diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed
index f8d559bf226..112bb4b4817 100644
--- a/tests/ui/eta.fixed
+++ b/tests/ui/eta.fixed
@@ -303,3 +303,14 @@ fn not_general_enough() {
     fn f(_: impl FnMut(&Path) -> std::io::Result<()>) {}
     f(|path| std::fs::remove_file(path));
 }
+
+// https://github.com/rust-lang/rust-clippy/issues/9369
+pub fn mutable_impl_fn_mut(mut f: impl FnMut()) {
+    fn takes_fn_mut(_: impl FnMut()) {}
+    takes_fn_mut(&mut f);
+
+    fn takes_fn_once(_: impl FnOnce()) {}
+    takes_fn_once(&mut f);
+
+    f();
+}
diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs
index f0fb55a1e5f..970144e06dd 100644
--- a/tests/ui/eta.rs
+++ b/tests/ui/eta.rs
@@ -303,3 +303,14 @@ fn not_general_enough() {
     fn f(_: impl FnMut(&Path) -> std::io::Result<()>) {}
     f(|path| std::fs::remove_file(path));
 }
+
+// https://github.com/rust-lang/rust-clippy/issues/9369
+pub fn mutable_impl_fn_mut(mut f: impl FnMut()) {
+    fn takes_fn_mut(_: impl FnMut()) {}
+    takes_fn_mut(|| f());
+
+    fn takes_fn_once(_: impl FnOnce()) {}
+    takes_fn_once(|| f());
+
+    f();
+}
diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr
index bf2e97e744a..f7ead7257f6 100644
--- a/tests/ui/eta.stderr
+++ b/tests/ui/eta.stderr
@@ -116,5 +116,17 @@ error: redundant closure
 LL |         Some(1).map(|n| in_loop(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
 
-error: aborting due to 19 previous errors
+error: redundant closure
+  --> $DIR/eta.rs:310:18
+   |
+LL |     takes_fn_mut(|| f());
+   |                  ^^^^^^ help: replace the closure with the function itself: `&mut f`
+
+error: redundant closure
+  --> $DIR/eta.rs:313:19
+   |
+LL |     takes_fn_once(|| f());
+   |                   ^^^^^^ help: replace the closure with the function itself: `&mut f`
+
+error: aborting due to 21 previous errors