Skip to content

Commit 797d50d

Browse files
borsbors
committed
Auto merge of #12562 - m-rph:12501, r=y21
Allow `filter_map_identity` when the closure is typed This extends the `filter_map_identity` lint to support typed closures. For untyped closures, we know that the program compiles, and therefore we can safely suggest using flatten. For typed closures, they may participate in type resolution. In this case we use `Applicability::MaybeIncorrect`. Details: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Should.20.60filter_map_identity.60.20lint.20when.20closures.20are.20typed.3F changelog: `filter_map_identity` will now suggest using flatten for typed closures. r? `@y21` && `@Centri3`
2 parents 3787a0c + 60937bf commit 797d50d

File tree

4 files changed

+288
-36
lines changed

4 files changed

+288
-36
lines changed

‎clippy_lints/src/methods/filter_map_identity.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,34 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::{is_expr_untyped_identity_function, is_trait_method};
2+
use clippy_utils::{is_expr_identity_function, is_expr_untyped_identity_function, is_trait_method};
33
use rustc_errors::Applicability;
44
use rustc_hir as hir;
55
use rustc_lint::LateContext;
66
use rustc_span::{sym, Span};
77

88
use super::FILTER_MAP_IDENTITY;
99

10+
fn is_identity(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Applicability> {
11+
if is_expr_untyped_identity_function(cx, expr) {
12+
return Some(Applicability::MachineApplicable);
13+
}
14+
if is_expr_identity_function(cx, expr) {
15+
return Some(Applicability::Unspecified);
16+
}
17+
None
18+
}
19+
1020
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) {
11-
if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, filter_map_arg) {
21+
if is_trait_method(cx, expr, sym::Iterator)
22+
&& let Some(applicability) = is_identity(cx, filter_map_arg)
23+
{
1224
span_lint_and_sugg(
1325
cx,
1426
FILTER_MAP_IDENTITY,
1527
filter_map_span.with_hi(expr.span.hi()),
1628
"use of `filter_map` with an identity function",
1729
"try",
1830
"flatten()".to_string(),
19-
Applicability::MachineApplicable,
31+
applicability,
2032
);
2133
}
2234
}

‎tests/ui/filter_map_identity.fixed

+76-10
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,83 @@
1-
#![allow(unused_imports, clippy::needless_return)]
1+
#![allow(unused_imports, clippy::needless_return, clippy::useless_vec)]
22
#![warn(clippy::filter_map_identity)]
3+
#![feature(stmt_expr_attributes)]
4+
5+
use std::option::Option;
6+
struct NonCopy;
7+
use std::convert::identity;
8+
9+
fn non_copy_vec() -> Vec<Option<NonCopy>> {
10+
todo!()
11+
}
12+
13+
fn copy_vec<T: Copy>() -> Vec<Option<T>> {
14+
todo!()
15+
}
16+
17+
fn copy_vec_non_inferred() -> Vec<Option<i32>> {
18+
todo!()
19+
}
20+
21+
fn opaque<T: Default>() -> impl IntoIterator<Item = Option<T>> {
22+
vec![Some(T::default())]
23+
}
324

425
fn main() {
5-
let iterator = vec![Some(1), None, Some(2)].into_iter();
6-
let _ = iterator.flatten();
26+
{
27+
// into_iter
28+
copy_vec_non_inferred().into_iter().flatten();
29+
//~^ ERROR: use of
30+
copy_vec_non_inferred().into_iter().flatten();
31+
//~^ ERROR: use of
32+
copy_vec_non_inferred().into_iter().flatten();
33+
//~^ ERROR: use of
34+
copy_vec_non_inferred().into_iter().flatten();
35+
//~^ ERROR: use of
36+
copy_vec_non_inferred().into_iter().flatten();
37+
//~^ ERROR: use of
38+
39+
non_copy_vec().into_iter().flatten();
40+
//~^ ERROR: use of
41+
non_copy_vec().into_iter().flatten();
42+
//~^ ERROR: use of
43+
44+
non_copy_vec().into_iter().flatten();
45+
//~^ ERROR: use of
46+
non_copy_vec().into_iter().flatten();
47+
//~^ ERROR: use of
48+
non_copy_vec().into_iter().flatten();
49+
//~^ ERROR: use of
50+
non_copy_vec().into_iter().flatten();
51+
//~^ ERROR: use of
752

8-
let iterator = vec![Some(1), None, Some(2)].into_iter();
9-
let _ = iterator.flatten();
53+
copy_vec::<i32>().into_iter().flatten();
54+
//~^ ERROR: use of
55+
copy_vec::<i32>().into_iter().flatten();
56+
//~^ ERROR: use of
57+
copy_vec::<i32>().into_iter().flatten();
58+
//~^ ERROR: use of
59+
copy_vec::<i32>().into_iter().flatten();
60+
//~^ ERROR: use of
1061

11-
use std::convert::identity;
12-
let iterator = vec![Some(1), None, Some(2)].into_iter();
13-
let _ = iterator.flatten();
62+
// we are forced to pass the type in the call.
63+
copy_vec::<i32>().into_iter().flatten();
64+
//~^ ERROR: use of
65+
copy_vec::<i32>().into_iter().flatten();
66+
//~^ ERROR: use of
67+
copy_vec::<i32>().into_iter().flatten();
68+
//~^ ERROR: use of
69+
copy_vec::<i32>().into_iter().flatten();
70+
//~^ ERROR: use of
71+
#[rustfmt::skip]
72+
copy_vec::<i32>().into_iter().flatten();
73+
//~^ ERROR: use of
74+
#[rustfmt::skip]
75+
copy_vec::<i32>().into_iter().flatten();
76+
//~^ ERROR: use of
1477

15-
let iterator = vec![Some(1), None, Some(2)].into_iter();
16-
let _ = iterator.flatten();
78+
// note, the compiler requires that we pass the type to `opaque`. This is mostly for reference,
79+
// it behaves the same as copy_vec.
80+
opaque::<i32>().into_iter().flatten();
81+
//~^ ERROR: use of
82+
}
1783
}

