]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/copies.rs
Auto merge of #4087 - phansch:move_tests, r=matthiaskrgr
[rust.git] / clippy_lints / src / copies.rs
index 26669d8c4c2a9b32335b3bc419e710ae103f50ae..d85cf97a6bdde3380b8e746ab3f3cce852a15ae1 100644 (file)
-use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use crate::rustc::{declare_tool_lint, lint_array};
-use crate::rustc::ty::Ty;
-use crate::rustc::hir::*;
-use crate::rustc_data_structures::fx::FxHashMap;
+use crate::utils::{get_parent_expr, higher, in_macro_or_desugar, 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::collections::hash_map::Entry;
 use std::hash::BuildHasherDefault;
-use crate::syntax::symbol::LocalInternedString;
-use smallvec::SmallVec;
-use crate::utils::{SpanlessEq, SpanlessHash};
-use crate::utils::{get_parent_expr, in_macro, snippet, span_lint_and_then, span_note_and_lint};
+use syntax::symbol::LocalInternedString;
 
-/// **What it does:** Checks for consecutive `if`s with the same condition.
-///
-/// **Why is this bad?** This is probably a copy & paste error.
-///
-/// **Known problems:** Hopefully none.
-///
-/// **Example:**
-/// ```rust
-/// if a == b {
-///     …
-/// } else if a == b {
-///     …
-/// }
-/// ```
-///
-/// Note that this lint ignores all conditions with a function call as it could
-/// have side effects:
-///
-/// ```rust
-/// if foo() {
-///     …
-/// } else if foo() { // not linted
-///     …
-/// }
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Checks for consecutive `if`s with the same condition.
+    ///
+    /// **Why is this bad?** This is probably a copy & paste error.
+    ///
+    /// **Known problems:** Hopefully none.
+    ///
+    /// **Example:**
+    /// ```ignore
+    /// if a == b {
+    ///     …
+    /// } else if a == b {
+    ///     …
+    /// }
+    /// ```
+    ///
+    /// Note that this lint ignores all conditions with a function call as it could
+    /// have side effects:
+    ///
+    /// ```ignore
+    /// if foo() {
+    ///     …
+    /// } else if foo() { // not linted
+    ///     …
+    /// }
+    /// ```
     pub IFS_SAME_COND,
     correctness,
     "consecutive `ifs` with the same condition"
 }
 
-/// **What it does:** Checks for `if/else` with the same body as the *then* part
-/// and the *else* part.
-///
-/// **Why is this bad?** This is probably a copy & paste error.
-///
-/// **Known problems:** Hopefully none.
-///
-/// **Example:**
-/// ```rust
-/// let foo = if … {
-///     42
-/// } else {
-///     42
-/// };
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Checks for `if/else` with the same body as the *then* part
+    /// and the *else* part.
+    ///
+    /// **Why is this bad?** This is probably a copy & paste error.
+    ///
+    /// **Known problems:** Hopefully none.
+    ///
+    /// **Example:**
+    /// ```ignore
+    /// let foo = if … {
+    ///     42
+    /// } else {
+    ///     42
+    /// };
+    /// ```
     pub IF_SAME_THEN_ELSE,
     correctness,
     "if with the same *then* and *else* blocks"
 }
 
-/// **What it does:** Checks for `match` with identical arm bodies.
-///
-/// **Why is this bad?** This is probably a copy & paste error. If arm bodies
-/// are the same on purpose, you can factor them
-/// [using `|`](https://doc.rust-lang.org/book/patterns.html#multiple-patterns).
-///
-/// **Known problems:** False positive possible with order dependent `match`
-/// (see issue
-/// [#860](https://github.com/rust-lang-nursery/rust-clippy/issues/860)).
-///
-/// **Example:**
-/// ```rust,ignore
-/// match foo {
-///     Bar => bar(),
-///     Quz => quz(),
-///     Baz => bar(), // <= oops
-/// }
-/// ```
-///
-/// This should probably be
-/// ```rust,ignore
-/// match foo {
-///     Bar => bar(),
-///     Quz => quz(),
-///     Baz => baz(), // <= fixed
-/// }
-/// ```
-///
-/// or if the original code was not a typo:
-/// ```rust,ignore
-/// match foo {
-///     Bar | Baz => bar(), // <= shows the intent better
-///     Quz => quz(),
-/// }
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Checks for `match` with identical arm bodies.
+    ///
+    /// **Why is this bad?** This is probably a copy & paste error. If arm bodies
+    /// are the same on purpose, you can factor them
+    /// [using `|`](https://doc.rust-lang.org/book/patterns.html#multiple-patterns).
+    ///
+    /// **Known problems:** False positive possible with order dependent `match`
+    /// (see issue
+    /// [#860](https://github.com/rust-lang/rust-clippy/issues/860)).
+    ///
+    /// **Example:**
+    /// ```rust,ignore
+    /// match foo {
+    ///     Bar => bar(),
+    ///     Quz => quz(),
+    ///     Baz => bar(), // <= oops
+    /// }
+    /// ```
+    ///
+    /// This should probably be
+    /// ```rust,ignore
+    /// match foo {
+    ///     Bar => bar(),
+    ///     Quz => quz(),
+    ///     Baz => baz(), // <= fixed
+    /// }
+    /// ```
+    ///
+    /// or if the original code was not a typo:
+    /// ```rust,ignore
+    /// match foo {
+    ///     Bar | Baz => bar(), // <= shows the intent better
+    ///     Quz => quz(),
+    /// }
+    /// ```
     pub MATCH_SAME_ARMS,
     pedantic,
     "`match` with identical arm bodies"
 }
 
-#[derive(Copy, Clone, Debug)]
-pub struct CopyAndPaste;
-
-impl LintPass for CopyAndPaste {
-    fn get_lints(&self) -> LintArray {
-        lint_array![IFS_SAME_COND, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]
-    }
-}
+declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
-        if !in_macro(expr.span) {
+        if !in_macro_or_desugar(expr.span) {
             // skip ifs directly in else, it will be checked in the parent if
-            if let Some(&Expr {
-                node: ExprKind::If(_, _, Some(ref else_expr)),
-                ..
-            }) = get_parent_expr(cx, expr)
-            {
-                if else_expr.id == expr.id {
-                    return;
+            if let Some(expr) = get_parent_expr(cx, expr) {
+                if let Some((_, _, Some(ref else_expr))) = higher::if_block(&expr) {
+                    if else_expr.hir_id == expr.hir_id {
+                        return;
+                    }
                 }
             }
 
@@ -158,7 +149,8 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
         h.finish()
     };
 
-    let eq: &dyn Fn(&&Expr, &&Expr) -> bool = &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) };
+    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) {
         span_note_and_lint(
@@ -202,7 +194,7 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
                 |db| {
                     db.span_note(i.body.span, "same as this");
 
-                    // Note: this does not use `span_suggestion_with_applicability` on purpose:
+                    // Note: this does not use `span_suggestion` on purpose:
                     // there is no clean way
                     // to remove the other arm. Building a span and suggest to replace it to ""
                     // makes an even more confusing error message. Also in order not to make up a
@@ -219,7 +211,10 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
                             // hiding all the subsequent arms, and rust won't compile
                             db.span_note(
                                 i.body.span,
-                                &format!("`{}` has the same arm body as the `_` wildcard, consider removing it`", lhs),
+                                &format!(
+                                    "`{}` has the same arm body as the `_` wildcard, consider removing it`",
+                                    lhs
+                                ),
                             );
                         } else {
                             db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
@@ -231,15 +226,15 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
     }
 }
 
-/// Return the list of condition expressions and the list of blocks in a
+/// Returns the list of condition expressions and the list of blocks in a
 /// sequence of `if/else`.
-/// Eg. would return `([a, b], [c, d, e])` for the expression
+/// 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 ExprKind::If(ref cond, ref then_expr, ref else_expr) = expr.node {
+    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);
@@ -264,15 +259,21 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
     (conds, blocks)
 }
 
-/// Return the list of bindings in a pattern.
+/// 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_impl<'a, 'tcx>(
+        cx: &LateContext<'a, 'tcx>,
+        pat: &Pat,
+        map: &mut FxHashMap<LocalInternedString, Ty<'tcx>>,
+    ) {
         match pat.node {
             PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => bindings_impl(cx, pat, map),
-            PatKind::TupleStruct(_, ref pats, _) => for pat in pats {
-                bindings_impl(cx, pat, map);
+            PatKind::TupleStruct(_, ref pats, _) => {
+                for pat in pats {
+                    bindings_impl(cx, pat, map);
+                }
             },
-            PatKind::Binding(_, _, ident, ref as_pat) => {
+            PatKind::Binding(.., ident, ref as_pat) => {
                 if let Entry::Vacant(v) = map.entry(ident.as_str()) {
                     v.insert(cx.tables.pat_ty(pat));
                 }
@@ -280,11 +281,15 @@ fn bindings_impl<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat, map: &mut FxHa
                     bindings_impl(cx, as_pat, map);
                 }
             },
-            PatKind::Struct(_, ref fields, _) => for pat in fields {
-                bindings_impl(cx, &pat.node.pat, map);
+            PatKind::Struct(_, ref fields, _) => {
+                for pat in fields {
+                    bindings_impl(cx, &pat.node.pat, map);
+                }
             },
-            PatKind::Tuple(ref fields, _) => for pat in fields {
-                bindings_impl(cx, pat, map);
+            PatKind::Tuple(ref fields, _) => {
+                for pat in fields {
+                    bindings_impl(cx, pat, map);
+                }
             },
             PatKind::Slice(ref lhs, ref mid, ref rhs) => {
                 for pat in lhs {
@@ -306,7 +311,6 @@ fn bindings_impl<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat, map: &mut FxHa
     result
 }
 
-
 fn search_same_sequenced<T, Eq>(exprs: &[T], eq: Eq) -> Option<(&T, &T)>
 where
     Eq: Fn(&T, &T) -> bool,
@@ -335,10 +339,8 @@ fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)>
         };
     }
 
-    let mut map: FxHashMap<_, Vec<&_>> = FxHashMap::with_capacity_and_hasher(
-        exprs.len(),
-        BuildHasherDefault::default()
-    );
+    let mut map: FxHashMap<_, Vec<&_>> =
+        FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
 
     for expr in exprs {
         match map.entry(hash(expr)) {