Skip to content

Commit

Permalink
Add new lint filter_map_identity
Browse files Browse the repository at this point in the history
This commit adds a new lint named `filter_map_identity`. This lint is
the same as `flat_map_identity` except that it checks for `filter_map`.

Closes #6643
  • Loading branch information
magurotuna committed Feb 6, 2021
1 parent a507c27 commit a60c143
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1955,6 +1955,7 @@ Released 2018-09-13
[`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
[`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity
[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
[`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next
[`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::EXPECT_USED,
&methods::FILETYPE_IS_FILE,
&methods::FILTER_MAP,
&methods::FILTER_MAP_IDENTITY,
&methods::FILTER_MAP_NEXT,
&methods::FILTER_NEXT,
&methods::FLAT_MAP_IDENTITY,
Expand Down Expand Up @@ -1531,6 +1532,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::CLONE_DOUBLE_REF),
LintId::of(&methods::CLONE_ON_COPY),
LintId::of(&methods::EXPECT_FUN_CALL),
LintId::of(&methods::FILTER_MAP_IDENTITY),
LintId::of(&methods::FILTER_NEXT),
LintId::of(&methods::FLAT_MAP_IDENTITY),
LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT),
Expand Down Expand Up @@ -1838,6 +1840,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
LintId::of(&methods::BIND_INSTEAD_OF_MAP),
LintId::of(&methods::CLONE_ON_COPY),
LintId::of(&methods::FILTER_MAP_IDENTITY),
LintId::of(&methods::FILTER_NEXT),
LintId::of(&methods::FLAT_MAP_IDENTITY),
LintId::of(&methods::INSPECT_FOR_EACH),
Expand Down
56 changes: 56 additions & 0 deletions clippy_lints/src/methods/filter_map_identity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use crate::utils::{match_qpath, match_trait_method, paths, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::source_map::Span;

use super::FILTER_MAP_IDENTITY;

pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
filter_map_args: &[hir::Expr<'_>],
filter_map_span: Span,
) {
if match_trait_method(cx, expr, &paths::ITERATOR) {
let arg_node = &filter_map_args[1].kind;

let apply_lint = |message: &str| {
span_lint_and_sugg(
cx,
FILTER_MAP_IDENTITY,
filter_map_span.with_hi(expr.span.hi()),
message,
"try",
"flatten()".to_string(),
Applicability::MachineApplicable,
);
};

if_chain! {
if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node;
let body = cx.tcx.hir().body(*body_id);

if let hir::PatKind::Binding(_, _, binding_ident, _) = body.params[0].pat.kind;
if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.kind;

if path.segments.len() == 1;
if path.segments[0].ident.name == binding_ident.name;

then {
apply_lint("called `filter_map(|x| x)` on an `Iterator`");
}
}

if_chain! {
if let hir::ExprKind::Path(ref qpath) = arg_node;

if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY);

then {
apply_lint("called `filter_map(std::convert::identity)` on an `Iterator`");
}
}
}
}
30 changes: 29 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod bind_instead_of_map;
mod filter_map_identity;
mod inefficient_to_string;
mod inspect_for_each;
mod manual_saturating_arithmetic;
Expand Down Expand Up @@ -1467,6 +1468,29 @@ declare_clippy_lint! {
"using `.inspect().for_each()`, which can be replaced with `.for_each()`"
}

declare_clippy_lint! {
/// **What it does:** Checks for usage of `filter_map(|x| x)`.
///
/// **Why is this bad?** Readability, this can be written more concisely by using `flatten`.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// # let iter = vec![Some(1)].into_iter();
/// iter.filter_map(|x| x);
/// ```
/// Use instead:
/// ```rust
/// # let iter = vec![Some(1)].into_iter();
/// iter.flatten();
/// ```
pub FILTER_MAP_IDENTITY,
complexity,
"call to `filter_map` where `flatten` is sufficient"
}

pub struct Methods {
msrv: Option<RustcVersion>,
}
Expand Down Expand Up @@ -1504,6 +1528,7 @@ impl_lint_pass!(Methods => [
FILTER_NEXT,
SKIP_WHILE_NEXT,
FILTER_MAP,
FILTER_MAP_IDENTITY,
MANUAL_FILTER_MAP,
MANUAL_FIND_MAP,
FILTER_MAP_NEXT,
Expand Down Expand Up @@ -1597,7 +1622,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
["as_ref"] => lint_asref(cx, expr, "as_ref", arg_lists[0]),
["as_mut"] => lint_asref(cx, expr, "as_mut", arg_lists[0]),
["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0], method_spans[0]),
["filter_map", ..] => unnecessary_filter_map::lint(cx, expr, arg_lists[0]),
["filter_map", ..] => {
unnecessary_filter_map::lint(cx, expr, arg_lists[0]);
filter_map_identity::check(cx, expr, arg_lists[0], method_spans[0]);
},
["count", "map"] => lint_suspicious_map(cx, expr),
["assume_init"] => lint_maybe_uninit(cx, &arg_lists[0][0], expr),
["unwrap_or", arith @ ("checked_add" | "checked_sub" | "checked_mul")] => {
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/filter_map_identity.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// run-rustfix

#![allow(unused_imports)]
#![warn(clippy::filter_map_identity)]

fn main() {
let iterator = vec![Some(1), None, Some(2)].into_iter();
let _ = iterator.flatten();

let iterator = vec![Some(1), None, Some(2)].into_iter();
let _ = iterator.flatten();

use std::convert::identity;
let iterator = vec![Some(1), None, Some(2)].into_iter();
let _ = iterator.flatten();
}
16 changes: 16 additions & 0 deletions tests/ui/filter_map_identity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// run-rustfix

#![allow(unused_imports)]
#![warn(clippy::filter_map_identity)]

fn main() {
let iterator = vec![Some(1), None, Some(2)].into_iter();
let _ = iterator.filter_map(|x| x);

let iterator = vec![Some(1), None, Some(2)].into_iter();
let _ = iterator.filter_map(std::convert::identity);

use std::convert::identity;
let iterator = vec![Some(1), None, Some(2)].into_iter();
let _ = iterator.filter_map(identity);
}
22 changes: 22 additions & 0 deletions tests/ui/filter_map_identity.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: called `filter_map(|x| x)` on an `Iterator`
--> $DIR/filter_map_identity.rs:8:22
|
LL | let _ = iterator.filter_map(|x| x);
| ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
|
= note: `-D clippy::filter-map-identity` implied by `-D warnings`

error: called `filter_map(std::convert::identity)` on an `Iterator`
--> $DIR/filter_map_identity.rs:11:22
|
LL | let _ = iterator.filter_map(std::convert::identity);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`

error: called `filter_map(std::convert::identity)` on an `Iterator`
--> $DIR/filter_map_identity.rs:15:22
|
LL | let _ = iterator.filter_map(identity);
| ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`

error: aborting due to 3 previous errors

0 comments on commit a60c143

Please sign in to comment.