‎tests/ui/filter_map_identity.rs

+76-10
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,83 @@
1-
#![allow(unused_imports, clippy::needless_return)]
1+
#![allow(unused_imports, clippy::needless_return, clippy::useless_vec)]
22
#![warn(clippy::filter_map_identity)]
3+
#![feature(stmt_expr_attributes)]
4+
5+
use std::option::Option;
6+
struct NonCopy;
7+
use std::convert::identity;
8+
9+
fn non_copy_vec() -> Vec<Option<NonCopy>> {
10+
todo!()
11+
}
12+
13+
fn copy_vec<T: Copy>() -> Vec<Option<T>> {
14+
todo!()
15+
}
16+
17+
fn copy_vec_non_inferred() -> Vec<Option<i32>> {
18+
todo!()
19+
}
20+
21+
fn opaque<T: Default>() -> impl IntoIterator<Item = Option<T>> {
22+
vec![Some(T::default())]
23+
}
324

425
fn main() {
5-
let iterator = vec![Some(1), None, Some(2)].into_iter();
6-
let _ = iterator.filter_map(|x| x);
26+
{
27+
// into_iter
28+
copy_vec_non_inferred().into_iter().filter_map(|x| x);
29+
//~^ ERROR: use of
30+
copy_vec_non_inferred().into_iter().filter_map(std::convert::identity);
31+
//~^ ERROR: use of
32+
copy_vec_non_inferred().into_iter().filter_map(identity);
33+
//~^ ERROR: use of
34+
copy_vec_non_inferred().into_iter().filter_map(|x| return x);
35+
//~^ ERROR: use of
36+
copy_vec_non_inferred().into_iter().filter_map(|x| return x);
37+
//~^ ERROR: use of
38+
39+
non_copy_vec().into_iter().filter_map(|x| x);
40+
//~^ ERROR: use of
41+
non_copy_vec().into_iter().filter_map(|x| x);
42+
//~^ ERROR: use of
43+
44+
non_copy_vec().into_iter().filter_map(std::convert::identity);
45+
//~^ ERROR: use of
46+
non_copy_vec().into_iter().filter_map(identity);
47+
//~^ ERROR: use of
48+
non_copy_vec().into_iter().filter_map(|x| return x);
49+
//~^ ERROR: use of
50+
non_copy_vec().into_iter().filter_map(|x| return x);
51+
//~^ ERROR: use of
752

8-
let iterator = vec![Some(1), None, Some(2)].into_iter();
9-
let _ = iterator.filter_map(std::convert::identity);
53+
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
54+
//~^ ERROR: use of
55+
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
56+
//~^ ERROR: use of
57+
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
58+
//~^ ERROR: use of
59+
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
60+
//~^ ERROR: use of
1061

11-
use std::convert::identity;
12-
let iterator = vec![Some(1), None, Some(2)].into_iter();
13-
let _ = iterator.filter_map(identity);
62+
// we are forced to pass the type in the call.
63+
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
64+
//~^ ERROR: use of
65+
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
66+
//~^ ERROR: use of
67+
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
68+
//~^ ERROR: use of
69+
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
70+
//~^ ERROR: use of
71+
#[rustfmt::skip]
72+
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ x }});
73+
//~^ ERROR: use of
74+
#[rustfmt::skip]
75+
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ return x }});
76+
//~^ ERROR: use of
1477

15-
let iterator = vec![Some(1), None, Some(2)].into_iter();
16-
let _ = iterator.filter_map(|x| return x);
78+
// note, the compiler requires that we pass the type to `opaque`. This is mostly for reference,
79+
// it behaves the same as copy_vec.
80+
opaque::<i32>().into_iter().filter_map(|x| x);
81+
//~^ ERROR: use of
82+
}
1783
}

‎tests/ui/filter_map_identity.stderr

