Skip to content

Commit ca168c0

Browse files
dswijdswij
authored
unnecessary_{find,filter}_map: make diagnostic spans more precise (#15929)
changelog: [`unnecessary_find_map`]: make diagnostic spans more precise changelog: [`unnecessary_filter_map`]: make diagnostic spans more precise
2 parents 0029a91 + 9145cee commit ca168c0

File tree

5 files changed

+83
-63
lines changed

5 files changed

+83
-63
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5217,7 +5217,7 @@ impl Methods {
52175217
},
52185218
(sym::filter_map, [arg]) => {
52195219
unused_enumerate_index::check(cx, expr, recv, arg);
5220-
unnecessary_filter_map::check(cx, expr, arg, name);
5220+
unnecessary_filter_map::check(cx, expr, arg, call_span, unnecessary_filter_map::Kind::FilterMap);
52215221
filter_map_bool_then::check(cx, expr, arg, call_span);
52225222
filter_map_identity::check(cx, expr, arg, span);
52235223
lines_filter_map_ok::check_filter_or_flat_map(
@@ -5232,7 +5232,7 @@ impl Methods {
52325232
},
52335233
(sym::find_map, [arg]) => {
52345234
unused_enumerate_index::check(cx, expr, recv, arg);
5235-
unnecessary_filter_map::check(cx, expr, arg, name);
5235+
unnecessary_filter_map::check(cx, expr, arg, call_span, unnecessary_filter_map::Kind::FindMap);
52365236
},
52375237
(sym::flat_map, [arg]) => {
52385238
unused_enumerate_index::check(cx, expr, recv, arg);

‎clippy_lints/src/methods/unnecessary_filter_map.rs‎

Lines changed: 59 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,24 @@ use super::utils::clone_or_copy_needed;
22
use clippy_utils::diagnostics::span_lint;
33
use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath, MaybeTypeckRes};
44
use clippy_utils::sym;
5-
use clippy_utils::ty::is_copy;
5+
use clippy_utils::ty::{is_copy, option_arg_ty};
66
use clippy_utils::usage::mutated_variables;
77
use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
88
use core::ops::ControlFlow;
99
use rustc_hir as hir;
1010
use rustc_hir::LangItem::{OptionNone, OptionSome};
1111
use rustc_lint::LateContext;
12-
use rustc_middle::ty;
13-
use rustc_span::Symbol;
12+
use rustc_span::Span;
13+
use std::fmt::Display;
1414

1515
use super::{UNNECESSARY_FILTER_MAP, UNNECESSARY_FIND_MAP};
1616

1717
pub(super) fn check<'tcx>(
1818
cx: &LateContext<'tcx>,
1919
expr: &'tcx hir::Expr<'tcx>,
2020
arg: &'tcx hir::Expr<'tcx>,
21-
name: Symbol,
21+
call_span: Span,
22+
kind: Kind,
2223
) {
2324
if !cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
2425
return;
@@ -45,61 +46,88 @@ pub(super) fn check<'tcx>(
4546
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
4647
let sugg = if !found_filtering {
4748
// Check if the closure is .filter_map(|x| Some(x))
48-
if name == sym::filter_map
49-
&& let hir::ExprKind::Call(expr, args) = body.value.kind
49+
if kind.is_filter_map()
50+
&& let hir::ExprKind::Call(expr, [arg]) = body.value.kind
5051
&& expr.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome)
51-
&& let hir::ExprKind::Path(_) = args[0].kind
52+
&& let hir::ExprKind::Path(_) = arg.kind
5253
{
5354
span_lint(
5455
cx,
5556
UNNECESSARY_FILTER_MAP,
56-
expr.span,
57+
call_span,
5758
String::from("this call to `.filter_map(..)` is unnecessary"),
5859
);
5960
return;
6061
}
61-
if name == sym::filter_map {
62-
"map(..)"
63-
} else {
64-
"map(..).next()"
62+
match kind {
63+
Kind::FilterMap => "map(..)",
64+
Kind::FindMap => "map(..).next()",
6565
}
6666
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
67-
match cx.typeck_results().expr_ty(body.value).kind() {
68-
ty::Adt(adt, subst)
69-
if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) && in_ty == subst.type_at(0) =>
70-
{
71-
if name == sym::filter_map {
72-
"filter(..)"
73-
} else {
74-
"find(..)"
75-
}
76-
},
77-
_ => return,
67+
let ty = cx.typeck_results().expr_ty(body.value);
68+
if option_arg_ty(cx, ty).is_some_and(|t| t == in_ty) {
69+
match kind {
70+
Kind::FilterMap => "filter(..)",
71+
Kind::FindMap => "find(..)",
72+
}
73+
} else {
74+
return;
7875
}
7976
} else {
8077
return;
8178
};
8279
span_lint(
8380
cx,
84-
if name == sym::filter_map {
85-
UNNECESSARY_FILTER_MAP
86-
} else {
87-
UNNECESSARY_FIND_MAP
81+
match kind {
82+
Kind::FilterMap => UNNECESSARY_FILTER_MAP,
83+
Kind::FindMap => UNNECESSARY_FIND_MAP,
8884
},
89-
expr.span,
90-
format!("this `.{name}(..)` can be written more simply using `.{sugg}`"),
85+
call_span,
86+
format!("this `.{kind}(..)` can be written more simply using `.{sugg}`"),
9187
);
9288
}
9389
}
9490

91+
#[derive(Clone, Copy)]
92+
pub(super) enum Kind {
93+
FilterMap,
94+
FindMap,
95+
}
96+
97+
impl Kind {
98+
fn is_filter_map(self) -> bool {
99+
matches!(self, Self::FilterMap)
100+
}
101+
}
102+
103+
impl Display for Kind {
104+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
105+
match self {
106+
Self::FilterMap => f.write_str("filter_map"),
107+
Self::FindMap => f.write_str("find_map"),
108+
}
109+
}
110+
}
111+
95112
// returns (found_mapping, found_filtering)
96113
fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tcx hir::Expr<'_>) -> (bool, bool) {
97114
match expr.kind {
115+
hir::ExprKind::Path(ref path)
116+
if cx
117+
.qpath_res(path, expr.hir_id)
118+
.ctor_parent(cx)
119+
.is_lang_item(cx, OptionNone) =>
120+
{
121+
// None
122+
(false, true)
123+
},
98124
hir::ExprKind::Call(func, args) => {
99125
if func.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome) {
100126
if args[0].res_local_id() == Some(arg_id) {
127+
// Some(arg_id)
101128
return (false, false);
102129
}
130+
// Some(not arg_id)
103131
return (true, false);
104132
}
105133
(true, true)
@@ -109,8 +137,10 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
109137
&& cx.typeck_results().expr_ty(recv).is_bool()
110138
&& arg.res_local_id() == Some(arg_id)
111139
{
140+
// bool.then_some(arg_id)
112141
(false, true)
113142
} else {
143+
// bool.then_some(not arg_id)
114144
(true, true)
115145
}
116146
},
@@ -134,14 +164,6 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
134164
let else_check = check_expression(cx, arg_id, else_arm);
135165
(if_check.0 | else_check.0, if_check.1 | else_check.1)
136166
},
137-
hir::ExprKind::Path(ref path)
138-
if cx
139-
.qpath_res(path, expr.hir_id)
140-
.ctor_parent(cx)
141-
.is_lang_item(cx, OptionNone) =>
142-
{
143-
(false, true)
144-
},
145167
_ => (true, true),
146168
}
147169
}

