Auto merge of #8862 - Alexendoo:get-last-with-len, r=Jarcho,xFrednet

`get_last_with_len`: lint `VecDeque` and any deref to slice

changelog: [`get_last_with_len`]: lint `VecDeque` and any deref to slice

Previously only `Vec`s were linted, this will now catch any usages on slices, arrays, etc. It also suggests `.back()` for `VecDeque`s

Also moves the lint into `methods/`

10 files changed, 172 insertions, 125 deletions

diff --git a/clippy_lints/src/get_last_with_len.rs b/clippy_lints/src/get_last_with_len.rs
deleted file mode 100644
index df29d9308e7..00000000000
--- a/clippy_lints/src/get_last_with_len.rs
+++ /dev/null
@@ -1,107 +0,0 @@
-//! lint on using `x.get(x.len() - 1)` instead of `x.last()`
-
-use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::SpanlessEq;
-use if_chain::if_chain;
-use rustc_ast::ast::LitKind;
-use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind};
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::source_map::Spanned;
-use rustc_span::sym;
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for using `x.get(x.len() - 1)` instead of
-    /// `x.last()`.
-    ///
-    /// ### Why is this bad?
-    /// Using `x.last()` is easier to read and has the same
-    /// result.
-    ///
-    /// Note that using `x[x.len() - 1]` is semantically different from
-    /// `x.last()`.  Indexing into the array will panic on out-of-bounds
-    /// accesses, while `x.get()` and `x.last()` will return `None`.
-    ///
-    /// There is another lint (get_unwrap) that covers the case of using
-    /// `x.get(index).unwrap()` instead of `x[index]`.
-    ///
-    /// ### Example
-    /// ```rust
-    /// // Bad
-    /// let x = vec![2, 3, 5];
-    /// let last_element = x.get(x.len() - 1);
-    ///
-    /// // Good
-    /// let x = vec![2, 3, 5];
-    /// let last_element = x.last();
-    /// ```
-    #[clippy::version = "1.37.0"]
-    pub GET_LAST_WITH_LEN,
-    complexity,
-    "Using `x.get(x.len() - 1)` when `x.last()` is correct and simpler"
-}
-
-declare_lint_pass!(GetLastWithLen => [GET_LAST_WITH_LEN]);
-
-impl<'tcx> LateLintPass<'tcx> for GetLastWithLen {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if_chain! {
-            // Is a method call
-            if let ExprKind::MethodCall(path, args, _) = expr.kind;
-
-            // Method name is "get"
-            if path.ident.name == sym!(get);
-
-            // Argument 0 (the struct we're calling the method on) is a vector
-            if let Some(struct_calling_on) = args.get(0);
-            let struct_ty = cx.typeck_results().expr_ty(struct_calling_on);
-            if is_type_diagnostic_item(cx, struct_ty, sym::Vec);
-
-            // Argument to "get" is a subtraction
-            if let Some(get_index_arg) = args.get(1);
-            if let ExprKind::Binary(
-                Spanned {
-                    node: BinOpKind::Sub,
-                    ..
-                },
-                lhs,
-                rhs,
-            ) = &get_index_arg.kind;
-
-            // LHS of subtraction is "x.len()"
-            if let ExprKind::MethodCall(arg_lhs_path, lhs_args, _) = &lhs.kind;
-            if arg_lhs_path.ident.name == sym::len;
-            if let Some(arg_lhs_struct) = lhs_args.get(0);
-
-            // The two vectors referenced (x in x.get(...) and in x.len())
-            if SpanlessEq::new(cx).eq_expr(struct_calling_on, arg_lhs_struct);
-
-            // RHS of subtraction is 1
-            if let ExprKind::Lit(rhs_lit) = &rhs.kind;
-            if let LitKind::Int(1, ..) = rhs_lit.node;
-
-            then {
-                let mut applicability = Applicability::MachineApplicable;
-                let vec_name = snippet_with_applicability(
-                    cx,
-                    struct_calling_on.span, "vec",
-                    &mut applicability,
-                );
-
-                span_lint_and_sugg(
-                    cx,
-                    GET_LAST_WITH_LEN,
-                    expr.span,
-                    &format!("accessing last element with `{0}.get({0}.len() - 1)`", vec_name),
-                    "try",
-                    format!("{}.last()", vec_name),
-                    applicability,
-                );
-            }
-        }
-    }
-}
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index 3b8d11449d2..5aba6040b8c 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -91,7 +91,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(functions::NOT_UNSAFE_PTR_ARG_DEREF),
     LintId::of(functions::RESULT_UNIT_ERR),
     LintId::of(functions::TOO_MANY_ARGUMENTS),
-    LintId::of(get_last_with_len::GET_LAST_WITH_LEN),
     LintId::of(identity_op::IDENTITY_OP),
     LintId::of(if_let_mutex::IF_LET_MUTEX),
     LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
@@ -166,6 +165,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(methods::FILTER_MAP_IDENTITY),
     LintId::of(methods::FILTER_NEXT),
     LintId::of(methods::FLAT_MAP_IDENTITY),
+    LintId::of(methods::GET_LAST_WITH_LEN),
     LintId::of(methods::INSPECT_FOR_EACH),
     LintId::of(methods::INTO_ITER_ON_REF),
     LintId::of(methods::IS_DIGIT_ASCII_RADIX),
diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs
index b15c979d0c7..d5dfcd10a66 100644
--- a/clippy_lints/src/lib.register_complexity.rs
+++ b/clippy_lints/src/lib.register_complexity.rs
@@ -15,7 +15,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(explicit_write::EXPLICIT_WRITE),
     LintId::of(format::USELESS_FORMAT),
     LintId::of(functions::TOO_MANY_ARGUMENTS),
-    LintId::of(get_last_with_len::GET_LAST_WITH_LEN),
     LintId::of(identity_op::IDENTITY_OP),
     LintId::of(int_plus_one::INT_PLUS_ONE),
     LintId::of(lifetimes::EXTRA_UNUSED_LIFETIMES),
@@ -37,6 +36,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(methods::FILTER_MAP_IDENTITY),
     LintId::of(methods::FILTER_NEXT),
     LintId::of(methods::FLAT_MAP_IDENTITY),
