user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Do not make incomplete or invalid suggestions (#14487)

Browse source 
The `unnecessary_filter_map` and `unnecessary_find_map` lints were
making partial suggestions, proposing to replace the whole expression by
only the method name, or a subexpression which contained explicit
placeholders.

Since even `MaybeIncorrect` suggestions must generate code that
compiles, this changes those lints to recommandation lints with no code
suggestion.

Fixes #14486

changelog: [`unnecessary_find_map`, `unnecessary_filter_map`]: do not
make suggestions that will not compile
This commit is contained in:
Manish Goregaokar 2025-03-27 21:15:55 +00:00 • committed by GitHub
commit 61b38e76b2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 48 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

26
clippy_lints/src/methods/unnecessary_filter_map.rs
View file

@ -1,11 +1,10 @@
use super::utils::clone_or_copy_needed;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::is_copy;
use clippy_utils::usage::mutated_variables;
use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id};
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_lint::LateContext;
@ -45,30 +44,32 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
&& is_res_lang_ctor(cx, path_res(cx, expr), OptionSome)
&& let hir::ExprKind::Path(_) = args[0].kind
{
span_lint_and_sugg(
span_lint(
cx,
UNNECESSARY_FILTER_MAP,
expr.span,
format!("{name} is unnecessary"),
"try removing the filter_map",
String::new(),
Applicability::MaybeIncorrect,
String::from("this call to `.filter_map(..)` is unnecessary"),
);
return;
}
if name == "filter_map" {
"map(..)"
} else {
"map(..).next()"
}
if name == "filter_map" { "map" } else { "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 == "filter_map" { "filter" } else { "find" }
if name == "filter_map" { "filter(..)" } else { "find(..)" }
},
_ => return,
}
} else {
return;
};
span_lint_and_sugg(
span_lint(
cx,
if name == "filter_map" {
UNNECESSARY_FILTER_MAP
@ -76,10 +77,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
UNNECESSARY_FIND_MAP
},
expr.span,
format!("this `.{name}` can be written more simply"),
"try instead",
sugg.to_string(),
Applicability::MaybeIncorrect,
format!("this `.{name}(..)` can be written more simply using `.{sugg}`"),
);
}
}

7
tests/ui/unnecessary_filter_map.rs
View file

@ -1,5 +1,4 @@
//@no-rustfix
#![allow(dead_code)]
#![allow(clippy::redundant_closure)]
fn main() {
let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
@ -27,9 +26,7 @@ fn main() {
let _ = (0..4).filter_map(Some);
let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
//~^ redundant_closure
//~| unnecessary_filter_map
//~| unnecessary_filter_map
//~^ unnecessary_filter_map
}
fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> {

53
tests/ui/unnecessary_filter_map.stderr
View file

@ -1,14 +1,14 @@
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:5:13
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
--> tests/ui/unnecessary_filter_map.rs:4:13
|
LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `filter`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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
--> tests/ui/unnecessary_filter_map.rs:8:13
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
--> tests/ui/unnecessary_filter_map.rs:7:13
|
LL | let _ = (0..4).filter_map(|x| {
| _____________^
@ -18,10 +18,10 @@ LL | | if x > 1 {
... |
LL | | None
LL | | });
| |______^ help: try instead: `filter`
| |______^
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:16:13
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
--> tests/ui/unnecessary_filter_map.rs:15:13
|
LL | let _ = (0..4).filter_map(|x| match x {
| _____________^
@ -29,40 +29,25 @@ LL | |
LL | | 0 | 1 => None,
LL | | _ => Some(x),
LL | | });
| |______^ help: try instead: `filter`
| |______^
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:22:13
error: this `.filter_map(..)` can be written more simply using `.map(..)`
--> tests/ui/unnecessary_filter_map.rs:21:13
|
LL | let _ = (0..4).filter_map(|x| Some(x + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: redundant closure
--> tests/ui/unnecessary_filter_map.rs:29:57
error: this call to `.filter_map(..)` is unnecessary
--> tests/ui/unnecessary_filter_map.rs:28:61
|
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
| ^^^^^^^^^^^ help: replace the closure with the function itself: `Some`
|
= note: `-D clippy::redundant-closure` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::redundant_closure)]`
| ^^^^
error: filter_map is unnecessary
--> tests/ui/unnecessary_filter_map.rs:29:61
|
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
| ^^^^ help: try removing the filter_map
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:29:13
|
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map`
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:169:14
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
--> tests/ui/unnecessary_filter_map.rs:166:14
|
LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `filter`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 8 previous errors
error: aborting due to 6 previous errors

1
tests/ui/unnecessary_find_map.rs
View file

@ -1,4 +1,3 @@
//@no-rustfix
#![allow(dead_code)]
fn main() {

30
tests/ui/unnecessary_find_map.stderr
View file

@ -1,14 +1,14 @@
error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:5:13
error: this `.find_map(..)` can be written more simply using `.find(..)`
--> tests/ui/unnecessary_find_map.rs:4:13
|
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `find`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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
--> tests/ui/unnecessary_find_map.rs:8:13
error: this `.find_map(..)` can be written more simply using `.find(..)`
--> tests/ui/unnecessary_find_map.rs:7:13
|
LL | let _ = (0..4).find_map(|x| {
| _____________^
@ -18,10 +18,10 @@ LL | | if x > 1 {
... |
LL | | None
LL | | });
| |______^ help: try instead: `find`
| |______^
error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:16:13
error: this `.find_map(..)` can be written more simply using `.find(..)`
--> tests/ui/unnecessary_find_map.rs:15:13
|
LL | let _ = (0..4).find_map(|x| match x {
| _____________^
@ -29,19 +29,19 @@ LL | |
LL | | 0 | 1 => None,
LL | | _ => Some(x),
LL | | });
| |______^ help: try instead: `find`
| |______^
error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:22:13
error: this `.find_map(..)` can be written more simply using `.map(..).next()`
--> tests/ui/unnecessary_find_map.rs:21:13
|
LL | let _ = (0..4).find_map(|x| Some(x + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map(..).next()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:34:14
error: this `.find_map(..)` can be written more simply using `.find(..)`
--> tests/ui/unnecessary_find_map.rs:33:14
|
LL | let _x = std::iter::once(1).find_map(|n| (n > 1).then_some(n));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `find`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 5 previous errors