Merge branch 'master' of github.com:HarrisonMc555/rust-clippy

Author: HarrisonMc555

CHANGELOG.md

@@ -1048,6 +1048,7 @@ All notable changes to this project will be documented in this file.
 [`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label
 [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit
 [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
+[`use_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_last
 [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
 [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
 [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref

clippy_lints/src/lib.rs

@@ -242,6 +242,7 @@ pub mod unsafe_removed_from_name;
 pub mod unused_io_amount;
 pub mod unused_label;
 pub mod unwrap;
+pub mod use_last;
 pub mod use_self;
 pub mod vec;
 pub mod wildcard_dependencies;
@@ -843,6 +844,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
         unused_io_amount::UNUSED_IO_AMOUNT,
         unused_label::UNUSED_LABEL,
+        use_last::USE_LAST,
         vec::USELESS_VEC,
         write::PRINTLN_EMPTY_STRING,
         write::PRINT_LITERAL,
@@ -1006,6 +1008,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         types::UNNECESSARY_CAST,
         types::VEC_BOX,
         unused_label::UNUSED_LABEL,
+        use_last::USE_LAST,
         zero_div_zero::ZERO_DIVIDED_BY_ZERO,
     ]);
 

clippy_lints/src/use_last.rs

@@ -0,0 +1,96 @@
+//! lint on using `x.get(x.len() - 1)` instead of `x.last()`
+
+use crate::utils::{match_type, paths, span_lint};
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::{declare_tool_lint, lint_array};
+use rustc::hir::{Expr, ExprKind};
+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.
+///
+/// **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();
+/// ```
+
+declare_clippy_lint! {
+    pub USE_LAST,
+    complexity,
+    "using `x.get(x.len() - 1)` instead of `x.last()`"
+}
+
+#[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"
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseLast {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if_chain! {
+            // Is a method call
+            if let ExprKind::MethodCall(ref path, _, ref args) = expr.node;
+
+            // Method name is "get"
+            if path.ident.name == "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.tables.expr_ty(struct_calling_on);
+            if match_type(cx, struct_ty, &paths::VEC);
+
+            // Argument to "get" is a binary operation
+            if let Some(get_index_arg) = args.get(1);
+            if let rustc::hir::ExprKind::Binary(ref op, ref lhs, ref rhs) = get_index_arg.node;
+
+            // Binary operation is a subtraction
+            if op.node == rustc::hir::BinOpKind::Sub;
+
+            // LHS of subtraction is "x.len()"
+            if let ExprKind::MethodCall(ref arg_lhs_path, _, ref lhs_args) = lhs.node;
+            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 arg_lhs_struct.hir_id == struct_calling_on.hir_id;
+
+            // RHS of subtraction is 1
+            if let ExprKind::Lit(ref rhs_lit) = rhs.node;
+            if let LitKind::Int(rhs_value, ..) = rhs_lit.node;
+            if rhs_value == 1;
+
+            // TODO: Figure out how to get name of variable for lint message
+            let vec_name = "x";
+
+            then {
+                span_lint(cx,
+                          USE_LAST,
+                          expr.span,
+                          &format!("Use `{}.last()` instead of `{}.get({}.len() - 1)`",
+                                   vec_name, vec_name, vec_name));
+            }
+        }
+    }
+}

tests/ui/use_last.rs

@@ -0,0 +1,20 @@
+#![warn(clippy::all)]
+// #![warn(clippy::use_last)]
+
+fn dont_use_last() -> Option<i32> {
+    let x = vec![2, 3, 5];
+    let last_element = x.get(x.len() - 1); // ~ERROR Use _.last()
+    last_element.map(|val| val + 1) // 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;
+    assert_eq!(dont_use_last(), Some(expected_value));
+    assert_eq!(index_into_last(), 5);
+}