Update Clippy

Author: JohnTitor

CHANGELOG.md

@@ -1285,6 +1285,7 @@ Released 2018-09-13
 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
 [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
+[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 347 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 348 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_dev/src/stderr_length_check.rs

@@ -7,7 +7,7 @@ use std::io::prelude::*;
 // The maximum length allowed for stderr files.
 //
 // We limit this because small files are easier to deal with than bigger files.
-const LIMIT: usize = 275;
+const LIMIT: usize = 245;
 
 pub fn check() {
     let stderr_files = stderr_files();

clippy_lints/src/assertions_on_constants.rs

@@ -33,12 +33,16 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
-        let lint_true = || {
+        let lint_true = |is_debug: bool| {
             span_help_and_lint(
                 cx,
                 ASSERTIONS_ON_CONSTANTS,
                 e.span,
-                "`assert!(true)` will be optimized out by the compiler",
+                if is_debug {
+                    "`debug_assert!(true)` will be optimized out by the compiler"
+                } else {
+                    "`assert!(true)` will be optimized out by the compiler"
+                },
                 "remove it",
             );
         };
@@ -70,7 +74,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
                 if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, lit);
                 if is_true;
                 then {
-                    lint_true();
+                    lint_true(true);
                 }
             };
         } else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") {
@@ -81,7 +85,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
                 match assert_match {
                     // matched assert but not message
                     AssertKind::WithoutMessage(false) => lint_false_without_message(),
-                    AssertKind::WithoutMessage(true) | AssertKind::WithMessage(_, true) => lint_true(),
+                    AssertKind::WithoutMessage(true) | AssertKind::WithMessage(_, true) => lint_true(false),
                     AssertKind::WithMessage(panic_message, false) => lint_false_with_message(panic_message),
                 };
             }

clippy_lints/src/atomic_ordering.rs

@@ -8,7 +8,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of invalid atomic
-    /// ordering in Atomic*::{load, store} calls.
+    /// ordering in atomic loads/stores and memory fences.
     ///
     /// **Why is this bad?** Using an invalid atomic ordering
     /// will cause a panic at run-time.
