Skip to content

Commit d4ba561

Browse files
dima74dima74
committed
Review fixes
1 parent a427c99 commit d4ba561

File tree

4 files changed

+71
-31
lines changed

4 files changed

+71
-31
lines changed

‎clippy_lints/src/methods/mod.rs

+16-20
Original file line numberDiff line numberDiff line change
@@ -2570,11 +2570,16 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
25702570
// lint if caller of `.map().flatten()` is an Iterator
25712571
if match_trait_method(cx, expr, &paths::ITERATOR) {
25722572
let map_closure_ty = cx.typeck_results().expr_ty(&map_args[1]);
2573-
let is_map_to_option = if let ty::Closure(_def_id, substs) = map_closure_ty.kind {
2574-
let map_closure_return_ty = cx.tcx.erase_late_bound_regions(&substs.as_closure().sig().output());
2575-
is_type_diagnostic_item(cx, map_closure_return_ty, sym!(option_type))
2576-
} else {
2577-
false
2573+
let is_map_to_option = match map_closure_ty.kind {
2574+
ty::Closure(_, _) | ty::FnDef(_, _) | ty::FnPtr(_) => {
2575+
let map_closure_sig = match map_closure_ty.kind {
2576+
ty::Closure(_, substs) => substs.as_closure().sig(),
2577+
_ => map_closure_ty.fn_sig(cx.tcx),
2578+
};
2579+
let map_closure_return_ty = cx.tcx.erase_late_bound_regions(&map_closure_sig.output());
2580+
is_type_diagnostic_item(cx, map_closure_return_ty, sym!(option_type))
2581+
},
2582+
_ => false,
25782583
};
25792584

25802585
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
25842589
// `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>>
25852590
"flat_map"
25862591
};
2587-
let msg = &format!(
2588-
"called `map(..).flatten()` on an `Iterator`. \
2589-
This is more succinctly expressed by calling `.{}(..)`",
2590-
method_to_use
2591-
);
2592-
let self_snippet = snippet(cx, map_args[0].span, "..");
25932592
let func_snippet = snippet(cx, map_args[1].span, "..");
2594-
let hint = format!("{0}.{1}({2})", self_snippet, method_to_use, func_snippet);
2593+
let hint = format!(".{0}({1})", method_to_use, func_snippet);
25952594
span_lint_and_sugg(
25962595
cx,
25972596
MAP_FLATTEN,
2598-
expr.span,
2599-
msg,
2597+
expr.span.with_lo(map_args[0].span.hi()),
2598+
"called `map(..).flatten()` on an `Iterator`",
26002599
&format!("try using `{}` instead", method_to_use),
26012600
hint,
26022601
Applicability::MachineApplicable,
@@ -2605,16 +2604,13 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
26052604

26062605
// lint if caller of `.map().flatten()` is an Option
26072606
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym!(option_type)) {
2608-
let msg = "called `map(..).flatten()` on an `Option`. \
2609-
This is more succinctly expressed by calling `.and_then(..)`";
2610-
let self_snippet = snippet(cx, map_args[0].span, "..");
26112607
let func_snippet = snippet(cx, map_args[1].span, "..");
2612-
let hint = format!("{0}.and_then({1})", self_snippet, func_snippet);
2608+
let hint = format!(".and_then({})", func_snippet);
26132609
span_lint_and_sugg(
26142610
cx,
26152611
MAP_FLATTEN,
2616-
expr.span,
2617-
msg,
2612+
expr.span.with_lo(map_args[0].span.hi()),
2613+
"called `map(..).flatten()` on an `Option`",
26182614
"try using `and_then` instead",
26192615
hint,
26202616
Applicability::MachineApplicable,

‎tests/ui/map_flatten.fixed

+13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,20 @@
55
#![allow(clippy::map_identity)]
66

77
fn main() {
8+
// mapping to Option on Iterator
9+
fn option_id(x: i8) -> Option<i8> {
10+
Some(x)
11+
}
12+
let option_id_ref: fn(i8) -> Option<i8> = option_id;
13+
let option_id_closure = |x| Some(x);
14+
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id).collect();
15+
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_ref).collect();
16+
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_closure).collect();
817
let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect();
18+
19+
// mapping to Iterator on Iterator
920
let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect();
21+
22+
// mapping to Option on Option
1023
let _: Option<_> = (Some(Some(1))).and_then(|x| x);
1124
}

‎tests/ui/map_flatten.rs

+13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,20 @@
55
#![allow(clippy::map_identity)]
66

77
fn main() {
8+
// mapping to Option on Iterator
9+
fn option_id(x: i8) -> Option<i8> {
10+
Some(x)
11+
}
12+
let option_id_ref: fn(i8) -> Option<i8> = option_id;
13+
let option_id_closure = |x| Some(x);
14+
let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
15+
let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
16+
let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
817
let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
18+
19+
// mapping to Iterator on Iterator
920
let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
21+
22+
// mapping to Option on Option
1023
let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
1124
}

‎tests/ui/map_flatten.stderr

+29-11
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,40 @@
1-
error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)`
2-
--> $DIR/map_flatten.rs:8:21
1+
error: called `map(..).flatten()` on an `Iterator`
2+
--> $DIR/map_flatten.rs:14:46
33
|
4-
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1))`
4+
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)`
66
|
77
= note: `-D clippy::map-flatten` implied by `-D warnings`
88

9-
error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)`
10-
--> $DIR/map_flatten.rs:9:21
9+
error: called `map(..).flatten()` on an `Iterator`
10+
--> $DIR/map_flatten.rs:15:46
11+
|
12+
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)`
14+
15+
error: called `map(..).flatten()` on an `Iterator`
16+
--> $DIR/map_flatten.rs:16:46
17+
|
18+
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)`
20+
21+
error: called `map(..).flatten()` on an `Iterator`
22+
--> $DIR/map_flatten.rs:17:46
23+
|
24+
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))`
26+
27+
error: called `map(..).flatten()` on an `Iterator`
28+
--> $DIR/map_flatten.rs:20:46
1129
|
1230
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `vec![5_i8; 6].into_iter().flat_map(|x| 0..x)`
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)`
1432

15-
error: called `map(..).flatten()` on an `Option`. This is more succinctly expressed by calling `.and_then(..)`
16-
--> $DIR/map_flatten.rs:10:24
33+
error: called `map(..).flatten()` on an `Option`
34+
--> $DIR/map_flatten.rs:23:39
1735
|
1836
LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `(Some(Some(1))).and_then(|x| x)`
37+
| ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)`
2038

21-
error: aborting due to 3 previous errors
39+
error: aborting due to 6 previous errors
2240

0 commit comments

Comments
 (0)