Skip to content

Commit 542d474

Browse files
borsbors
committed
Auto merge of #8930 - kyoto7250:issue_8920, r=Alexendoo
fix(manual_find_map and manual_filter_map): check clone method close #8920 Added conditional branching when the clone method is used. Thank you in advance. --- changelog: check `clone()` and other variant preserving methods in [`manual_find_map`] and [`manual_filter_map`]
2 parents d9ddce8 + 42cf985 commit 542d474

7 files changed

+389
-4
lines changed

‎clippy_lints/src/methods/filter_map.rs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use if_chain::if_chain;
66
use rustc_errors::Applicability;
77
use rustc_hir as hir;
88
use rustc_hir::def::Res;
9-
use rustc_hir::{Expr, ExprKind, PatKind, QPath, UnOp};
9+
use rustc_hir::{Expr, ExprKind, PatKind, PathSegment, QPath, UnOp};
1010
use rustc_lint::LateContext;
1111
use rustc_span::source_map::Span;
1212
use rustc_span::symbol::{sym, Symbol};
@@ -155,7 +155,15 @@ pub(super) fn check<'tcx>(
155155
}
156156
false
157157
};
158-
if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
158+
159+
if match map_arg.kind {
160+
ExprKind::MethodCall(method, [original_arg], _) => {
161+
acceptable_methods(method)
162+
&& SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, original_arg)
163+
},
164+
_ => SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg)
165+
};
166+
159167
then {
160168
let span = filter_span.with_hi(expr.span.hi());
161169
let (filter_name, lint) = if is_find {
@@ -171,3 +179,18 @@ pub(super) fn check<'tcx>(
171179
}
172180
}
173181
}
182+
183+
fn acceptable_methods(method: &PathSegment<'_>) -> bool {
184+
let methods: [Symbol; 8] = [
185+
sym::clone,
186+
sym::as_ref,
187+
sym!(copied),
188+
sym!(cloned),
189+
sym!(as_deref),
190+
sym!(as_mut),
191+
sym!(as_deref_mut),
192+
sym!(to_owned),
193+
];
194+
195+
methods.contains(&method.ident.name)
196+
}

‎tests/ui/manual_filter_map.fixed

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,53 @@ fn to_opt<T>(_: T) -> Option<T> {
3535
fn to_res<T>(_: T) -> Result<T, ()> {
3636
unimplemented!()
3737
}
38+
39+
struct Issue8920<'a> {
40+
option_field: Option<String>,
41+
result_field: Result<String, ()>,
42+
ref_field: Option<&'a usize>,
43+
}
44+
45+
fn issue_8920() {
46+
let mut vec = vec![Issue8920 {
47+
option_field: Some(String::from("str")),
48+
result_field: Ok(String::from("str")),
49+
ref_field: Some(&1),
50+
}];
51+
52+
let _ = vec
53+
.iter()
54+
.filter_map(|f| f.option_field.clone());
55+
56+
let _ = vec
57+
.iter()
58+
.filter_map(|f| f.ref_field.cloned());
59+
60+
let _ = vec
61+
.iter()
62+
.filter_map(|f| f.ref_field.copied());
63+
64+
let _ = vec
65+
.iter()
66+
.filter_map(|f| f.result_field.clone().ok());
67+
68+
let _ = vec
69+
.iter()
70+
.filter_map(|f| f.result_field.as_ref().ok());
71+
72+
let _ = vec
73+
.iter()
74+
.filter_map(|f| f.result_field.as_deref().ok());
75+
76+
let _ = vec
77+
.iter_mut()
78+
.filter_map(|f| f.result_field.as_mut().ok());
79+
80+
let _ = vec
81+
.iter_mut()
82+
.filter_map(|f| f.result_field.as_deref_mut().ok());
83+
84+
let _ = vec
85+
.iter()
86+
.filter_map(|f| f.result_field.to_owned().ok());
87+
}

