Skip to content

Commit

Permalink
Backport #4101
Browse files Browse the repository at this point in the history
#4101

Splits up redundant_closure's method checking into a pedantic lint
  • Loading branch information
Manishearth authored and flip1995 committed May 17, 2019
1 parent 37f5c1e commit 1100773
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
26 changes: 24 additions & 2 deletions clippy_lints/src/eta_reduction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/eta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
clippy::option_map_unit_fn,
clippy::trivially_copy_pass_by_ref
)]
#![warn(clippy::redundant_closure, clippy::needless_borrow)]
#![warn(
clippy::redundant_closure,
clippy::redundant_closure_for_method_calls,
clippy::needless_borrow
)]

use std::path::PathBuf;

Expand Down
24 changes: 13 additions & 11 deletions tests/ui/eta.stderr
Original file line number Diff line number Diff line change
@@ -1,69 +1,71 @@
error: redundant closure found
--> $DIR/eta.rs:15:27
--> $DIR/eta.rs:19:27
|
LL | let a = Some(1u8).map(|a| foo(a));
| ^^^^^^^^^^ help: remove closure as shown: `foo`
|
= note: `-D clippy::redundant-closure` implied by `-D warnings`

error: redundant closure found
--> $DIR/eta.rs:16:10
--> $DIR/eta.rs:20:10
|
LL | meta(|a| foo(a));
| ^^^^^^^^^^ help: remove closure as shown: `foo`

error: redundant closure found
--> $DIR/eta.rs:17:27
--> $DIR/eta.rs:21: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
--> $DIR/eta.rs:23: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
--> $DIR/eta.rs:30: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
--> $DIR/eta.rs:73:51
|
LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo());
| ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo`
|
= note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings`

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.trait_foo());
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `TestTrait::trait_foo`

error: redundant closure found
--> $DIR/eta.rs:74:42
--> $DIR/eta.rs:78: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
--> $DIR/eta.rs:83: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
--> $DIR/eta.rs:85: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
--> $DIR/eta.rs:88:65
|
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`
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/map_clone.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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_closure_for_method_calls)]

fn main() {
let _: Vec<i8> = vec![5_i8; 6].iter().cloned().collect();
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/map_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_closure_for_method_calls)]

fn main() {
let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
Expand Down

0 comments on commit 1100773

Please sign in to comment.