Review fixes

This commit is contained in:
Dmitry Murzin 2020-07-30 22:20:31 +03:00
parent a427c99f3d
commit d4ba561aaf
No known key found for this signature in database
GPG Key ID: 3DA7EB8E3131A454
4 changed files with 71 additions and 31 deletions

View File

@ -2570,11 +2570,16 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
// lint if caller of `.map().flatten()` is an Iterator // lint if caller of `.map().flatten()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) { if match_trait_method(cx, expr, &paths::ITERATOR) {
let map_closure_ty = cx.typeck_results().expr_ty(&map_args[1]); 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 is_map_to_option = match map_closure_ty.kind {
let map_closure_return_ty = cx.tcx.erase_late_bound_regions(&substs.as_closure().sig().output()); ty::Closure(_, _) | ty::FnDef(_, _) | ty::FnPtr(_) => {
is_type_diagnostic_item(cx, map_closure_return_ty, sym!(option_type)) let map_closure_sig = match map_closure_ty.kind {
} else { ty::Closure(_, substs) => substs.as_closure().sig(),
false _ => map_closure_ty.fn_sig(cx.tcx),
};
let map_closure_return_ty = cx.tcx.erase_late_bound_regions(&map_closure_sig.output());
is_type_diagnostic_item(cx, map_closure_return_ty, sym!(option_type))
},
_ => false,
}; };
let method_to_use = if is_map_to_option { let method_to_use = if is_map_to_option {
@ -2584,19 +2589,13 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
// `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>> // `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>>
"flat_map" "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 func_snippet = snippet(cx, map_args[1].span, "..");
let hint = format!("{0}.{1}({2})", self_snippet, method_to_use, func_snippet); let hint = format!(".{0}({1})", method_to_use, func_snippet);
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
MAP_FLATTEN, MAP_FLATTEN,
expr.span, expr.span.with_lo(map_args[0].span.hi()),
msg, "called `map(..).flatten()` on an `Iterator`",
&format!("try using `{}` instead", method_to_use), &format!("try using `{}` instead", method_to_use),
hint, hint,
Applicability::MachineApplicable, Applicability::MachineApplicable,
@ -2605,16 +2604,13 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
// lint if caller of `.map().flatten()` is an Option // lint if caller of `.map().flatten()` is an Option
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym!(option_type)) { if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym!(option_type)) {
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 func_snippet = snippet(cx, map_args[1].span, "..");
let hint = format!("{0}.and_then({1})", self_snippet, func_snippet); let hint = format!(".and_then({})", func_snippet);
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
MAP_FLATTEN, MAP_FLATTEN,
expr.span, expr.span.with_lo(map_args[0].span.hi()),
msg, "called `map(..).flatten()` on an `Option`",
"try using `and_then` instead", "try using `and_then` instead",
hint, hint,
Applicability::MachineApplicable, Applicability::MachineApplicable,

View File

@ -5,7 +5,20 @@
#![allow(clippy::map_identity)] #![allow(clippy::map_identity)]
fn main() { fn main() {
// mapping to Option on Iterator
fn option_id(x: i8) -> Option<i8> {
Some(x)
}
let option_id_ref: fn(i8) -> Option<i8> = option_id;
let option_id_closure = |x| Some(x);
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id).collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_ref).collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_closure).collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect(); let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect();
// mapping to Iterator on Iterator
let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect(); let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect();
// mapping to Option on Option
let _: Option<_> = (Some(Some(1))).and_then(|x| x); let _: Option<_> = (Some(Some(1))).and_then(|x| x);
} }

View File

@ -5,7 +5,20 @@
#![allow(clippy::map_identity)] #![allow(clippy::map_identity)]
fn main() { fn main() {
// mapping to Option on Iterator
fn option_id(x: i8) -> Option<i8> {
Some(x)
}
let option_id_ref: fn(i8) -> Option<i8> = option_id;
let option_id_closure = |x| Some(x);
let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
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| x.checked_add(1)).flatten().collect();
// mapping to Iterator on Iterator
let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
// mapping to Option on Option
let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
} }

View File

@ -1,22 +1,40 @@
error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)` error: called `map(..).flatten()` on an `Iterator`
--> $DIR/map_flatten.rs:8:21 --> $DIR/map_flatten.rs:14:46
| |
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1))` | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)`
| |
= note: `-D clippy::map-flatten` implied by `-D warnings` = 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(..)` error: called `map(..).flatten()` on an `Iterator`
--> $DIR/map_flatten.rs:9:21 --> $DIR/map_flatten.rs:15:46
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)`
error: called `map(..).flatten()` on an `Iterator`
--> $DIR/map_flatten.rs:16:46
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)`
error: called `map(..).flatten()` on an `Iterator`
--> $DIR/map_flatten.rs:17:46
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))`
error: called `map(..).flatten()` on an `Iterator`
--> $DIR/map_flatten.rs:20:46
| |
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); 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)` | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)`
error: called `map(..).flatten()` on an `Option`. This is more succinctly expressed by calling `.and_then(..)` error: called `map(..).flatten()` on an `Option`
--> $DIR/map_flatten.rs:10:24 --> $DIR/map_flatten.rs:23:39
| |
LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `(Some(Some(1))).and_then(|x| x)` | ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)`
error: aborting due to 3 previous errors error: aborting due to 6 previous errors