From c20afbdfe017a904180f01499139adfafe296bbf Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Wed, 15 Apr 2020 18:37:37 +0200 Subject: [PATCH 1/2] Lint map_flatten if caller is an Option --- clippy_lints/src/methods/mod.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 5255fec2cec5..608b351c3e1f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2484,7 +2484,7 @@ fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, ok_args: &[hir } } -/// lint use of `map().flatten()` for `Iterators` +/// lint use of `map().flatten()` for `Iterators` and 'Options' fn lint_map_flatten<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>, map_args: &'tcx [hir::Expr<'_>]) { // lint if caller of `.map().flatten()` is an Iterator if match_trait_method(cx, expr, &paths::ITERATOR) { @@ -2503,6 +2503,24 @@ fn lint_map_flatten<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr< Applicability::MachineApplicable, ); } + + // lint if caller of `.map().flatten()` is an Option + if match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION) { + let msg = "called `map(..).flatten()` on an `Option`. \ + This is more succinctly expressed by calling `.and_then(..)`"; + let self_snippet = snippet(cx, map_args[0].span, ".."); + let func_snippet = snippet(cx, map_args[1].span, ".."); + let hint = format!("{0}.and_then({1})", self_snippet, func_snippet); + span_lint_and_sugg( + cx, + MAP_FLATTEN, + expr.span, + msg, + "try using `and_then` instead", + hint, + Applicability::MachineApplicable, + ); + } } /// lint use of `map().unwrap_or_else()` for `Option`s and `Result`s From 72a8fc24e600c7a2b01799b824af8c9c8d67de1f Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Wed, 15 Apr 2020 19:06:41 +0200 Subject: [PATCH 2/2] Add test to map_flatten with an Option --- tests/ui/map_flatten.fixed | 1 + tests/ui/map_flatten.rs | 1 + tests/ui/map_flatten.stderr | 8 +++++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed index 51732e02be4f..7ac368878ab7 100644 --- a/tests/ui/map_flatten.fixed +++ b/tests/ui/map_flatten.fixed @@ -5,4 +5,5 @@ fn main() { let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect(); + let _: Option<_> = (Some(Some(1))).and_then(|x| x); } diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs index 66137a50ae8d..a608601039ce 100644 --- a/tests/ui/map_flatten.rs +++ b/tests/ui/map_flatten.rs @@ -5,4 +5,5 @@ fn main() { let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); + let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); } diff --git a/tests/ui/map_flatten.stderr b/tests/ui/map_flatten.stderr index e2ebd89b4c46..3cf2abd5b6d8 100644 --- a/tests/ui/map_flatten.stderr +++ b/tests/ui/map_flatten.stderr @@ -6,5 +6,11 @@ LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().colle | = note: `-D clippy::map-flatten` implied by `-D warnings` -error: aborting due to previous error +error: called `map(..).flatten()` on an `Option`. This is more succinctly expressed by calling `.and_then(..)` + --> $DIR/map_flatten.rs:8:24 + | +LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `(Some(Some(1))).and_then(|x| x)` + +error: aborting due to 2 previous errors