‎tests/ui/unnecessary_filter_map.stderr‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
2-
--> tests/ui/unnecessary_filter_map.rs:4:13
2+
--> tests/ui/unnecessary_filter_map.rs:4:20
33
|
44
LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::unnecessary-filter-map` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_filter_map)]`
99

1010
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
11-
--> tests/ui/unnecessary_filter_map.rs:7:13
11+
--> tests/ui/unnecessary_filter_map.rs:7:20
1212
|
1313
LL | let _ = (0..4).filter_map(|x| {
14-
| _____________^
14+
| ____________________^
1515
LL | |
1616
LL | |
1717
LL | | if x > 1 {
@@ -21,33 +21,33 @@ LL | | });
2121
| |______^
2222

2323
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
24-
--> tests/ui/unnecessary_filter_map.rs:15:13
24+
--> tests/ui/unnecessary_filter_map.rs:15:20
2525
|
2626
LL | let _ = (0..4).filter_map(|x| match x {
27-
| _____________^
27+
| ____________________^
2828
LL | |
2929
LL | | 0 | 1 => None,
3030
LL | | _ => Some(x),
3131
LL | | });
3232
| |______^
3333

3434
error: this `.filter_map(..)` can be written more simply using `.map(..)`
35-
--> tests/ui/unnecessary_filter_map.rs:21:13
35+
--> tests/ui/unnecessary_filter_map.rs:21:20
3636
|
3737
LL | let _ = (0..4).filter_map(|x| Some(x + 1));
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
3939

4040
error: this call to `.filter_map(..)` is unnecessary
41-
--> tests/ui/unnecessary_filter_map.rs:28:61
41+
--> tests/ui/unnecessary_filter_map.rs:28:46
4242
|
4343
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
44-
| ^^^^
44+
| ^^^^^^^^^^^^^^^^^^^^^^^
4545

4646
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
47-
--> tests/ui/unnecessary_filter_map.rs:166:14
47+
--> tests/ui/unnecessary_filter_map.rs:166:33
4848
|
4949
LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n));
50-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5151

