Skip to content

Commit ad05bc0

Browse files
llogiqllogiq
authored
Include generic arguments when suggesting a closure η-reduction (rust-lang#14105)
Fix rust-lang#14096 changelog: [`redundant_closure_for_method_calls`]: repeat generic args in suggestion
2 parents 88a00a8 + e4505fb commit ad05bc0

File tree

4 files changed

+49
-28
lines changed

4 files changed

+49
-28
lines changed

‎clippy_lints/src/eta_reduction.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::higher::VecArgs;
3-
use clippy_utils::source::snippet_opt;
3+
use clippy_utils::source::{snippet_opt, snippet_with_applicability};
44
use clippy_utils::ty::get_type_diagnostic_name;
55
use clippy_utils::usage::{local_used_after_expr, local_used_in};
66
use clippy_utils::{
77
get_path_from_caller_to_method_type, is_adjusted, is_no_std_crate, path_to_local, path_to_local_id,
88
};
99
use rustc_errors::Applicability;
10-
use rustc_hir::{BindingMode, Expr, ExprKind, FnRetTy, Param, PatKind, QPath, Safety, TyKind};
10+
use rustc_hir::{BindingMode, Expr, ExprKind, FnRetTy, GenericArgs, Param, PatKind, QPath, Safety, TyKind};
1111
use rustc_infer::infer::TyCtxtInferExt;
1212
use rustc_lint::{LateContext, LateLintPass};
1313
use rustc_middle::ty::{
@@ -239,6 +239,11 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
239239
&& !cx.tcx.has_attr(method_def_id, sym::track_caller)
240240
&& check_sig(closure_sig, cx.tcx.fn_sig(method_def_id).skip_binder().skip_binder())
241241
{
242+
let mut app = Applicability::MachineApplicable;
243+
let generic_args = match path.args.and_then(GenericArgs::span_ext) {
244+
Some(span) => format!("::{}", snippet_with_applicability(cx, span, "<..>", &mut app)),
245+
None => String::new(),
246+
};
242247
span_lint_and_then(
243248
cx,
244249
REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
@@ -251,8 +256,8 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
251256
diag.span_suggestion(
252257
expr.span,
253258
"replace the closure with the method itself",
254-
format!("{}::{}", type_name, path.ident.name),
255-
Applicability::MachineApplicable,
259+
format!("{}::{}{}", type_name, path.ident.name, generic_args),
260+
app,
256261
);
257262
},
258263
);

‎tests/ui/eta.fixed

+5
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ fn test_redundant_closures_containing_method_calls() {
116116
t.iter().filter(|x| x.trait_foo_ref());
117117
t.iter().map(|x| x.trait_foo_ref());
118118
}
119+
120+
fn issue14096() {
121+
let x = Some("42");
122+
let _ = x.map(str::parse::<i16>);
123+
}
119124
}
120125

121126
struct Thunk<T>(Box<dyn FnMut() -> T>);

‎tests/ui/eta.rs

+5
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ fn test_redundant_closures_containing_method_calls() {
116116
t.iter().filter(|x| x.trait_foo_ref());
117117
t.iter().map(|x| x.trait_foo_ref());
118118
}
119+
120+
fn issue14096() {
121+
let x = Some("42");
122+
let _ = x.map(|x| x.parse::<i16>());
123+
}
119124
}
120125

121126
struct Thunk<T>(Box<dyn FnMut() -> T>);

‎tests/ui/eta.stderr

+30-24
Original file line numberDiff line numberDiff line change
@@ -71,142 +71,148 @@ LL | let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_as
7171
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase`
7272

7373
error: redundant closure
74-
--> tests/ui/eta.rs:169:22
74+
--> tests/ui/eta.rs:122:23
75+
|
76+
LL | let _ = x.map(|x| x.parse::<i16>());
77+
| ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `str::parse::<i16>`
78+
79+
error: redundant closure
80+
--> tests/ui/eta.rs:174:22
7581
|
7682
LL | requires_fn_once(|| x());
7783
| ^^^^^^ help: replace the closure with the function itself: `x`
7884

7985
error: redundant closure
80-
--> tests/ui/eta.rs:176:27
86+
--> tests/ui/eta.rs:181:27
8187
|
8288
LL | let a = Some(1u8).map(|a| foo_ptr(a));
8389
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr`
8490

8591
error: redundant closure
86-
--> tests/ui/eta.rs:181:27
92+
--> tests/ui/eta.rs:186:27
8793
|
8894
LL | let a = Some(1u8).map(|a| closure(a));
8995
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure`
9096

9197
error: redundant closure
92-
--> tests/ui/eta.rs:213:28
98+
--> tests/ui/eta.rs:218:28
9399
|
94100
LL | x.into_iter().for_each(|x| add_to_res(x));
95101
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
96102

97103
error: redundant closure
98-
--> tests/ui/eta.rs:214:28
104+
--> tests/ui/eta.rs:219:28
99105
|
100106
LL | y.into_iter().for_each(|x| add_to_res(x));
101107
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
102108

103109
error: redundant closure
104-
--> tests/ui/eta.rs:215:28
110+
--> tests/ui/eta.rs:220:28
105111
|
106112
LL | z.into_iter().for_each(|x| add_to_res(x));
107113
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res`
108114