@@ -17,7 +17,7 @@ declare_clippy_lint! {
     ///
     /// **Example:**
     /// ```rust,no_run
-    /// # use std::sync::atomic::{AtomicBool, Ordering};
+    /// # use std::sync::atomic::{self, AtomicBool, Ordering};
     ///
     /// let x = AtomicBool::new(true);
     ///
@@ -26,10 +26,13 @@ declare_clippy_lint! {
     ///
     /// x.store(false, Ordering::Acquire);
     /// x.store(false, Ordering::AcqRel);
+    ///
+    /// atomic::fence(Ordering::Relaxed);
+    /// atomic::compiler_fence(Ordering::Relaxed);
     /// ```
     pub INVALID_ATOMIC_ORDERING,
     correctness,
-    "usage of invalid atomic ordering in atomic load/store calls"
+    "usage of invalid atomic ordering in atomic loads/stores and memory fences"
 }
 
 declare_lint_pass!(AtomicOrdering => [INVALID_ATOMIC_ORDERING]);
@@ -65,37 +68,65 @@ fn match_ordering_def_path(cx: &LateContext<'_, '_>, did: DefId, orderings: &[&s
         .any(|ordering| match_def_path(cx, did, &["core", "sync", "atomic", "Ordering", ordering]))
 }
 
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AtomicOrdering {
-    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
-        if_chain! {
-            if let ExprKind::MethodCall(ref method_path, _, args) = &expr.kind;
-            let method = method_path.ident.name.as_str();
-            if type_is_atomic(cx, &args[0]);
-            if method == "load" || method == "store";
-            let ordering_arg = if method == "load" { &args[1] } else { &args[2] };
-            if let ExprKind::Path(ref ordering_qpath) = ordering_arg.kind;
-            if let Some(ordering_def_id) = cx.tables.qpath_res(ordering_qpath, ordering_arg.hir_id).opt_def_id();
-            then {
-                if method == "load" &&
-                    match_ordering_def_path(cx, ordering_def_id, &["Release", "AcqRel"]) {
-                    span_help_and_lint(
-                        cx,
-                        INVALID_ATOMIC_ORDERING,
-                        ordering_arg.span,
-                        "atomic loads cannot have `Release` and `AcqRel` ordering",
-                        "consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`"
-                    );
-                } else if method == "store" &&
-                    match_ordering_def_path(cx, ordering_def_id, &["Acquire", "AcqRel"]) {
-                    span_help_and_lint(
-                        cx,
-                        INVALID_ATOMIC_ORDERING,
-                        ordering_arg.span,
-                        "atomic stores cannot have `Acquire` and `AcqRel` ordering",
-                        "consider using ordering modes `Release`, `SeqCst` or `Relaxed`"
-                    );
-                }
+fn check_atomic_load_store(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+    if_chain! {
+        if let ExprKind::MethodCall(ref method_path, _, args) = &expr.kind;
+        let method = method_path.ident.name.as_str();
+        if type_is_atomic(cx, &args[0]);
+        if method == "load" || method == "store";
+        let ordering_arg = if method == "load" { &args[1] } else { &args[2] };
+        if let ExprKind::Path(ref ordering_qpath) = ordering_arg.kind;
+        if let Some(ordering_def_id) = cx.tables.qpath_res(ordering_qpath, ordering_arg.hir_id).opt_def_id();
+        then {
+            if method == "load" &&
+                match_ordering_def_path(cx, ordering_def_id, &["Release", "AcqRel"]) {
+                span_help_and_lint(
+                    cx,
+                    INVALID_ATOMIC_ORDERING,
+                    ordering_arg.span,
+                    "atomic loads cannot have `Release` and `AcqRel` ordering",
+                    "consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`"
+                );
+            } else if method == "store" &&
+                match_ordering_def_path(cx, ordering_def_id, &["Acquire", "AcqRel"]) {
+                span_help_and_lint(
+                    cx,
+                    INVALID_ATOMIC_ORDERING,
+                    ordering_arg.span,
+                    "atomic stores cannot have `Acquire` and `AcqRel` ordering",
+                    "consider using ordering modes `Release`, `SeqCst` or `Relaxed`"
+                );
             }
         }
     }
 }
+
+fn check_memory_fence(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+    if_chain! {
+        if let ExprKind::Call(ref func, ref args) = expr.kind;
+        if let ExprKind::Path(ref func_qpath) = func.kind;
+        if let Some(def_id) = cx.tables.qpath_res(func_qpath, func.hir_id).opt_def_id();
+        if ["fence", "compiler_fence"]
+            .iter()
+            .any(|func| match_def_path(cx, def_id, &["core", "sync", "atomic", func]));
+        if let ExprKind::Path(ref ordering_qpath) = &args[0].kind;
+        if let Some(ordering_def_id) = cx.tables.qpath_res(ordering_qpath, args[0].hir_id).opt_def_id();
+        if match_ordering_def_path(cx, ordering_def_id, &["Relaxed"]);
+        then {
+            span_help_and_lint(
+                cx,
+                INVALID_ATOMIC_ORDERING,
+                args[0].span,
+                "memory fences cannot have `Relaxed` ordering",
+                "consider using ordering modes `Acquire`, `Release`, `AcqRel` or `SeqCst`"
+            );
+        }
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AtomicOrdering {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
+        check_atomic_load_store(cx, expr);
+        check_memory_fence(cx, expr);
+    }
+}

clippy_lints/src/lib.rs

@@ -645,6 +645,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::SEARCH_IS_SOME,
         &methods::SHOULD_IMPLEMENT_TRAIT,
         &methods::SINGLE_CHAR_PATTERN,
+        &methods::SKIP_WHILE_NEXT,
         &methods::STRING_EXTEND_CHARS,
         &methods::SUSPICIOUS_MAP,
         &methods::TEMPORARY_CSTRING_AS_PTR,
@@ -1223,6 +1224,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
         LintId::of(&methods::SINGLE_CHAR_PATTERN),
+        LintId::of(&methods::SKIP_WHILE_NEXT),
         LintId::of(&methods::STRING_EXTEND_CHARS),
         LintId::of(&methods::SUSPICIOUS_MAP),
         LintId::of(&methods::TEMPORARY_CSTRING_AS_PTR),
@@ -1475,6 +1477,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::FLAT_MAP_IDENTITY),
         LintId::of(&methods::OPTION_AND_THEN_SOME),
         LintId::of(&methods::SEARCH_IS_SOME),
+        LintId::of(&methods::SKIP_WHILE_NEXT),
         LintId::of(&methods::SUSPICIOUS_MAP),
         LintId::of(&methods::UNNECESSARY_FILTER_MAP),
         LintId::of(&methods::USELESS_ASREF),

clippy_lints/src/methods/mod.rs

@@ -375,6 +375,29 @@ declare_clippy_lint! {
     "using `filter(p).next()`, which is more succinctly expressed as `.find(p)`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `_.skip_while(condition).next()`.
+    ///
+    /// **Why is this bad?** Readability, this can be written more concisely as
+    /// `_.find(!condition)`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # let vec = vec![1];
+    /// vec.iter().skip_while(|x| **x == 0).next();
+    /// ```
+    /// Could be written as
+    /// ```rust
+    /// # let vec = vec![1];
+    /// vec.iter().find(|x| **x != 0);
+    /// ```
+    pub SKIP_WHILE_NEXT,
+    complexity,
+    "using `skip_while(p).next()`, which is more succinctly expressed as `.find(!p)`"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `_.map(_).flatten(_)`,
     ///
@@ -1041,7 +1064,8 @@ declare_clippy_lint! {
     /// **What it does:** Checks for calls to `map` followed by a `count`.
     ///
     /// **Why is this bad?** It looks suspicious. Maybe `map` was confused with `filter`.
-    /// If the `map` call is intentional, this should be rewritten.
+    /// If the `map` call is intentional, this should be rewritten. Or, if you intend to
+    /// drive the iterator to completion, you can just use `for_each` instead.
     ///
     /// **Known problems:** None
     ///
@@ -1192,6 +1216,7 @@ declare_lint_pass!(Methods => [
     SEARCH_IS_SOME,
     TEMPORARY_CSTRING_AS_PTR,
     FILTER_NEXT,
+    SKIP_WHILE_NEXT,
     FILTER_MAP,
     FILTER_MAP_NEXT,
     FLAT_MAP_IDENTITY,
@@ -1237,6 +1262,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
             ["and_then", ..] => lint_option_and_then_some(cx, expr, arg_lists[0]),
             ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
+            ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
             ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]),
@@ -2530,6 +2556,24 @@ fn lint_filter_next<'a, 'tcx>(
     }
 }
 
+/// lint use of `skip_while().next()` for `Iterators`
+fn lint_skip_while_next<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    expr: &'tcx hir::Expr<'_>,
+    _skip_while_args: &'tcx [hir::Expr<'_>],
+) {
+    // lint if caller of `.skip_while().next()` is an Iterator
+    if match_trait_method(cx, expr, &paths::ITERATOR) {
+        span_help_and_lint(
+            cx,
+            SKIP_WHILE_NEXT,
+            expr.span,
+            "called `skip_while(p).next()` on an `Iterator`",
+            "this is more succinctly expressed by calling `.find(!p)` instead",
+        );
+    }
+}
+
 /// lint use of `filter().map()` for `Iterators`
 fn lint_filter_map<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
@@ -3014,7 +3058,7 @@ fn lint_suspicious_map(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) {
         SUSPICIOUS_MAP,
         expr.span,
         "this call to `map()` won't have an effect on the call to `count()`",
-        "make sure you did not confuse `map` with `filter`",
+        "make sure you did not confuse `map` with `filter` or `for_each`",
     );
 }
 

clippy_lints/src/mut_key.rs

@@ -1,5 +1,5 @@
 use crate::utils::{match_def_path, paths, span_lint, trait_ref_of_method, walk_ptrs_ty};
-use rustc::ty::{Adt, Dynamic, Opaque, Param, RawPtr, Ref, Ty, TypeAndMut};
+use rustc::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut};
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -101,21 +101,24 @@ fn check_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, ty: Ty<'tcx>) {
         if [&paths::HASHMAP, &paths::BTREEMAP, &paths::HASHSET, &paths::BTREESET]
             .iter()
             .any(|path| match_def_path(cx, def.did, &**path))
+            && is_mutable_type(cx, substs.type_at(0), span)
         {
-            let key_type = concrete_type(substs.type_at(0));
-            if let Some(key_type) = key_type {
-                if !key_type.is_freeze(cx.tcx, cx.param_env, span) {
-                    span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
-                }
-            }
+            span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
         }
     }
 }
 
