From 329dc4715bb152c98d4a5d59747604f3e79a2bb6 Mon Sep 17 00:00:00 2001
From: Matthew Ingwersen <matttpt@gmail.com>
Date: Fri, 28 Oct 2022 15:48:04 -0400
Subject: [PATCH] Fix `redundant_closure_for_method_calls` suggestion
Certain types must be enclosed in angle brackets and must have generic
arguments substituted to create a working suggestion. For example, if
`s` has type `&[u8]`, then `|s| s.len()` may be replaced with
`<[u8]>::len`. Previously, Clippy erroneously suggested `[T]::len`.
---
clippy_lints/src/eta_reduction.rs | 15 ++++++++++++---
tests/ui/eta.fixed | 22 ++++++++++++++++++++++
tests/ui/eta.rs | 22 ++++++++++++++++++++++
tests/ui/eta.stderr | 26 +++++++++++++++++++++++++-
4 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index 7b9786d7e57..9588be7364d 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/clippy_lints/src/eta_reduction.rs
@@ -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, Ty, TypeVisitable};
+use rustc_middle::ty::{self, EarlyBinder, SubstsRef, Ty, TypeVisitable};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::sym;
@@ -150,7 +150,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
if check_sig(cx, closure_ty, call_ty);
then {
span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure", |diag| {
- let name = get_ufcs_type_name(cx, method_def_id);
+ let name = get_ufcs_type_name(cx, method_def_id, substs);
diag.span_suggestion(
expr.span,
"replace the closure with the method itself",
@@ -220,7 +220,7 @@ fn check_sig<'tcx>(cx: &LateContext<'tcx>, closure_ty: Ty<'tcx>, call_ty: Ty<'tc
cx.tcx.erase_late_bound_regions(closure_sig) == cx.tcx.erase_late_bound_regions(call_sig)
}
-fn get_ufcs_type_name(cx: &LateContext<'_>, method_def_id: DefId) -> String {
+fn get_ufcs_type_name<'tcx>(cx: &LateContext<'tcx>, method_def_id: DefId, substs: SubstsRef<'tcx>) -> String {
let assoc_item = cx.tcx.associated_item(method_def_id);
let def_id = assoc_item.container_id(cx.tcx);
match assoc_item.container {
@@ -229,6 +229,15 @@ fn get_ufcs_type_name(cx: &LateContext<'_>, method_def_id: DefId) -> String {
let ty = cx.tcx.type_of(def_id);
match ty.kind() {
ty::Adt(adt, _) => cx.tcx.def_path_str(adt.did()),
+ ty::Array(..)
+ | ty::Dynamic(..)
+ | ty::Never
+ | ty::RawPtr(_)
+ | ty::Ref(..)
+ | ty::Slice(_)
+ | ty::Tuple(_) => {
+ format!("<{}>", EarlyBinder(ty).subst(cx.tcx, substs))
+ },
_ => ty.to_string(),
}
},
diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed
index a9cc80aaaf6..dc129591eac 100644
--- a/tests/ui/eta.fixed
+++ b/tests/ui/eta.fixed
@@ -316,3 +316,25 @@ pub fn mutable_impl_fn_mut(mut f: impl FnMut(), mut f_used_once: impl FnMut()) -
move || takes_fn_mut(&mut f_used_once)
}
+
+impl dyn TestTrait + '_ {
+ fn method_on_dyn(&self) -> bool {
+ false
+ }
+}
+
+// https://github.com/rust-lang/rust-clippy/issues/7746
+fn angle_brackets_and_substs() {
+ let array_opt: Option<&[u8; 3]> = Some(&[4, 8, 7]);
+ array_opt.map(<[u8; 3]>::as_slice);
+
+ let slice_opt: Option<&[u8]> = Some(b"slice");
+ slice_opt.map(<[u8]>::len);
+
+ let ptr_opt: Option<*const usize> = Some(&487);
+ ptr_opt.map(<*const usize>::is_null);
+
+ let test_struct = TestStruct { some_ref: &487 };
+ let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
+ dyn_opt.map(<dyn TestTrait>::method_on_dyn);
+}
diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs
index cc99906ccd6..025fd6a0b7a 100644
--- a/tests/ui/eta.rs
+++ b/tests/ui/eta.rs
@@ -316,3 +316,25 @@ pub fn mutable_impl_fn_mut(mut f: impl FnMut(), mut f_used_once: impl FnMut()) -
move || takes_fn_mut(|| f_used_once())
}
+
+impl dyn TestTrait + '_ {
+ fn method_on_dyn(&self) -> bool {
+ false
+ }
+}
+
+// https://github.com/rust-lang/rust-clippy/issues/7746
+fn angle_brackets_and_substs() {
+ let array_opt: Option<&[u8; 3]> = Some(&[4, 8, 7]);
+ array_opt.map(|a| a.as_slice());
+
+ let slice_opt: Option<&[u8]> = Some(b"slice");
+ slice_opt.map(|s| s.len());
+
+ let ptr_opt: Option<*const usize> = Some(&487);
+ ptr_opt.map(|p| p.is_null());
+
+ let test_struct = TestStruct { some_ref: &487 };
+ let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
+ dyn_opt.map(|d| d.method_on_dyn());
+}
diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr
index 434706b7e25..a521fb86860 100644
--- a/tests/ui/eta.stderr
+++ b/tests/ui/eta.stderr
@@ -134,5 +134,29 @@ error: redundant closure
LL | move || takes_fn_mut(|| f_used_once())
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
-error: aborting due to 22 previous errors
+error: redundant closure
+ --> $DIR/eta.rs:329:19
+ |
+LL | array_opt.map(|a| a.as_slice());
+ | ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
+
+error: redundant closure
+ --> $DIR/eta.rs:332:19
+ |
+LL | slice_opt.map(|s| s.len());
+ | ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
+
+error: redundant closure
+ --> $DIR/eta.rs:335:17
+ |
+LL | ptr_opt.map(|p| p.is_null());
+ | ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
+
+error: redundant closure
+ --> $DIR/eta.rs:339:17
+ |
+LL | dyn_opt.map(|d| d.method_on_dyn());
+ | ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`
+
+error: aborting due to 26 previous errors