diff --git a/CHANGELOG.md b/CHANGELOG.md index 7188e6f93241..397916fe409e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -896,7 +896,9 @@ All notable changes to this project will be documented in this file. [`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map +[`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 [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic [`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const diff --git a/README.md b/README.md index 9ca5761a2ba4..4ed388db1763 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 299 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 301 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4220e6a738bb..0f09f678e4bb 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -622,6 +622,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { loops::EXPLICIT_ITER_LOOP, matches::SINGLE_MATCH_ELSE, methods::FILTER_MAP, + methods::FILTER_MAP_NEXT, + methods::FIND_MAP, methods::MAP_FLATTEN, methods::OPTION_MAP_UNWRAP_OR, methods::OPTION_MAP_UNWRAP_OR_ELSE, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d8595ea9004c..2904fd4c98dc 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -288,6 +288,50 @@ declare_clippy_lint! { "using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call" } +declare_clippy_lint! { + /// **What it does:** Checks for usage of `_.filter_map(_).next()`. + /// + /// **Why is this bad?** Readability, this can be written more concisely as a + /// single method call. + /// + /// **Known problems:** None + /// + /// **Example:** + /// ```rust + /// (0..3).filter_map(|x| if x == 2 { Some(x) } else { None }).next(); + /// ``` + /// Can be written as + /// + /// ```rust + /// (0..3).find_map(|x| if x == 2 { Some(x) } else { None }); + /// ``` + pub FILTER_MAP_NEXT, + pedantic, + "using combination of `filter_map` and `next` which can usually be written as a single method call" +} + +declare_clippy_lint! { + /// **What it does:** Checks for usage of `_.find(_).map(_)`. + /// + /// **Why is this bad?** Readability, this can be written more concisely as a + /// single method call. + /// + /// **Known problems:** Often requires a condition + Option/Iterator creation + /// inside the closure. + /// + /// **Example:** + /// ```rust + /// (0..3).find(|x| x == 2).map(|x| x * 2); + /// ``` + /// Can be written as + /// ```rust + /// (0..3).find_map(|x| if x == 2 { Some(x * 2) } else { None }); + /// ``` + pub FIND_MAP, + pedantic, + "using a combination of `find` and `map` can usually be written as a single method call" +} + declare_clippy_lint! { /// **What it does:** Checks for an iterator search (such as `find()`, /// `position()`, or `rposition()`) followed by a call to `is_some()`. @@ -798,6 +842,8 @@ declare_lint_pass!(Methods => [ TEMPORARY_CSTRING_AS_PTR, FILTER_NEXT, FILTER_MAP, + FILTER_MAP_NEXT, + FIND_MAP, MAP_FLATTEN, ITER_NTH, ITER_SKIP_NEXT, @@ -833,6 +879,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]), ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]), ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]), + ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]), + ["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]), ["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]), @@ -1911,6 +1959,42 @@ fn lint_filter_map<'a, 'tcx>( } } +/// lint use of `filter_map().next()` for `Iterators` +fn lint_filter_map_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, filter_args: &'tcx [hir::Expr]) { + if match_trait_method(cx, expr, &paths::ITERATOR) { + let msg = "called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling \ + `.find_map(p)` instead."; + let filter_snippet = snippet(cx, filter_args[1].span, ".."); + if filter_snippet.lines().count() <= 1 { + span_note_and_lint( + cx, + FILTER_MAP_NEXT, + expr.span, + msg, + expr.span, + &format!("replace `filter_map({0}).next()` with `find_map({0})`", filter_snippet), + ); + } else { + span_lint(cx, FILTER_MAP_NEXT, expr.span, msg); + } + } +} + +/// lint use of `find().map()` for `Iterators` +fn lint_find_map<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + expr: &'tcx hir::Expr, + _find_args: &'tcx [hir::Expr], + map_args: &'tcx [hir::Expr], +) { + // lint if caller of `.filter().map()` is an Iterator + if match_trait_method(cx, &map_args[0], &paths::ITERATOR) { + let msg = "called `find(p).map(q)` on an `Iterator`. \ + This is more succinctly expressed by calling `.find_map(..)` instead."; + span_lint(cx, FIND_MAP, expr.span, msg); + } +} + /// lint use of `filter().map()` for `Iterators` fn lint_filter_map_map<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index e43b591a9cdc..63d75be1da20 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -186,7 +186,9 @@ fn check_pat<'a, 'tcx>( if let ExprKind::Struct(_, ref efields, _) = init_struct.node { for field in pfields { let name = field.node.ident.name; - let efield = efields.iter().find(|f| f.ident.name == name).map(|f| &*f.expr); + let efield = efields + .iter() + .find_map(|f| if f.ident.name == name { Some(&*f.expr) } else { None }); check_pat(cx, &field.node.pat, efield, span, bindings); } } else { diff --git a/clippy_lints/src/utils/attrs.rs b/clippy_lints/src/utils/attrs.rs index fbd36ba71dec..bec8d53714e2 100644 --- a/clippy_lints/src/utils/attrs.rs +++ b/clippy_lints/src/utils/attrs.rs @@ -58,10 +58,16 @@ pub fn get_attr<'a>( attrs.iter().filter(move |attr| { let attr_segments = &attr.path.segments; if attr_segments.len() == 2 && attr_segments[0].ident.to_string() == "clippy" { - if let Some(deprecation_status) = BUILTIN_ATTRIBUTES - .iter() - .find(|(builtin_name, _)| *builtin_name == attr_segments[1].ident.to_string()) - .map(|(_, deprecation_status)| deprecation_status) + if let Some(deprecation_status) = + BUILTIN_ATTRIBUTES + .iter() + .find_map(|(builtin_name, deprecation_status)| { + if *builtin_name == attr_segments[1].ident.to_string() { + Some(deprecation_status) + } else { + None + } + }) { let mut db = sess.struct_span_err(attr_segments[1].ident.span, "Usage of deprecated attribute"); match deprecation_status { diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 65561b7fbd76..663e55d8ea2b 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -61,10 +61,13 @@ fn config(mode: &str, dir: PathBuf) -> compiletest::Config { let name = path.file_name()?.to_string_lossy(); // NOTE: This only handles a single dep // https://github.com/laumann/compiletest-rs/issues/101 - needs_disambiguation - .iter() - .find(|dep| name.starts_with(&format!("lib{}-", dep))) - .map(|dep| format!("--extern {}={}", dep, path.display())) + needs_disambiguation.iter().find_map(|dep| { + if name.starts_with(&format!("lib{}-", dep)) { + Some(format!("--extern {}={}", dep, path.display())) + } else { + None + } + }) }) .collect::>(); diff --git a/tests/ui/filter_map_next.rs b/tests/ui/filter_map_next.rs new file mode 100644 index 000000000000..f5d051be198e --- /dev/null +++ b/tests/ui/filter_map_next.rs @@ -0,0 +1,20 @@ +#![warn(clippy::all, clippy::pedantic)] + +fn main() { + let a = ["1", "lol", "3", "NaN", "5"]; + + let element: Option = a.iter().filter_map(|s| s.parse().ok()).next(); + assert_eq!(element, Some(1)); + + #[rustfmt::skip] + let _: Option = vec![1, 2, 3, 4, 5, 6] + .into_iter() + .filter_map(|x| { + if x == 2 { + Some(x * 2) + } else { + None + } + }) + .next(); +} diff --git a/tests/ui/filter_map_next.stderr b/tests/ui/filter_map_next.stderr new file mode 100644 index 000000000000..d69ae212414f --- /dev/null +++ b/tests/ui/filter_map_next.stderr @@ -0,0 +1,24 @@ +error: called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(p)` instead. + --> $DIR/filter_map_next.rs:6:32 + | +LL | let element: Option = a.iter().filter_map(|s| s.parse().ok()).next(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::filter-map-next` implied by `-D warnings` + = note: replace `filter_map(|s| s.parse().ok()).next()` with `find_map(|s| s.parse().ok())` + +error: called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(p)` instead. + --> $DIR/filter_map_next.rs:10:26 + | +LL | let _: Option = vec![1, 2, 3, 4, 5, 6] + | __________________________^ +LL | | .into_iter() +LL | | .filter_map(|x| { +LL | | if x == 2 { +... | +LL | | }) +LL | | .next(); + | |_______________^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/find_map.rs b/tests/ui/find_map.rs new file mode 100644 index 000000000000..c28cca144ca3 --- /dev/null +++ b/tests/ui/find_map.rs @@ -0,0 +1,32 @@ +#![warn(clippy::all, clippy::pedantic)] + +#[derive(Debug, Copy, Clone)] +enum Flavor { + Chocolate, +} + +#[derive(Debug, Copy, Clone)] +enum Dessert { + Banana, + Pudding, + Cake(Flavor), +} + +fn main() { + let desserts_of_the_week = vec![Dessert::Banana, Dessert::Cake(Flavor::Chocolate), Dessert::Pudding]; + + let a = ["lol", "NaN", "2", "5", "Xunda"]; + + let _: Option = a.iter().find(|s| s.parse::().is_ok()).map(|s| s.parse().unwrap()); + + let _: Option = desserts_of_the_week + .iter() + .find(|dessert| match *dessert { + Dessert::Cake(_) => true, + _ => false, + }) + .map(|dessert| match *dessert { + Dessert::Cake(ref flavor) => *flavor, + _ => unreachable!(), + }); +} diff --git a/tests/ui/find_map.stderr b/tests/ui/find_map.stderr new file mode 100644 index 000000000000..0417c7f3dc9a --- /dev/null +++ b/tests/ui/find_map.stderr @@ -0,0 +1,23 @@ +error: called `find(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead. + --> $DIR/find_map.rs:20:26 + | +LL | let _: Option = a.iter().find(|s| s.parse::().is_ok()).map(|s| s.parse().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::find-map` implied by `-D warnings` + +error: called `find(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead. + --> $DIR/find_map.rs:22:29 + | +LL | let _: Option = desserts_of_the_week + | _____________________________^ +LL | | .iter() +LL | | .find(|dessert| match *dessert { +LL | | Dessert::Cake(_) => true, +... | +LL | | _ => unreachable!(), +LL | | }); + | |__________^ + +error: aborting due to 2 previous errors +