Update clippy

Author: Manishearth

.travis.yml

@@ -85,8 +85,6 @@ matrix:
   allow_failures:
     - os: windows
       env: CARGO_INCREMENTAL=0 BASE_TESTS=true OS_WINDOWS=true
-    - os: osx # run base tests on both platforms
-      env: BASE_TESTS=true
   # prevent these jobs with default env vars
   exclude:
     - os: linux

CHANGELOG.md

@@ -962,6 +962,7 @@ Released 2018-09-13
 [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned
 [`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
 [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
+[`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
 [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
 [`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro

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 316 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 317 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/booleans.rs

@@ -1,5 +1,6 @@
 use crate::utils::{
-    get_trait_def_id, implements_trait, in_macro, match_type, paths, snippet_opt, span_lint_and_then, SpanlessEq,
+    get_trait_def_id, implements_trait, in_macro, match_type, paths, snippet_opt, span_lint_and_sugg,
+    span_lint_and_then, SpanlessEq,
 };
 use if_chain::if_chain;
 use rustc::hir::intravisit::*;
@@ -159,46 +160,6 @@ struct SuggestContext<'a, 'tcx, 'v> {
 }
 
 impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
-    fn snip(&self, e: &Expr) -> Option<String> {
-        snippet_opt(self.cx, e.span)
-    }
-
-    fn simplify_not(&self, expr: &Expr) -> Option<String> {
-        match &expr.node {
-            ExprKind::Binary(binop, lhs, rhs) => {
-                if !implements_ord(self.cx, lhs) {
-                    return None;
-                }
-
-                match binop.node {
-                    BinOpKind::Eq => Some(" != "),
-                    BinOpKind::Ne => Some(" == "),
-                    BinOpKind::Lt => Some(" >= "),
-                    BinOpKind::Gt => Some(" <= "),
-                    BinOpKind::Le => Some(" > "),
-                    BinOpKind::Ge => Some(" < "),
-                    _ => None,
-                }
-                .and_then(|op| Some(format!("{}{}{}", self.snip(lhs)?, op, self.snip(rhs)?)))
-            },
-            ExprKind::MethodCall(path, _, args) if args.len() == 1 => {
-                let type_of_receiver = self.cx.tables.expr_ty(&args[0]);
-                if !match_type(self.cx, type_of_receiver, &paths::OPTION)
-                    && !match_type(self.cx, type_of_receiver, &paths::RESULT)
-                {
-                    return None;
-                }
-                METHODS_WITH_NEGATION
-                    .iter()
-                    .cloned()
-                    .flat_map(|(a, b)| vec![(a, b), (b, a)])
-                    .find(|&(a, _)| a == path.ident.name.as_str())
-                    .and_then(|(_, neg_method)| Some(format!("{}.{}()", self.snip(&args[0])?, neg_method)))
-            },
-            _ => None,
-        }
-    }
-
     fn recurse(&mut self, suggestion: &Bool) -> Option<()> {
         use quine_mc_cluskey::Bool::*;
         match suggestion {
@@ -217,12 +178,12 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
                 },
                 Term(n) => {
                     let terminal = self.terminals[n as usize];
-                    if let Some(str) = self.simplify_not(terminal) {
+                    if let Some(str) = simplify_not(self.cx, terminal) {
                         self.simplified = true;
                         self.output.push_str(&str)
                     } else {
                         self.output.push('!');
-                        let snip = self.snip(terminal)?;
+                        let snip = snippet_opt(self.cx, terminal.span)?;
                         self.output.push_str(&snip);
                     }
                 },
@@ -254,14 +215,55 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
                 }
             },
             &Term(n) => {
-                let snip = self.snip(self.terminals[n as usize])?;
+                let snip = snippet_opt(self.cx, self.terminals[n as usize].span)?;
                 self.output.push_str(&snip);
             },
         }
         Some(())
     }
 }
 
+fn simplify_not(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<String> {
+    match &expr.node {
+        ExprKind::Binary(binop, lhs, rhs) => {
+            if !implements_ord(cx, lhs) {
+                return None;
+            }
+
+            match binop.node {
+                BinOpKind::Eq => Some(" != "),
+                BinOpKind::Ne => Some(" == "),
+                BinOpKind::Lt => Some(" >= "),
+                BinOpKind::Gt => Some(" <= "),
+                BinOpKind::Le => Some(" > "),
+                BinOpKind::Ge => Some(" < "),
+                _ => None,
+            }
+            .and_then(|op| {
+                Some(format!(
+                    "{}{}{}",
+                    snippet_opt(cx, lhs.span)?,
+                    op,
+                    snippet_opt(cx, rhs.span)?
+                ))
+            })
+        },
+        ExprKind::MethodCall(path, _, args) if args.len() == 1 => {
+            let type_of_receiver = cx.tables.expr_ty(&args[0]);
+            if !match_type(cx, type_of_receiver, &paths::OPTION) && !match_type(cx, type_of_receiver, &paths::RESULT) {
+                return None;
+            }
+            METHODS_WITH_NEGATION
+                .iter()
+                .cloned()
+                .flat_map(|(a, b)| vec![(a, b), (b, a)])
+                .find(|&(a, _)| a == path.ident.name.as_str())
+                .and_then(|(_, neg_method)| Some(format!("{}.{}()", snippet_opt(cx, args[0].span)?, neg_method)))
+        },
+        _ => None,
+    }
+}
+
 // The boolean part of the return indicates whether some simplifications have been applied.
 fn suggest(cx: &LateContext<'_, '_>, suggestion: &Bool, terminals: &[&Expr]) -> (String, bool) {
     let mut suggest_context = SuggestContext {
@@ -330,7 +332,7 @@ fn terminal_stats(b: &Bool) -> Stats {
 }
 
 impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
-    fn bool_expr(&self, e: &Expr) {
+    fn bool_expr(&self, e: &'tcx Expr) {
         let mut h2q = Hir2Qmm {
             terminals: Vec::new(),
             cx: self.cx,
@@ -420,10 +422,8 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
                 );
             };
             if improvements.is_empty() {
-                let suggest = suggest(self.cx, &expr, &h2q.terminals);
-                if suggest.1 {
-                    nonminimal_bool_lint(vec![suggest.0])
-                }
+                let mut visitor = NotSimplificationVisitor { cx: self.cx };
+                visitor.visit_expr(e);
             } else {
                 nonminimal_bool_lint(
                     improvements
@@ -464,3 +464,30 @@ fn implements_ord<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &Expr) -> bool
     let ty = cx.tables.expr_ty(expr);
     get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[]))
 }
+
+struct NotSimplificationVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'a, 'tcx>,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for NotSimplificationVisitor<'a, 'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx Expr) {
+        if let ExprKind::Unary(UnNot, inner) = &expr.node {
+            if let Some(suggestion) = simplify_not(self.cx, inner) {
+                span_lint_and_sugg(
+                    self.cx,
+                    NONMINIMAL_BOOL,
+                    expr.span,
+                    "this boolean expression can be simplified",
+                    "try",
+                    suggestion,
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+
+        walk_expr(self, expr);
+    }
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::None
+    }
+}

clippy_lints/src/bytecount.rs

@@ -62,7 +62,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount {
                                 _ => { return; }
                             }
                         };
-                        if ty::Uint(UintTy::U8) != walk_ptrs_ty(cx.tables.expr_ty(needle)).sty {
+                        if ty::Uint(UintTy::U8) != walk_ptrs_ty(cx.tables.expr_ty(needle)).kind {
                             return;
                         }
                         let haystack = if let ExprKind::MethodCall(ref path, _, ref args) =

clippy_lints/src/comparison_chain.rs

@@ -0,0 +1,107 @@
+use crate::utils::{if_sequence, parent_node_is_if_expr, span_help_and_lint, SpanlessEq};
+use rustc::hir::*;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// **What it does:** Checks comparison chains written with `if` that can be
+    /// rewritten with `match` and `cmp`.
+    ///
+    /// **Why is this bad?** `if` is not guaranteed to be exhaustive and conditionals can get
+    /// repetitive
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust,ignore
+    /// # fn a() {}
+    /// # fn b() {}
+    /// # fn c() {}
+    /// fn f(x: u8, y: u8) {
+    ///     if x > y {
+    ///         a()
+    ///     } else if x < y {
+    ///         b()
+    ///     } else {
+    ///         c()
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// Could be written:
+    ///
+    /// ```rust,ignore
+    /// use std::cmp::Ordering;
+    /// # fn a() {}
+    /// # fn b() {}
+    /// # fn c() {}
+    /// fn f(x: u8, y: u8) {
+    ///      match x.cmp(&y) {
+    ///          Ordering::Greater => a(),
+    ///          Ordering::Less => b(),
+    ///          Ordering::Equal => c()
+    ///      }
+    /// }
+    /// ```
+    pub COMPARISON_CHAIN,
+    style,
+    "`if`s that can be rewritten with `match` and `cmp`"
+}
+
+declare_lint_pass!(ComparisonChain => [COMPARISON_CHAIN]);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if expr.span.from_expansion() {
+            return;
+        }
+
+        // We only care about the top-most `if` in the chain
+        if parent_node_is_if_expr(expr, cx) {
+            return;
+        }
+
+        // Check that there exists at least one explicit else condition
+        let (conds, _) = if_sequence(expr);
+        if conds.len() < 2 {
+            return;
+        }
+
+        for cond in conds.windows(2) {
+            if let (
+                &ExprKind::Binary(ref kind1, ref lhs1, ref rhs1),
+                &ExprKind::Binary(ref kind2, ref lhs2, ref rhs2),
+            ) = (&cond[0].node, &cond[1].node)
+            {
+                if !kind_is_cmp(kind1.node) || !kind_is_cmp(kind2.node) {
+                    return;
+                }
+
+                // Check that both sets of operands are equal
+                let mut spanless_eq = SpanlessEq::new(cx);
+                if (!spanless_eq.eq_expr(lhs1, lhs2) || !spanless_eq.eq_expr(rhs1, rhs2))
+                    && (!spanless_eq.eq_expr(lhs1, rhs2) || !spanless_eq.eq_expr(rhs1, lhs2))
+                {
+                    return;
+                }
+            } else {
+                // We only care about comparison chains
+                return;
+            }
+        }
+        span_help_and_lint(
+            cx,
+            COMPARISON_CHAIN,
+            expr.span,
+            "`if` chain can be rewritten with `match`",
+            "Consider rewriting the `if` chain to use `cmp` and `match`.",
+        )
+    }
+}
+
+fn kind_is_cmp(kind: BinOpKind) -> bool {
+    match kind {
+        BinOpKind::Lt | BinOpKind::Gt | BinOpKind::Eq => true,
+        _ => false,
+    }
+}