Add snippet, suggestion, update stderr, add test case

Author: HarrisonMc555

clippy_lints/src/use_last.rs

@@ -1,19 +1,23 @@
 //! lint on using `x.get(x.len() - 1)` instead of `x.last()`
 
-use crate::utils::{match_type, paths, span_lint};
+use crate::utils::{match_type, paths, span_lint_and_sugg, snippet_with_applicability};
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_tool_lint, lint_array};
 use rustc::hir::{Expr, ExprKind};
+use rustc_errors::Applicability;
 use syntax::ast::{LitKind};
 use if_chain::if_chain;
 
 /// **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(),
-/// since indexing into the array will panic on out-of-bounds accesses, while
-/// x.get() and x.last() will return None.
+/// 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]`.
 ///
 /// **Known problems:** None.
 ///
@@ -79,11 +83,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseLast {
 
             // LHS of subtraction is "x.len()"
             if let ExprKind::MethodCall(ref arg_lhs_path, _, ref lhs_args) = lhs.node;
-            // let _ = println!("LHS of sub is a method call");
+             // let _ = println!("LHS of sub is a method call");
             if arg_lhs_path.ident.name == "len";
             // let _ = println!("LHS of sub was method named len");
-            // if let Some(arg_lhs_struct) = lhs_args.get(0);
+            if let Some(_arg_lhs_struct) = lhs_args.get(0);
             // let _ = println!("LHS of sub method has an arg");
+
             // TODO: Is this a valid way to check if they reference the same vector?
             // if arg_lhs_struct.hir_id == struct_calling_on.hir_id;
             // let _ = println!("The vector in .get and .len were the same");
@@ -96,24 +101,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseLast {
             if rhs_value == 1;
             // let _ = println!("RHS of sub was 1");
 
-            // TODO: Figure out how to get name of variable for lint message
-            // Can't do this (cannot move out of borrowed content (context?))
-            // if let ExprKind::Struct(ref struct_calling_on_path, _, _) = struct_calling_on.node;
-            // let _ = println!("It was a struct");
-            // let vec_name = match struct_calling_on_path.into_inner() {
-            //     rustc::hir::QPath::Resolved(_, path) =>
-            //         path.segments.last().map(|path_seg| path_seg.ident.name.as_str().get()).unwrap_or("x"),
-            //     rustc::hir::QPath::TypeRelative(_, path_seg) => path_seg.ident.name.as_str().get(),
-            // };
-            let vec_name = "x";
+            let mut applicability = Applicability::MachineApplicable;
+            let vec_name = snippet_with_applicability(
+                cx, struct_calling_on.span, "x", &mut applicability);
             // let _ = println!("About to span_lint on \"{}\"", vec_name);
 
             then {
-                span_lint(cx,
-                          USE_LAST,
-                          expr.span,
-                          &format!("Use `{}.last()` instead of `{}.get({}.len() - 1)`",
-                                   vec_name, vec_name, vec_name));
+                span_lint_and_sugg(
+                    cx,
+                    USE_LAST,
+                    expr.span,
+                    &format!("Use `{}.last()` instead of `{}.get({}.len() - 1)`",
+                             vec_name, vec_name, vec_name),
+                    "try",
+                    format!("{}.last()", vec_name),
+                    applicability,
+                );
             }
         }
     }

tests/ui/use_last.rs

@@ -2,19 +2,26 @@
 
 fn dont_use_last() -> Option<i32> {
     let x = vec![2, 3, 5];
-    let last_element = x.get(x.len() - 1); // ~ERROR Use _.last()
+    let last_element = x.get(x.len() - 1); // ~ERROR Use x.last()
     last_element.map(|val| val + 1) // To avoid warnings
 }
 
+fn indexing_two_from_end() -> Option<i32> {
+    let x = vec![2, 3, 5];
+    let last_element = x.get(x.len() - 2);
+    last_element.map(|val| val + 3) // To avoid warnings
+}
+
 fn index_into_last() -> i32 {
     let x = vec![2, 3, 5];
     let last_element = x[x.len() - 1];
     last_element + 1 // To avoid warnings
 }
 
 fn main() {
-    let expected_value: i32 = 5;
+    let expected_value: i32 = 6;
     println!("Working...");
     assert_eq!(dont_use_last(), Some(expected_value));
+    assert_eq!(indexing_two_from_end(), Some(expected_value));
     assert_eq!(index_into_last(), expected_value);
 }

tests/ui/use_last.stderr

@@ -1,8 +1,8 @@
 error: Use `x.last()` instead of `x.get(x.len() - 1)`
   --> $DIR/use_last.rs:5:24
    |
-LL |     let last_element = x.get(x.len() - 1); // ~ERROR Use _.last()
-   |                        ^^^^^^^^^^^^^^^^^^
+LL |     let last_element = x.get(x.len() - 1); // ~ERROR Use x.last()
+   |                        ^^^^^^^^^^^^^^^^^^ help: try: `x.last()`
    |
    = note: `-D clippy::use-last` implied by `-D warnings`