109115
error: redundant closure
110-
--> tests/ui/eta.rs:222:21
116+
--> tests/ui/eta.rs:227:21
111117
|
112118
LL | Some(1).map(|n| closure(n));
113119
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
114120

115121
error: redundant closure
116-
--> tests/ui/eta.rs:226:21
122+
--> tests/ui/eta.rs:231:21
117123
|
118124
LL | Some(1).map(|n| in_loop(n));
119125
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
120126

121127
error: redundant closure
122-
--> tests/ui/eta.rs:319:18
128+
--> tests/ui/eta.rs:324:18
123129
|
124130
LL | takes_fn_mut(|| f());
125131
| ^^^^^^ help: replace the closure with the function itself: `&mut f`
126132

127133
error: redundant closure
128-
--> tests/ui/eta.rs:322:19
134+
--> tests/ui/eta.rs:327:19
129135
|
130136
LL | takes_fn_once(|| f());
131137
| ^^^^^^ help: replace the closure with the function itself: `&mut f`
132138

133139
error: redundant closure
134-
--> tests/ui/eta.rs:326:26
140+
--> tests/ui/eta.rs:331:26
135141
|
136142
LL | move || takes_fn_mut(|| f_used_once())
137143
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
138144

139145
error: redundant closure
140-
--> tests/ui/eta.rs:338:19
146+
--> tests/ui/eta.rs:343:19
141147
|
142148
LL | array_opt.map(|a| a.as_slice());
143149
| ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
144150

145151
error: redundant closure
146-
--> tests/ui/eta.rs:341:19
152+
--> tests/ui/eta.rs:346:19
147153
|
148154
LL | slice_opt.map(|s| s.len());
149155
| ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
150156

151157
error: redundant closure
152-
--> tests/ui/eta.rs:344:17
158+
--> tests/ui/eta.rs:349:17
153159
|
154160
LL | ptr_opt.map(|p| p.is_null());
155161
| ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
156162

157163
error: redundant closure
158-
--> tests/ui/eta.rs:348:17
164+
--> tests/ui/eta.rs:353:17
159165
|
160166
LL | dyn_opt.map(|d| d.method_on_dyn());
161167
| ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`
162168

163169
error: redundant closure
164-
--> tests/ui/eta.rs:408:19
170+
--> tests/ui/eta.rs:413:19
165171
|
166172
LL | let _ = f(&0, |x, y| f2(x, y));
167173
| ^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `f2`
168174

169175
error: redundant closure
170-
--> tests/ui/eta.rs:436:22
176+
--> tests/ui/eta.rs:441:22
171177
|
172178
LL | test.map(|t| t.method())
173179
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `Test::method`
174180

175181
error: redundant closure
176-
--> tests/ui/eta.rs:440:22
182+
--> tests/ui/eta.rs:445:22
177183
|
178184
LL | test.map(|t| t.method())
179185
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `super::Outer::method`
180186

181187
error: redundant closure
182-
--> tests/ui/eta.rs:453:18
188+
--> tests/ui/eta.rs:458:18
183189
|
184190
LL | test.map(|t| t.method())
185191
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `test_mod::Test::method`
186192

187193
error: redundant closure
188-
--> tests/ui/eta.rs:460:30
194+
--> tests/ui/eta.rs:465:30
189195
|
190196
LL | test.map(|t| t.method())
191197
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `crate::issue_10854::d::Test::method`
192198

193199
error: redundant closure
194-
--> tests/ui/eta.rs:479:38
200+
--> tests/ui/eta.rs:484:38
195201
|
196202
LL | let x = Box::new(|| None.map(|x| f(x)));
197203
| ^^^^^^^^ help: replace the closure with the function itself: `&f`
198204

199205
error: redundant closure
200-
--> tests/ui/eta.rs:483:38
206+
--> tests/ui/eta.rs:488:38
201207
|
202208
LL | let x = Box::new(|| None.map(|x| f(x)));
203209
| ^^^^^^^^ help: replace the closure with the function itself: `f`
204210

205211
error: redundant closure
206-
--> tests/ui/eta.rs:500:35
212+
--> tests/ui/eta.rs:505:35
207213
|
208214
LL | let _field = bind.or_else(|| get_default()).unwrap();
209215
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `get_default`
210216

211-
error: aborting due to 34 previous errors
217+
error: aborting due to 35 previous errors
212218

0 commit comments

Comments
 (0)