+121-13
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,137 @@
11
error: use of `filter_map` with an identity function
2-
--> tests/ui/filter_map_identity.rs:6:22
2+
--> tests/ui/filter_map_identity.rs:28:45
33
|
4-
LL | let _ = iterator.filter_map(|x| x);
5-
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
4+
LL | copy_vec_non_inferred().into_iter().filter_map(|x| x);
5+
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
66
|
77
= note: `-D clippy::filter-map-identity` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::filter_map_identity)]`
99

1010
error: use of `filter_map` with an identity function
11-
--> tests/ui/filter_map_identity.rs:9:22
11+
--> tests/ui/filter_map_identity.rs:30:45
1212
|
13-
LL | let _ = iterator.filter_map(std::convert::identity);
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
13+
LL | copy_vec_non_inferred().into_iter().filter_map(std::convert::identity);
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
1515

1616
error: use of `filter_map` with an identity function
17-
--> tests/ui/filter_map_identity.rs:13:22
17+
--> tests/ui/filter_map_identity.rs:32:45
1818
|
19-
LL | let _ = iterator.filter_map(identity);
20-
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
19+
LL | copy_vec_non_inferred().into_iter().filter_map(identity);
20+
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
2121

2222
error: use of `filter_map` with an identity function
23-
--> tests/ui/filter_map_identity.rs:16:22
23+
--> tests/ui/filter_map_identity.rs:34:45
2424
|
25-
LL | let _ = iterator.filter_map(|x| return x);
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
25+
LL | copy_vec_non_inferred().into_iter().filter_map(|x| return x);
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
2727

28-
error: aborting due to 4 previous errors
28+
error: use of `filter_map` with an identity function
29+
--> tests/ui/filter_map_identity.rs:36:45
30+
|
31+
LL | copy_vec_non_inferred().into_iter().filter_map(|x| return x);
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
33+
34+
error: use of `filter_map` with an identity function
35+
--> tests/ui/filter_map_identity.rs:39:36
36+
|
37+
LL | non_copy_vec().into_iter().filter_map(|x| x);
38+
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
39+
40+
error: use of `filter_map` with an identity function
41+
--> tests/ui/filter_map_identity.rs:41:36
42+
|
43+
LL | non_copy_vec().into_iter().filter_map(|x| x);
44+
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
45+
46+
error: use of `filter_map` with an identity function
47+
--> tests/ui/filter_map_identity.rs:44:36
48+
|
49+
LL | non_copy_vec().into_iter().filter_map(std::convert::identity);
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
51+
52+
error: use of `filter_map` with an identity function
53+
--> tests/ui/filter_map_identity.rs:46:36
54+
|
55+
LL | non_copy_vec().into_iter().filter_map(identity);
56+
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
57+
58+
error: use of `filter_map` with an identity function
59+
--> tests/ui/filter_map_identity.rs:48:36
60+
|
61+
LL | non_copy_vec().into_iter().filter_map(|x| return x);
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
63+
64+
error: use of `filter_map` with an identity function
65+
--> tests/ui/filter_map_identity.rs:50:36
66+
|
67+
LL | non_copy_vec().into_iter().filter_map(|x| return x);
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
69+
70+
error: use of `filter_map` with an identity function
71+
--> tests/ui/filter_map_identity.rs:53:39
72+
|
73+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
75+
76+
error: use of `filter_map` with an identity function
77+
--> tests/ui/filter_map_identity.rs:55:39
78+
|
79+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
80+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
81+
82+
error: use of `filter_map` with an identity function
83+
--> tests/ui/filter_map_identity.rs:57:39
84+
|
85+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
87+
88+
error: use of `filter_map` with an identity function
89+
--> tests/ui/filter_map_identity.rs:59:39
90+
|
91+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
92+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
93+
94+
error: use of `filter_map` with an identity function
95+
--> tests/ui/filter_map_identity.rs:63:39
96+
|
97+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
98+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
99+
100+
error: use of `filter_map` with an identity function
101+
--> tests/ui/filter_map_identity.rs:65:39
102+
|
103+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
104+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
105+
106+
error: use of `filter_map` with an identity function
107+
--> tests/ui/filter_map_identity.rs:67:39
108+
|
109+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
110+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
111+
112+
error: use of `filter_map` with an identity function
113+
--> tests/ui/filter_map_identity.rs:69:39
114+
|
115+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
116+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
117+
118+
error: use of `filter_map` with an identity function
119+
--> tests/ui/filter_map_identity.rs:72:43
120+
|
121+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ x }});
122+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
123+
124+
error: use of `filter_map` with an identity function
125+
--> tests/ui/filter_map_identity.rs:75:43
126+
|
127+
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ return x }});
128+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
129+
130+
error: use of `filter_map` with an identity function
131+
--> tests/ui/filter_map_identity.rs:80:37
132+
|
133+
LL | opaque::<i32>().into_iter().filter_map(|x| x);
134+
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
135+
136+
error: aborting due to 22 previous errors
29137

0 commit comments

Comments
 (0)