‎tests/ui/manual_filter_map.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,62 @@ fn to_opt<T>(_: T) -> Option<T> {
3535
fn to_res<T>(_: T) -> Result<T, ()> {
3636
unimplemented!()
3737
}
38+
39+
struct Issue8920<'a> {
40+
option_field: Option<String>,
41+
result_field: Result<String, ()>,
42+
ref_field: Option<&'a usize>,
43+
}
44+
45+
fn issue_8920() {
46+
let mut vec = vec![Issue8920 {
47+
option_field: Some(String::from("str")),
48+
result_field: Ok(String::from("str")),
49+
ref_field: Some(&1),
50+
}];
51+
52+
let _ = vec
53+
.iter()
54+
.filter(|f| f.option_field.is_some())
55+
.map(|f| f.option_field.clone().unwrap());
56+
57+
let _ = vec
58+
.iter()
59+
.filter(|f| f.ref_field.is_some())
60+
.map(|f| f.ref_field.cloned().unwrap());
61+
62+
let _ = vec
63+
.iter()
64+
.filter(|f| f.ref_field.is_some())
65+
.map(|f| f.ref_field.copied().unwrap());
66+
67+
let _ = vec
68+
.iter()
69+
.filter(|f| f.result_field.is_ok())
70+
.map(|f| f.result_field.clone().unwrap());
71+
72+
let _ = vec
73+
.iter()
74+
.filter(|f| f.result_field.is_ok())
75+
.map(|f| f.result_field.as_ref().unwrap());
76+
77+
let _ = vec
78+
.iter()
79+
.filter(|f| f.result_field.is_ok())
80+
.map(|f| f.result_field.as_deref().unwrap());
81+
82+
let _ = vec
83+
.iter_mut()
84+
.filter(|f| f.result_field.is_ok())
85+
.map(|f| f.result_field.as_mut().unwrap());
86+
87+
let _ = vec
88+
.iter_mut()
89+
.filter(|f| f.result_field.is_ok())
90+
.map(|f| f.result_field.as_deref_mut().unwrap());
91+
92+
let _ = vec
93+
.iter()
94+
.filter(|f| f.result_field.is_ok())
95+
.map(|f| f.result_field.to_owned().unwrap());
96+
}

