Include generic arguments when suggesting a closure η-reduction (#14105)

Fix #14096 changelog: [`redundant_closure_for_method_calls`]: repeat generic args in suggestion
rust-lang · Jan 30, 2025 · ad05bc0 · ad05bc0
2 parents 88a00a8 + e4505fb
commit ad05bc0
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 28 deletions.
diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
@@ -1,13 +1,13 @@
 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::source::{snippet_opt, snippet_with_applicability};
 use clippy_utils::ty::get_type_diagnostic_name;
 use clippy_utils::usage::{local_used_after_expr, local_used_in};
 use clippy_utils::{
     get_path_from_caller_to_method_type, is_adjusted, is_no_std_crate, path_to_local, path_to_local_id,
 };
 use rustc_errors::Applicability;
-use rustc_hir::{BindingMode, Expr, ExprKind, FnRetTy, Param, PatKind, QPath, Safety, TyKind};
+use rustc_hir::{BindingMode, Expr, ExprKind, FnRetTy, GenericArgs, Param, PatKind, QPath, Safety, TyKind};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{
@@ -239,6 +239,11 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
                 && !cx.tcx.has_attr(method_def_id, sym::track_caller)
                 && check_sig(closure_sig, cx.tcx.fn_sig(method_def_id).skip_binder().skip_binder())
             {
+                let mut app = Applicability::MachineApplicable;
+                let generic_args = match path.args.and_then(GenericArgs::span_ext) {
+                    Some(span) => format!("::{}", snippet_with_applicability(cx, span, "<..>", &mut app)),
+                    None => String::new(),
+                };
                 span_lint_and_then(
                     cx,
                     REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
@@ -251,8 +256,8 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
                         diag.span_suggestion(
                             expr.span,
                             "replace the closure with the method itself",
-                            format!("{}::{}", type_name, path.ident.name),
-                            Applicability::MachineApplicable,
+                            format!("{}::{}{}", type_name, path.ident.name, generic_args),
+                            app,
                         );
                     },
                 );

diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed
@@ -116,6 +116,11 @@ fn test_redundant_closures_containing_method_calls() {
         t.iter().filter(|x| x.trait_foo_ref());
         t.iter().map(|x| x.trait_foo_ref());
     }
+
+    fn issue14096() {
+        let x = Some("42");
+        let _ = x.map(str::parse::<i16>);
+    }
 }
 
 struct Thunk<T>(Box<dyn FnMut() -> T>);

diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs
@@ -116,6 +116,11 @@ fn test_redundant_closures_containing_method_calls() {
         t.iter().filter(|x| x.trait_foo_ref());
         t.iter().map(|x| x.trait_foo_ref());
     }
+
+    fn issue14096() {
+        let x = Some("42");
+        let _ = x.map(|x| x.parse::<i16>());
+    }
 }
 
 struct Thunk<T>(Box<dyn FnMut() -> T>);

diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr
@@ -71,142 +71,148 @@ LL |     let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_as
    |                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase`
 
 error: redundant closure
-  --> tests/ui/eta.rs:169:22
+  --> tests/ui/eta.rs:122:23
+   |
+LL |         let _ = x.map(|x| x.parse::<i16>());
+   |                       ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `str::parse::<i16>`
+
+error: redundant closure
+  --> tests/ui/eta.rs:174:22
    |
 LL |     requires_fn_once(|| x());
    |                      ^^^^^^ help: replace the closure with the function itself: `x`
 
 error: redundant closure
-  --> tests/ui/eta.rs:176:27
+  --> tests/ui/eta.rs:181:27
    |
 LL |     let a = Some(1u8).map(|a| foo_ptr(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr`
 
 error: redundant closure
