Skip to content

Commit e56e325

Browse files
borsbors
committed
Auto merge of #4039 - andrehjr:add-find-map-lints, r=flip1995
Add lints for find_map changelog: adds lints for find_map and filter_map_next. Closes issue #3474 Hope I got everything correctly this time! Let me know if I missed something.
2 parents 86f7347 + e428862 commit e56e325

File tree

11 files changed

+208
-10
lines changed

11 files changed

+208
-10
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,9 @@ All notable changes to this project will be documented in this file.
896896
[`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes
897897
[`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
898898
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
899+
[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
899900
[`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next
901+
[`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map
900902
[`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic
901903
[`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp
902904
[`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const

‎README.md‎

Lines changed: 1 addition & 1 deletion
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 299 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
10+
[There are 301 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/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
622622
loops::EXPLICIT_ITER_LOOP,
623623
matches::SINGLE_MATCH_ELSE,
624624
methods::FILTER_MAP,
625+
methods::FILTER_MAP_NEXT,
626+
methods::FIND_MAP,
625627
methods::MAP_FLATTEN,
626628
methods::OPTION_MAP_UNWRAP_OR,
627629
methods::OPTION_MAP_UNWRAP_OR_ELSE,

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,50 @@ declare_clippy_lint! {
288288
"using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call"
289289
}
290290

291+
declare_clippy_lint! {
292+
/// **What it does:** Checks for usage of `_.filter_map(_).next()`.
293+
///
294+
/// **Why is this bad?** Readability, this can be written more concisely as a
295+
/// single method call.
296+
///
297+
/// **Known problems:** None
298+
///
299+
/// **Example:**
300+
/// ```rust
301+
/// (0..3).filter_map(|x| if x == 2 { Some(x) } else { None }).next();
302+
/// ```
303+
/// Can be written as
304+
///
305+
/// ```rust
306+
/// (0..3).find_map(|x| if x == 2 { Some(x) } else { None });
307+
/// ```
308+
pub FILTER_MAP_NEXT,
309+
pedantic,
310+
"using combination of `filter_map` and `next` which can usually be written as a single method call"
311+
}
312+
313+
declare_clippy_lint! {
314+
/// **What it does:** Checks for usage of `_.find(_).map(_)`.
315+
///
316+
/// **Why is this bad?** Readability, this can be written more concisely as a
317+
/// single method call.
318+
///
319+
/// **Known problems:** Often requires a condition + Option/Iterator creation
320+
/// inside the closure.
321+
///
322+
/// **Example:**
323+
/// ```rust
324+
/// (0..3).find(|x| x == 2).map(|x| x * 2);
325+
/// ```
326+
/// Can be written as
327+
/// ```rust
328+
/// (0..3).find_map(|x| if x == 2 { Some(x * 2) } else { None });
329+
/// ```
330+
pub FIND_MAP,
331+
pedantic,
332+
"using a combination of `find` and `map` can usually be written as a single method call"
333+
}
334+
291335
declare_clippy_lint! {
292336
/// **What it does:** Checks for an iterator search (such as `find()`,
293337
/// `position()`, or `rposition()`) followed by a call to `is_some()`.
@@ -798,6 +842,8 @@ declare_lint_pass!(Methods => [
798842
TEMPORARY_CSTRING_AS_PTR,
799843
FILTER_NEXT,
800844
FILTER_MAP,
845+
FILTER_MAP_NEXT,
846+
FIND_MAP,
801847
MAP_FLATTEN,
802848
ITER_NTH,
803849
ITER_SKIP_NEXT,
@@ -833,6 +879,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
833879
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
834880
["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
835881
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
882+
["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]),
883+
["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]),
836884
["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
837885
["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
838886
["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]),
@@ -1911,6 +1959,42 @@ fn lint_filter_map<'a, 'tcx>(
19111959
}
19121960
}
19131961

1962+
/// lint use of `filter_map().next()` for `Iterators`
1963+
fn lint_filter_map_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, filter_args: &'tcx [hir::Expr]) {
1964+
if match_trait_method(cx, expr, &paths::ITERATOR) {
1965+
let msg = "called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling \
1966+
`.find_map(p)` instead.";
1967+
let filter_snippet = snippet(cx, filter_args[1].span, "..");
1968+
if filter_snippet.lines().count() <= 1 {
1969+
span_note_and_lint(
1970+
cx,
1971+
FILTER_MAP_NEXT,
1972+
expr.span,
1973+
msg,
1974+
expr.span,
1975+
&format!("replace `filter_map({0}).next()` with `find_map({0})`", filter_snippet),
1976+
);
1977+
} else {
1978+
span_lint(cx, FILTER_MAP_NEXT, expr.span, msg);
1979+
}
1980+
}
1981+
}
1982+
1983+
/// lint use of `find().map()` for `Iterators`
1984+
fn lint_find_map<'a, 'tcx>(
1985+
cx: &LateContext<'a, 'tcx>,
1986+
expr: &'tcx hir::Expr,
1987+
_find_args: &'tcx [hir::Expr],
1988+
map_args: &'tcx [hir::Expr],
1989+
) {
1990+
// lint if caller of `.filter().map()` is an Iterator
1991+
if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
1992+
let msg = "called `find(p).map(q)` on an `Iterator`. \
1993+
This is more succinctly expressed by calling `.find_map(..)` instead.";
1994+
span_lint(cx, FIND_MAP, expr.span, msg);
1995+
}
1996+
}
1997+
19141998
/// lint use of `filter().map()` for `Iterators`
19151999
fn lint_filter_map_map<'a, 'tcx>(
19162000
cx: &LateContext<'a, 'tcx>,

‎clippy_lints/src/shadow.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,9 @@ fn check_pat<'a, 'tcx>(
186186
if let ExprKind::Struct(_, ref efields, _) = init_struct.node {
187187
for field in pfields {
188188
let name = field.node.ident.name;
189-
let efield = efields.iter().find(|f| f.ident.name == name).map(|f| &*f.expr);
189+
let efield = efields
190+
.iter()
191+
.find_map(|f| if f.ident.name == name { Some(&*f.expr) } else { None });
190192
check_pat(cx, &field.node.pat, efield, span, bindings);
191193
}
192194
} else {

‎clippy_lints/src/utils/attrs.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,16 @@ pub fn get_attr<'a>(
5858
attrs.iter().filter(move |attr| {
5959
let attr_segments = &attr.path.segments;
6060
if attr_segments.len() == 2 && attr_segments[0].ident.to_string() == "clippy" {
61-
if let Some(deprecation_status) = BUILTIN_ATTRIBUTES
62-
.iter()
63-
.find(|(builtin_name, _)| *builtin_name == attr_segments[1].ident.to_string())
64-
.map(|(_, deprecation_status)| deprecation_status)
61+
if let Some(deprecation_status) =
62+
BUILTIN_ATTRIBUTES
63+
.iter()
64+
.find_map(|(builtin_name, deprecation_status)| {
65+
if *builtin_name == attr_segments[1].ident.to_string() {
66+
Some(deprecation_status)
67+
} else {
68+
None
69+
}
70+
})
6571
{
6672
let mut db = sess.struct_span_err(attr_segments[1].ident.span, "Usage of deprecated attribute");
6773
match deprecation_status {

‎tests/compile-test.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,13 @@ fn config(mode: &str, dir: PathBuf) -> compiletest::Config {
6161
let name = path.file_name()?.to_string_lossy();
6262
// NOTE: This only handles a single dep
6363
// https://github.com/laumann/compiletest-rs/issues/101
64-
needs_disambiguation
65-
.iter()
66-
.find(|dep| name.starts_with(&format!("lib{}-", dep)))
67-
.map(|dep| format!("--extern {}={}", dep, path.display()))
64+
needs_disambiguation.iter().find_map(|dep| {
65+
if name.starts_with(&format!("lib{}-", dep)) {
66+
Some(format!("--extern {}={}", dep, path.display()))
67+
} else {
68+
None
69+
}
70+
})
6871
})
6972
.collect::<Vec<_>>();
7073

‎tests/ui/filter_map_next.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#![warn(clippy::all, clippy::pedantic)]
2+
3+
fn main() {
4+
let a = ["1", "lol", "3", "NaN", "5"];
5+
6+
let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
7+
assert_eq!(element, Some(1));
8+
9+
#[rustfmt::skip]
10+
let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
11+
.into_iter()
12+
.filter_map(|x| {
13+
if x == 2 {
14+
Some(x * 2)
15+
} else {
16+
None
17+
}
18+
})
19+
.next();
20+
}

‎tests/ui/filter_map_next.stderr‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
error: called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(p)` instead.
2+
--> $DIR/filter_map_next.rs:6:32
3+
|
4+
LL | let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::filter-map-next` implied by `-D warnings`
8+
= note: replace `filter_map(|s| s.parse().ok()).next()` with `find_map(|s| s.parse().ok())`
9+
10+
error: called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(p)` instead.
11+
--> $DIR/filter_map_next.rs:10:26
12+
|
13+
LL | let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
14+
| __________________________^
15+
LL | | .into_iter()
16+
LL | | .filter_map(|x| {
17+
LL | | if x == 2 {
18+
... |
19+
LL | | })
20+
LL | | .next();
21+
| |_______________^
22+
23+
error: aborting due to 2 previous errors
24+

‎tests/ui/find_map.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![warn(clippy::all, clippy::pedantic)]
2+
3+
#[derive(Debug, Copy, Clone)]
4+
enum Flavor {
5+
Chocolate,
6+
}
7+
8+
#[derive(Debug, Copy, Clone)]
9+
enum Dessert {
10+
Banana,
11+
Pudding,
12+
Cake(Flavor),
13+
}
14+
15+
fn main() {
16+
let desserts_of_the_week = vec![Dessert::Banana, Dessert::Cake(Flavor::Chocolate), Dessert::Pudding];
17+
18+
let a = ["lol", "NaN", "2", "5", "Xunda"];
19+
20+
let _: Option<i32> = a.iter().find(|s| s.parse::<i32>().is_ok()).map(|s| s.parse().unwrap());
21+
22+
let _: Option<Flavor> = desserts_of_the_week
23+
.iter()
24+
.find(|dessert| match *dessert {
25+
Dessert::Cake(_) => true,
26+
_ => false,
27+
})
28+
.map(|dessert| match *dessert {
29+
Dessert::Cake(ref flavor) => *flavor,
30+
_ => unreachable!(),
31+
});
32+
}

0 commit comments

Comments
 (0)