Auto merge of #10905 - y21:issue10684, r=Alexendoo

[`redundant_closure`]: special case inclusive ranges

Fixes #10684.

`x..=y` ranges need a bit of special handling in this lint because it desugars to a call to the lang item `RangeInclusiveNew`, where the callee span would be the same as the range expression itself, so the suggestion looked a bit weird. It now correctly suggests `RangeInclusive::new`.

changelog: [`redundant_closure`]: special case `RangeInclusive`
This commit is contained in:
bors 2023-06-08 12:15:35 +00:00
commit b7c330fc78
4 changed files with 41 additions and 21 deletions

View file

@ -120,6 +120,13 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
if !is_type_diagnostic_item(cx, callee_ty_unadjusted, sym::Arc); if !is_type_diagnostic_item(cx, callee_ty_unadjusted, sym::Arc);
if !is_type_diagnostic_item(cx, callee_ty_unadjusted, sym::Rc); if !is_type_diagnostic_item(cx, callee_ty_unadjusted, sym::Rc);
if let ty::Closure(_, substs) = *closure_ty.kind(); if let ty::Closure(_, substs) = *closure_ty.kind();
// Don't lint if this is an inclusive range expression.
// They desugar to a call to `RangeInclusiveNew` which would have odd suggestions. (#10684)
if !matches!(higher::Range::hir(body.value), Some(higher::Range {
start: Some(_),
end: Some(_),
limits: rustc_ast::RangeLimits::Closed
}));
then { then {
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| { span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
if let Some(mut snippet) = snippet_opt(cx, callee.span) { if let Some(mut snippet) = snippet_opt(cx, callee.span) {
@ -136,6 +143,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
// Mutable closure is used after current expr; we cannot consume it. // Mutable closure is used after current expr; we cannot consume it.
snippet = format!("&mut {snippet}"); snippet = format!("&mut {snippet}");
} }
diag.span_suggestion( diag.span_suggestion(
expr.span, expr.span,
"replace the closure with the function itself", "replace the closure with the function itself",

View file

@ -46,6 +46,12 @@ fn main() {
// issue #7224 // issue #7224
let _: Option<Vec<u32>> = Some(0).map(|_| vec![]); let _: Option<Vec<u32>> = Some(0).map(|_| vec![]);
// issue #10684
fn test<T>(x: impl Fn(usize, usize) -> T) -> T {
x(1, 2)
}
test(|start, end| start..=end);
} }
trait TestTrait { trait TestTrait {

View file

@ -46,6 +46,12 @@ fn main() {
// issue #7224 // issue #7224
let _: Option<Vec<u32>> = Some(0).map(|_| vec![]); let _: Option<Vec<u32>> = Some(0).map(|_| vec![]);
// issue #10684
fn test<T>(x: impl Fn(usize, usize) -> T) -> T {
x(1, 2)
}
test(|start, end| start..=end);
} }
trait TestTrait { trait TestTrait {

View file

@ -31,7 +31,7 @@ LL | let e = Some(1u8).map(|a| generic(a));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic` | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:87:51 --> $DIR/eta.rs:93:51
| |
LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo());
| ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo` | ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo`
@ -39,121 +39,121 @@ LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo());
= note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:88:51 --> $DIR/eta.rs:94:51
| |
LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo());
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo` | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:90:42 --> $DIR/eta.rs:96:42
| |
LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear());
| ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear` | ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:94:29 --> $DIR/eta.rs:100:29
| |
LL | let e = Some("str").map(|s| s.to_string()); LL | let e = Some("str").map(|s| s.to_string());
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string` | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:95:27 --> $DIR/eta.rs:101:27
| |
LL | let e = Some('a').map(|s| s.to_uppercase()); LL | let e = Some('a').map(|s| s.to_uppercase());
| ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase` | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:97:65 --> $DIR/eta.rs:103:65
| |
LL | let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); LL | let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase` | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:160:22 --> $DIR/eta.rs:166:22
| |
LL | requires_fn_once(|| x()); LL | requires_fn_once(|| x());
| ^^^^^^ help: replace the closure with the function itself: `x` | ^^^^^^ help: replace the closure with the function itself: `x`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:167:27 --> $DIR/eta.rs:173:27
| |
LL | let a = Some(1u8).map(|a| foo_ptr(a)); LL | let a = Some(1u8).map(|a| foo_ptr(a));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr` | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:172:27 --> $DIR/eta.rs:178:27
| |
LL | let a = Some(1u8).map(|a| closure(a)); LL | let a = Some(1u8).map(|a| closure(a));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure` | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:204:28 --> $DIR/eta.rs:210:28
| |
LL | x.into_iter().for_each(|x| add_to_res(x)); LL | x.into_iter().for_each(|x| add_to_res(x));
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:205:28 --> $DIR/eta.rs:211:28
| |
LL | y.into_iter().for_each(|x| add_to_res(x)); LL | y.into_iter().for_each(|x| add_to_res(x));
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:206:28 --> $DIR/eta.rs:212:28
| |
LL | z.into_iter().for_each(|x| add_to_res(x)); LL | z.into_iter().for_each(|x| add_to_res(x));
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res` | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:213:21 --> $DIR/eta.rs:219:21
| |
LL | Some(1).map(|n| closure(n)); LL | Some(1).map(|n| closure(n));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure` | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:217:21 --> $DIR/eta.rs:223:21
| |
LL | Some(1).map(|n| in_loop(n)); LL | Some(1).map(|n| in_loop(n));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop` | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:310:18 --> $DIR/eta.rs:316:18
| |
LL | takes_fn_mut(|| f()); LL | takes_fn_mut(|| f());
| ^^^^^^ help: replace the closure with the function itself: `&mut f` | ^^^^^^ help: replace the closure with the function itself: `&mut f`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:313:19 --> $DIR/eta.rs:319:19
| |
LL | takes_fn_once(|| f()); LL | takes_fn_once(|| f());
| ^^^^^^ help: replace the closure with the function itself: `&mut f` | ^^^^^^ help: replace the closure with the function itself: `&mut f`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:317:26 --> $DIR/eta.rs:323:26
| |
LL | move || takes_fn_mut(|| f_used_once()) LL | move || takes_fn_mut(|| f_used_once())
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once` | ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:329:19 --> $DIR/eta.rs:335:19
| |
LL | array_opt.map(|a| a.as_slice()); LL | array_opt.map(|a| a.as_slice());
| ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice` | ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:332:19 --> $DIR/eta.rs:338:19
| |
LL | slice_opt.map(|s| s.len()); LL | slice_opt.map(|s| s.len());
| ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len` | ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:335:17 --> $DIR/eta.rs:341:17
| |
LL | ptr_opt.map(|p| p.is_null()); LL | ptr_opt.map(|p| p.is_null());
| ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null` | ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
error: redundant closure error: redundant closure
--> $DIR/eta.rs:339:17 --> $DIR/eta.rs:345:17
| |
LL | dyn_opt.map(|d| d.method_on_dyn()); LL | dyn_opt.map(|d| d.method_on_dyn());
| ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn` | ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`