-  --> tests/ui/eta.rs:181:27
+  --> tests/ui/eta.rs:186:27
    |
 LL |     let a = Some(1u8).map(|a| closure(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure`
 
 error: redundant closure
-  --> tests/ui/eta.rs:213:28
+  --> tests/ui/eta.rs:218:28
    |
 LL |     x.into_iter().for_each(|x| add_to_res(x));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
 
 error: redundant closure
-  --> tests/ui/eta.rs:214:28
+  --> tests/ui/eta.rs:219:28
    |
 LL |     y.into_iter().for_each(|x| add_to_res(x));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
 
 error: redundant closure
-  --> tests/ui/eta.rs:215:28
+  --> tests/ui/eta.rs:220:28
    |
 LL |     z.into_iter().for_each(|x| add_to_res(x));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res`
 
 error: redundant closure
-  --> tests/ui/eta.rs:222:21
+  --> tests/ui/eta.rs:227:21
    |
 LL |         Some(1).map(|n| closure(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
 
 error: redundant closure
-  --> tests/ui/eta.rs:226:21
+  --> tests/ui/eta.rs:231:21
    |
 LL |         Some(1).map(|n| in_loop(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
 
 error: redundant closure
-  --> tests/ui/eta.rs:319:18
+  --> tests/ui/eta.rs:324:18
    |
 LL |     takes_fn_mut(|| f());
    |                  ^^^^^^ help: replace the closure with the function itself: `&mut f`
 
 error: redundant closure
-  --> tests/ui/eta.rs:322:19
+  --> tests/ui/eta.rs:327:19
    |
 LL |     takes_fn_once(|| f());
    |                   ^^^^^^ help: replace the closure with the function itself: `&mut f`
 
 error: redundant closure
-  --> tests/ui/eta.rs:326:26
+  --> tests/ui/eta.rs:331:26
    |
 LL |     move || takes_fn_mut(|| f_used_once())
    |                          ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
 
 error: redundant closure
-  --> tests/ui/eta.rs:338:19
+  --> tests/ui/eta.rs:343:19
    |
 LL |     array_opt.map(|a| a.as_slice());
    |                   ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
 
 error: redundant closure
-  --> tests/ui/eta.rs:341:19
+  --> tests/ui/eta.rs:346:19
    |
 LL |     slice_opt.map(|s| s.len());
    |                   ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
 
 error: redundant closure
-  --> tests/ui/eta.rs:344:17
+  --> tests/ui/eta.rs:349:17
    |
 LL |     ptr_opt.map(|p| p.is_null());
    |                 ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
 
 error: redundant closure
-  --> tests/ui/eta.rs:348:17
+  --> tests/ui/eta.rs:353:17
    |
 LL |     dyn_opt.map(|d| d.method_on_dyn());
    |                 ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`
 
 error: redundant closure
-  --> tests/ui/eta.rs:408:19
+  --> tests/ui/eta.rs:413:19
    |
 LL |     let _ = f(&0, |x, y| f2(x, y));
    |                   ^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `f2`
 
 error: redundant closure
-  --> tests/ui/eta.rs:436:22
+  --> tests/ui/eta.rs:441:22
    |
 LL |             test.map(|t| t.method())
    |                      ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `Test::method`
 
 error: redundant closure
-  --> tests/ui/eta.rs:440:22
+  --> tests/ui/eta.rs:445:22
    |
 LL |             test.map(|t| t.method())
    |                      ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `super::Outer::method`
 
 error: redundant closure
-  --> tests/ui/eta.rs:453:18
+  --> tests/ui/eta.rs:458:18
    |
 LL |         test.map(|t| t.method())
    |                  ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `test_mod::Test::method`
 
 error: redundant closure
-  --> tests/ui/eta.rs:460:30
+  --> tests/ui/eta.rs:465:30
    |
 LL |                     test.map(|t| t.method())
    |                              ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `crate::issue_10854::d::Test::method`
 
 error: redundant closure
-  --> tests/ui/eta.rs:479:38
+  --> tests/ui/eta.rs:484: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:483:38
+  --> tests/ui/eta.rs:488: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:500:35
+  --> tests/ui/eta.rs:505:35
    |
 LL |         let _field = bind.or_else(|| get_default()).unwrap();
    |                                   ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `get_default`
 
-error: aborting due to 34 previous errors
+error: aborting due to 35 previous errors