mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-09-20 14:41:58 +00:00
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
This commit is contained in:
Quinn Sinclair
parent
ca6a647490
commit
9655231eb8
4 changed files with 288 additions and 36 deletions
|
|
@ -1,5 +1,5 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::{is_expr_untyped_identity_function, is_trait_method};
|
||||
use clippy_utils::{is_expr_identity_function, is_expr_untyped_identity_function, is_trait_method};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir as hir;
|
||||
use rustc_lint::LateContext;
|
||||
|
|
@ -7,8 +7,20 @@ use rustc_span::{sym, Span};
|
|||
|
||||
use super::FILTER_MAP_IDENTITY;
|
||||
|
||||
fn is_identity(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Applicability> {
|
||||
if is_expr_untyped_identity_function(cx, expr) {
|
||||
return Some(Applicability::MachineApplicable);
|
||||
}
|
||||
if is_expr_identity_function(cx, expr) {
|
||||
return Some(Applicability::MaybeIncorrect);
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) {
|
||||
if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, filter_map_arg) {
|
||||
if is_trait_method(cx, expr, sym::Iterator)
|
||||
&& let Some(applicability) = is_identity(cx, filter_map_arg)
|
||||
{
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
FILTER_MAP_IDENTITY,
|
||||
|
|
@ -16,7 +28,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg:
|
|||
"use of `filter_map` with an identity function",
|
||||
"try",
|
||||
"flatten()".to_string(),
|
||||
Applicability::MachineApplicable,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,17 +1,83 @@
|
|||
#![allow(unused_imports, clippy::needless_return)]
|
||||
#![allow(unused_imports, clippy::needless_return, clippy::useless_vec)]
|
||||
#![warn(clippy::filter_map_identity)]
|
||||
#![feature(stmt_expr_attributes)]
|
||||
|
||||
use std::option::Option;
|
||||
struct NonCopy;
|
||||
use std::convert::identity;
|
||||
|
||||
fn non_copy_vec() -> Vec<Option<NonCopy>> {
|
||||
todo!()
|
||||
}
|
||||
|
||||
fn copy_vec<T: Copy>() -> Vec<Option<T>> {
|
||||
todo!()
|
||||
}
|
||||
|
||||
fn copy_vec_non_inferred() -> Vec<Option<i32>> {
|
||||
todo!()
|
||||
}
|
||||
|
||||
fn opaque<T: Default>() -> impl IntoIterator<Item = Option<T>> {
|
||||
vec![Some(T::default())]
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.flatten();
|
||||
{
|
||||
// into_iter
|
||||
copy_vec_non_inferred().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.flatten();
|
||||
non_copy_vec().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
|
||||
use std::convert::identity;
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.flatten();
|
||||
non_copy_vec().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.flatten();
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
|
||||
// we are forced to pass the type in the call.
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
#[rustfmt::skip]
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
#[rustfmt::skip]
|
||||
copy_vec::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
|
||||
// note, the compiler requires that we pass the type to `opaque`. This is mostly for reference,
|
||||
// it behaves the same as copy_vec.
|
||||
opaque::<i32>().into_iter().flatten();
|
||||
//~^ ERROR: use of
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,17 +1,83 @@
|
|||
#![allow(unused_imports, clippy::needless_return)]
|
||||
#![allow(unused_imports, clippy::needless_return, clippy::useless_vec)]
|
||||
#![warn(clippy::filter_map_identity)]
|
||||
#![feature(stmt_expr_attributes)]
|
||||
|
||||
use std::option::Option;
|
||||
struct NonCopy;
|
||||
use std::convert::identity;
|
||||
|
||||
fn non_copy_vec() -> Vec<Option<NonCopy>> {
|
||||
todo!()
|
||||
}
|
||||
|
||||
fn copy_vec<T: Copy>() -> Vec<Option<T>> {
|
||||
todo!()
|
||||
}
|
||||
|
||||
fn copy_vec_non_inferred() -> Vec<Option<i32>> {
|
||||
todo!()
|
||||
}
|
||||
|
||||
fn opaque<T: Default>() -> impl IntoIterator<Item = Option<T>> {
|
||||
vec![Some(T::default())]
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.filter_map(|x| x);
|
||||
{
|
||||
// into_iter
|
||||
copy_vec_non_inferred().into_iter().filter_map(|x| x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().filter_map(std::convert::identity);
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().filter_map(identity);
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().filter_map(|x| return x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec_non_inferred().into_iter().filter_map(|x| return x);
|
||||
//~^ ERROR: use of
|
||||
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.filter_map(std::convert::identity);
|
||||
non_copy_vec().into_iter().filter_map(|x| x);
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().filter_map(|x| x);
|
||||
//~^ ERROR: use of
|
||||
|
||||
use std::convert::identity;
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.filter_map(identity);
|
||||
non_copy_vec().into_iter().filter_map(std::convert::identity);
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().filter_map(identity);
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().filter_map(|x| return x);
|
||||
//~^ ERROR: use of
|
||||
non_copy_vec().into_iter().filter_map(|x| return x);
|
||||
//~^ ERROR: use of
|
||||
|
||||
let iterator = vec![Some(1), None, Some(2)].into_iter();
|
||||
let _ = iterator.filter_map(|x| return x);
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
|
||||
//~^ ERROR: use of
|
||||
|
||||
// we are forced to pass the type in the call.
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
|
||||
//~^ ERROR: use of
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
|
||||
//~^ ERROR: use of
|
||||
#[rustfmt::skip]
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ x }});
|
||||
//~^ ERROR: use of
|
||||
#[rustfmt::skip]
|
||||
copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ return x }});
|
||||
//~^ ERROR: use of
|
||||
|
||||
// note, the compiler requires that we pass the type to `opaque`. This is mostly for reference,
|
||||
// it behaves the same as copy_vec.
|
||||
opaque::<i32>().into_iter().filter_map(|x| x);
|
||||
//~^ ERROR: use of
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,29 +1,137 @@
|
|||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:6:22
|
||||
--> tests/ui/filter_map_identity.rs:28:45
|
||||
|
|
||||
LL | let _ = iterator.filter_map(|x| x);
|
||||
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
LL | copy_vec_non_inferred().into_iter().filter_map(|x| x);
|
||||
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
|
||||
= note: `-D clippy::filter-map-identity` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::filter_map_identity)]`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:9:22
|
||||
--> tests/ui/filter_map_identity.rs:30:45
|
||||
|
|
||||
LL | let _ = iterator.filter_map(std::convert::identity);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
LL | copy_vec_non_inferred().into_iter().filter_map(std::convert::identity);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:13:22
|
||||
--> tests/ui/filter_map_identity.rs:32:45
|
||||
|
|
||||
LL | let _ = iterator.filter_map(identity);
|
||||
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
LL | copy_vec_non_inferred().into_iter().filter_map(identity);
|
||||
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:16:22
|
||||
--> tests/ui/filter_map_identity.rs:34:45
|
||||
|
|
||||
LL | let _ = iterator.filter_map(|x| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
LL | copy_vec_non_inferred().into_iter().filter_map(|x| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:36:45
|
||||
|
|
||||
LL | copy_vec_non_inferred().into_iter().filter_map(|x| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:39:36
|
||||
|
|
||||
LL | non_copy_vec().into_iter().filter_map(|x| x);
|
||||
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:41:36
|
||||
|
|
||||
LL | non_copy_vec().into_iter().filter_map(|x| x);
|
||||
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:44:36
|
||||
|
|
||||
LL | non_copy_vec().into_iter().filter_map(std::convert::identity);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:46:36
|
||||
|
|
||||
LL | non_copy_vec().into_iter().filter_map(identity);
|
||||
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:48:36
|
||||
|
|
||||
LL | non_copy_vec().into_iter().filter_map(|x| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:50:36
|
||||
|
|
||||
LL | non_copy_vec().into_iter().filter_map(|x| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:53:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:55:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:57:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:59:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:63:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:65:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:67:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:69:39
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:72:43
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ x }});
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:75:43
|
||||
|
|
||||
LL | copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ return x }});
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: use of `filter_map` with an identity function
|
||||
--> tests/ui/filter_map_identity.rs:80:37
|
||||
|
|
||||
LL | opaque::<i32>().into_iter().filter_map(|x| x);
|
||||
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
||||
|
||||
error: aborting due to 22 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue