Skip to content

Commit

Permalink
Handle mapping to Option in map_flatten lint
Browse files Browse the repository at this point in the history
  • Loading branch information
dima74 committed Jul 30, 2020
1 parent 2e0f8b6 commit a427c99
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
26 changes: 22 additions & 4 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2569,17 +2569,35 @@ fn lint_ok_expect(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ok_args: &[hir::Ex
fn lint_map_flatten<'tcx>(cx: &LateContext<'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) {
let msg = "called `map(..).flatten()` on an `Iterator`. \
This is more succinctly expressed by calling `.flat_map(..)`";
let map_closure_ty = cx.typeck_results().expr_ty(&map_args[1]);
let is_map_to_option = if let ty::Closure(_def_id, substs) = map_closure_ty.kind {
let map_closure_return_ty = cx.tcx.erase_late_bound_regions(&substs.as_closure().sig().output());
is_type_diagnostic_item(cx, map_closure_return_ty, sym!(option_type))
} else {
false
};

let method_to_use = if is_map_to_option {
// `(...).map(...)` has type `impl Iterator<Item=Option<...>>
"filter_map"
} else {
// `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>>
"flat_map"
};
let msg = &format!(
"called `map(..).flatten()` on an `Iterator`. \
This is more succinctly expressed by calling `.{}(..)`",
method_to_use
);
let self_snippet = snippet(cx, map_args[0].span, "..");
let func_snippet = snippet(cx, map_args[1].span, "..");
let hint = format!("{0}.flat_map({1})", self_snippet, func_snippet);
let hint = format!("{0}.{1}({2})", self_snippet, method_to_use, func_snippet);
span_lint_and_sugg(
cx,
MAP_FLATTEN,
expr.span,
msg,
"try using `flat_map` instead",
&format!("try using `{}` instead", method_to_use),
hint,
Applicability::MachineApplicable,
);
Expand Down
1 change: 1 addition & 0 deletions tests/ui/map_flatten.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![allow(clippy::map_identity)]

fn main() {
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect();
let _: Option<_> = (Some(Some(1))).and_then(|x| x);
}
1 change: 1 addition & 0 deletions tests/ui/map_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![allow(clippy::map_identity)]

fn main() {
let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
}
16 changes: 11 additions & 5 deletions tests/ui/map_flatten.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)`
error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)`
--> $DIR/map_flatten.rs:8:21
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `vec![5_i8; 6].into_iter().flat_map(|x| 0..x)`
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1))`
|
= note: `-D clippy::map-flatten` implied by `-D warnings`

error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)`
--> $DIR/map_flatten.rs:9:21
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `vec![5_i8; 6].into_iter().flat_map(|x| 0..x)`

error: called `map(..).flatten()` on an `Option`. This is more succinctly expressed by calling `.and_then(..)`
--> $DIR/map_flatten.rs:9:24
--> $DIR/map_flatten.rs:10: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
error: aborting due to 3 previous errors

0 comments on commit a427c99

Please sign in to comment.