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()`

Author: bors

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,
+                    );
+            }
         }
     }
 }

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.
 

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.
 

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