about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-06-08 12:15:35 +0000
committerbors <bors@rust-lang.org>2023-06-08 12:15:35 +0000
commitb7c330fc7834fde83a804ff85c39c8a11833c1cc (patch)
treef876b29bc290f2ad70c26e0866926460f20209f5
parent60258b061d0e77f97bb40a377a53bb705178c0ba (diff)
parent9ff34acf211c1b2aa7b4a282b2dd38927e137169 (diff)
downloadrust-b7c330fc7834fde83a804ff85c39c8a11833c1cc.tar.gz
rust-b7c330fc7834fde83a804ff85c39c8a11833c1cc.zip
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`
-rw-r--r--clippy_lints/src/eta_reduction.rs8
-rw-r--r--tests/ui/eta.fixed6
-rw-r--r--tests/ui/eta.rs6
-rw-r--r--tests/ui/eta.stderr42
4 files changed, 41 insertions, 21 deletions
diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index c919b4de65d..58e62d1f3d3 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/clippy_lints/src/eta_reduction.rs
@@ -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::Rc);
             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 {
                 span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
                     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.
                                 snippet = format!("&mut {snippet}");
                         }
+
                         diag.span_suggestion(
                             expr.span,
                             "replace the closure with the function itself",
diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed
index b1baf462c0f..31a94cb10e8 100644
--- a/tests/ui/eta.fixed
+++ b/tests/ui/eta.fixed
@@ -46,6 +46,12 @@ fn main() {
 
     // issue #7224
     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 {
diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs
index e113c3d6cd6..978aecd24b3 100644
--- a/tests/ui/eta.rs
+++ b/tests/ui/eta.rs
@@ -46,6 +46,12 @@ fn main() {
 
     // issue #7224
     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 {
diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr
index a521fb86860..19be5fc7389 100644
--- a/tests/ui/eta.stderr
+++ b/tests/ui/eta.stderr
@@ -31,7 +31,7 @@ LL |     let e = Some(1u8).map(|a| generic(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic`
 
 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());
    |                                                   ^^^^^^^^^^^ 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`
 
 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());
    |                                                   ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo`
 
 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());
    |                                          ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear`
 
 error: redundant closure
-  --> $DIR/eta.rs:94:29
+  --> $DIR/eta.rs:100:29
    |
 LL |     let e = Some("str").map(|s| s.to_string());
    |                             ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string`
 
 error: redundant closure
-  --> $DIR/eta.rs:95:27
+  --> $DIR/eta.rs:101:27
    |
 LL |     let e = Some('a').map(|s| s.to_uppercase());
    |                           ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase`
 
 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();
    |                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase`
 
 error: redundant closure
-  --> $DIR/eta.rs:160:22
+  --> $DIR/eta.rs:166:22
    |
 LL |     requires_fn_once(|| x());
    |                      ^^^^^^ help: replace the closure with the function itself: `x`
 
 error: redundant closure
-  --> $DIR/eta.rs:167:27
+  --> $DIR/eta.rs:173:27
    |
 LL |     let a = Some(1u8).map(|a| foo_ptr(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr`
 
 error: redundant closure
-  --> $DIR/eta.rs:172:27
+  --> $DIR/eta.rs:178:27
    |
 LL |     let a = Some(1u8).map(|a| closure(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `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));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
 
 error: redundant closure
-  --> $DIR/eta.rs:205:28
+  --> $DIR/eta.rs:211: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
-  --> $DIR/eta.rs:206:28
+  --> $DIR/eta.rs:212: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
-  --> $DIR/eta.rs:213:21
+  --> $DIR/eta.rs:219:21
    |
 LL |         Some(1).map(|n| closure(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
 
 error: redundant closure
-  --> $DIR/eta.rs:217:21
+  --> $DIR/eta.rs:223:21
    |
 LL |         Some(1).map(|n| in_loop(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
 
 error: redundant closure
-  --> $DIR/eta.rs:310:18
+  --> $DIR/eta.rs:316:18
    |
 LL |     takes_fn_mut(|| f());
    |                  ^^^^^^ help: replace the closure with the function itself: `&mut f`
 
 error: redundant closure
-  --> $DIR/eta.rs:313:19
+  --> $DIR/eta.rs:319:19
    |
 LL |     takes_fn_once(|| f());
    |                   ^^^^^^ help: replace the closure with the function itself: `&mut f`
 
 error: redundant closure
-  --> $DIR/eta.rs:317:26
+  --> $DIR/eta.rs:323:26
    |
 LL |     move || takes_fn_mut(|| f_used_once())
    |                          ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
 
 error: redundant closure
-  --> $DIR/eta.rs:329:19
+  --> $DIR/eta.rs:335: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
+  --> $DIR/eta.rs:338: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
+  --> $DIR/eta.rs:341: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
+  --> $DIR/eta.rs:345:17
    |
 LL |     dyn_opt.map(|d| d.method_on_dyn());
    |                 ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`