Skip to content

Commit aa3eb07

Browse files
borsbors
committed
Auto merge of #4101 - mikerite:redundant_closures_for_method_calls, r=Manishearth
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 changelog: Move method checking from `redundant_closure` to a new `pedantic` lint called `redundant_closure_for_method_calls`.
2 parents f49d878 + ce63f3a commit aa3eb07

File tree

9 files changed

+56
-21
lines changed

9 files changed

+56
-21
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,7 @@ All notable changes to this project will be documented in this file.
10351035
[`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
10361036
[`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
10371037
[`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call
1038+
[`redundant_closure_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls
10381039
[`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
10391040
[`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
10401041
[`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
99

10-
[There are 301 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
10+
[There are 302 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1111

1212
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1313

‎clippy_lints/src/eta_reduction.rs

+25-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,30 @@ declare_clippy_lint! {
3232
"redundant closures, i.e., `|a| foo(a)` (which can be written as just `foo`)"
3333
}
3434

35-
declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE]);
35+
declare_clippy_lint! {
36+
/// **What it does:** Checks for closures which only invoke a method on the closure
37+
/// argument and can be replaced by referencing the method directly.
38+
///
39+
/// **Why is this bad?** It's unnecessary to create the closure.
40+
///
41+
/// **Known problems:** rust-lang/rust-clippy#3071, rust-lang/rust-clippy#4002,
42+
/// rust-lang/rust-clippy#3942
43+
///
44+
///
45+
/// **Example:**
46+
/// ```rust,ignore
47+
/// Some('a').map(|s| s.to_uppercase());
48+
/// ```
49+
/// may be rewritten as
50+
/// ```rust,ignore
51+
/// Some('a').map(char::to_uppercase);
52+
/// ```
53+
pub REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
54+
pedantic,
55+
"redundant closures for method calls"
56+
}
57+
58+
declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE, REDUNDANT_CLOSURE_FOR_METHOD_CALLS]);
3659

3760
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EtaReduction {
3861
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@@ -104,7 +127,7 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr) {
104127
if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]);
105128

106129
then {
107-
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| {
130+
span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure found", |db| {
108131
db.span_suggestion(
109132
expr.span,
110133
"remove closure as shown",

‎clippy_lints/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
614614
enum_glob_use::ENUM_GLOB_USE,
615615
enum_variants::MODULE_NAME_REPETITIONS,
616616
enum_variants::PUB_ENUM_VARIANT_NAMES,
617+
eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
617618
functions::TOO_MANY_LINES,
618619
if_not_else::IF_NOT_ELSE,
619620
infinite_iter::MAYBE_INFINITE_ITER,

‎tests/ui/eta.fixed

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
clippy::option_map_unit_fn,
1010
clippy::trivially_copy_pass_by_ref
1111
)]
12-
#![warn(clippy::redundant_closure, clippy::needless_borrow)]
12+
#![warn(
13+
clippy::redundant_closure,
14+
clippy::redundant_closures_for_method_calls,
15+
clippy::needless_borrow
16+
)]
1317

1418
use std::path::PathBuf;
1519

‎tests/ui/eta.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
clippy::option_map_unit_fn,
1010
clippy::trivially_copy_pass_by_ref
1111
)]
12-
#![warn(clippy::redundant_closure, clippy::needless_borrow)]
12+
#![warn(
13+
clippy::redundant_closure,
14+
clippy::redundant_closures_for_method_calls,
15+
clippy::needless_borrow
16+
)]
1317

1418
use std::path::PathBuf;
1519

‎tests/ui/eta.stderr

+16-14
Original file line numberDiff line numberDiff line change
@@ -1,87 +1,89 @@
11
error: redundant closure found
2-
--> $DIR/eta.rs:17:27
2+
--> $DIR/eta.rs:21:27
33
|
44
LL | let a = Some(1u8).map(|a| foo(a));
55
| ^^^^^^^^^^ help: remove closure as shown: `foo`
66
|
77
= note: `-D clippy::redundant-closure` implied by `-D warnings`
88

99
error: redundant closure found
10-
--> $DIR/eta.rs:18:10
10+
--> $DIR/eta.rs:22:10
1111
|
1212
LL | meta(|a| foo(a));
1313
| ^^^^^^^^^^ help: remove closure as shown: `foo`
1414

1515
error: redundant closure found
16-
--> $DIR/eta.rs:19:27
16+
--> $DIR/eta.rs:23:27
1717
|
1818
LL | let c = Some(1u8).map(|a| {1+2; foo}(a));
1919
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `{1+2; foo}`
2020

2121
error: this expression borrows a reference that is immediately dereferenced by the compiler
22-
--> $DIR/eta.rs:21:21
22+
--> $DIR/eta.rs:25:21
2323
|
2424
LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted
2525
| ^^^ help: change this to: `&2`
2626
|
2727
= note: `-D clippy::needless-borrow` implied by `-D warnings`
2828

2929
error: redundant closure found
30-
--> $DIR/eta.rs:28:27
30+
--> $DIR/eta.rs:32:27
3131
|
3232
LL | let e = Some(1u8).map(|a| generic(a));
3333
| ^^^^^^^^^^^^^^ help: remove closure as shown: `generic`
3434

3535
error: redundant closure found
36-
--> $DIR/eta.rs:71:51
36+
--> $DIR/eta.rs:75:51
3737
|
3838
LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo());
3939
| ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo`
40+
|
41+
= note: `-D clippy::redundant-closures-for-method-calls` implied by `-D warnings`
4042

4143
error: redundant closure found
42-
--> $DIR/eta.rs:73:51
44+
--> $DIR/eta.rs:77:51
4345
|
4446
LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo());
4547
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `TestTrait::trait_foo`
4648

4749
error: redundant closure found
48-
--> $DIR/eta.rs:76:42
50+
--> $DIR/eta.rs:80:42
4951
|
5052
LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear());
5153
| ^^^^^^^^^^^^^ help: remove closure as shown: `std::vec::Vec::clear`
5254

5355
error: redundant closure found
54-
--> $DIR/eta.rs:81:29
56+
--> $DIR/eta.rs:85:29
5557
|
5658
LL | let e = Some("str").map(|s| s.to_string());
5759
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string`
5860

5961
error: redundant closure found
60-
--> $DIR/eta.rs:83:27
62+
--> $DIR/eta.rs:87:27
6163
|
6264
LL | let e = Some('a').map(|s| s.to_uppercase());
6365
| ^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_uppercase`
6466

6567
error: redundant closure found
66-
--> $DIR/eta.rs:86:65
68+
--> $DIR/eta.rs:90:65
6769
|
6870
LL | let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect();
6971
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase`
7072

7173
error: redundant closure found
72-
--> $DIR/eta.rs:104:50
74+
--> $DIR/eta.rs:108:50
7375
|
7476
LL | let _: Vec<_> = arr.iter().map(|x| x.map_err(|e| some.take().unwrap()(e))).collect();
7577
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `some.take().unwrap()`
7678

7779
error: redundant closure found
78-
--> $DIR/eta.rs:169:27
80+
--> $DIR/eta.rs:173:27
7981
|
8082
LL | let a = Some(1u8).map(|a| foo_ptr(a));
8183
| ^^^^^^^^^^^^^^ help: remove closure as shown: `foo_ptr`
8284

8385
error: redundant closure found
84-
--> $DIR/eta.rs:174:27
86+
--> $DIR/eta.rs:178:27
8587
|
8688
LL | let a = Some(1u8).map(|a| closure(a));
8789
| ^^^^^^^^^^^^^^ help: remove closure as shown: `closure`

‎tests/ui/map_clone.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
#![allow(clippy::iter_cloned_collect)]
44
#![allow(clippy::clone_on_copy)]
55
#![allow(clippy::missing_docs_in_private_items)]
6-
#![allow(clippy::redundant_closure)]
6+
#![allow(clippy::redundant_closures_for_method_calls)]
77

88
fn main() {
99
let _: Vec<i8> = vec![5_i8; 6].iter().copied().collect();

‎tests/ui/map_clone.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
#![allow(clippy::iter_cloned_collect)]
44
#![allow(clippy::clone_on_copy)]
55
#![allow(clippy::missing_docs_in_private_items)]
6-
#![allow(clippy::redundant_closure)]
6+
#![allow(clippy::redundant_closures_for_method_calls)]
77

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

0 commit comments

Comments
 (0)