From bf98aa6fb8e02db15ad18d517145f13a5bed2921 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 8 Mar 2021 12:59:58 -0600 Subject: [PATCH] Fix redundant closure with macros --- clippy_lints/src/eta_reduction.rs | 24 ++++++++++++++++++++++- tests/ui/eta.fixed | 15 +++++++++++++++ tests/ui/eta.rs | 15 +++++++++++++++ tests/ui/eta.stderr | 32 ++++++++++++++++++------------- 4 files changed, 72 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index 1a722d39f73..ab385fc1f8a 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -10,6 +10,8 @@ use crate::utils::{ implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_sugg, span_lint_and_then, type_is_unsafe_function, }; +use clippy_utils::higher; +use clippy_utils::higher::VecArgs; declare_clippy_lint! { /// **What it does:** Checks for closures which just call another function where @@ -74,7 +76,10 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction { match expr.kind { ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) => { for arg in args { - check_closure(cx, arg) + // skip `foo(macro!())` + if arg.span.ctxt() == expr.span.ctxt() { + check_closure(cx, arg) + } } }, _ => (), @@ -87,6 +92,23 @@ fn check_closure(cx: &LateContext<'_>, expr: &Expr<'_>) { let body = cx.tcx.hir().body(eid); let ex = &body.value; + if ex.span.ctxt() != expr.span.ctxt() { + if let Some(VecArgs::Vec(&[])) = higher::vec_macro(cx, ex) { + // replace `|| vec![]` with `Vec::new` + span_lint_and_sugg( + cx, + REDUNDANT_CLOSURE, + expr.span, + "redundant closure found", + "remove closure as shown", + "std::vec::Vec::new".into(), + Applicability::MachineApplicable, + ); + } + // skip `foo(|| macro!())` + return; + } + if_chain!( if let ExprKind::Call(ref caller, ref args) = ex.kind; diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index 1b34c2f74eb..2be2283e3fd 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -16,10 +16,25 @@ use std::path::PathBuf; +macro_rules! mac { + () => { + foobar() + }; +} + +macro_rules! closure_mac { + () => { + |n| foo(n) + }; +} + fn main() { let a = Some(1u8).map(foo); meta(foo); let c = Some(1u8).map(|a| {1+2; foo}(a)); + true.then(|| mac!()); // don't lint function in macro expansion + Some(1).map(closure_mac!()); // don't lint closure in macro expansion + let _: Option> = true.then(std::vec::Vec::new); // special case vec! let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? all(&[1, 2, 3], &2, |x, y| below(x, y)); //is adjusted unsafe { diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index 4f050bd8479..f0373f9ccf6 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -16,10 +16,25 @@ use std::path::PathBuf; +macro_rules! mac { + () => { + foobar() + }; +} + +macro_rules! closure_mac { + () => { + |n| foo(n) + }; +} + fn main() { let a = Some(1u8).map(|a| foo(a)); meta(|a| foo(a)); let c = Some(1u8).map(|a| {1+2; foo}(a)); + true.then(|| mac!()); // don't lint function in macro expansion + Some(1).map(closure_mac!()); // don't lint closure in macro expansion + let _: Option> = true.then(|| vec![]); // special case vec! let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted unsafe { diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index 16aa1b07733..34d6dd46311 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -1,5 +1,5 @@ error: redundant closure found - --> $DIR/eta.rs:20:27 + --> $DIR/eta.rs:32:27 | LL | let a = Some(1u8).map(|a| foo(a)); | ^^^^^^^^^^ help: remove closure as shown: `foo` @@ -7,13 +7,19 @@ LL | let a = Some(1u8).map(|a| foo(a)); = note: `-D clippy::redundant-closure` implied by `-D warnings` error: redundant closure found - --> $DIR/eta.rs:21:10 + --> $DIR/eta.rs:33:10 | LL | meta(|a| foo(a)); | ^^^^^^^^^^ help: remove closure as shown: `foo` +error: redundant closure found + --> $DIR/eta.rs:37:40 + | +LL | let _: Option> = true.then(|| vec![]); // special case vec! + | ^^^^^^^^^ help: remove closure as shown: `std::vec::Vec::new` + error: this expression borrows a reference (`&u8`) that is immediately dereferenced by the compiler - --> $DIR/eta.rs:24:21 + --> $DIR/eta.rs:39:21 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted | ^^^ help: change this to: `&2` @@ -21,13 +27,13 @@ LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted = note: `-D clippy::needless-borrow` implied by `-D warnings` error: redundant closure found - --> $DIR/eta.rs:31:27 + --> $DIR/eta.rs:46:27 | LL | let e = Some(1u8).map(|a| generic(a)); | ^^^^^^^^^^^^^^ help: remove closure as shown: `generic` error: redundant closure found - --> $DIR/eta.rs:74:51 + --> $DIR/eta.rs:89:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); | ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo` @@ -35,46 +41,46 @@ 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 found - --> $DIR/eta.rs:76:51 + --> $DIR/eta.rs:91:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `TestTrait::trait_foo` error: redundant closure found - --> $DIR/eta.rs:79:42 + --> $DIR/eta.rs:94:42 | LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); | ^^^^^^^^^^^^^ help: remove closure as shown: `std::vec::Vec::clear` error: redundant closure found - --> $DIR/eta.rs:84:29 + --> $DIR/eta.rs:99:29 | LL | let e = Some("str").map(|s| s.to_string()); | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string` error: redundant closure found - --> $DIR/eta.rs:86:27 + --> $DIR/eta.rs:101:27 | LL | let e = Some('a').map(|s| s.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_uppercase` error: redundant closure found - --> $DIR/eta.rs:89:65 + --> $DIR/eta.rs:104:65 | LL | let e: std::vec::Vec = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase` error: redundant closure found - --> $DIR/eta.rs:172:27 + --> $DIR/eta.rs:187:27 | LL | let a = Some(1u8).map(|a| foo_ptr(a)); | ^^^^^^^^^^^^^^ help: remove closure as shown: `foo_ptr` error: redundant closure found - --> $DIR/eta.rs:177:27 + --> $DIR/eta.rs:192:27 | LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: remove closure as shown: `closure` -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors