Split redundant_closure lint

Move the method checking into a new lint called
`redundant_closures_for_method_calls` and put it in the pedantic group.

This aspect of the lint seems more controversial than the rest.

cc #3942
This commit is contained in:
Michael Wright 2019-05-16 08:25:39 +02:00
parent f49d878ce5
commit 4fcaab3d62
9 changed files with 52 additions and 21 deletions

View file

@ -1035,6 +1035,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_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`]: 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_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call
[`redundant_closures_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closures_for_method_calls
[`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names [`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`]: 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 [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching

View file

@ -7,7 +7,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
[There are 301 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) [There are 302 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: We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

View file

@ -32,7 +32,26 @@ declare_clippy_lint! {
"redundant closures, i.e., `|a| foo(a)` (which can be written as just `foo`)" "redundant closures, i.e., `|a| foo(a)` (which can be written as just `foo`)"
} }
declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE]); 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.
///
/// **Example:**
/// ```rust,ignore
/// Some('a').map(|s| s.to_uppercase());
/// ```
/// may be rewritten as
/// ```rust,ignore
/// Some('a').map(char::to_uppercase);
/// ```
pub REDUNDANT_CLOSURES_FOR_METHOD_CALLS,
pedantic,
"redundant closures for method calls"
}
declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE, REDUNDANT_CLOSURES_FOR_METHOD_CALLS]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EtaReduction { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EtaReduction {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@ -104,7 +123,7 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr) {
if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]); if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]);
then { then {
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| { span_lint_and_then(cx, REDUNDANT_CLOSURES_FOR_METHOD_CALLS, expr.span, "redundant closure found", |db| {
db.span_suggestion( db.span_suggestion(
expr.span, expr.span,
"remove closure as shown", "remove closure as shown",

View file

@ -614,6 +614,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
enum_glob_use::ENUM_GLOB_USE, enum_glob_use::ENUM_GLOB_USE,
enum_variants::MODULE_NAME_REPETITIONS, enum_variants::MODULE_NAME_REPETITIONS,
enum_variants::PUB_ENUM_VARIANT_NAMES, enum_variants::PUB_ENUM_VARIANT_NAMES,
eta_reduction::REDUNDANT_CLOSURES_FOR_METHOD_CALLS,
functions::TOO_MANY_LINES, functions::TOO_MANY_LINES,
if_not_else::IF_NOT_ELSE, if_not_else::IF_NOT_ELSE,
infinite_iter::MAYBE_INFINITE_ITER, infinite_iter::MAYBE_INFINITE_ITER,

View file

@ -9,7 +9,11 @@
clippy::option_map_unit_fn, clippy::option_map_unit_fn,
clippy::trivially_copy_pass_by_ref clippy::trivially_copy_pass_by_ref
)] )]
#![warn(clippy::redundant_closure, clippy::needless_borrow)] #![warn(
clippy::redundant_closure,
clippy::redundant_closures_for_method_calls,
clippy::needless_borrow
)]
use std::path::PathBuf; use std::path::PathBuf;

View file

@ -9,7 +9,11 @@
clippy::option_map_unit_fn, clippy::option_map_unit_fn,
clippy::trivially_copy_pass_by_ref clippy::trivially_copy_pass_by_ref
)] )]
#![warn(clippy::redundant_closure, clippy::needless_borrow)] #![warn(
clippy::redundant_closure,
clippy::redundant_closures_for_method_calls,
clippy::needless_borrow
)]
use std::path::PathBuf; use std::path::PathBuf;

View file

@ -1,5 +1,5 @@
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:17:27 --> $DIR/eta.rs:21:27
| |
LL | let a = Some(1u8).map(|a| foo(a)); LL | let a = Some(1u8).map(|a| foo(a));
| ^^^^^^^^^^ help: remove closure as shown: `foo` | ^^^^^^^^^^ help: remove closure as shown: `foo`
@ -7,19 +7,19 @@ LL | let a = Some(1u8).map(|a| foo(a));
= note: `-D clippy::redundant-closure` implied by `-D warnings` = note: `-D clippy::redundant-closure` implied by `-D warnings`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:18:10 --> $DIR/eta.rs:22:10
| |
LL | meta(|a| foo(a)); LL | meta(|a| foo(a));
| ^^^^^^^^^^ help: remove closure as shown: `foo` | ^^^^^^^^^^ help: remove closure as shown: `foo`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:19:27 --> $DIR/eta.rs:23:27
| |
LL | let c = Some(1u8).map(|a| {1+2; foo}(a)); LL | let c = Some(1u8).map(|a| {1+2; foo}(a));
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `{1+2; foo}` | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `{1+2; foo}`
error: this expression borrows a reference that is immediately dereferenced by the compiler error: this expression borrows a reference that is immediately dereferenced by the compiler
--> $DIR/eta.rs:21:21 --> $DIR/eta.rs:25:21
| |
LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted
| ^^^ help: change this to: `&2` | ^^^ help: change this to: `&2`
@ -27,61 +27,63 @@ LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted
= note: `-D clippy::needless-borrow` implied by `-D warnings` = note: `-D clippy::needless-borrow` implied by `-D warnings`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:28:27 --> $DIR/eta.rs:32:27
| |
LL | let e = Some(1u8).map(|a| generic(a)); LL | let e = Some(1u8).map(|a| generic(a));
| ^^^^^^^^^^^^^^ help: remove closure as shown: `generic` | ^^^^^^^^^^^^^^ help: remove closure as shown: `generic`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:71:51 --> $DIR/eta.rs:75: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: remove closure as shown: `TestStruct::foo` | ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo`
|
= note: `-D clippy::redundant-closures-for-method-calls` implied by `-D warnings`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:73:51 --> $DIR/eta.rs:77: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: remove closure as shown: `TestTrait::trait_foo` | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `TestTrait::trait_foo`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:76:42 --> $DIR/eta.rs:80: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: remove closure as shown: `std::vec::Vec::clear` | ^^^^^^^^^^^^^ help: remove closure as shown: `std::vec::Vec::clear`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:81:29 --> $DIR/eta.rs:85:29
| |
LL | let e = Some("str").map(|s| s.to_string()); LL | let e = Some("str").map(|s| s.to_string());
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string` | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:83:27 --> $DIR/eta.rs:87:27
| |
LL | let e = Some('a').map(|s| s.to_uppercase()); LL | let e = Some('a').map(|s| s.to_uppercase());
| ^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_uppercase` | ^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_uppercase`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:86:65 --> $DIR/eta.rs:90: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: remove closure as shown: `char::to_ascii_uppercase` | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:104:50 --> $DIR/eta.rs:108:50
| |
LL | let _: Vec<_> = arr.iter().map(|x| x.map_err(|e| some.take().unwrap()(e))).collect(); LL | let _: Vec<_> = arr.iter().map(|x| x.map_err(|e| some.take().unwrap()(e))).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `some.take().unwrap()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `some.take().unwrap()`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:169: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: remove closure as shown: `foo_ptr` | ^^^^^^^^^^^^^^ help: remove closure as shown: `foo_ptr`
error: redundant closure found error: redundant closure found
--> $DIR/eta.rs:174: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: remove closure as shown: `closure` | ^^^^^^^^^^^^^^ help: remove closure as shown: `closure`

View file

@ -3,7 +3,7 @@
#![allow(clippy::iter_cloned_collect)] #![allow(clippy::iter_cloned_collect)]
#![allow(clippy::clone_on_copy)] #![allow(clippy::clone_on_copy)]
#![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::missing_docs_in_private_items)]
#![allow(clippy::redundant_closure)] #![allow(clippy::redundant_closures_for_method_calls)]
fn main() { fn main() {
let _: Vec<i8> = vec![5_i8; 6].iter().copied().collect(); let _: Vec<i8> = vec![5_i8; 6].iter().copied().collect();

View file

@ -3,7 +3,7 @@
#![allow(clippy::iter_cloned_collect)] #![allow(clippy::iter_cloned_collect)]
#![allow(clippy::clone_on_copy)] #![allow(clippy::clone_on_copy)]
#![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::missing_docs_in_private_items)]
#![allow(clippy::redundant_closure)] #![allow(clippy::redundant_closures_for_method_calls)]
fn main() { fn main() {
let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect(); let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();