clippy_lints/src/copies.rs

   1 // Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
   2 // file at the top-level directory of this distribution.
   3 //
   4 // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
   5 // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
   6 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
   7 // option. This file may not be copied, modified, or distributed
   8 // except according to those terms.
   9
  10
  11 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
  12 use crate::rustc::{declare_tool_lint, lint_array};
  13 use crate::rustc::ty::Ty;
  14 use crate::rustc::hir::*;
  15 use crate::rustc_data_structures::fx::FxHashMap;
  16 use std::collections::hash_map::Entry;
  17 use std::hash::BuildHasherDefault;
  18 use crate::syntax::symbol::LocalInternedString;
  19 use smallvec::SmallVec;
  20 use crate::utils::{SpanlessEq, SpanlessHash};
  21 use crate::utils::{get_parent_expr, in_macro, snippet, span_lint_and_then, span_note_and_lint};
  22
  23 /// **What it does:** Checks for consecutive `if`s with the same condition.
  24 ///
  25 /// **Why is this bad?** This is probably a copy & paste error.
  26 ///
  27 /// **Known problems:** Hopefully none.
  28 ///
  29 /// **Example:**
  30 /// ```rust
  31 /// if a == b {
  32 ///     …
  33 /// } else if a == b {
  34 ///     …
  35 /// }
  36 /// ```
  37 ///
  38 /// Note that this lint ignores all conditions with a function call as it could
  39 /// have side effects:
  40 ///
  41 /// ```rust
  42 /// if foo() {
  43 ///     …
  44 /// } else if foo() { // not linted
  45 ///     …
  46 /// }
  47 /// ```
  48 declare_clippy_lint! {
  49     pub IFS_SAME_COND,
  50     correctness,
  51     "consecutive `ifs` with the same condition"
  52 }
  53
  54 /// **What it does:** Checks for `if/else` with the same body as the *then* part
  55 /// and the *else* part.
  56 ///
  57 /// **Why is this bad?** This is probably a copy & paste error.
  58 ///
  59 /// **Known problems:** Hopefully none.
  60 ///
  61 /// **Example:**
  62 /// ```rust
  63 /// let foo = if … {
  64 ///     42
  65 /// } else {
  66 ///     42
  67 /// };
  68 /// ```
  69 declare_clippy_lint! {
  70     pub IF_SAME_THEN_ELSE,
  71     correctness,
  72     "if with the same *then* and *else* blocks"
  73 }
  74
  75 /// **What it does:** Checks for `match` with identical arm bodies.
  76 ///
  77 /// **Why is this bad?** This is probably a copy & paste error. If arm bodies
  78 /// are the same on purpose, you can factor them
  79 /// [using `|`](https://doc.rust-lang.org/book/patterns.html#multiple-patterns).
  80 ///
  81 /// **Known problems:** False positive possible with order dependent `match`
  82 /// (see issue
  83 /// [#860](https://github.com/rust-lang/rust-clippy/issues/860)).
  84 ///
  85 /// **Example:**
  86 /// ```rust,ignore
  87 /// match foo {
  88 ///     Bar => bar(),
  89 ///     Quz => quz(),
  90 ///     Baz => bar(), // <= oops
  91 /// }
  92 /// ```
  93 ///
  94 /// This should probably be
  95 /// ```rust,ignore
  96 /// match foo {
  97 ///     Bar => bar(),
  98 ///     Quz => quz(),
  99 ///     Baz => baz(), // <= fixed
 100 /// }
 101 /// ```
 102 ///
 103 /// or if the original code was not a typo:
 104 /// ```rust,ignore
 105 /// match foo {
 106 ///     Bar | Baz => bar(), // <= shows the intent better
 107 ///     Quz => quz(),
 108 /// }
 109 /// ```
 110 declare_clippy_lint! {
 111     pub MATCH_SAME_ARMS,
 112     pedantic,
 113     "`match` with identical arm bodies"
 114 }
 115
 116 #[derive(Copy, Clone, Debug)]
 117 pub struct CopyAndPaste;
 118
 119 impl LintPass for CopyAndPaste {
 120     fn get_lints(&self) -> LintArray {
 121         lint_array![IFS_SAME_COND, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]
 122     }
 123 }
 124
 125 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
 126     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
 127         if !in_macro(expr.span) {
 128             // skip ifs directly in else, it will be checked in the parent if
 129             if let Some(&Expr {
 130                 node: ExprKind::If(_, _, Some(ref else_expr)),
 131                 ..
 132             }) = get_parent_expr(cx, expr)
 133             {
 134                 if else_expr.id == expr.id {
 135                     return;
 136                 }
 137             }
 138
 139             let (conds, blocks) = if_sequence(expr);
 140             lint_same_then_else(cx, &blocks);
 141             lint_same_cond(cx, &conds);
 142             lint_match_arms(cx, expr);
 143         }
 144     }
 145 }
 146
 147 /// Implementation of `IF_SAME_THEN_ELSE`.
 148 fn lint_same_then_else(cx: &LateContext<'_, '_>, blocks: &[&Block]) {
 149     let eq: &dyn Fn(&&Block, &&Block) -> bool = &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).eq_block(lhs, rhs) };
 150
 151     if let Some((i, j)) = search_same_sequenced(blocks, eq) {
 152         span_note_and_lint(
 153             cx,
 154             IF_SAME_THEN_ELSE,
 155             j.span,
 156             "this `if` has identical blocks",
 157             i.span,
 158             "same as this",
 159         );
 160     }
 161 }
 162
 163 /// Implementation of `IFS_SAME_COND`.
 164 fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
 165     let hash: &dyn Fn(&&Expr) -> u64 = &|expr| -> u64 {
 166         let mut h = SpanlessHash::new(cx, cx.tables);
 167         h.hash_expr(expr);
 168         h.finish()
 169     };
 170
 171     let eq: &dyn Fn(&&Expr, &&Expr) -> bool = &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) };
 172
 173     if let Some((i, j)) = search_same(conds, hash, eq) {
 174         span_note_and_lint(
 175             cx,
 176             IFS_SAME_COND,
 177             j.span,
 178             "this `if` has the same condition as a previous if",
 179             i.span,
 180             "same as this",
 181         );
 182     }
 183 }
 184
 185 /// Implementation of `MATCH_SAME_ARMS`.
 186 fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
 187     if let ExprKind::Match(_, ref arms, MatchSource::Normal) = expr.node {
 188         let hash = |&(_, arm): &(usize, &Arm)| -> u64 {
 189             let mut h = SpanlessHash::new(cx, cx.tables);
 190             h.hash_expr(&arm.body);
 191             h.finish()
 192         };
 193
 194         let eq = |&(lindex, lhs): &(usize, &Arm), &(rindex, rhs): &(usize, &Arm)| -> bool {
 195             let min_index = usize::min(lindex, rindex);
 196             let max_index = usize::max(lindex, rindex);
 197             // Arms with a guard are ignored, those can’t always be merged together
 198             // This is also the case for arms in-between each there is an arm with a guard
 199             (min_index..=max_index).all(|index| arms[index].guard.is_none()) &&
 200                 SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) &&
 201                 // all patterns should have the same bindings
 202                 bindings(cx, &lhs.pats[0]) == bindings(cx, &rhs.pats[0])
 203         };
 204
 205         let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect();
 206         if let Some((&(_, i), &(_, j))) = search_same(&indexed_arms, hash, eq) {
 207             span_lint_and_then(
 208                 cx,
 209                 MATCH_SAME_ARMS,
 210                 j.body.span,
 211                 "this `match` has identical arm bodies",
 212                 |db| {
 213                     db.span_note(i.body.span, "same as this");
 214
 215                     // Note: this does not use `span_suggestion_with_applicability` on purpose:
 216                     // there is no clean way
 217                     // to remove the other arm. Building a span and suggest to replace it to ""
 218                     // makes an even more confusing error message. Also in order not to make up a
 219                     // span for the whole pattern, the suggestion is only shown when there is only
 220                     // one pattern. The user should know about `|` if they are already using it…
 221
 222                     if i.pats.len() == 1 && j.pats.len() == 1 {
 223                         let lhs = snippet(cx, i.pats[0].span, "<pat1>");
 224                         let rhs = snippet(cx, j.pats[0].span, "<pat2>");
 225
 226                         if let PatKind::Wild = j.pats[0].node {
 227                             // if the last arm is _, then i could be integrated into _
 228                             // note that i.pats[0] cannot be _, because that would mean that we're
 229                             // hiding all the subsequent arms, and rust won't compile
 230                             db.span_note(
 231                                 i.body.span,
 232                                 &format!("`{}` has the same arm body as the `_` wildcard, consider removing it`", lhs),
 233                             );
 234                         } else {
 235                             db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
 236                         }
 237                     }
 238                 },
 239             );
 240         }
 241     }
 242 }
 243
 244 /// Return the list of condition expressions and the list of blocks in a
 245 /// sequence of `if/else`.
 246 /// Eg. would return `([a, b], [c, d, e])` for the expression
 247 /// `if a { c } else if b { d } else { e }`.
 248 fn if_sequence(mut expr: &Expr) -> (SmallVec<[&Expr; 1]>, SmallVec<[&Block; 1]>) {
 249     let mut conds = SmallVec::new();
 250     let mut blocks: SmallVec<[&Block; 1]> = SmallVec::new();
 251
 252     while let ExprKind::If(ref cond, ref then_expr, ref else_expr) = expr.node {
 253         conds.push(&**cond);
 254         if let ExprKind::Block(ref block, _) = then_expr.node {
 255             blocks.push(block);
 256         } else {
 257             panic!("ExprKind::If node is not an ExprKind::Block");
 258         }
 259
 260         if let Some(ref else_expr) = *else_expr {
 261             expr = else_expr;
 262         } else {
 263             break;
 264         }
 265     }
 266
 267     // final `else {..}`
 268     if !blocks.is_empty() {
 269         if let ExprKind::Block(ref block, _) = expr.node {
 270             blocks.push(&**block);
 271         }
 272     }
 273
 274     (conds, blocks)
 275 }
 276
 277 /// Return the list of bindings in a pattern.
 278 fn bindings<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat) -> FxHashMap<LocalInternedString, Ty<'tcx>> {
 279     fn bindings_impl<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat, map: &mut FxHashMap<LocalInternedString, Ty<'tcx>>) {
 280         match pat.node {
 281             PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => bindings_impl(cx, pat, map),
 282             PatKind::TupleStruct(_, ref pats, _) => for pat in pats {
 283                 bindings_impl(cx, pat, map);
 284             },
 285             PatKind::Binding(_, _, ident, ref as_pat) => {
 286                 if let Entry::Vacant(v) = map.entry(ident.as_str()) {
 287                     v.insert(cx.tables.pat_ty(pat));
 288                 }
 289                 if let Some(ref as_pat) = *as_pat {
 290                     bindings_impl(cx, as_pat, map);
 291                 }
 292             },
 293             PatKind::Struct(_, ref fields, _) => for pat in fields {
 294                 bindings_impl(cx, &pat.node.pat, map);
 295             },
 296             PatKind::Tuple(ref fields, _) => for pat in fields {
 297                 bindings_impl(cx, pat, map);
 298             },
 299             PatKind::Slice(ref lhs, ref mid, ref rhs) => {
 300                 for pat in lhs {
 301                     bindings_impl(cx, pat, map);
 302                 }
 303                 if let Some(ref mid) = *mid {
 304                     bindings_impl(cx, mid, map);
 305                 }
 306                 for pat in rhs {
 307                     bindings_impl(cx, pat, map);
 308                 }
 309             },
 310             PatKind::Lit(..) | PatKind::Range(..) | PatKind::Wild | PatKind::Path(..) => (),
 311         }
 312     }
 313
 314     let mut result = FxHashMap::default();
 315     bindings_impl(cx, pat, &mut result);
 316     result
 317 }
 318
 319
 320 fn search_same_sequenced<T, Eq>(exprs: &[T], eq: Eq) -> Option<(&T, &T)>
 321 where
 322     Eq: Fn(&T, &T) -> bool,
 323 {
 324     for win in exprs.windows(2) {
 325         if eq(&win[0], &win[1]) {
 326             return Some((&win[0], &win[1]));
 327         }
 328     }
 329     None
 330 }
 331
 332 fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)>
 333 where
 334     Hash: Fn(&T) -> u64,
 335     Eq: Fn(&T, &T) -> bool,
 336 {
 337     // common cases
 338     if exprs.len() < 2 {
 339         return None;
 340     } else if exprs.len() == 2 {
 341         return if eq(&exprs[0], &exprs[1]) {
 342             Some((&exprs[0], &exprs[1]))
 343         } else {
 344             None
 345         };
 346     }
 347
 348     let mut map: FxHashMap<_, Vec<&_>> = FxHashMap::with_capacity_and_hasher(
 349         exprs.len(),
 350         BuildHasherDefault::default()
 351     );
 352
 353     for expr in exprs {
 354         match map.entry(hash(expr)) {
 355             Entry::Occupied(mut o) => {
 356                 for o in o.get() {
 357                     if eq(o, expr) {
 358                         return Some((o, expr));
 359                     }
 360                 }
 361                 o.get_mut().push(expr);
 362             },
 363             Entry::Vacant(v) => {
 364                 v.insert(vec![expr]);
 365             },
 366         }
 367     }
 368
 369     None
 370 }