5252
error: aborting due to 6 previous errors
5353

‎tests/ui/unnecessary_find_map.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#![allow(dead_code)]
2-
31
fn main() {
42
let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
53
//~^ unnecessary_find_map

‎tests/ui/unnecessary_find_map.stderr‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
error: this `.find_map(..)` can be written more simply using `.find(..)`
2-
--> tests/ui/unnecessary_find_map.rs:4:13
2+
--> tests/ui/unnecessary_find_map.rs:2:20
33
|
44
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::unnecessary-find-map` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_find_map)]`
99

1010
error: this `.find_map(..)` can be written more simply using `.find(..)`
11-
--> tests/ui/unnecessary_find_map.rs:7:13
11+
--> tests/ui/unnecessary_find_map.rs:5:20
1212
|
1313
LL | let _ = (0..4).find_map(|x| {
14-
| _____________^
14+
| ____________________^
1515
LL | |
1616
LL | |
1717
LL | | if x > 1 {
@@ -21,27 +21,27 @@ LL | | });
2121
| |______^
2222

2323
error: this `.find_map(..)` can be written more simply using `.find(..)`
24-
--> tests/ui/unnecessary_find_map.rs:15:13
24+
--> tests/ui/unnecessary_find_map.rs:13:20
2525
|
2626
LL | let _ = (0..4).find_map(|x| match x {
27-
| _____________^
27+
| ____________________^
2828
LL | |
2929
LL | | 0 | 1 => None,
3030
LL | | _ => Some(x),
3131
LL | | });
3232
| |______^
3333

3434
error: this `.find_map(..)` can be written more simply using `.map(..).next()`
35-
--> tests/ui/unnecessary_find_map.rs:21:13
35+
--> tests/ui/unnecessary_find_map.rs:19:20
3636
|
3737
LL | let _ = (0..4).find_map(|x| Some(x + 1));
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
3939

4040
error: this `.find_map(..)` can be written more simply using `.find(..)`
41-
--> tests/ui/unnecessary_find_map.rs:33:14
41+
--> tests/ui/unnecessary_find_map.rs:31:33
4242
|
4343
LL | let _x = std::iter::once(1).find_map(|n| (n > 1).then_some(n));
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4545

4646
error: aborting due to 5 previous errors
4747

0 commit comments

Comments
 (0)