diff options
| author | bors <bors@rust-lang.org> | 2023-11-02 09:23:39 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-11-02 09:23:39 +0000 |
| commit | 01a0a3666681b88c3cab5ffc440bdd74352093dd (patch) | |
| tree | fb23302ae2ba07b9800ebc277a4010e10098df6d | |
| parent | 919f698da0f5917d9f9620e510b69a81234249bc (diff) | |
| parent | 61c76dd4ffd8c96acb45bfcd290f95ae0938513e (diff) | |
| download | rust-01a0a3666681b88c3cab5ffc440bdd74352093dd.tar.gz rust-01a0a3666681b88c3cab5ffc440bdd74352093dd.zip | |
Auto merge of #11744 - matthri:get-first-deque-fix, r=Jarcho
Fix get_first false negative for VecDeque fixes #11695 Also run the lint on `VecDeque` and suggest using `.front()` instead of `.get(0)` when trying to access the first element. PS: At first I implemented the VecDeque Lint in a separate `if_chain` (see the previous commit). Let me know if thats the preferred way, then I will remove the refactoring into one block. changelog: [`get_first`]: fix false negative: Also lint `VecDeque` and suggest using `front()`
| -rw-r--r-- | clippy_lints/src/methods/get_first.rs | 40 | ||||
| -rw-r--r-- | tests/ui/get_first.fixed | 5 | ||||
| -rw-r--r-- | tests/ui/get_first.rs | 5 | ||||
| -rw-r--r-- | tests/ui/get_first.stderr | 10 |
4 files changed, 44 insertions, 16 deletions
diff --git a/clippy_lints/src/methods/get_first.rs b/clippy_lints/src/methods/get_first.rs index e7b4564c651..2e1dd3ec649 100644 --- a/clippy_lints/src/methods/get_first.rs +++ b/clippy_lints/src/methods/get_first.rs @@ -1,11 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::source_map::Spanned; +use rustc_span::sym; use super::GET_FIRST; @@ -18,20 +20,34 @@ pub(super) fn check<'tcx>( if_chain! { if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); if let Some(impl_id) = cx.tcx.impl_of_method(method_id); - if cx.tcx.type_of(impl_id).instantiate_identity().is_slice(); + let identity = cx.tcx.type_of(impl_id).instantiate_identity(); if let hir::ExprKind::Lit(Spanned { node: LitKind::Int(0, _), .. }) = arg.kind; then { - let mut app = Applicability::MachineApplicable; - let slice_name = snippet_with_applicability(cx, recv.span, "..", &mut app); - span_lint_and_sugg( - cx, - GET_FIRST, - expr.span, - &format!("accessing first element with `{slice_name}.get(0)`"), - "try", - format!("{slice_name}.first()"), - app, - ); + if identity.is_slice() { + let mut app = Applicability::MachineApplicable; + let slice_name = snippet_with_applicability(cx, recv.span, "..", &mut app); + span_lint_and_sugg( + cx, + GET_FIRST, + expr.span, + &format!("accessing first element with `{slice_name}.get(0)`"), + "try", + format!("{slice_name}.first()"), + app, + ); + } else if is_type_diagnostic_item(cx, identity, sym::VecDeque){ + let mut app = Applicability::MachineApplicable; + let slice_name = snippet_with_applicability(cx, recv.span, "..", &mut app); + span_lint_and_sugg( + cx, + GET_FIRST, + expr.span, + &format!("accessing first element with `{slice_name}.get(0)`"), + "try", + format!("{slice_name}.front()"), + app, + ); + } } } } diff --git a/tests/ui/get_first.fixed b/tests/ui/get_first.fixed index a7cdd2a93ba..710ebab1ef2 100644 --- a/tests/ui/get_first.fixed +++ b/tests/ui/get_first.fixed @@ -32,9 +32,12 @@ fn main() { let _ = z[0]; let vecdeque: VecDeque<_> = x.iter().cloned().collect(); + let _ = vecdeque.front(); + //~^ ERROR: accessing first element with `vecdeque.get(0)` + let _ = vecdeque.get(1); + let hashmap: HashMap<u8, char> = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]); let btreemap: BTreeMap<u8, char> = BTreeMap::from_iter(vec![(0, 'a'), (1, 'b')]); - let _ = vecdeque.get(0); // Do not lint, because VecDeque is not slice. let _ = hashmap.get(&0); // Do not lint, because HashMap is not slice. let _ = btreemap.get(&0); // Do not lint, because BTreeMap is not slice. diff --git a/tests/ui/get_first.rs b/tests/ui/get_first.rs index cca743c4bf5..ad2ba6ce2c3 100644 --- a/tests/ui/get_first.rs +++ b/tests/ui/get_first.rs @@ -32,9 +32,12 @@ fn main() { let _ = z[0]; let vecdeque: VecDeque<_> = x.iter().cloned().collect(); + let _ = vecdeque.get(0); + //~^ ERROR: accessing first element with `vecdeque.get(0)` + let _ = vecdeque.get(1); + let hashmap: HashMap<u8, char> = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]); let btreemap: BTreeMap<u8, char> = BTreeMap::from_iter(vec![(0, 'a'), (1, 'b')]); - let _ = vecdeque.get(0); // Do not lint, because VecDeque is not slice. let _ = hashmap.get(&0); // Do not lint, because HashMap is not slice. let _ = btreemap.get(&0); // Do not lint, because BTreeMap is not slice. diff --git a/tests/ui/get_first.stderr b/tests/ui/get_first.stderr index 8ee66e33cc8..7474a2ada66 100644 --- a/tests/ui/get_first.stderr +++ b/tests/ui/get_first.stderr @@ -19,11 +19,17 @@ error: accessing first element with `z.get(0)` LL | let _ = z.get(0); | ^^^^^^^^ help: try: `z.first()` +error: accessing first element with `vecdeque.get(0)` + --> $DIR/get_first.rs:35:13 + | +LL | let _ = vecdeque.get(0); + | ^^^^^^^^^^^^^^^ help: try: `vecdeque.front()` + error: accessing first element with `non_primitives.get(0)` - --> $DIR/get_first.rs:45:13 + --> $DIR/get_first.rs:48:13 | LL | let _ = non_primitives.get(0); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `non_primitives.first()` -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors |
