Skip to content

Commit 91dd9c4

Browse files
committed
Handle other projection kinds
1 parent 97783a8 commit 91dd9c4

7 files changed

+231
-14
lines changed

clippy_lints/src/methods/search_is_some.rs

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
220220
let ident_str = map.name(id).to_string();
221221
let span = map.span(cmt.hir_id);
222222
let start_span = Span::new(self.next_pos, span.lo(), span.ctxt());
223-
let start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
223+
let mut start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
224224

225225
if cmt.place.projections.is_empty() {
226226
// handle item without any projection, that needs an explicit borrowing
@@ -255,19 +255,75 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
255255
// handle item projections by removing one explicit deref
256256
// i.e.: suggest `*x` instead of `**x`
257257
let mut replacement_str = ident_str;
258-
let last_deref = cmt
259-
.place
260-
.projections
261-
.iter()
262-
.rposition(|proj| proj.kind == ProjectionKind::Deref);
263258

264-
if let Some(pos) = last_deref {
265-
let mut projections = cmt.place.projections.clone();
266-
projections.truncate(pos);
259+
// handle index projection first
260+
let index_handled = cmt.place.projections.iter().any(|proj| match proj.kind {
261+
// Index projection like `|x| foo[x]`
262+
// the index is dropped so we can't get it to build the suggestion,
263+
// so the span is set-up again to get more code, using `span.hi()` (i.e.: `foo[x]`)
264+
// instead of `span.lo()` (i.e.: `foo`)
265+
ProjectionKind::Index => {
266+
let start_span = Span::new(self.next_pos, span.hi(), span.ctxt());
267+
start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
268+
replacement_str.clear();
269+
true
270+
},
271+
_ => false,
272+
});
273+
274+
// looking for projections other that need to be handled differently
275+
let other_projections_handled = cmt.place.projections.iter().enumerate().any(|(i, proj)| {
276+
match proj.kind {
277+
// Field projection like `|v| v.foo`
278+
ProjectionKind::Field(idx, variant) => match cmt.place.ty_before_projection(i).kind() {
279+
ty::Adt(def, ..) => {
280+
replacement_str = format!(
281+
"{}.{}",
282+
replacement_str,
283+
def.variants[variant].fields[idx as usize].ident.name.as_str()
284+
);
285+
true
286+
},
287+
ty::Tuple(_) => {
288+
replacement_str = format!("{}.{}", replacement_str, idx);
289+
true
290+
},
291+
_ => false,
292+
},
293+
ProjectionKind::Index => false, /* handled previously */
294+
// note: unable to capture `Subslice` kind in tests
295+
ProjectionKind::Subslice => false,
296+
ProjectionKind::Deref => {
297+
// explicit deref for arrays should be avoided in the suggestion
298+
// i.e.: `|sub| *sub[1..4].len() == 3` is not expected
299+
match cmt.place.ty_before_projection(i).kind() {
300+
// dereferencing an array (i.e.: `|sub| sub[1..4].len() == 3`)
301+
ty::Ref(_, inner, _) => match inner.kind() {
302+
ty::Ref(_, innermost, _) if innermost.is_array() => true,
303+
_ => false,
304+
},
305+
_ => false,
306+
}
307+
},
308+
}
309+
});
267310

268-
for item in projections {
269-
if item.kind == ProjectionKind::Deref {
270-
replacement_str = format!("*{}", replacement_str);
311+
// handle `ProjectionKind::Deref` if no special case detected
312+
if !index_handled && !other_projections_handled {
313+
let last_deref = cmt
314+
.place
315+
.projections
316+
.iter()
317+
.rposition(|proj| proj.kind == ProjectionKind::Deref);
318+
319+
if let Some(pos) = last_deref {
320+
let mut projections = cmt.place.projections.clone();
321+
projections.truncate(pos);
322+
323+
for item in projections {
324+
if item.kind == ProjectionKind::Deref {
325+
replacement_str = format!("*{}", replacement_str);
326+
}
271327
}
272328
}
273329
}

tests/ui/search_is_some_fixable_none.fixed

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,28 @@ mod issue7392 {
7373
.map(|c| c.clone())
7474
.collect::<Vec<_>>();
7575
}
76+
77+
fn field_projection() {
78+
struct Foo {
79+
foo: i32,
80+
bar: u32,
81+
}
82+
let vfoo = vec![Foo { foo: 1, bar: 2 }];
83+
let _ = !vfoo.iter().any(|v| v.foo == 1 && v.bar == 2);
84+
85+
let vfoo = vec![(42, Foo { foo: 1, bar: 2 })];
86+
let _ = !vfoo
87+
.iter().any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2);
88+
}
89+
90+
fn index_projection() {
91+
let vfoo = vec![[0, 1, 2, 3]];
92+
let _ = !vfoo.iter().any(|a| a[0] == 42);
93+
}
94+
95+
#[allow(clippy::match_like_matches_macro)]
96+
fn slice_projection() {
97+
let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]];
98+
let _ = !vfoo.iter().any(|sub| sub[1..4].len() == 3);
99+
}
76100
}

tests/ui/search_is_some_fixable_none.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,30 @@ mod issue7392 {
7575
.map(|c| c.clone())
7676
.collect::<Vec<_>>();
7777
}
78+
79+
fn field_projection() {
80+
struct Foo {
81+
foo: i32,
82+
bar: u32,
83+
}
84+
let vfoo = vec![Foo { foo: 1, bar: 2 }];
85+
let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_none();
86+
87+
let vfoo = vec![(42, Foo { foo: 1, bar: 2 })];
88+
let _ = vfoo
89+
.iter()
90+
.find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2)
91+
.is_none();
92+
}
93+
94+
fn index_projection() {
95+
let vfoo = vec![[0, 1, 2, 3]];
96+
let _ = vfoo.iter().find(|a| a[0] == 42).is_none();
97+
}
98+
99+
#[allow(clippy::match_like_matches_macro)]
100+
fn slice_projection() {
101+
let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]];
102+
let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_none();
103+
}
78104
}

