Skip to content

Commit 9fd359b

Browse files
committed
clean-up
- introduce `Kind`: a bit more type-safe and hopefully a bit faster
1 parent fb82de5 commit 9fd359b

File tree

2 files changed

+57
-42
lines changed

2 files changed

+57
-42
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5162,13 +5162,13 @@ impl Methods {
51625162
},
51635163
(sym::filter_map, [arg]) => {
51645164
unused_enumerate_index::check(cx, expr, recv, arg);
5165-
unnecessary_filter_map::check(cx, expr, arg, name);
5165+
unnecessary_filter_map::check(cx, expr, arg, unnecessary_filter_map::Kind::FilterMap);
51665166
filter_map_bool_then::check(cx, expr, arg, call_span);
51675167
filter_map_identity::check(cx, expr, arg, span);
51685168
},
51695169
(sym::find_map, [arg]) => {
51705170
unused_enumerate_index::check(cx, expr, recv, arg);
5171-
unnecessary_filter_map::check(cx, expr, arg, name);
5171+
unnecessary_filter_map::check(cx, expr, arg, unnecessary_filter_map::Kind::FindMap);
51725172
},
51735173
(sym::flat_map, [arg]) => {
51745174
unused_enumerate_index::check(cx, expr, recv, arg);

clippy_lints/src/methods/unnecessary_filter_map.rs

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,18 @@ 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 std::fmt::Display;
1413

1514
use super::{UNNECESSARY_FILTER_MAP, UNNECESSARY_FIND_MAP};
1615

17-
pub(super) fn check<'tcx>(
18-
cx: &LateContext<'tcx>,
19-
expr: &'tcx hir::Expr<'tcx>,
20-
arg: &'tcx hir::Expr<'tcx>,
21-
name: Symbol,
22-
) {
16+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>, kind: Kind) {
2317
if !cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
2418
return;
2519
}
@@ -45,10 +39,10 @@ pub(super) fn check<'tcx>(
4539
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
4640
let sugg = if !found_filtering {
4741
// 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
42+
if kind.is_filter_map()
43+
&& let hir::ExprKind::Call(expr, [arg]) = body.value.kind
5044
&& expr.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome)
51-
&& let hir::ExprKind::Path(_) = args[0].kind
45+
&& let hir::ExprKind::Path(_) = arg.kind
5246
{
5347
span_lint(
5448
cx,
@@ -58,48 +52,75 @@ pub(super) fn check<'tcx>(
5852
);
5953
return;
6054
}
61-
if name == sym::filter_map {
62-
"map(..)"
63-
} else {
64-
"map(..).next()"
55+
match kind {
56+
Kind::FilterMap => "map(..)",
57+
Kind::FindMap => "map(..).next()",
6558
}
6659
} 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,
60+
let ty = cx.typeck_results().expr_ty(body.value);
61+
if option_arg_ty(cx, ty).is_some_and(|t| t == in_ty) {
62+
match kind {
63+
Kind::FilterMap => "filter(..)",
64+
Kind::FindMap => "find(..)",
65+
}
66+
} else {
67+
return;
7868
}
7969
} else {
8070
return;
8171
};
8272
span_lint(
8373
cx,
84-
if name == sym::filter_map {
85-
UNNECESSARY_FILTER_MAP
86-
} else {
87-
UNNECESSARY_FIND_MAP
74+
match kind {
75+
Kind::FilterMap => UNNECESSARY_FILTER_MAP,
76+
Kind::FindMap => UNNECESSARY_FIND_MAP,
8877
},
8978
expr.span,
90-
format!("this `.{name}(..)` can be written more simply using `.{sugg}`"),
79+
format!("this `.{kind}(..)` can be written more simply using `.{sugg}`"),
9180
);
9281
}
9382
}
9483

84+
#[derive(Clone, Copy)]
85+
pub(super) enum Kind {
86+
FilterMap,
87+
FindMap,
88+
}
89+
90+
impl Kind {
91+
fn is_filter_map(self) -> bool {
92+
matches!(self, Self::FilterMap)
93+
}
94+
}
95+
96+
impl Display for Kind {
97+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
98+
match self {
99+
Self::FilterMap => f.write_str("filter_map"),
100+
Self::FindMap => f.write_str("find_map"),
101+
}
102+
}
103+
}
104+
95105
// returns (found_mapping, found_filtering)
96106
fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tcx hir::Expr<'_>) -> (bool, bool) {
97107
match expr.kind {
108+
hir::ExprKind::Path(ref path)
109+
if cx
110+
.qpath_res(path, expr.hir_id)
111+
.ctor_parent(cx)
112+
.is_lang_item(cx, OptionNone) =>
113+
{
114+
// None
115+
(false, true)
116+
},
98117
hir::ExprKind::Call(func, args) => {
99118
if func.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome) {
100119
if args[0].res_local_id() == Some(arg_id) {
120+
// Some(arg_id)
101121
return (false, false);
102122
}
123+
// Some(not arg_id)
103124
return (true, false);
104125
}
105126
(true, true)
@@ -109,8 +130,10 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
109130
&& cx.typeck_results().expr_ty(recv).is_bool()
110131
&& arg.res_local_id() == Some(arg_id)
111132
{
133+
// bool.then_some(arg_id)
112134
(false, true)
113135
} else {
136+
// bool.then_some(not arg_id)
114137
(true, true)
115138
}
116139
},
@@ -134,14 +157,6 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
134157
let else_check = check_expression(cx, arg_id, else_arm);
135158
(if_check.0 | else_check.0, if_check.1 | else_check.1)
136159
},
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-
},
145160
_ => (true, true),
146161
}
147162
}

0 commit comments

Comments
 (0)