about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-11-02 09:23:39 +0000
committerbors <bors@rust-lang.org>2023-11-02 09:23:39 +0000
commit01a0a3666681b88c3cab5ffc440bdd74352093dd (patch)
treefb23302ae2ba07b9800ebc277a4010e10098df6d
parent919f698da0f5917d9f9620e510b69a81234249bc (diff)
parent61c76dd4ffd8c96acb45bfcd290f95ae0938513e (diff)
downloadrust-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.rs40
-rw-r--r--tests/ui/get_first.fixed5
-rw-r--r--tests/ui/get_first.rs5
-rw-r--r--tests/ui/get_first.stderr10
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