tests/ui/search_is_some_fixable_none.stderr

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,5 +135,39 @@ error: called `is_none()` after searching an `Iterator` with `find`
135135
LL | .filter(|(c, _)| filter_hand.iter().find(|cc| c == *cc).is_none())
136136
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!filter_hand.iter().any(|cc| c == cc)`
137137

138-
error: aborting due to 22 previous errors
138+
error: called `is_none()` after searching an `Iterator` with `find`
139+
--> $DIR/search_is_some_fixable_none.rs:85:17
140+
|
141+
LL | let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_none();
142+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|v| v.foo == 1 && v.bar == 2)`
143+
144+
error: called `is_none()` after searching an `Iterator` with `find`
145+
--> $DIR/search_is_some_fixable_none.rs:88:17
146+
|
147+
LL | let _ = vfoo
148+
| _________________^
149+
LL | | .iter()
150+
LL | | .find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2)
151+
LL | | .is_none();
152+
| |______________________^
153+
|
154+
help: use `!_.any()` instead
155+
|
156+
LL ~ let _ = !vfoo
157+
LL ~ .iter().any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2);
158+
|
159+
160+
error: called `is_none()` after searching an `Iterator` with `find`
161+
--> $DIR/search_is_some_fixable_none.rs:96:17
162+
|
163+
LL | let _ = vfoo.iter().find(|a| a[0] == 42).is_none();
164+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|a| a[0] == 42)`
165+
166+
error: called `is_none()` after searching an `Iterator` with `find`
167+
--> $DIR/search_is_some_fixable_none.rs:102:17
168+
|
169+
LL | let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_none();
170+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|sub| sub[1..4].len() == 3)`
171+
172+
error: aborting due to 26 previous errors
139173

tests/ui/search_is_some_fixable_some.fixed

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,29 @@ mod issue7392 {
7373
.map(|c| c.clone())
7474
.collect::<Vec<_>>();
7575
}
76+
77+
fn field_projection() {
78+
struct Foo {
79+
foo: i32,
80+
bar: u32,
81+
}
82+
let vfoo = vec![Foo { foo: 1, bar: 2 }];
83+
let _ = vfoo.iter().any(|v| v.foo == 1 && v.bar == 2);
84+
85+
let vfoo = vec![(42, Foo { foo: 1, bar: 2 })];
86+
let _ = vfoo
87+
.iter()
88+
.any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2);
89+
}
90+
91+
fn index_projection() {
92+
let vfoo = vec![[0, 1, 2, 3]];
93+
let _ = vfoo.iter().any(|a| a[0] == 42);
94+
}
95+
96+
#[allow(clippy::match_like_matches_macro)]
97+
fn slice_projection() {
98+
let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]];
99+
let _ = vfoo.iter().any(|sub| sub[1..4].len() == 3);
100+
}
76101
}

tests/ui/search_is_some_fixable_some.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,30 @@ mod issue7392 {
7474
.map(|c| c.clone())
7575
.collect::<Vec<_>>();
7676
}
77+
78+
fn field_projection() {
79+
struct Foo {
80+
foo: i32,
81+
bar: u32,
82+
}
83+
let vfoo = vec![Foo { foo: 1, bar: 2 }];
84+
let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_some();
85+
86+
let vfoo = vec![(42, Foo { foo: 1, bar: 2 })];
87+
let _ = vfoo
88+
.iter()
89+
.find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2)
90+
.is_some();
91+
}
92+
93+
fn index_projection() {
94+
let vfoo = vec![[0, 1, 2, 3]];
95+
let _ = vfoo.iter().find(|a| a[0] == 42).is_some();
96+
}
97+
98+
#[allow(clippy::match_like_matches_macro)]
99+
fn slice_projection() {
100+
let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]];
101+
let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_some();
102+
}
77103
}

tests/ui/search_is_some_fixable_some.stderr

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,5 +134,31 @@ error: called `is_some()` after searching an `Iterator` with `find`
134134
LL | .filter(|(c, _)| filter_hand.iter().find(|cc| c == *cc).is_some())
135135
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|cc| c == cc)`
136136

137-
error: aborting due to 22 previous errors
137+
error: called `is_some()` after searching an `Iterator` with `find`
138+
--> $DIR/search_is_some_fixable_some.rs:84:29
139+
|
140+
LL | let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_some();
141+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|v| v.foo == 1 && v.bar == 2)`
142+
143+
error: called `is_some()` after searching an `Iterator` with `find`
144+
--> $DIR/search_is_some_fixable_some.rs:89:14
145+
|
146+
LL | .find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2)
147+
| ______________^
148+
LL | | .is_some();
149+
| |______________________^ help: use `any()` instead: `any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2)`
150+
151+
error: called `is_some()` after searching an `Iterator` with `find`
152+
--> $DIR/search_is_some_fixable_some.rs:95:29
153+
|
154+
LL | let _ = vfoo.iter().find(|a| a[0] == 42).is_some();
155+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|a| a[0] == 42)`
156+
157+
error: called `is_some()` after searching an `Iterator` with `find`
158+
--> $DIR/search_is_some_fixable_some.rs:101:29
159+
|
160+
LL | let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_some();
161+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|sub| sub[1..4].len() == 3)`
162+
163+
error: aborting due to 26 previous errors
138164

0 commit comments

Comments
 (0)