Skip to content

Commit 61b38e7

Browse files
ManishearthManishearth
authored
Do not make incomplete or invalid suggestions (#14487)
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
2 parents a895265 + 01820a9 commit 61b38e7

File tree

5 files changed

+48
-69
lines changed

5 files changed

+48
-69
lines changed

‎clippy_lints/src/methods/unnecessary_filter_map.rs

+12-14
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
use super::utils::clone_or_copy_needed;
2-
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::diagnostics::span_lint;
33
use clippy_utils::ty::is_copy;
44
use clippy_utils::usage::mutated_variables;
55
use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
66
use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id};
77
use core::ops::ControlFlow;
8-
use rustc_errors::Applicability;
98
use rustc_hir as hir;
109
use rustc_hir::LangItem::{OptionNone, OptionSome};
1110
use rustc_lint::LateContext;
@@ -45,41 +44,40 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
4544
&& is_res_lang_ctor(cx, path_res(cx, expr), OptionSome)
4645
&& let hir::ExprKind::Path(_) = args[0].kind
4746
{
48-
span_lint_and_sugg(
47+
span_lint(
4948
cx,
5049
UNNECESSARY_FILTER_MAP,
5150
expr.span,
52-
format!("{name} is unnecessary"),
53-
"try removing the filter_map",
54-
String::new(),
55-
Applicability::MaybeIncorrect,
51+
String::from("this call to `.filter_map(..)` is unnecessary"),
5652
);
53+
return;
54+
}
55+
if name == "filter_map" {
56+
"map(..)"
57+
} else {
58+
"map(..).next()"
5759
}
58-
if name == "filter_map" { "map" } else { "map(..).next()" }
5960
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
6061
match cx.typeck_results().expr_ty(body.value).kind() {
6162
ty::Adt(adt, subst)
6263
if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) && in_ty == subst.type_at(0) =>
6364
{
64-
if name == "filter_map" { "filter" } else { "find" }
65+
if name == "filter_map" { "filter(..)" } else { "find(..)" }
6566
},
6667
_ => return,
6768
}
6869
} else {
6970
return;
7071
};
71-
span_lint_and_sugg(
72+
span_lint(
7273
cx,
7374
if name == "filter_map" {
7475
UNNECESSARY_FILTER_MAP
7576
} else {
7677
UNNECESSARY_FIND_MAP
7778
},
7879
expr.span,
79-
format!("this `.{name}` can be written more simply"),
80-
"try instead",
81-
sugg.to_string(),
82-
Applicability::MaybeIncorrect,
80+
format!("this `.{name}(..)` can be written more simply using `.{sugg}`"),
8381
);
8482
}
8583
}

‎tests/ui/unnecessary_filter_map.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//@no-rustfix
2-
#![allow(dead_code)]
1+
#![allow(clippy::redundant_closure)]
32

43
fn main() {
54
let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
@@ -27,9 +26,7 @@ fn main() {
2726
let _ = (0..4).filter_map(Some);
2827

2928
let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
30-
//~^ redundant_closure
31-
//~| unnecessary_filter_map
32-
//~| unnecessary_filter_map
29+
//~^ unnecessary_filter_map
3330
}
3431

3532
fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> {

‎tests/ui/unnecessary_filter_map.stderr

+19-34
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
error: this `.filter_map` can be written more simply
2-
--> tests/ui/unnecessary_filter_map.rs:5:13
1+
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
2+
--> tests/ui/unnecessary_filter_map.rs:4:13
33
|
44
LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `filter`
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

10-
error: this `.filter_map` can be written more simply
11-
--> tests/ui/unnecessary_filter_map.rs:8:13
10+
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
11+
--> tests/ui/unnecessary_filter_map.rs:7:13
1212
|
1313
LL | let _ = (0..4).filter_map(|x| {
1414
| _____________^
@@ -18,51 +18,36 @@ LL | | if x > 1 {
1818
... |
1919
LL | | None
2020
LL | | });
21-
| |______^ help: try instead: `filter`
21+
| |______^
2222

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

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

40-
error: redundant closure
41-
--> tests/ui/unnecessary_filter_map.rs:29:57
40+
error: this call to `.filter_map(..)` is unnecessary
41+
--> tests/ui/unnecessary_filter_map.rs:28:61
4242
|
4343
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
44-
| ^^^^^^^^^^^ help: replace the closure with the function itself: `Some`
45-
|
46-
= note: `-D clippy::redundant-closure` implied by `-D warnings`
47-
= help: to override `-D warnings` add `#[allow(clippy::redundant_closure)]`
48-
49-
error: filter_map is unnecessary
50-
--> tests/ui/unnecessary_filter_map.rs:29:61
51-
|
52-
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
53-
| ^^^^ help: try removing the filter_map
54-
55-
error: this `.filter_map` can be written more simply
56-
--> tests/ui/unnecessary_filter_map.rs:29:13
57-
|
58-
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
59-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map`
44+
| ^^^^
6045

61-
error: this `.filter_map` can be written more simply
62-
--> tests/ui/unnecessary_filter_map.rs:169:14
46+
error: this `.filter_map(..)` can be written more simply using `.filter(..)`
47+
--> tests/ui/unnecessary_filter_map.rs:166:14
6348
|
6449
LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n));
65-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `filter`
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6651

67-
error: aborting due to 8 previous errors
52+
error: aborting due to 6 previous errors
6853

‎tests/ui/unnecessary_find_map.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
//@no-rustfix
21
#![allow(dead_code)]
32

43
fn main() {

‎tests/ui/unnecessary_find_map.stderr

+15-15
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
error: this `.find_map` can be written more simply
2-
--> tests/ui/unnecessary_find_map.rs:5:13
1+
error: this `.find_map(..)` can be written more simply using `.find(..)`
2+
--> tests/ui/unnecessary_find_map.rs:4:13
33
|
44
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `find`
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

10-
error: this `.find_map` can be written more simply
11-
--> tests/ui/unnecessary_find_map.rs:8:13
10+
error: this `.find_map(..)` can be written more simply using `.find(..)`
11+
--> tests/ui/unnecessary_find_map.rs:7:13
1212
|
1313
LL | let _ = (0..4).find_map(|x| {
1414
| _____________^
@@ -18,30 +18,30 @@ LL | | if x > 1 {
1818
... |
1919
LL | | None
2020
LL | | });
21-
| |______^ help: try instead: `find`
21+
| |______^
2222

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

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

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

4646
error: aborting due to 5 previous errors
4747

0 commit comments

Comments
 (0)