Add a new lint for comparison chains

[rust.git] / clippy_lints / src / copies.rs

diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
index 3894b56c0494b0775d89dff5214f316bab6a40d3..38654c753bf80a80eddde76885ee3c655bc5a28d 100644 (file)
--- a/clippy_lints/src/copies.rs
+++ b/clippy_lints/src/copies.rs
@@ -1,16 +1,14 @@
-use crate::utils::{
-    get_parent_expr, higher, in_macro_or_desugar, same_tys, snippet, span_lint_and_then, span_note_and_lint,
-};
+use crate::utils::{get_parent_expr, higher, if_sequence, same_tys, snippet, span_lint_and_then, span_note_and_lint};
 use crate::utils::{SpanlessEq, SpanlessHash};
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::ty::Ty;
 use rustc::{declare_lint_pass, declare_tool_lint};
 use rustc_data_structures::fx::FxHashMap;
-use smallvec::SmallVec;
+use std::cmp::Ordering;
 use std::collections::hash_map::Entry;
 use std::hash::BuildHasherDefault;
-use syntax::symbol::LocalInternedString;
+use syntax::symbol::Symbol;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for consecutive `if`s with the same condition.
@@ -109,7 +107,7 @@
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
-        if !in_macro_or_desugar(expr.span) {
+        if !expr.span.from_expansion() {
             // skip ifs directly in else, it will be checked in the parent if
             if let Some(expr) = get_parent_expr(cx, expr) {
                 if let Some((_, _, Some(ref else_expr))) = higher::if_block(&expr) {
@@ -170,8 +168,8 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
 fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
     fn same_bindings<'tcx>(
         cx: &LateContext<'_, 'tcx>,
-        lhs: &FxHashMap<LocalInternedString, Ty<'tcx>>,
-        rhs: &FxHashMap<LocalInternedString, Ty<'tcx>>,
+        lhs: &FxHashMap<Symbol, Ty<'tcx>>,
+        rhs: &FxHashMap<Symbol, Ty<'tcx>>,
     ) -> bool {
         lhs.len() == rhs.len()
             && lhs
@@ -243,46 +241,9 @@ fn same_bindings<'tcx>(
     }
 }
 
-/// Returns the list of condition expressions and the list of blocks in a
-/// sequence of `if/else`.
-/// E.g., this returns `([a, b], [c, d, e])` for the expression
-/// `if a { c } else if b { d } else { e }`.
-fn if_sequence(mut expr: &Expr) -> (SmallVec<[&Expr; 1]>, SmallVec<[&Block; 1]>) {
-    let mut conds = SmallVec::new();
-    let mut blocks: SmallVec<[&Block; 1]> = SmallVec::new();
-
-    while let Some((ref cond, ref then_expr, ref else_expr)) = higher::if_block(&expr) {
-        conds.push(&**cond);
-        if let ExprKind::Block(ref block, _) = then_expr.node {
-            blocks.push(block);
-        } else {
-            panic!("ExprKind::If node is not an ExprKind::Block");
-        }
-
-        if let Some(ref else_expr) = *else_expr {
-            expr = else_expr;
-        } else {
-            break;
-        }
-    }
-
-    // final `else {..}`
-    if !blocks.is_empty() {
-        if let ExprKind::Block(ref block, _) = expr.node {
-            blocks.push(&**block);
-        }
-    }
-
-    (conds, blocks)
-}
-
 /// Returns the list of bindings in a pattern.
-fn bindings<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat) -> FxHashMap<LocalInternedString, Ty<'tcx>> {
-    fn bindings_impl<'a, 'tcx>(
-        cx: &LateContext<'a, 'tcx>,
-        pat: &Pat,
-        map: &mut FxHashMap<LocalInternedString, Ty<'tcx>>,
-    ) {
+fn bindings<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat) -> FxHashMap<Symbol, Ty<'tcx>> {
+    fn bindings_impl<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat, map: &mut FxHashMap<Symbol, Ty<'tcx>>) {
         match pat.node {
             PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => bindings_impl(cx, pat, map),
             PatKind::TupleStruct(_, ref pats, _) => {
@@ -291,21 +252,21 @@ fn bindings_impl<'a, 'tcx>(
                 }
             },
             PatKind::Binding(.., ident, ref as_pat) => {
-                if let Entry::Vacant(v) = map.entry(ident.as_str()) {
+                if let Entry::Vacant(v) = map.entry(ident.name) {
                     v.insert(cx.tables.pat_ty(pat));
                 }
                 if let Some(ref as_pat) = *as_pat {
                     bindings_impl(cx, as_pat, map);
                 }
             },
-            PatKind::Struct(_, ref fields, _) => {
+            PatKind::Or(ref fields) | PatKind::Tuple(ref fields, _) => {
                 for pat in fields {
-                    bindings_impl(cx, &pat.node.pat, map);
+                    bindings_impl(cx, pat, map);
                 }
             },
-            PatKind::Tuple(ref fields, _) => {
+            PatKind::Struct(_, ref fields, _) => {
                 for pat in fields {
-                    bindings_impl(cx, pat, map);
+                    bindings_impl(cx, &pat.pat, map);
                 }
             },
             PatKind::Slice(ref lhs, ref mid, ref rhs) => {
@@ -344,16 +305,15 @@ fn search_common_cases<'a, T, Eq>(exprs: &'a [T], eq: &Eq) -> Option<(&'a T, &'a
 where
     Eq: Fn(&T, &T) -> bool,
 {
-    if exprs.len() < 2 {
-        None
-    } else if exprs.len() == 2 {
-        if eq(&exprs[0], &exprs[1]) {
-            Some((&exprs[0], &exprs[1]))
-        } else {
-            None
-        }
-    } else {
-        None
+    match exprs.len().cmp(&2) {
+        Ordering::Greater | Ordering::Less => None,
+        Ordering::Equal => {
+            if eq(&exprs[0], &exprs[1]) {
+                Some((&exprs[0], &exprs[1]))
+            } else {
+                None
+            }
+        },
     }
 }