Update clippy submodule

Author: oli-obk

CHANGELOG.md

@@ -936,6 +936,7 @@ All notable changes to this project will be documented in this file.
 [`for_loop_over_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loop_over_result
 [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
 [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
+[`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
 [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
 [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion
 [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

README.md

@@ -7,7 +7,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 303 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 304 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/consts.rs

@@ -14,7 +14,7 @@ use std::cmp::PartialOrd;
 use std::convert::TryInto;
 use std::hash::{Hash, Hasher};
 use syntax::ast::{FloatTy, LitKind};
-use syntax_pos::symbol::{LocalInternedString, Symbol};
+use syntax_pos::symbol::Symbol;
 
 /// A `LitKind`-like enum to fold constant `Expr`s into.
 #[derive(Debug, Clone)]
@@ -250,14 +250,18 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
                     if let ExprKind::Path(qpath) = &callee.node;
                     let res = self.tables.qpath_res(qpath, callee.hir_id);
                     if let Some(def_id) = res.opt_def_id();
-                    let get_def_path = self.lcx.get_def_path(def_id);
+                    let get_def_path = self.lcx.get_def_path(def_id, );
                     let def_path = get_def_path
                         .iter()
-                        .map(LocalInternedString::get)
+                        .copied()
+                        .map(Symbol::as_str)
                         .collect::<Vec<_>>();
-                    if let &["core", "num", impl_ty, "max_value"] = &def_path[..];
+                    if def_path[0] == "core";
+                    if def_path[1] == "num";
+                    if def_path[3] == "max_value";
+                    if def_path.len() == 4;
                     then {
-                       let value = match impl_ty {
+                       let value = match &*def_path[2] {
                            "<impl i8>" => i8::max_value() as u128,
                            "<impl i16>" => i16::max_value() as u128,
                            "<impl i32>" => i32::max_value() as u128,
@@ -326,7 +330,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
 
         let res = self.tables.qpath_res(qpath, id);
         match res {
-            Res::Def(DefKind::Const, def_id) | Res::Def(DefKind::AssociatedConst, def_id) => {
+            Res::Def(DefKind::Const, def_id) | Res::Def(DefKind::AssocConst, def_id) => {
                 let substs = self.tables.node_substs(id);
                 let substs = if self.substs.is_empty() {
                     substs

clippy_lints/src/copies.rs

@@ -152,7 +152,7 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
     let eq: &dyn Fn(&&Expr, &&Expr) -> bool =
         &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) };
 
-    if let Some((i, j)) = search_same(conds, hash, eq) {
+    for (i, j) in search_same(conds, hash, eq) {
         span_note_and_lint(
             cx,
             IFS_SAME_COND,
@@ -185,7 +185,7 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
         };
 
         let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect();
-        if let Some((&(_, i), &(_, j))) = search_same(&indexed_arms, hash, eq) {
+        for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) {
             span_lint_and_then(
                 cx,
                 MATCH_SAME_ARMS,
@@ -217,7 +217,10 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
                                 ),
                             );
                         } else {
-                            db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
+                            db.span_help(
+                                i.pats[0].span,
+                                &format!("consider refactoring into `{} | {}`", lhs, rhs),
+                            );
                         }
                     }
                 },
@@ -323,21 +326,33 @@ where
     None
 }
 
-fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)>
+fn search_common_cases<'a, T, Eq>(exprs: &'a [T], eq: &Eq) -> Option<(&'a T, &'a T)>
 where
-    Hash: Fn(&T) -> u64,
     Eq: Fn(&T, &T) -> bool,
 {
-    // common cases
     if exprs.len() < 2 {
-        return None;
+        None
     } else if exprs.len() == 2 {
-        return if eq(&exprs[0], &exprs[1]) {
+        if eq(&exprs[0], &exprs[1]) {
             Some((&exprs[0], &exprs[1]))
         } else {
             None
-        };
+        }
+    } else {
+        None
     }
+}
+
+fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Vec<(&T, &T)>
+where
+    Hash: Fn(&T) -> u64,
+    Eq: Fn(&T, &T) -> bool,
+{
+    if let Some(expr) = search_common_cases(&exprs, &eq) {
+        return vec![expr];
+    }
+
+    let mut match_expr_list: Vec<(&T, &T)> = Vec::new();
 
     let mut map: FxHashMap<_, Vec<&_>> =
         FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
@@ -347,7 +362,7 @@ where
             Entry::Occupied(mut o) => {
                 for o in o.get() {
                     if eq(o, expr) {
-                        return Some((o, expr));
+                        match_expr_list.push((o, expr));
                     }
                 }
                 o.get_mut().push(expr);
@@ -358,5 +373,5 @@ where
         }
     }
 
-    None
+    match_expr_list
 }

clippy_lints/src/get_last_with_len.rs

@@ -0,0 +1,105 @@
+//! lint on using `x.get(x.len() - 1)` instead of `x.last()`
+
+use crate::utils::{match_type, paths, snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
+use if_chain::if_chain;
+use rustc::hir::{BinOpKind, 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 syntax::source_map::Spanned;
+use syntax::symbol::Symbol;
+
+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 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<'a, 'tcx> LateLintPass<'a, 'tcx> for GetLastWithLen {
+    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 == Symbol::intern("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 subtraction
+            if let Some(get_index_arg) = args.get(1);
+            if let ExprKind::Binary(
+                Spanned {
+                    node: BinOpKind::Sub,
+                    ..
+                },
+                lhs,
+                rhs,
+            ) = &get_index_arg.node;
+
+            // LHS of subtraction is "x.len()"
+            if let ExprKind::MethodCall(arg_lhs_path, _, lhs_args) = &lhs.node;
+            if arg_lhs_path.ident.name == Symbol::intern("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.node;
+            if let LitKind::Int(rhs_value, ..) = rhs_lit.node;
+            if rhs_value == 1;
+
+            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,
+                );
+            }
+        }
+    }
+}

clippy_lints/src/len_zero.rs

@@ -120,7 +120,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero {
 fn check_trait_items(cx: &LateContext<'_, '_>, visited_trait: &Item, trait_items: &[TraitItemRef]) {
     fn is_named_self(cx: &LateContext<'_, '_>, item: &TraitItemRef, name: &str) -> bool {
         item.ident.name.as_str() == name
-            && if let AssociatedItemKind::Method { has_self } = item.kind {
+            && if let AssocItemKind::Method { has_self } = item.kind {
                 has_self && {
                     let did = cx.tcx.hir().local_def_id_from_hir_id(item.id.hir_id);
                     cx.tcx.fn_sig(did).inputs().skip_binder().len() == 1
@@ -148,7 +148,7 @@ fn check_trait_items(cx: &LateContext<'_, '_>, visited_trait: &Item, trait_items
             .iter()
             .flat_map(|&i| cx.tcx.associated_items(i))
             .any(|i| {
-                i.kind == ty::AssociatedKind::Method
+                i.kind == ty::AssocKind::Method
                     && i.method_has_self_argument
                     && i.ident.name == sym!(is_empty)
                     && cx.tcx.fn_sig(i.def_id).inputs().skip_binder().len() == 1
@@ -171,7 +171,7 @@ fn check_trait_items(cx: &LateContext<'_, '_>, visited_trait: &Item, trait_items
 fn check_impl_items(cx: &LateContext<'_, '_>, item: &Item, impl_items: &[ImplItemRef]) {
     fn is_named_self(cx: &LateContext<'_, '_>, item: &ImplItemRef, name: &str) -> bool {
         item.ident.name.as_str() == name
-            && if let AssociatedItemKind::Method { has_self } = item.kind {
+            && if let AssocItemKind::Method { has_self } = item.kind {
                 has_self && {
                     let did = cx.tcx.hir().local_def_id_from_hir_id(item.id.hir_id);
                     cx.tcx.fn_sig(did).inputs().skip_binder().len() == 1
@@ -258,9 +258,9 @@ fn check_len(
 
 /// Checks if this type has an `is_empty` method.
 fn has_is_empty(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
-    /// Gets an `AssociatedItem` and return true if it matches `is_empty(self)`.
-    fn is_is_empty(cx: &LateContext<'_, '_>, item: &ty::AssociatedItem) -> bool {
-        if let ty::AssociatedKind::Method = item.kind {
+    /// Gets an `AssocItem` and return true if it matches `is_empty(self)`.
+    fn is_is_empty(cx: &LateContext<'_, '_>, item: &ty::AssocItem) -> bool {
+        if let ty::AssocKind::Method = item.kind {
             if item.ident.name.as_str() == "is_empty" {
                 let sig = cx.tcx.fn_sig(item.def_id);
                 let ty = sig.skip_binder();

clippy_lints/src/lib.rs

@@ -186,6 +186,7 @@ pub mod fallible_impl_from;
 pub mod format;
 pub mod formatting;
 pub mod functions;
+pub mod get_last_with_len;
 pub mod identity_conversion;
 pub mod identity_op;
 pub mod if_not_else;
@@ -492,6 +493,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box types::CharLitAsU8);
     reg.register_late_lint_pass(box vec::UselessVec);
     reg.register_late_lint_pass(box drop_bounds::DropBounds);
+    reg.register_late_lint_pass(box get_last_with_len::GetLastWithLen);
     reg.register_late_lint_pass(box drop_forget_ref::DropForgetRef);
     reg.register_late_lint_pass(box empty_enum::EmptyEnum);
     reg.register_late_lint_pass(box types::AbsurdExtremeComparisons);
@@ -710,6 +712,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         formatting::SUSPICIOUS_ELSE_FORMATTING,
         functions::NOT_UNSAFE_PTR_ARG_DEREF,
         functions::TOO_MANY_ARGUMENTS,
+        get_last_with_len::GET_LAST_WITH_LEN,
         identity_conversion::IDENTITY_CONVERSION,
         identity_op::IDENTITY_OP,
         indexing_slicing::OUT_OF_BOUNDS_INDEXING,
@@ -981,6 +984,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         explicit_write::EXPLICIT_WRITE,
         format::USELESS_FORMAT,
         functions::TOO_MANY_ARGUMENTS,
+        get_last_with_len::GET_LAST_WITH_LEN,
         identity_conversion::IDENTITY_CONVERSION,
         identity_op::IDENTITY_OP,
         int_plus_one::INT_PLUS_ONE,

clippy_lints/src/methods/mod.rs

@@ -2157,7 +2157,7 @@ fn lint_binary_expr_with_method_call(cx: &LateContext<'_, '_>, info: &mut Binary
     lint_with_both_lhs_and_rhs!(lint_chars_last_cmp_with_unwrap, cx, info);
 }
 
-/// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_NEXT_CMP` lints.
+/// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_LAST_CMP` lints.
 fn lint_chars_cmp(
     cx: &LateContext<'_, '_>,
     info: &BinaryExprInfo<'_>,

clippy_lints/src/new_without_default.rs

@@ -95,7 +95,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
         if let hir::ItemKind::Impl(_, _, _, _, None, _, ref items) = item.node {
             for assoc_item in items {
-                if let hir::AssociatedItemKind::Method { has_self: false } = assoc_item.kind {
+                if let hir::AssocItemKind::Method { has_self: false } = assoc_item.kind {
                     let impl_item = cx.tcx.hir().impl_item(assoc_item.id);
                     if in_external_macro(cx.sess(), impl_item.span) {
                         return;

clippy_lints/src/non_copy_const.rs

@@ -195,7 +195,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonCopyConst {
 
             // Make sure it is a const item.
             match cx.tables.qpath_res(qpath, expr.hir_id) {
-                Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssociatedConst, _) => {},
+                Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => {},
                 _ => return,
             };
 

clippy_lints/src/returns.rs

@@ -7,7 +7,7 @@ use syntax::source_map::Span;
 use syntax::visit::FnKind;
 use syntax_pos::BytePos;
 
-use crate::utils::{in_macro_or_desugar, match_path_ast, snippet_opt, span_lint_and_then, span_note_and_lint};
+use crate::utils::{in_macro_or_desugar, match_path_ast, snippet_opt, span_lint_and_then};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for return statements at the end of a block.
@@ -164,13 +164,28 @@ impl Return {
             if match_path_ast(path, &[&*ident.name.as_str()]);
             if !in_external_macro(cx.sess(), initexpr.span);
             then {
-                    span_note_and_lint(cx,
-                                       LET_AND_RETURN,
-                                       retexpr.span,
-                                       "returning the result of a let binding from a block. \
-                                       Consider returning the expression directly.",
-                                       initexpr.span,
-                                       "this expression can be directly returned");
+                span_lint_and_then(
+                    cx,
+                    LET_AND_RETURN,
+                    retexpr.span,
+                    "returning the result of a let binding from a block",
+                    |err| {
+                        err.span_label(local.span, "unnecessary let binding");
+
+                        if let Some(snippet) = snippet_opt(cx, initexpr.span) {
+                            err.multipart_suggestion(
+                                "return the expression directly",
+                                vec![
+                                    (local.span, String::new()),
+                                    (retexpr.span, snippet),
+                                ],
+                                Applicability::MachineApplicable,
+                            );
+                        } else {
+                            err.span_help(initexpr.span, "this expression can be directly returned");
+                        }
+                    },
+                );
             }
         }
     }