Merged

Author: HarrisonMc555

clippy_lints/src/lib.rs

@@ -6,8 +6,7 @@
 #![feature(slice_patterns)]
 #![feature(stmt_expr_attributes)]
 #![allow(clippy::missing_docs_in_private_items)]
-// #![recursion_limit = "256"]
-#![recursion_limit = "512"]
+#![recursion_limit = "256"]
 #![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)]
 #![deny(internal)]
 #![feature(crate_visibility_modifier)]

clippy_lints/src/use_last.rs

@@ -1,56 +1,46 @@
 //! lint on using `x.get(x.len() - 1)` instead of `x.last()`
 
-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 crate::utils::{match_type, paths, snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
+use if_chain::if_chain;
 use rustc::hir::{Expr, ExprKind};
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::{declare_lint_pass, declare_tool_lint};
 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()`.
-/// 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.
-///
-/// **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();
-/// ```
+use syntax::ast::LitKind;
 
 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]`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **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();
+    /// ```
     pub USE_LAST,
     complexity,
-    "using `x.get(x.len() - 1)` instead of `x.last()`"
+    "Using `x.get(x.len() - 1)` when `x.last()` is correct and simpler"
 }
 
-#[derive(Copy, Clone, Debug)]
-pub struct UseLast;
-
-impl LintPass for UseLast {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(USE_LAST)
-    }
-
-    fn name(&self) -> &'static str {
-        "UseLast"
-    }
-}
+declare_lint_pass!(UseLast => [USE_LAST]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseLast {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@@ -78,10 +68,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseLast {
             if arg_lhs_path.ident.name == "len";
             if let Some(arg_lhs_struct) = lhs_args.get(0);
 
-            // TODO: Is this a valid way to check if they reference the same vector?
-            if let ExprKind::Path(arg_lhs_struct_path) = arg_lhs_struct.node;
-            if let ExprKind::Path(struct_calling_on_path) = struct_calling_on.nod
-            if arg_lhs_struct_path == struct_calling_on_path;
+            if SpanlessEq::new(cx).eq_expr(struct_calling_on, arg_lhs_struct);
 
             // RHS of subtraction is 1
             if let ExprKind::Lit(ref rhs_lit) = rhs.node;

tests/ui/use_last.rs

@@ -1,36 +1,29 @@
 #![warn(clippy::use_last)]
 
-fn dont_use_last() -> Option<i32> {
+fn dont_use_last() {
     let x = vec![2, 3, 5];
-    let last_element = x.get(x.len() - 1); // ~ERROR Use x.last()
-    last_element.map(|val| val + 1) // To avoid warnings
+    let _ = x.get(x.len() - 1); // ~ERROR Use x.last()
 }
 
-fn indexing_two_from_end() -> Option<i32> {
+fn indexing_two_from_end() {
     let x = vec![2, 3, 5];
-    let last_element = x.get(x.len() - 2);
-    last_element.map(|val| val + 3) // To avoid warnings
+    let _ = x.get(x.len() - 2);
 }
 
-fn index_into_last() -> i32 {
+fn index_into_last() {
     let x = vec![2, 3, 5];
-    let last_element = x[x.len() - 1];
-    last_element + 1 // To avoid warnings
+    let _ = x[x.len() - 1];
 }
 
-// False positive test (currently failing)
-// fn use_last_with_different_vec_length() -> Option<i32> {
-//     let x = vec![2, 3, 5];
-//     let y = vec!['a', 'b', 'c'];
-//     let last_element = x.get(y.len() - 1);
-//     last_element.map(|val| val + 1)
-// }
+fn use_last_with_different_vec_length() {
+    let x = vec![2, 3, 5];
+    let y = vec!['a', 'b', 'c'];
+    let _ = x.get(y.len() - 1);
+}
 
 fn main() {
-    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);
-    // assert_eq!(use_last_with_different_vec_length(), Some(expected_value));
+    dont_use_last();
+    indexing_two_from_end();
+    index_into_last();
+    use_last_with_different_vec_length();
 }

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
+  --> $DIR/use_last.rs:5:13
    |
-LL |     let last_element = x.get(x.len() - 1); // ~ERROR Use x.last()
-   |                        ^^^^^^^^^^^^^^^^^^ help: try: `x.last()`
+LL |     let _ = x.get(x.len() - 1); // ~ERROR Use x.last()
+   |             ^^^^^^^^^^^^^^^^^^ help: try: `x.last()`
    |
    = note: `-D clippy::use-last` implied by `-D warnings`