clippy_lints/src/copies.rs

   1 use crate::utils::{both, count_eq, eq_expr_value, in_macro, search_same, SpanlessEq, SpanlessHash};
   2 use crate::utils::{
   3     first_line_of_span, get_parent_expr, higher, if_sequence, indent_of, parent_node_is_if_expr, reindent_multiline,
   4     snippet, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
   5 };
   6 use rustc_data_structures::fx::FxHashSet;
   7 use rustc_errors::Applicability;
   8 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
   9 use rustc_hir::{Block, Expr, HirId};
  10 use rustc_lint::{LateContext, LateLintPass};
  11 use rustc_middle::hir::map::Map;
  12 use rustc_session::{declare_lint_pass, declare_tool_lint};
  13 use rustc_span::source_map::Span;
  14 use std::borrow::Cow;
  15
  16 declare_clippy_lint! {
  17     /// **What it does:** Checks for consecutive `if`s with the same condition.
  18     ///
  19     /// **Why is this bad?** This is probably a copy & paste error.
  20     ///
  21     /// **Known problems:** Hopefully none.
  22     ///
  23     /// **Example:**
  24     /// ```ignore
  25     /// if a == b {
  26     ///     …
  27     /// } else if a == b {
  28     ///     …
  29     /// }
  30     /// ```
  31     ///
  32     /// Note that this lint ignores all conditions with a function call as it could
  33     /// have side effects:
  34     ///
  35     /// ```ignore
  36     /// if foo() {
  37     ///     …
  38     /// } else if foo() { // not linted
  39     ///     …
  40     /// }
  41     /// ```
  42     pub IFS_SAME_COND,
  43     correctness,
  44     "consecutive `if`s with the same condition"
  45 }
  46
  47 declare_clippy_lint! {
  48     /// **What it does:** Checks for consecutive `if`s with the same function call.
  49     ///
  50     /// **Why is this bad?** This is probably a copy & paste error.
  51     /// Despite the fact that function can have side effects and `if` works as
  52     /// intended, such an approach is implicit and can be considered a "code smell".
  53     ///
  54     /// **Known problems:** Hopefully none.
  55     ///
  56     /// **Example:**
  57     /// ```ignore
  58     /// if foo() == bar {
  59     ///     …
  60     /// } else if foo() == bar {
  61     ///     …
  62     /// }
  63     /// ```
  64     ///
  65     /// This probably should be:
  66     /// ```ignore
  67     /// if foo() == bar {
  68     ///     …
  69     /// } else if foo() == baz {
  70     ///     …
  71     /// }
  72     /// ```
  73     ///
  74     /// or if the original code was not a typo and called function mutates a state,
  75     /// consider move the mutation out of the `if` condition to avoid similarity to
  76     /// a copy & paste error:
  77     ///
  78     /// ```ignore
  79     /// let first = foo();
  80     /// if first == bar {
  81     ///     …
  82     /// } else {
  83     ///     let second = foo();
  84     ///     if second == bar {
  85     ///     …
  86     ///     }
  87     /// }
  88     /// ```
  89     pub SAME_FUNCTIONS_IN_IF_CONDITION,
  90     pedantic,
  91     "consecutive `if`s with the same function call"
  92 }
  93
  94 declare_clippy_lint! {
  95     /// **What it does:** Checks for `if/else` with the same body as the *then* part
  96     /// and the *else* part.
  97     ///
  98     /// **Why is this bad?** This is probably a copy & paste error.
  99     ///
 100     /// **Known problems:** Hopefully none.
 101     ///
 102     /// **Example:**
 103     /// ```ignore
 104     /// let foo = if … {
 105     ///     42
 106     /// } else {
 107     ///     42
 108     /// };
 109     /// ```
 110     pub IF_SAME_THEN_ELSE,
 111     correctness,
 112     "`if` with the same `then` and `else` blocks"
 113 }
 114
 115 declare_clippy_lint! {
 116     /// **What it does:** Checks if the `if` and `else` block contain shared code that can be
 117     /// moved out of the blocks.
 118     ///
 119     /// **Why is this bad?** Duplicate code is less maintainable.
 120     ///
 121     /// **Known problems:** Hopefully none.
 122     ///
 123     /// **Example:**
 124     /// ```ignore
 125     /// let foo = if … {
 126     ///     println!("Hello World");
 127     ///     13
 128     /// } else {
 129     ///     println!("Hello World");
 130     ///     42
 131     /// };
 132     /// ```
 133     ///
 134     /// Could be written as:
 135     /// ```ignore
 136     /// println!("Hello World");
 137     /// let foo = if … {
 138     ///     13
 139     /// } else {
 140     ///     42
 141     /// };
 142     /// ```
 143     pub SHARED_CODE_IN_IF_BLOCKS,
 144     nursery,
 145     "`if` statement with shared code in all blocks"
 146 }
 147
 148 declare_lint_pass!(CopyAndPaste => [
 149     IFS_SAME_COND,
 150     SAME_FUNCTIONS_IN_IF_CONDITION,
 151     IF_SAME_THEN_ELSE,
 152     SHARED_CODE_IN_IF_BLOCKS
 153 ]);
 154
 155 impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
 156     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 157         if !expr.span.from_expansion() {
 158             // skip ifs directly in else, it will be checked in the parent if
 159             if let Some(&Expr {
 160                 kind: ExprKind::If(_, _, Some(ref else_expr)),
 161                 ..
 162             }) = get_parent_expr(cx, expr)
 163             {
 164                 if else_expr.hir_id == expr.hir_id {
 165                     return;
 166                 }
 167             }
 168
 169             let (conds, blocks) = if_sequence(expr);
 170             // Conditions
 171             lint_same_cond(cx, &conds);
 172             lint_same_fns_in_if_cond(cx, &conds);
 173             // Block duplication
 174             lint_same_then_else(cx, &blocks, conds.len() != blocks.len(), expr);
 175         }
 176     }
 177 }
 178
 179 /// Implementation of `SHARED_CODE_IN_IF_BLOCKS` and `IF_SAME_THEN_ELSE` if the blocks are equal.
 180 fn lint_same_then_else<'tcx>(
 181     cx: &LateContext<'tcx>,
 182     blocks: &[&Block<'tcx>],
 183     has_unconditional_else: bool,
 184     expr: &'tcx Expr<'_>,
 185 ) {
 186     // We only lint ifs with multiple blocks
 187     // TODO xFrednet 2021-01-01: Check if it's an else if block
 188     if blocks.len() < 2 || parent_node_is_if_expr(expr, cx) {
 189         return;
 190     }
 191
 192     let has_expr = blocks[0].expr.is_some();
 193
 194     // Check if each block has shared code
 195     let mut start_eq = usize::MAX;
 196     let mut end_eq = usize::MAX;
 197     let mut expr_eq = true;
 198     for win in blocks.windows(2) {
 199         let l_stmts = win[0].stmts;
 200         let r_stmts = win[1].stmts;
 201
 202         let mut evaluator = SpanlessEq::new(cx);
 203         let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
 204         let current_end_eq = count_eq(&mut l_stmts.iter().rev(), &mut r_stmts.iter().rev(), |l, r| {
 205             evaluator.eq_stmt(l, r)
 206         });
 207         let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
 208
 209         // IF_SAME_THEN_ELSE
 210         if block_expr_eq && l_stmts.len() == r_stmts.len() && l_stmts.len() == current_start_eq {
 211             span_lint_and_note(
 212                 cx,
 213                 IF_SAME_THEN_ELSE,
 214                 win[0].span,
 215                 "this `if` has identical blocks",
 216                 Some(win[1].span),
 217                 "same as this",
 218             );
 219
 220             return;
 221         } else {
 222             println!(
 223                 "{:?}\n - expr_eq: {:10}, l_stmts.len(): {:10}, r_stmts.len(): {:10}",
 224                 win[0].span,
 225                 block_expr_eq,
 226                 l_stmts.len(),
 227                 r_stmts.len()
 228             )
 229         }
 230
 231         start_eq = start_eq.min(current_start_eq);
 232         end_eq = end_eq.min(current_end_eq);
 233         expr_eq &= block_expr_eq;
 234     }
 235
 236     // SHARED_CODE_IN_IF_BLOCKS prerequisites
 237     if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
 238         return;
 239     }
 240
 241     if has_expr && !expr_eq {
 242         end_eq = 0;
 243     }
 244
 245     // Check if the regions are overlapping. Set `end_eq` to prevent the overlap
 246     let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
 247     if (start_eq + end_eq) > min_block_size {
 248         end_eq = min_block_size - start_eq;
 249     }
 250
 251     // Only the start is the same
 252     if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
 253         emit_shared_code_in_if_blocks_lint(cx, start_eq, 0, false, blocks, expr);
 254     } else if end_eq != 0 && (!has_expr || !expr_eq) {
 255         let block = blocks[blocks.len() - 1];
 256         let stmts = block.stmts.split_at(start_eq).1;
 257         let (block_stmts, moved_stmts) = stmts.split_at(stmts.len() - end_eq);
 258
 259         // Scan block
 260         let mut walker = SymbolFinderVisitor::new(cx);
 261         for stmt in block_stmts {
 262             intravisit::walk_stmt(&mut walker, stmt);
 263         }
 264         let mut block_defs = walker.defs;
 265
 266         // Scan moved stmts
 267         let mut moved_start: Option<usize> = None;
 268         let mut walker = SymbolFinderVisitor::new(cx);
 269         for (index, stmt) in moved_stmts.iter().enumerate() {
 270             intravisit::walk_stmt(&mut walker, stmt);
 271
 272             for value in &walker.uses {
 273                 // Well we can't move this and all prev statements. So reset
 274                 if block_defs.contains(&value) {
 275                     moved_start = Some(index + 1);
 276                     walker.defs.drain().for_each(|x| {
 277                         block_defs.insert(x);
 278                     });
 279                 }
 280             }
 281
 282             walker.uses.clear();
 283         }
 284
 285         if let Some(moved_start) = moved_start {
 286             end_eq -= moved_start;
 287         }
 288
 289         let end_linable = if let Some(expr) = block.expr {
 290             intravisit::walk_expr(&mut walker, expr);
 291             walker.uses.iter().any(|x| !block_defs.contains(x))
 292         } else if end_eq == 0 {
 293             false
 294         } else {
 295             true
 296         };
 297
 298         emit_shared_code_in_if_blocks_lint(cx, start_eq, end_eq, end_linable, blocks, expr);
 299     }
 300 }
 301
 302 fn emit_shared_code_in_if_blocks_lint(
 303     cx: &LateContext<'tcx>,
 304     start_stmts: usize,
 305     end_stmts: usize,
 306     lint_end: bool,
 307     blocks: &[&Block<'tcx>],
 308     if_expr: &'tcx Expr<'_>,
 309 ) {
 310     if start_stmts == 0 && !lint_end {
 311         return;
 312     }
 313
 314     // (help, span, suggestion)
 315     let mut suggestions: Vec<(&str, Span, String)> = vec![];
 316
 317     if start_stmts > 0 {
 318         let block = blocks[0];
 319         let span_start = first_line_of_span(cx, if_expr.span).shrink_to_lo();
 320         let span_end = block.stmts[start_stmts - 1].span.source_callsite();
 321
 322         let cond_span = first_line_of_span(cx, if_expr.span).until(block.span);
 323         let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
 324         let cond_indent = indent_of(cx, cond_span);
 325         let moved_span = block.stmts[0].span.source_callsite().to(span_end);
 326         let moved_snippet = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
 327         let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
 328         let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
 329
 330         let span = span_start.to(span_end);
 331         suggestions.push(("START HELP", span, suggestion.to_string()));
 332     }
 333
 334     if lint_end {
 335         let block = blocks[blocks.len() - 1];
 336         let span_end = block.span.shrink_to_hi();
 337
 338         let moved_start = if end_stmts == 0 && block.expr.is_some() {
 339             block.expr.unwrap().span
 340         } else {
 341             block.stmts[block.stmts.len() - end_stmts].span
 342         }
 343         .source_callsite();
 344         let moved_end = if let Some(expr) = block.expr {
 345             expr.span
 346         } else {
 347             block.stmts[block.stmts.len() - 1].span
 348         }
 349         .source_callsite();
 350
 351         let moved_span = moved_start.to(moved_end);
 352         let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
 353         let indent = indent_of(cx, if_expr.span.shrink_to_hi());
 354         let suggestion = "}\n".to_string() + &moved_snipped;
 355         let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
 356
 357         let span = moved_start.to(span_end);
 358         suggestions.push(("END_RANGE", span, suggestion.to_string()));
 359     }
 360
 361     if suggestions.len() == 1 {
 362         let (_, span, sugg) = &suggestions[0];
 363         span_lint_and_sugg(
 364             cx,
 365             SHARED_CODE_IN_IF_BLOCKS,
 366             *span,
 367             "All code blocks contain the same code",
 368             "Consider moving the statements out like this",
 369             sugg.clone(),
 370             Applicability::Unspecified,
 371         );
 372     } else {
 373         span_lint_and_then(
 374             cx,
 375             SHARED_CODE_IN_IF_BLOCKS,
 376             if_expr.span,
 377             "All if blocks contain the same code",
 378             move |diag| {
 379                 for (help, span, sugg) in suggestions {
 380                     diag.span_suggestion(span, help, sugg, Applicability::Unspecified);
 381                 }
 382             },
 383         );
 384     }
 385 }
 386
 387 pub struct SymbolFinderVisitor<'a, 'tcx> {
 388     cx: &'a LateContext<'tcx>,
 389     defs: FxHashSet<HirId>,
 390     uses: FxHashSet<HirId>,
 391 }
 392
 393 impl<'a, 'tcx> SymbolFinderVisitor<'a, 'tcx> {
 394     fn new(cx: &'a LateContext<'tcx>) -> Self {
 395         SymbolFinderVisitor {
 396             cx,
 397             defs: FxHashSet::default(),
 398             uses: FxHashSet::default(),
 399         }
 400     }
 401 }
 402
 403 impl<'a, 'tcx> Visitor<'tcx> for SymbolFinderVisitor<'a, 'tcx> {
 404     type Map = Map<'tcx>;
 405
 406     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 407         NestedVisitorMap::All(self.cx.tcx.hir())
 408     }
 409
 410     fn visit_local(&mut self, l: &'tcx rustc_hir::Local<'tcx>) {
 411         let local_id = l.pat.hir_id;
 412         self.defs.insert(local_id);
 413         if let Some(expr) = l.init {
 414             intravisit::walk_expr(self, expr);
 415         }
 416     }
 417
 418     fn visit_qpath(&mut self, qpath: &'tcx rustc_hir::QPath<'tcx>, id: HirId, _span: rustc_span::Span) {
 419         if let rustc_hir::QPath::Resolved(_, ref path) = *qpath {
 420             if path.segments.len() == 1 {
 421                 if let rustc_hir::def::Res::Local(var) = self.cx.qpath_res(qpath, id) {
 422                     self.uses.insert(var);
 423                 }
 424             }
 425         }
 426     }
 427 }
 428
 429 /// Implementation of `IFS_SAME_COND`.
 430 fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 431     let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
 432         let mut h = SpanlessHash::new(cx);
 433         h.hash_expr(expr);
 434         h.finish()
 435     };
 436
 437     let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };
 438
 439     for (i, j) in search_same(conds, hash, eq) {
 440         span_lint_and_note(
 441             cx,
 442             IFS_SAME_COND,
 443             j.span,
 444             "this `if` has the same condition as a previous `if`",
 445             Some(i.span),
 446             "same as this",
 447         );
 448     }
 449 }
 450
 451 /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
 452 fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 453     let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
 454         let mut h = SpanlessHash::new(cx);
 455         h.hash_expr(expr);
 456         h.finish()
 457     };
 458
 459     let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
 460         // Do not lint if any expr originates from a macro
 461         if in_macro(lhs.span) || in_macro(rhs.span) {
 462             return false;
 463         }
 464         // Do not spawn warning if `IFS_SAME_COND` already produced it.
 465         if eq_expr_value(cx, lhs, rhs) {
 466             return false;
 467         }
 468         SpanlessEq::new(cx).eq_expr(lhs, rhs)
 469     };
 470
 471     for (i, j) in search_same(conds, hash, eq) {
 472         span_lint_and_note(
 473             cx,
 474             SAME_FUNCTIONS_IN_IF_CONDITION,
 475             j.span,
 476             "this `if` has the same function call as a previous `if`",
 477             Some(i.span),
 478             "same as this",
 479         );
 480     }
 481 }