clippy_lints/src/copies.rs

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