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
This commit is contained in:
commit
ca168c07a6
5 changed files with 83 additions and 63 deletions
|
|
@ -5217,7 +5217,7 @@ 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, call_span, unnecessary_filter_map::Kind::FilterMap);
|
||||
filter_map_bool_then::check(cx, expr, arg, call_span);
|
||||
filter_map_identity::check(cx, expr, arg, span);
|
||||
lines_filter_map_ok::check_filter_or_flat_map(
|
||||
|
|
@ -5232,7 +5232,7 @@ impl Methods {
|
|||
},
|
||||
(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, call_span, unnecessary_filter_map::Kind::FindMap);
|
||||
},
|
||||
(sym::flat_map, [arg]) => {
|
||||
unused_enumerate_index::check(cx, expr, recv, arg);
|
||||
|
|
|
|||
|
|
@ -2,15 +2,15 @@ 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 rustc_span::Span;
|
||||
use std::fmt::Display;
|
||||
|
||||
use super::{UNNECESSARY_FILTER_MAP, UNNECESSARY_FIND_MAP};
|
||||
|
||||
|
|
@ -18,7 +18,8 @@ pub(super) fn check<'tcx>(
|
|||
cx: &LateContext<'tcx>,
|
||||
expr: &'tcx hir::Expr<'tcx>,
|
||||
arg: &'tcx hir::Expr<'tcx>,
|
||||
name: Symbol,
|
||||
call_span: Span,
|
||||
kind: Kind,
|
||||
) {
|
||||
if !cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
|
||||
return;
|
||||
|
|
@ -45,61 +46,88 @@ 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,
|
||||
UNNECESSARY_FILTER_MAP,
|
||||
expr.span,
|
||||
call_span,
|
||||
String::from("this call to `.filter_map(..)` is unnecessary"),
|
||||
);
|
||||
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}`"),
|
||||
call_span,
|
||||
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 +137,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 +164,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),
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,17 +1,17 @@
|
|||
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
|
||||
--> tests/ui/unnecessary_filter_map.rs:4:13
|
||||
--> tests/ui/unnecessary_filter_map.rs:4:20
|
||||
|
|
||||
LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::unnecessary-filter-map` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_filter_map)]`
|
||||
|
||||
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
|
||||
--> tests/ui/unnecessary_filter_map.rs:7:13
|
||||
--> tests/ui/unnecessary_filter_map.rs:7:20
|
||||
|
|
||||
LL | let _ = (0..4).filter_map(|x| {
|
||||
| _____________^
|
||||
| ____________________^
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | if x > 1 {
|
||||
|
|
@ -21,10 +21,10 @@ LL | | });
|
|||
| |______^
|
||||
|
||||
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
|
||||
--> tests/ui/unnecessary_filter_map.rs:15:13
|
||||
--> tests/ui/unnecessary_filter_map.rs:15:20
|
||||
|
|
||||
LL | let _ = (0..4).filter_map(|x| match x {
|
||||
| _____________^
|
||||
| ____________________^
|
||||
LL | |
|
||||
LL | | 0 | 1 => None,
|
||||
LL | | _ => Some(x),
|
||||
|
|
@ -32,22 +32,22 @@ LL | | });
|
|||
| |______^
|
||||
|
||||
error: this `.filter_map(..)` can be written more simply using `.map(..)`
|
||||
--> tests/ui/unnecessary_filter_map.rs:21:13
|
||||
--> tests/ui/unnecessary_filter_map.rs:21:20
|
||||
|
|
||||
LL | let _ = (0..4).filter_map(|x| Some(x + 1));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: this call to `.filter_map(..)` is unnecessary
|
||||
--> tests/ui/unnecessary_filter_map.rs:28:61
|
||||
--> tests/ui/unnecessary_filter_map.rs:28:46
|
||||
|
|
||||
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
|
||||
| ^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
|
||||
--> tests/ui/unnecessary_filter_map.rs:166:14
|
||||
--> tests/ui/unnecessary_filter_map.rs:166:33
|
||||
|
|
||||
LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 6 previous errors
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,3 @@
|
|||
#![allow(dead_code)]
|
||||
|
||||
fn main() {
|
||||
let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
|
||||
//~^ unnecessary_find_map
|
||||
|
|
|
|||
|
|
@ -1,17 +1,17 @@
|
|||
error: this `.find_map(..)` can be written more simply using `.find(..)`
|
||||
--> tests/ui/unnecessary_find_map.rs:4:13
|
||||
--> tests/ui/unnecessary_find_map.rs:2:20
|
||||
|
|
||||
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::unnecessary-find-map` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_find_map)]`
|
||||
|
||||
error: this `.find_map(..)` can be written more simply using `.find(..)`
|
||||
--> tests/ui/unnecessary_find_map.rs:7:13
|
||||
--> tests/ui/unnecessary_find_map.rs:5:20
|
||||
|
|
||||
LL | let _ = (0..4).find_map(|x| {
|
||||
| _____________^
|
||||
| ____________________^
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | if x > 1 {
|
||||
|
|
@ -21,10 +21,10 @@ LL | | });
|
|||
| |______^
|
||||
|
||||
error: this `.find_map(..)` can be written more simply using `.find(..)`
|
||||
--> tests/ui/unnecessary_find_map.rs:15:13
|
||||
--> tests/ui/unnecessary_find_map.rs:13:20
|
||||
|
|
||||
LL | let _ = (0..4).find_map(|x| match x {
|
||||
| _____________^
|
||||
| ____________________^
|
||||
LL | |
|
||||
LL | | 0 | 1 => None,
|
||||
LL | | _ => Some(x),
|
||||
|
|
@ -32,16 +32,16 @@ LL | | });
|
|||
| |______^
|
||||
|
||||
error: this `.find_map(..)` can be written more simply using `.map(..).next()`
|
||||
--> tests/ui/unnecessary_find_map.rs:21:13
|
||||
--> tests/ui/unnecessary_find_map.rs:19:20
|
||||
|
|
||||
LL | let _ = (0..4).find_map(|x| Some(x + 1));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: this `.find_map(..)` can be written more simply using `.find(..)`
|
||||
--> tests/ui/unnecessary_find_map.rs:33:14
|
||||
--> tests/ui/unnecessary_find_map.rs:31:33
|
||||
|
|
||||
LL | let _x = std::iter::once(1).find_map(|n| (n > 1).then_some(n));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 5 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue