From b975be090ded01490af466ed6399bc9e6b7869ae Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 16 May 2019 09:47:21 -0700 Subject: [PATCH] Backport #4101 /~https://github.com/rust-lang/rust-clippy/pull/4101 Splits up redundant_closure's method checking into a pedantic lint --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/eta_reduction.rs | 26 ++++++++++- clippy_lints/src/lib.rs | 1 + tests/ui/eta.rs | 4 +- tests/ui/eta.stderr | 76 ++++--------------------------- tests/ui/map_clone.fixed | 2 +- tests/ui/map_clone.rs | 2 +- tests/ui/map_clone.stderr | 18 +++++++- 9 files changed, 59 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7837b899b533..aca02b53d017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -978,6 +978,7 @@ All notable changes to this project will be documented in this file. [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call +[`redundant_closure_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching diff --git a/README.md b/README.md index 43b477d2cef3..523704a86f33 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](/~https://github.com/rust-lang/rust) code. -[There are 298 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 299 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index 425f1e671477..afbee6f8ded2 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -33,9 +33,31 @@ declare_clippy_lint! { "redundant closures, i.e., `|a| foo(a)` (which can be written as just `foo`)" } +declare_clippy_lint! { + /// **What it does:** Checks for closures which only invoke a method on the closure + /// argument and can be replaced by referencing the method directly. + /// + /// **Why is this bad?** It's unnecessary to create the closure. + /// + /// **Known problems:** rust-lang/rust-clippy#3071, rust-lang/rust-clippy#4002, + /// rust-lang/rust-clippy#3942 + /// + /// **Example:** + /// ```rust,ignore + /// Some('a').map(|s| s.to_uppercase()); + /// ``` + /// may be rewritten as + /// ```rust,ignore + /// Some('a').map(char::to_uppercase); + /// ``` + pub REDUNDANT_CLOSURE_FOR_METHOD_CALLS, + pedantic, + "redundant closures for method calls" +} + impl LintPass for EtaPass { fn get_lints(&self) -> LintArray { - lint_array!(REDUNDANT_CLOSURE) + lint_array!(REDUNDANT_CLOSURE, REDUNDANT_CLOSURE_FOR_METHOD_CALLS) } fn name(&self) -> &'static str { @@ -110,7 +132,7 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr) { if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]); then { - span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| { + span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure found", |db| { db.span_suggestion( expr.span, "remove closure as shown", diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d547e0a99b79..dca0badc29f2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -611,6 +611,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { enum_glob_use::ENUM_GLOB_USE, enum_variants::MODULE_NAME_REPETITIONS, enum_variants::PUB_ENUM_VARIANT_NAMES, + eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS, functions::TOO_MANY_LINES, if_not_else::IF_NOT_ELSE, infinite_iter::MAYBE_INFINITE_ITER, diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index f777939c67d2..e71a0d0a1ac7 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -7,7 +7,9 @@ clippy::option_map_unit_fn, clippy::trivially_copy_pass_by_ref )] -#![warn(clippy::redundant_closure, clippy::needless_borrow)] +#![warn(clippy::redundant_closure, + redundant_closures_for_method_calls, + clippy::needless_borrow)] use std::path::PathBuf; diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index 5f56cd7912a7..50985fdbcd32 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -1,72 +1,16 @@ -error: redundant closure found - --> $DIR/eta.rs:15:27 +error: unknown lint: `redundant_closures_for_method_calls` + --> $DIR/eta.rs:11:9 | -LL | let a = Some(1u8).map(|a| foo(a)); - | ^^^^^^^^^^ help: remove closure as shown: `foo` +LL | redundant_closures_for_method_calls, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::redundant_closure_for_method_calls` | - = note: `-D clippy::redundant-closure` implied by `-D warnings` + = note: `-D unknown-lints` implied by `-D warnings` -error: redundant closure found - --> $DIR/eta.rs:16:10 +error: unknown lint: `redundant_closures_for_method_calls` + --> $DIR/eta.rs:11:9 | -LL | meta(|a| foo(a)); - | ^^^^^^^^^^ help: remove closure as shown: `foo` +LL | redundant_closures_for_method_calls, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::redundant_closure_for_method_calls` -error: redundant closure found - --> $DIR/eta.rs:17:27 - | -LL | let c = Some(1u8).map(|a| {1+2; foo}(a)); - | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `{1+2; foo}` - -error: this expression borrows a reference that is immediately dereferenced by the compiler - --> $DIR/eta.rs:19:21 - | -LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted - | ^^^ help: change this to: `&2` - | - = note: `-D clippy::needless-borrow` implied by `-D warnings` - -error: redundant closure found - --> $DIR/eta.rs:26:27 - | -LL | let e = Some(1u8).map(|a| generic(a)); - | ^^^^^^^^^^^^^^ help: remove closure as shown: `generic` - -error: redundant closure found - --> $DIR/eta.rs:69:51 - | -LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); - | ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo` - -error: redundant closure found - --> $DIR/eta.rs:71: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:74: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:79: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:81: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:84: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: aborting due to 11 previous errors +error: aborting due to 2 previous errors diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed index d804e838d5a6..2dd9e80dcaeb 100644 --- a/tests/ui/map_clone.fixed +++ b/tests/ui/map_clone.fixed @@ -3,7 +3,7 @@ #![allow(clippy::iter_cloned_collect)] #![allow(clippy::clone_on_copy)] #![allow(clippy::missing_docs_in_private_items)] -#![allow(clippy::redundant_closure)] +#![allow(clippy::redundant_closures_for_method_calls)] fn main() { let _: Vec = vec![5_i8; 6].iter().cloned().collect(); diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs index d98cd939d8cc..49fba3d98198 100644 --- a/tests/ui/map_clone.rs +++ b/tests/ui/map_clone.rs @@ -3,7 +3,7 @@ #![allow(clippy::iter_cloned_collect)] #![allow(clippy::clone_on_copy)] #![allow(clippy::missing_docs_in_private_items)] -#![allow(clippy::redundant_closure)] +#![allow(clippy::redundant_closures_for_method_calls)] fn main() { let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr index db7fa4f52fce..f41d496df954 100644 --- a/tests/ui/map_clone.stderr +++ b/tests/ui/map_clone.stderr @@ -6,6 +6,14 @@ LL | let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); | = note: `-D clippy::map-clone` implied by `-D warnings` +error: redundant closure found + --> $DIR/map_clone.rs:10:57 + | +LL | let _: Vec = vec![String::new()].iter().map(|x| x.clone()).collect(); + | ^^^^^^^^^^^^^ help: remove closure as shown: `std::clone::Clone::clone` + | + = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` + error: You are using an explicit closure for cloning elements --> $DIR/map_clone.rs:10:26 | @@ -24,5 +32,13 @@ error: You are needlessly cloning iterator elements LL | let _ = std::env::args().map(|v| v.clone()); | ^^^^^^^^^^^^^^^^^^^ help: Remove the map call -error: aborting due to 4 previous errors +error: unknown clippy lint: clippy::redundant_closures_for_method_calls + --> $DIR/map_clone.rs:6:10 + | +LL | #![allow(clippy::redundant_closures_for_method_calls)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unknown-clippy-lints` implied by `-D warnings` + +error: aborting due to 6 previous errors