diff options
| author | ThibsG <thibsg@pm.me> | 2021-09-12 11:31:11 +0200 |
|---|---|---|
| committer | ThibsG <thibsg@pm.me> | 2021-11-20 09:40:11 +0100 |
| commit | 91dd9c46de11b9abeb391d5b19707c5e7ddfef98 (patch) | |
| tree | fc9a436a8d71d293995be063b79dd3857617cf13 | |
| parent | 97783a8cb9b154b17abbf0084fe8a16d490cf801 (diff) | |
| download | rust-91dd9c46de11b9abeb391d5b19707c5e7ddfef98.tar.gz rust-91dd9c46de11b9abeb391d5b19707c5e7ddfef98.zip | |
Handle other projection kinds
| -rw-r--r-- | clippy_lints/src/methods/search_is_some.rs | 80 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable_none.fixed | 24 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable_none.rs | 26 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable_none.stderr | 36 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable_some.fixed | 25 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable_some.rs | 26 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable_some.stderr | 28 |
7 files changed, 231 insertions, 14 deletions
diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs index a1206199460..d09baf50e49 100644 --- a/clippy_lints/src/methods/search_is_some.rs +++ b/clippy_lints/src/methods/search_is_some.rs @@ -220,7 +220,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { let ident_str = map.name(id).to_string(); let span = map.span(cmt.hir_id); let start_span = Span::new(self.next_pos, span.lo(), span.ctxt()); - let start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability); + let mut start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability); if cmt.place.projections.is_empty() { // handle item without any projection, that needs an explicit borrowing @@ -255,19 +255,75 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { // handle item projections by removing one explicit deref // i.e.: suggest `*x` instead of `**x` let mut replacement_str = ident_str; - let last_deref = cmt - .place - .projections - .iter() - .rposition(|proj| proj.kind == ProjectionKind::Deref); - if let Some(pos) = last_deref { - let mut projections = cmt.place.projections.clone(); - projections.truncate(pos); + // handle index projection first + let index_handled = cmt.place.projections.iter().any(|proj| match proj.kind { + // Index projection like `|x| foo[x]` + // the index is dropped so we can't get it to build the suggestion, + // so the span is set-up again to get more code, using `span.hi()` (i.e.: `foo[x]`) + // instead of `span.lo()` (i.e.: `foo`) + ProjectionKind::Index => { + let start_span = Span::new(self.next_pos, span.hi(), span.ctxt()); + start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability); + replacement_str.clear(); + true + }, + _ => false, + }); + + // looking for projections other that need to be handled differently + let other_projections_handled = cmt.place.projections.iter().enumerate().any(|(i, proj)| { + match proj.kind { + // Field projection like `|v| v.foo` + ProjectionKind::Field(idx, variant) => match cmt.place.ty_before_projection(i).kind() { + ty::Adt(def, ..) => { + replacement_str = format!( + "{}.{}", + replacement_str, + def.variants[variant].fields[idx as usize].ident.name.as_str() + ); + true + }, + ty::Tuple(_) => { + replacement_str = format!("{}.{}", replacement_str, idx); + true + }, + _ => false, + }, + ProjectionKind::Index => false, /* handled previously */ + // note: unable to capture `Subslice` kind in tests + ProjectionKind::Subslice => false, + ProjectionKind::Deref => { + // explicit deref for arrays should be avoided in the suggestion + // i.e.: `|sub| *sub[1..4].len() == 3` is not expected + match cmt.place.ty_before_projection(i).kind() { + // dereferencing an array (i.e.: `|sub| sub[1..4].len() == 3`) + ty::Ref(_, inner, _) => match inner.kind() { + ty::Ref(_, innermost, _) if innermost.is_array() => true, + _ => false, + }, + _ => false, + } + }, + } + }); - for item in projections { - if item.kind == ProjectionKind::Deref { - replacement_str = format!("*{}", replacement_str); + // handle `ProjectionKind::Deref` if no special case detected + if !index_handled && !other_projections_handled { + let last_deref = cmt + .place + .projections + .iter() + .rposition(|proj| proj.kind == ProjectionKind::Deref); + + if let Some(pos) = last_deref { + let mut projections = cmt.place.projections.clone(); + projections.truncate(pos); + + for item in projections { + if item.kind == ProjectionKind::Deref { + replacement_str = format!("*{}", replacement_str); + } } } } diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed index 5d9953f9b3c..222731fd212 100644 --- a/tests/ui/search_is_some_fixable_none.fixed +++ b/tests/ui/search_is_some_fixable_none.fixed @@ -73,4 +73,28 @@ mod issue7392 { .map(|c| c.clone()) .collect::<Vec<_>>(); } + + fn field_projection() { + struct Foo { + foo: i32, + bar: u32, + } + let vfoo = vec![Foo { foo: 1, bar: 2 }]; + let _ = !vfoo.iter().any(|v| v.foo == 1 && v.bar == 2); + + let vfoo = vec![(42, Foo { foo: 1, bar: 2 })]; + let _ = !vfoo + .iter().any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2); + } + + fn index_projection() { + let vfoo = vec![[0, 1, 2, 3]]; + let _ = !vfoo.iter().any(|a| a[0] == 42); + } + + #[allow(clippy::match_like_matches_macro)] + fn slice_projection() { + let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; + let _ = !vfoo.iter().any(|sub| sub[1..4].len() == 3); + } } diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs index 2a8aadfd0e0..43ac1a07667 100644 --- a/tests/ui/search_is_some_fixable_none.rs +++ b/tests/ui/search_is_some_fixable_none.rs @@ -75,4 +75,30 @@ mod issue7392 { .map(|c| c.clone()) .collect::<Vec<_>>(); } + + fn field_projection() { + struct Foo { + foo: i32, + bar: u32, + } + let vfoo = vec![Foo { foo: 1, bar: 2 }]; + let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_none(); + + let vfoo = vec![(42, Foo { foo: 1, bar: 2 })]; + let _ = vfoo + .iter() + .find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2) + .is_none(); + } + + fn index_projection() { + let vfoo = vec![[0, 1, 2, 3]]; + let _ = vfoo.iter().find(|a| a[0] == 42).is_none(); + } + + #[allow(clippy::match_like_matches_macro)] + fn slice_projection() { + let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; + let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_none(); + } } diff --git a/tests/ui/search_is_some_fixable_none.stderr b/tests/ui/search_is_some_fixable_none.stderr index 34768feb6dd..fa344046aeb 100644 --- a/tests/ui/search_is_some_fixable_none.stderr +++ b/tests/ui/search_is_some_fixable_none.stderr @@ -135,5 +135,39 @@ error: called `is_none()` after searching an `Iterator` with `find` LL | .filter(|(c, _)| filter_hand.iter().find(|cc| c == *cc).is_none()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!filter_hand.iter().any(|cc| c == cc)` -error: aborting due to 22 previous errors +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_none.rs:85:17 + | +LL | let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|v| v.foo == 1 && v.bar == 2)` + +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_none.rs:88:17 + | +LL | let _ = vfoo + | _________________^ +LL | | .iter() +LL | | .find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2) +LL | | .is_none(); + | |______________________^ + | +help: use `!_.any()` instead + | +LL ~ let _ = !vfoo +LL ~ .iter().any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2); + | + +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_none.rs:96:17 + | +LL | let _ = vfoo.iter().find(|a| a[0] == 42).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|a| a[0] == 42)` + +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_none.rs:102:17 + | +LL | let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|sub| sub[1..4].len() == 3)` + +error: aborting due to 26 previous errors diff --git a/tests/ui/search_is_some_fixable_some.fixed b/tests/ui/search_is_some_fixable_some.fixed index 94a4c7b3639..ae9478a42e9 100644 --- a/tests/ui/search_is_some_fixable_some.fixed +++ b/tests/ui/search_is_some_fixable_some.fixed @@ -73,4 +73,29 @@ mod issue7392 { .map(|c| c.clone()) .collect::<Vec<_>>(); } + + fn field_projection() { + struct Foo { + foo: i32, + bar: u32, + } + let vfoo = vec![Foo { foo: 1, bar: 2 }]; + let _ = vfoo.iter().any(|v| v.foo == 1 && v.bar == 2); + + let vfoo = vec![(42, Foo { foo: 1, bar: 2 })]; + let _ = vfoo + .iter() + .any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2); + } + + fn index_projection() { + let vfoo = vec![[0, 1, 2, 3]]; + let _ = vfoo.iter().any(|a| a[0] == 42); + } + + #[allow(clippy::match_like_matches_macro)] + fn slice_projection() { + let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; + let _ = vfoo.iter().any(|sub| sub[1..4].len() == 3); + } } diff --git a/tests/ui/search_is_some_fixable_some.rs b/tests/ui/search_is_some_fixable_some.rs index 8887b0327e2..98c3e7c1a68 100644 --- a/tests/ui/search_is_some_fixable_some.rs +++ b/tests/ui/search_is_some_fixable_some.rs @@ -74,4 +74,30 @@ mod issue7392 { .map(|c| c.clone()) .collect::<Vec<_>>(); } + + fn field_projection() { + struct Foo { + foo: i32, + bar: u32, + } + let vfoo = vec![Foo { foo: 1, bar: 2 }]; + let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_some(); + + let vfoo = vec![(42, Foo { foo: 1, bar: 2 })]; + let _ = vfoo + .iter() + .find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2) + .is_some(); + } + + fn index_projection() { + let vfoo = vec![[0, 1, 2, 3]]; + let _ = vfoo.iter().find(|a| a[0] == 42).is_some(); + } + + #[allow(clippy::match_like_matches_macro)] + fn slice_projection() { + let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; + let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_some(); + } } diff --git a/tests/ui/search_is_some_fixable_some.stderr b/tests/ui/search_is_some_fixable_some.stderr index 90615a85ca3..4161b5eec0a 100644 --- a/tests/ui/search_is_some_fixable_some.stderr +++ b/tests/ui/search_is_some_fixable_some.stderr @@ -134,5 +134,31 @@ error: called `is_some()` after searching an `Iterator` with `find` LL | .filter(|(c, _)| filter_hand.iter().find(|cc| c == *cc).is_some()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|cc| c == cc)` -error: aborting due to 22 previous errors +error: called `is_some()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_some.rs:84:29 + | +LL | let _ = vfoo.iter().find(|v| v.foo == 1 && v.bar == 2).is_some(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|v| v.foo == 1 && v.bar == 2)` + +error: called `is_some()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_some.rs:89:14 + | +LL | .find(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2) + | ______________^ +LL | | .is_some(); + | |______________________^ help: use `any()` instead: `any(|(i, v)| *i == 42 && v.foo == 1 && v.bar == 2)` + +error: called `is_some()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_some.rs:95:29 + | +LL | let _ = vfoo.iter().find(|a| a[0] == 42).is_some(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|a| a[0] == 42)` + +error: called `is_some()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_some.rs:101:29 + | +LL | let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_some(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|sub| sub[1..4].len() == 3)` + +error: aborting due to 26 previous errors |
