Skip to content

Commit 329dc47

Browse files
committed
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`.
1 parent 33137dd commit 329dc47

File tree

4 files changed

+81
-4
lines changed

4 files changed

+81
-4
lines changed

clippy_lints/src/eta_reduction.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rustc_hir::{Closure, Expr, ExprKind, Param, PatKind, Unsafety};
1111
use rustc_lint::{LateContext, LateLintPass};
1212
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
1313
use rustc_middle::ty::binding::BindingMode;
14-
use rustc_middle::ty::{self, Ty, TypeVisitable};
14+
use rustc_middle::ty::{self, EarlyBinder, SubstsRef, Ty, TypeVisitable};
1515
use rustc_session::{declare_lint_pass, declare_tool_lint};
1616
use rustc_span::symbol::sym;
1717

@@ -150,7 +150,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
150150
if check_sig(cx, closure_ty, call_ty);
151151
then {
152152
span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure", |diag| {
153-
let name = get_ufcs_type_name(cx, method_def_id);
153+
let name = get_ufcs_type_name(cx, method_def_id, substs);
154154
diag.span_suggestion(
155155
expr.span,
156156
"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
220220
cx.tcx.erase_late_bound_regions(closure_sig) == cx.tcx.erase_late_bound_regions(call_sig)
221221
}
222222

223-
fn get_ufcs_type_name(cx: &LateContext<'_>, method_def_id: DefId) -> String {
223+
fn get_ufcs_type_name<'tcx>(cx: &LateContext<'tcx>, method_def_id: DefId, substs: SubstsRef<'tcx>) -> String {
224224
let assoc_item = cx.tcx.associated_item(method_def_id);
225225
let def_id = assoc_item.container_id(cx.tcx);
226226
match assoc_item.container {
@@ -229,6 +229,15 @@ fn get_ufcs_type_name(cx: &LateContext<'_>, method_def_id: DefId) -> String {
229229
let ty = cx.tcx.type_of(def_id);
230230
match ty.kind() {
231231
ty::Adt(adt, _) => cx.tcx.def_path_str(adt.did()),
232+
ty::Array(..)
233+
| ty::Dynamic(..)
234+
| ty::Never
235+
| ty::RawPtr(_)
236+
| ty::Ref(..)
237+
| ty::Slice(_)
238+
| ty::Tuple(_) => {
239+
format!("<{}>", EarlyBinder(ty).subst(cx.tcx, substs))
240+
},
232241
_ => ty.to_string(),
233242
}
234243
},

tests/ui/eta.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,25 @@ pub fn mutable_impl_fn_mut(mut f: impl FnMut(), mut f_used_once: impl FnMut()) -
316316

317317
move || takes_fn_mut(&mut f_used_once)
318318
}
319+
320+
impl dyn TestTrait + '_ {
321+
fn method_on_dyn(&self) -> bool {
322+
false
323+
}
324+
}
325+
326+
// https://github.com/rust-lang/rust-clippy/issues/7746
327+
fn angle_brackets_and_substs() {
328+
let array_opt: Option<&[u8; 3]> = Some(&[4, 8, 7]);
329+
array_opt.map(<[u8; 3]>::as_slice);
330+
331+
let slice_opt: Option<&[u8]> = Some(b"slice");
332+
slice_opt.map(<[u8]>::len);
333+
334+
let ptr_opt: Option<*const usize> = Some(&487);
335+
ptr_opt.map(<*const usize>::is_null);
336+
337+
let test_struct = TestStruct { some_ref: &487 };
338+
let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
339+
dyn_opt.map(<dyn TestTrait>::method_on_dyn);
340+
}

tests/ui/eta.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,25 @@ pub fn mutable_impl_fn_mut(mut f: impl FnMut(), mut f_used_once: impl FnMut()) -
316316

317317
move || takes_fn_mut(|| f_used_once())
318318
}
319+
320+
impl dyn TestTrait + '_ {
321+
fn method_on_dyn(&self) -> bool {
322+
false
323+
}
324+
}
325+
326+
// https://github.com/rust-lang/rust-clippy/issues/7746
327+
fn angle_brackets_and_substs() {
328+
let array_opt: Option<&[u8; 3]> = Some(&[4, 8, 7]);
329+
array_opt.map(|a| a.as_slice());
330+
331+
let slice_opt: Option<&[u8]> = Some(b"slice");
332+
slice_opt.map(|s| s.len());
333+
334+
let ptr_opt: Option<*const usize> = Some(&487);
335+
ptr_opt.map(|p| p.is_null());
336+
337+
let test_struct = TestStruct { some_ref: &487 };
338+
let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
339+
dyn_opt.map(|d| d.method_on_dyn());
340+
}

tests/ui/eta.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,5 +134,29 @@ error: redundant closure
134134
LL | move || takes_fn_mut(|| f_used_once())
135135
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
136136

137-
error: aborting due to 22 previous errors
137+
error: redundant closure
138+
--> $DIR/eta.rs:329:19
139+
|
140+
LL | array_opt.map(|a| a.as_slice());
141+
| ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
142+
143+
error: redundant closure
144+
--> $DIR/eta.rs:332:19
145+
|
146+
LL | slice_opt.map(|s| s.len());
147+
| ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
148+
149+
error: redundant closure
150+
--> $DIR/eta.rs:335:17
151+
|
152+
LL | ptr_opt.map(|p| p.is_null());
153+
| ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
154+
155+
error: redundant closure
156+
--> $DIR/eta.rs:339:17
157+
|
158+
LL | dyn_opt.map(|d| d.method_on_dyn());
159+
| ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`
160+
161+
error: aborting due to 26 previous errors
138162

0 commit comments

Comments
 (0)