- introduce `Kind`: a bit more type-safe and hopefully a bit faster
This commit is contained in:
Ada Alakbarova 2025-10-21 17:54:50 +02:00
parent fb82de5ed8
commit 9fd359b53e
No known key found for this signature in database
2 changed files with 57 additions and 42 deletions

View file

@ -5162,13 +5162,13 @@ impl Methods {
},
(sym::filter_map, [arg]) => {
unused_enumerate_index::check(cx, expr, recv, arg);
unnecessary_filter_map::check(cx, expr, arg, name);
unnecessary_filter_map::check(cx, expr, arg, unnecessary_filter_map::Kind::FilterMap);
filter_map_bool_then::check(cx, expr, arg, call_span);
filter_map_identity::check(cx, expr, arg, span);
},
(sym::find_map, [arg]) => {
unused_enumerate_index::check(cx, expr, recv, arg);
unnecessary_filter_map::check(cx, expr, arg, name);
unnecessary_filter_map::check(cx, expr, arg, unnecessary_filter_map::Kind::FindMap);
},
(sym::flat_map, [arg]) => {
unused_enumerate_index::check(cx, expr, recv, arg);

View file

@ -2,24 +2,18 @@ use super::utils::clone_or_copy_needed;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath, MaybeTypeckRes};
use clippy_utils::sym;
use clippy_utils::ty::is_copy;
use clippy_utils::ty::{is_copy, option_arg_ty};
use clippy_utils::usage::mutated_variables;
use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
use core::ops::ControlFlow;
use rustc_hir as hir;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::Symbol;
use std::fmt::Display;
use super::{UNNECESSARY_FILTER_MAP, UNNECESSARY_FIND_MAP};
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
arg: &'tcx hir::Expr<'tcx>,
name: Symbol,
) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>, kind: Kind) {
if !cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
return;
}
@ -45,10 +39,10 @@ pub(super) fn check<'tcx>(
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
let sugg = if !found_filtering {
// Check if the closure is .filter_map(|x| Some(x))
if name == sym::filter_map
&& let hir::ExprKind::Call(expr, args) = body.value.kind
if kind.is_filter_map()
&& let hir::ExprKind::Call(expr, [arg]) = body.value.kind
&& expr.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome)
&& let hir::ExprKind::Path(_) = args[0].kind
&& let hir::ExprKind::Path(_) = arg.kind
{
span_lint(
cx,
@ -58,48 +52,75 @@ pub(super) fn check<'tcx>(
);
return;
}
if name == sym::filter_map {
"map(..)"
} else {
"map(..).next()"
match kind {
Kind::FilterMap => "map(..)",
Kind::FindMap => "map(..).next()",
}
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
match cx.typeck_results().expr_ty(body.value).kind() {
ty::Adt(adt, subst)
if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) && in_ty == subst.type_at(0) =>
{
if name == sym::filter_map {
"filter(..)"
} else {
"find(..)"
}
},
_ => return,
let ty = cx.typeck_results().expr_ty(body.value);
if option_arg_ty(cx, ty).is_some_and(|t| t == in_ty) {
match kind {
Kind::FilterMap => "filter(..)",
Kind::FindMap => "find(..)",
}
} else {
return;
}
} else {
return;
};
span_lint(
cx,
if name == sym::filter_map {
UNNECESSARY_FILTER_MAP
} else {
UNNECESSARY_FIND_MAP
match kind {
Kind::FilterMap => UNNECESSARY_FILTER_MAP,
Kind::FindMap => UNNECESSARY_FIND_MAP,
},
expr.span,
format!("this `.{name}(..)` can be written more simply using `.{sugg}`"),
format!("this `.{kind}(..)` can be written more simply using `.{sugg}`"),
);
}
}
#[derive(Clone, Copy)]
pub(super) enum Kind {
FilterMap,
FindMap,
}
impl Kind {
fn is_filter_map(self) -> bool {
matches!(self, Self::FilterMap)
}
}
impl Display for Kind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::FilterMap => f.write_str("filter_map"),
Self::FindMap => f.write_str("find_map"),
}
}
}
// returns (found_mapping, found_filtering)
fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tcx hir::Expr<'_>) -> (bool, bool) {
match expr.kind {
hir::ExprKind::Path(ref path)
if cx
.qpath_res(path, expr.hir_id)
.ctor_parent(cx)
.is_lang_item(cx, OptionNone) =>
{
// None
(false, true)
},
hir::ExprKind::Call(func, args) => {
if func.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome) {
if args[0].res_local_id() == Some(arg_id) {
// Some(arg_id)
return (false, false);
}
// Some(not arg_id)
return (true, false);
}
(true, true)
@ -109,8 +130,10 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
&& cx.typeck_results().expr_ty(recv).is_bool()
&& arg.res_local_id() == Some(arg_id)
{
// bool.then_some(arg_id)
(false, true)
} else {
// bool.then_some(not arg_id)
(true, true)
}
},
@ -134,14 +157,6 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
let else_check = check_expression(cx, arg_id, else_arm);
(if_check.0 | else_check.0, if_check.1 | else_check.1)
},
hir::ExprKind::Path(ref path)
if cx
.qpath_res(path, expr.hir_id)
.ctor_parent(cx)
.is_lang_item(cx, OptionNone) =>
{
(false, true)
},
_ => (true, true),
}
}