-fn concrete_type(ty: Ty<'_>) -> Option<Ty<'_>> {
+fn is_mutable_type<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, span: Span) -> bool {
     match ty.kind {
-        RawPtr(TypeAndMut { ty: inner_ty, .. }) | Ref(_, inner_ty, _) => concrete_type(inner_ty),
-        Dynamic(..) | Opaque(..) | Param(..) => None,
-        _ => Some(ty),
+        RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => {
+            mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span)
+        },
+        Slice(inner_ty) => is_mutable_type(cx, inner_ty, span),
+        Array(inner_ty, size) => {
+            size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span)
+        },
+        Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)),
+        Adt(..) => cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx, cx.param_env, span),
+        _ => false,
     }
 }

clippy_lints/src/needless_pass_by_value.rs

@@ -114,7 +114,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
         let preds = traits::elaborate_predicates(cx.tcx, cx.param_env.caller_bounds.to_vec())
             .filter(|p| !p.is_global())
             .filter_map(|pred| {
-                if let ty::Predicate::Trait(poly_trait_ref) = pred {
+                if let ty::Predicate::Trait(poly_trait_ref, _) = pred {
                     if poly_trait_ref.def_id() == sized_trait || poly_trait_ref.skip_binder().has_escaping_bound_vars()
                     {
                         return None;

clippy_lints/src/utils/mod.rs

@@ -1300,7 +1300,7 @@ pub fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> boo
         Tuple(ref substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)),
         Opaque(ref def_id, _) => {
             for (predicate, _) in cx.tcx.predicates_of(*def_id).predicates {
-                if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
+                if let ty::Predicate::Trait(ref poly_trait_predicate, _) = predicate {
                     if must_use_attr(&cx.tcx.get_attrs(poly_trait_predicate.skip_binder().trait_ref.def_id)).is_some() {
                         return true;
                     }