+    LintId::of(methods::GET_LAST_WITH_LEN),
     LintId::of(methods::INSPECT_FOR_EACH),
     LintId::of(methods::ITER_COUNT),
     LintId::of(methods::MANUAL_FILTER_MAP),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 50c135bf74b..fea34fe66f4 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -183,7 +183,6 @@ store.register_lints(&[
     functions::TOO_MANY_ARGUMENTS,
     functions::TOO_MANY_LINES,
     future_not_send::FUTURE_NOT_SEND,
-    get_last_with_len::GET_LAST_WITH_LEN,
     identity_op::IDENTITY_OP,
     if_let_mutex::IF_LET_MUTEX,
     if_not_else::IF_NOT_ELSE,
@@ -302,6 +301,7 @@ store.register_lints(&[
     methods::FLAT_MAP_IDENTITY,
     methods::FLAT_MAP_OPTION,
     methods::FROM_ITER_INSTEAD_OF_COLLECT,
+    methods::GET_LAST_WITH_LEN,
     methods::GET_UNWRAP,
     methods::IMPLICIT_CLONE,
     methods::INEFFICIENT_TO_STRING,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 4ac834f7240..2f8533723ef 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -242,7 +242,6 @@ mod from_over_into;
 mod from_str_radix_10;
 mod functions;
 mod future_not_send;
-mod get_last_with_len;
 mod identity_op;
 mod if_let_mutex;
 mod if_not_else;
@@ -652,7 +651,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(strings::StringLitAsBytes));
     store.register_late_pass(|| Box::new(derive::Derive));
     store.register_late_pass(|| Box::new(derivable_impls::DerivableImpls));
-    store.register_late_pass(|| Box::new(get_last_with_len::GetLastWithLen));
     store.register_late_pass(|| Box::new(drop_forget_ref::DropForgetRef));
     store.register_late_pass(|| Box::new(empty_enum::EmptyEnum));
     store.register_late_pass(|| Box::new(absurd_extreme_comparisons::AbsurdExtremeComparisons));
diff --git a/clippy_lints/src/methods/get_last_with_len.rs b/clippy_lints/src/methods/get_last_with_len.rs
new file mode 100644
index 00000000000..23368238ef5
--- /dev/null
+++ b/clippy_lints/src/methods/get_last_with_len.rs
@@ -0,0 +1,55 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::SpanlessEq;
+use rustc_ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_span::source_map::Spanned;
+use rustc_span::sym;
+
+use super::GET_LAST_WITH_LEN;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) {
+    // Argument to "get" is a subtraction
+    if let ExprKind::Binary(
+        Spanned {
+            node: BinOpKind::Sub, ..
+        },
+        lhs,
+        rhs,
+    ) = arg.kind
+
+        // LHS of subtraction is "x.len()"
+        && let ExprKind::MethodCall(lhs_path, [lhs_recv], _) = &lhs.kind
+        && lhs_path.ident.name == sym::len
+
+        // RHS of subtraction is 1
+        && let ExprKind::Lit(rhs_lit) = &rhs.kind
+        && let LitKind::Int(1, ..) = rhs_lit.node
+
+        // check that recv == lhs_recv `recv.get(lhs_recv.len() - 1)`
+        && SpanlessEq::new(cx).eq_expr(recv, lhs_recv)
+        && !recv.can_have_side_effects()
+    {
+        let method = match cx.typeck_results().expr_ty_adjusted(recv).peel_refs().kind() {
+            ty::Adt(def, _) if cx.tcx.is_diagnostic_item(sym::VecDeque, def.did()) => "back",
+            ty::Slice(_) => "last",
+            _ => return,
+        };
+
+        let mut applicability = Applicability::MachineApplicable;
+        let recv_snippet = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
+
+        span_lint_and_sugg(
+            cx,
+            GET_LAST_WITH_LEN,
+            expr.span,
+            &format!("accessing last element with `{recv_snippet}.get({recv_snippet}.len() - 1)`"),
+            "try",
+            format!("{recv_snippet}.{method}()"),
+            applicability,
+        );
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 5cfb025ebfd..b820b740930 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -21,6 +21,7 @@ mod filter_next;
 mod flat_map_identity;
 mod flat_map_option;
 mod from_iter_instead_of_collect;
+mod get_last_with_len;
 mod get_unwrap;
 mod implicit_clone;
 mod inefficient_to_string;
@@ -1212,6 +1213,38 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
+    /// Checks for using `x.get(x.len() - 1)` instead of
+    /// `x.last()`.
+    ///
+    /// ### Why is this bad?
+    /// Using `x.last()` is easier to read and has the same
+    /// result.
+    ///
+    /// Note that using `x[x.len() - 1]` is semantically different from
+    /// `x.last()`.  Indexing into the array will panic on out-of-bounds
+    /// accesses, while `x.get()` and `x.last()` will return `None`.
+    ///
+    /// There is another lint (get_unwrap) that covers the case of using
+    /// `x.get(index).unwrap()` instead of `x[index]`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// // Bad
+    /// let x = vec![2, 3, 5];
+    /// let last_element = x.get(x.len() - 1);
+    ///
+    /// // Good
+    /// let x = vec![2, 3, 5];
+    /// let last_element = x.last();
+    /// ```
+    #[clippy::version = "1.37.0"]
+    pub GET_LAST_WITH_LEN,
+    complexity,
+    "Using `x.get(x.len() - 1)` when `x.last()` is correct and simpler"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
     /// Checks for use of `.get().unwrap()` (or
     /// `.get_mut().unwrap`) on a standard library type which implements `Index`
     ///
@@ -2283,6 +2316,7 @@ impl_lint_pass!(Methods => [
     BYTES_NTH,
     ITER_SKIP_NEXT,
     GET_UNWRAP,
+    GET_LAST_WITH_LEN,
     STRING_EXTEND_CHARS,
     ITER_CLONED_COLLECT,
     ITER_WITH_DRAIN,
@@ -2610,6 +2644,7 @@ impl Methods {
                         inspect_for_each::check(cx, expr, span2);
                     }
                 },
+                ("get", [arg]) => get_last_with_len::check(cx, expr, recv, arg),
                 ("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"),
                 ("is_file", []) => filetype_is_file::check(cx, expr, recv),
                 ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, self.msrv),
diff --git a/tests/ui/get_last_with_len.fixed b/tests/ui/get_last_with_len.fixed
index c8b363f9c38..1e90b37687a 100644
--- a/tests/ui/get_last_with_len.fixed
+++ b/tests/ui/get_last_with_len.fixed
@@ -1,10 +1,13 @@
 // run-rustfix
 
 #![warn(clippy::get_last_with_len)]
+#![allow(unused)]
+
+use std::collections::VecDeque;
 
 fn dont_use_last() {
     let x = vec![2, 3, 5];
-    let _ = x.last(); // ~ERROR Use x.last()
+    let _ = x.last();
 }
 
 fn indexing_two_from_end() {
@@ -23,9 +26,24 @@ fn use_last_with_different_vec_length() {
     let _ = x.get(y.len() - 1);
 }
 
+struct S {
+    field: Vec<usize>,
+}
+
+fn in_field(s: &S) {
+    let _ = s.field.last();
+}
+
 fn main() {
-    dont_use_last();
-    indexing_two_from_end();
-    index_into_last();
-    use_last_with_different_vec_length();
+    let slice = &[1, 2, 3];
+    let _ = slice.last();
+
+    let array = [4, 5, 6];
+    let _ = array.last();
+
+    let deq = VecDeque::from([7, 8, 9]);
+    let _ = deq.back();
+
+    let nested = [[1]];
+    let _ = nested[0].last();
 }
diff --git a/tests/ui/get_last_with_len.rs b/tests/ui/get_last_with_len.rs
index bf9cb2d7e0c..d63a731bd52 100644
--- a/tests/ui/get_last_with_len.rs
+++ b/tests/ui/get_last_with_len.rs
@@ -1,10 +1,13 @@
 // run-rustfix
 
 #![warn(clippy::get_last_with_len)]
+#![allow(unused)]
+
+use std::collections::VecDeque;
 
 fn dont_use_last() {
     let x = vec![2, 3, 5];
-    let _ = x.get(x.len() - 1); // ~ERROR Use x.last()
+    let _ = x.get(x.len() - 1);
 }
 
 fn indexing_two_from_end() {
@@ -23,9 +26,24 @@ fn use_last_with_different_vec_length() {
     let _ = x.get(y.len() - 1);
 }
 
+struct S {
+    field: Vec<usize>,
+}
+
+fn in_field(s: &S) {
+    let _ = s.field.get(s.field.len() - 1);
+}
+
 fn main() {
-    dont_use_last();
-    indexing_two_from_end();
-    index_into_last();
-    use_last_with_different_vec_length();
+    let slice = &[1, 2, 3];
+    let _ = slice.get(slice.len() - 1);
+
+    let array = [4, 5, 6];
+    let _ = array.get(array.len() - 1);
+
+    let deq = VecDeque::from([7, 8, 9]);
+    let _ = deq.get(deq.len() - 1);
+
+    let nested = [[1]];
+    let _ = nested[0].get(nested[0].len() - 1);
 }
diff --git a/tests/ui/get_last_with_len.stderr b/tests/ui/get_last_with_len.stderr
index 55baf87384a..ac8dd6c2e41 100644
--- a/tests/ui/get_last_with_len.stderr
+++ b/tests/ui/get_last_with_len.stderr
@@ -1,10 +1,40 @@
 error: accessing last element with `x.get(x.len() - 1)`
-  --> $DIR/get_last_with_len.rs:7:13
+  --> $DIR/get_last_with_len.rs:10:13
    |
-LL |     let _ = x.get(x.len() - 1); // ~ERROR Use x.last()
+LL |     let _ = x.get(x.len() - 1);
    |             ^^^^^^^^^^^^^^^^^^ help: try: `x.last()`
    |
    = note: `-D clippy::get-last-with-len` implied by `-D warnings`
 
-error: aborting due to previous error
+error: accessing last element with `s.field.get(s.field.len() - 1)`
+  --> $DIR/get_last_with_len.rs:34:13
+   |
+LL |     let _ = s.field.get(s.field.len() - 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `s.field.last()`
+
+error: accessing last element with `slice.get(slice.len() - 1)`
+  --> $DIR/get_last_with_len.rs:39:13
+   |
+LL |     let _ = slice.get(slice.len() - 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice.last()`
+
+error: accessing last element with `array.get(array.len() - 1)`
+  --> $DIR/get_last_with_len.rs:42:13
+   |
+LL |     let _ = array.get(array.len() - 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `array.last()`
+
+error: accessing last element with `deq.get(deq.len() - 1)`
+  --> $DIR/get_last_with_len.rs:45:13
+   |
+LL |     let _ = deq.get(deq.len() - 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: try: `deq.back()`
+
+error: accessing last element with `nested[0].get(nested[0].len() - 1)`
+  --> $DIR/get_last_with_len.rs:48:13
+   |
+LL |     let _ = nested[0].get(nested[0].len() - 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `nested[0].last()`
+
+error: aborting due to 6 previous errors