‎tests/ui/manual_filter_map.stderr

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,77 @@ error: `filter(..).map(..)` can be simplified as `filter_map(..)`
1818
LL | let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1));
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_res(a).ok())`
2020

21-
error: aborting due to 3 previous errors
21+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
22+
--> $DIR/manual_filter_map.rs:54:10
23+
|
24+
LL | .filter(|f| f.option_field.is_some())
25+
| __________^
26+
LL | | .map(|f| f.option_field.clone().unwrap());
27+
| |_________________________________________________^ help: try: `filter_map(|f| f.option_field.clone())`
28+
29+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
30+
--> $DIR/manual_filter_map.rs:59:10
31+
|
32+
LL | .filter(|f| f.ref_field.is_some())
33+
| __________^
34+
LL | | .map(|f| f.ref_field.cloned().unwrap());
35+
| |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.cloned())`
36+
37+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
38+
--> $DIR/manual_filter_map.rs:64:10
39+
|
40+
LL | .filter(|f| f.ref_field.is_some())
41+
| __________^
42+
LL | | .map(|f| f.ref_field.copied().unwrap());
43+
| |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.copied())`
44+
45+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
46+
--> $DIR/manual_filter_map.rs:69:10
47+
|
48+
LL | .filter(|f| f.result_field.is_ok())
49+
| __________^
50+
LL | | .map(|f| f.result_field.clone().unwrap());
51+
| |_________________________________________________^ help: try: `filter_map(|f| f.result_field.clone().ok())`
52+
53+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
54+
--> $DIR/manual_filter_map.rs:74:10
55+
|
56+
LL | .filter(|f| f.result_field.is_ok())
57+
| __________^
58+
LL | | .map(|f| f.result_field.as_ref().unwrap());
59+
| |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_ref().ok())`
60+
61+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
62+
--> $DIR/manual_filter_map.rs:79:10
63+
|
64+
LL | .filter(|f| f.result_field.is_ok())
65+
| __________^
66+
LL | | .map(|f| f.result_field.as_deref().unwrap());
67+
| |____________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref().ok())`
68+
69+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
70+
--> $DIR/manual_filter_map.rs:84:10
71+
|
72+
LL | .filter(|f| f.result_field.is_ok())
73+
| __________^
74+
LL | | .map(|f| f.result_field.as_mut().unwrap());
75+
| |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_mut().ok())`
76+
77+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
78+
--> $DIR/manual_filter_map.rs:89:10
79+
|
80+
LL | .filter(|f| f.result_field.is_ok())
81+
| __________^
82+
LL | | .map(|f| f.result_field.as_deref_mut().unwrap());
83+
| |________________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref_mut().ok())`
84+
85+
error: `filter(..).map(..)` can be simplified as `filter_map(..)`
86+
--> $DIR/manual_filter_map.rs:94:10
87+
|
88+
LL | .filter(|f| f.result_field.is_ok())
89+
| __________^
90+
LL | | .map(|f| f.result_field.to_owned().unwrap());
91+
| |____________________________________________________^ help: try: `filter_map(|f| f.result_field.to_owned().ok())`
92+
93+
error: aborting due to 12 previous errors
2294

‎tests/ui/manual_find_map.fixed

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,53 @@ fn to_opt<T>(_: T) -> Option<T> {
3535
fn to_res<T>(_: T) -> Result<T, ()> {
3636
unimplemented!()
3737
}
38+
39+
struct Issue8920<'a> {
40+
option_field: Option<String>,
41+
result_field: Result<String, ()>,
42+
ref_field: Option<&'a usize>,
43+
}
44+
45+
fn issue_8920() {
46+
let mut vec = vec![Issue8920 {
47+
option_field: Some(String::from("str")),
48+
result_field: Ok(String::from("str")),
49+
ref_field: Some(&1),
50+
}];
51+
52+
let _ = vec
53+
.iter()
54+
.find_map(|f| f.option_field.clone());
55+
56+
let _ = vec
57+
.iter()
58+
.find_map(|f| f.ref_field.cloned());
59+
60+
let _ = vec
61+
.iter()
62+
.find_map(|f| f.ref_field.copied());
63+
64+
let _ = vec
65+
.iter()
66+
.find_map(|f| f.result_field.clone().ok());
67+
68+
let _ = vec
69+
.iter()
70+
.find_map(|f| f.result_field.as_ref().ok());
71+
72+
let _ = vec
73+
.iter()
74+
.find_map(|f| f.result_field.as_deref().ok());
75+
76+
let _ = vec
77+
.iter_mut()
78+
.find_map(|f| f.result_field.as_mut().ok());
79+
80+
let _ = vec
81+
.iter_mut()
82+
.find_map(|f| f.result_field.as_deref_mut().ok());
83+
84+
let _ = vec
85+
.iter()
86+
.find_map(|f| f.result_field.to_owned().ok());
87+
}

‎tests/ui/manual_find_map.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,62 @@ fn to_opt<T>(_: T) -> Option<T> {
3535
fn to_res<T>(_: T) -> Result<T, ()> {
3636
unimplemented!()
3737
}
38+
39+
struct Issue8920<'a> {
40+
option_field: Option<String>,
41+
result_field: Result<String, ()>,
42+
ref_field: Option<&'a usize>,
43+
}
44+
45+
fn issue_8920() {
46+
let mut vec = vec![Issue8920 {
47+
option_field: Some(String::from("str")),
48+
result_field: Ok(String::from("str")),
49+
ref_field: Some(&1),
50+
}];
51+
52+
let _ = vec
53+
.iter()
54+
.find(|f| f.option_field.is_some())
55+
.map(|f| f.option_field.clone().unwrap());
56+
57+
let _ = vec
58+
.iter()
59+
.find(|f| f.ref_field.is_some())
60+
.map(|f| f.ref_field.cloned().unwrap());
61+
62+
let _ = vec
63+
.iter()
64+
.find(|f| f.ref_field.is_some())
65+
.map(|f| f.ref_field.copied().unwrap());
66+
67+
let _ = vec
68+
.iter()
69+
.find(|f| f.result_field.is_ok())
70+
.map(|f| f.result_field.clone().unwrap());
71+
72+
let _ = vec
73+
.iter()
74+
.find(|f| f.result_field.is_ok())
75+
.map(|f| f.result_field.as_ref().unwrap());
76+
77+
let _ = vec
78+
.iter()
79+
.find(|f| f.result_field.is_ok())
80+
.map(|f| f.result_field.as_deref().unwrap());
81+
82+
let _ = vec
83+
.iter_mut()
84+
.find(|f| f.result_field.is_ok())
85+
.map(|f| f.result_field.as_mut().unwrap());
86+
87+
let _ = vec
88+
.iter_mut()
89+
.find(|f| f.result_field.is_ok())
90+
.map(|f| f.result_field.as_deref_mut().unwrap());
91+
92+
let _ = vec
93+
.iter()
94+
.find(|f| f.result_field.is_ok())
95+
.map(|f| f.result_field.to_owned().unwrap());
96+
}

0 commit comments

Comments
 (0)