clippy_lints/src/copies.rs

   1 use crate::utils::{
   2     both, count_eq, eq_expr_value, first_line_of_span, get_enclosing_block, get_parent_expr, if_sequence, in_macro,
   3     indent_of, parent_node_is_if_expr, reindent_multiline, run_lints, search_same, snippet, snippet_opt,
   4     span_lint_and_note, span_lint_and_then, ContainsName, SpanlessEq, SpanlessHash,
   5 };
   6 use if_chain::if_chain;
   7 use rustc_data_structures::fx::FxHashSet;
   8 use rustc_errors::{Applicability, DiagnosticBuilder};
   9 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
  10 use rustc_hir::{Block, Expr, ExprKind, HirId};
  11 use rustc_lint::{LateContext, LateLintPass};
  12 use rustc_middle::hir::map::Map;
  13 use rustc_session::{declare_lint_pass, declare_tool_lint};
  14 use rustc_span::{source_map::Span, symbol::Symbol, BytePos};
  15 use std::borrow::Cow;
  16
  17 declare_clippy_lint! {
  18     /// **What it does:** Checks for consecutive `if`s with the same condition.
  19     ///
  20     /// **Why is this bad?** This is probably a copy & paste error.
  21     ///
  22     /// **Known problems:** Hopefully none.
  23     ///
  24     /// **Example:**
  25     /// ```ignore
  26     /// if a == b {
  27     ///     …
  28     /// } else if a == b {
  29     ///     …
  30     /// }
  31     /// ```
  32     ///
  33     /// Note that this lint ignores all conditions with a function call as it could
  34     /// have side effects:
  35     ///
  36     /// ```ignore
  37     /// if foo() {
  38     ///     …
  39     /// } else if foo() { // not linted
  40     ///     …
  41     /// }
  42     /// ```
  43     pub IFS_SAME_COND,
  44     correctness,
  45     "consecutive `if`s with the same condition"
  46 }
  47
  48 declare_clippy_lint! {
  49     /// **What it does:** Checks for consecutive `if`s with the same function call.
  50     ///
  51     /// **Why is this bad?** This is probably a copy & paste error.
  52     /// Despite the fact that function can have side effects and `if` works as
  53     /// intended, such an approach is implicit and can be considered a "code smell".
  54     ///
  55     /// **Known problems:** Hopefully none.
  56     ///
  57     /// **Example:**
  58     /// ```ignore
  59     /// if foo() == bar {
  60     ///     …
  61     /// } else if foo() == bar {
  62     ///     …
  63     /// }
  64     /// ```
  65     ///
  66     /// This probably should be:
  67     /// ```ignore
  68     /// if foo() == bar {
  69     ///     …
  70     /// } else if foo() == baz {
  71     ///     …
  72     /// }
  73     /// ```
  74     ///
  75     /// or if the original code was not a typo and called function mutates a state,
  76     /// consider move the mutation out of the `if` condition to avoid similarity to
  77     /// a copy & paste error:
  78     ///
  79     /// ```ignore
  80     /// let first = foo();
  81     /// if first == bar {
  82     ///     …
  83     /// } else {
  84     ///     let second = foo();
  85     ///     if second == bar {
  86     ///     …
  87     ///     }
  88     /// }
  89     /// ```
  90     pub SAME_FUNCTIONS_IN_IF_CONDITION,
  91     pedantic,
  92     "consecutive `if`s with the same function call"
  93 }
  94
  95 declare_clippy_lint! {
  96     /// **What it does:** Checks for `if/else` with the same body as the *then* part
  97     /// and the *else* part.
  98     ///
  99     /// **Why is this bad?** This is probably a copy & paste error.
 100     ///
 101     /// **Known problems:** Hopefully none.
 102     ///
 103     /// **Example:**
 104     /// ```ignore
 105     /// let foo = if … {
 106     ///     42
 107     /// } else {
 108     ///     42
 109     /// };
 110     /// ```
 111     pub IF_SAME_THEN_ELSE,
 112     correctness,
 113     "`if` with the same `then` and `else` blocks"
 114 }
 115
 116 declare_clippy_lint! {
 117     /// **What it does:** Checks if the `if` and `else` block contain shared code that can be
 118     /// moved out of the blocks.
 119     ///
 120     /// **Why is this bad?** Duplicate code is less maintainable.
 121     ///
 122     /// **Known problems:** Hopefully none.
 123     ///
 124     /// **Example:**
 125     /// ```ignore
 126     /// let foo = if … {
 127     ///     println!("Hello World");
 128     ///     13
 129     /// } else {
 130     ///     println!("Hello World");
 131     ///     42
 132     /// };
 133     /// ```
 134     ///
 135     /// Could be written as:
 136     /// ```ignore
 137     /// println!("Hello World");
 138     /// let foo = if … {
 139     ///     13
 140     /// } else {
 141     ///     42
 142     /// };
 143     /// ```
 144     pub SHARED_CODE_IN_IF_BLOCKS,
 145     complexity,
 146     "`if` statement with shared code in all blocks"
 147 }
 148
 149 declare_lint_pass!(CopyAndPaste => [
 150     IFS_SAME_COND,
 151     SAME_FUNCTIONS_IN_IF_CONDITION,
 152     IF_SAME_THEN_ELSE,
 153     SHARED_CODE_IN_IF_BLOCKS
 154 ]);
 155
 156 impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
 157     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 158         if !expr.span.from_expansion() {
 159             // skip ifs directly in else, it will be checked in the parent if
 160             if let Some(&Expr {
 161                 kind: ExprKind::If(_, _, Some(ref else_expr)),
 162                 ..
 163             }) = get_parent_expr(cx, expr)
 164             {
 165                 if else_expr.hir_id == expr.hir_id {
 166                     return;
 167                 }
 168             }
 169
 170             let (conds, blocks) = if_sequence(expr);
 171             // Conditions
 172             lint_same_cond(cx, &conds);
 173             lint_same_fns_in_if_cond(cx, &conds);
 174             // Block duplication
 175             lint_same_then_else(cx, &blocks, conds.len() != blocks.len(), expr);
 176         }
 177     }
 178 }
 179
 180 /// Implementation of `SHARED_CODE_IN_IF_BLOCKS` and `IF_SAME_THEN_ELSE` if the blocks are equal.
 181 fn lint_same_then_else<'tcx>(
 182     cx: &LateContext<'tcx>,
 183     blocks: &[&Block<'tcx>],
 184     has_unconditional_else: bool,
 185     expr: &'tcx Expr<'_>,
 186 ) {
 187     // We only lint ifs with multiple blocks
 188     if blocks.len() < 2 || parent_node_is_if_expr(expr, cx) {
 189         return;
 190     }
 191
 192     // Check if each block has shared code
 193     let has_expr = blocks[0].expr.is_some();
 194     let (start_eq, mut end_eq, expr_eq) = scan_block_for_eq(cx, blocks);
 195
 196     // SHARED_CODE_IN_IF_BLOCKS prerequisites
 197     if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
 198         return;
 199     }
 200
 201     // Only the start is the same
 202     if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
 203         let block = blocks[0];
 204         let start_stmts = block.stmts.split_at(start_eq).0;
 205
 206         let mut start_walker = UsedValueFinderVisitor::new(cx);
 207         for stmt in start_stmts {
 208             intravisit::walk_stmt(&mut start_walker, stmt);
 209         }
 210
 211         emit_shared_code_in_if_blocks_lint(
 212             cx,
 213             start_eq,
 214             0,
 215             false,
 216             check_for_warn_of_moved_symbol(cx, &start_walker.def_symbols, expr),
 217             blocks,
 218             expr,
 219         );
 220     } else if end_eq != 0 || (has_expr && expr_eq) {
 221         let block = blocks[blocks.len() - 1];
 222         let (start_stmts, block_stmts) = block.stmts.split_at(start_eq);
 223         let (block_stmts, end_stmts) = block_stmts.split_at(block_stmts.len() - end_eq);
 224
 225         // Scan start
 226         let mut start_walker = UsedValueFinderVisitor::new(cx);
 227         for stmt in start_stmts {
 228             intravisit::walk_stmt(&mut start_walker, stmt);
 229         }
 230         let mut moved_syms = start_walker.def_symbols;
 231
 232         // Scan block
 233         let mut block_walker = UsedValueFinderVisitor::new(cx);
 234         for stmt in block_stmts {
 235             intravisit::walk_stmt(&mut block_walker, stmt);
 236         }
 237         let mut block_defs = block_walker.defs;
 238
 239         // Scan moved stmts
 240         let mut moved_start: Option<usize> = None;
 241         let mut end_walker = UsedValueFinderVisitor::new(cx);
 242         for (index, stmt) in end_stmts.iter().enumerate() {
 243             intravisit::walk_stmt(&mut end_walker, stmt);
 244
 245             for value in &end_walker.uses {
 246                 // Well we can't move this and all prev statements. So reset
 247                 if block_defs.contains(&value) {
 248                     moved_start = Some(index + 1);
 249                     end_walker.defs.drain().for_each(|x| {
 250                         block_defs.insert(x);
 251                     });
 252
 253                     end_walker.def_symbols.clear();
 254                 }
 255             }
 256
 257             end_walker.uses.clear();
 258         }
 259
 260         if let Some(moved_start) = moved_start {
 261             end_eq -= moved_start;
 262         }
 263
 264         let end_linable = block.expr.map_or_else(
 265             || end_eq != 0,
 266             |expr| {
 267                 intravisit::walk_expr(&mut end_walker, expr);
 268                 end_walker.uses.iter().any(|x| !block_defs.contains(x))
 269             },
 270         );
 271
 272         if end_linable {
 273             end_walker.def_symbols.drain().for_each(|x| {
 274                 moved_syms.insert(x);
 275             });
 276         }
 277
 278         emit_shared_code_in_if_blocks_lint(
 279             cx,
 280             start_eq,
 281             end_eq,
 282             end_linable,
 283             check_for_warn_of_moved_symbol(cx, &moved_syms, expr),
 284             blocks,
 285             expr,
 286         );
 287     }
 288 }
 289
 290 fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, usize, bool) {
 291     let mut start_eq = usize::MAX;
 292     let mut end_eq = usize::MAX;
 293     let mut expr_eq = true;
 294     for win in blocks.windows(2) {
 295         let l_stmts = win[0].stmts;
 296         let r_stmts = win[1].stmts;
 297
 298         // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
 299         // The comparison therefor needs to be done in a way that builds the correct context.
 300         let mut evaluator = SpanlessEq::new(cx);
 301         let mut evaluator = evaluator.inter_expr();
 302
 303         let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
 304
 305         let current_end_eq = {
 306             // We skip the middle statements which can't be equal
 307             let end_comparison_count = l_stmts.len().min(r_stmts.len()) - current_start_eq;
 308             let it1 = l_stmts.iter().skip(l_stmts.len() - end_comparison_count);
 309             let it2 = r_stmts.iter().skip(r_stmts.len() - end_comparison_count);
 310             it1.zip(it2)
 311                 .fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
 312         };
 313         let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
 314
 315         // IF_SAME_THEN_ELSE
 316         if_chain! {
 317             if block_expr_eq;
 318             if l_stmts.len() == r_stmts.len();
 319             if l_stmts.len() == current_start_eq;
 320             if run_lints(cx, &[IF_SAME_THEN_ELSE], win[0].hir_id);
 321             if run_lints(cx, &[IF_SAME_THEN_ELSE], win[1].hir_id);
 322             then {
 323                 span_lint_and_note(
 324                     cx,
 325                     IF_SAME_THEN_ELSE,
 326                     win[0].span,
 327                     "this `if` has identical blocks",
 328                     Some(win[1].span),
 329                     "same as this",
 330                 );
 331
 332                 return (0, 0, false);
 333             }
 334         }
 335
 336         start_eq = start_eq.min(current_start_eq);
 337         end_eq = end_eq.min(current_end_eq);
 338         expr_eq &= block_expr_eq;
 339     }
 340
 341     let has_expr = blocks[0].expr.is_some();
 342     if has_expr && !expr_eq {
 343         end_eq = 0;
 344     }
 345
 346     // Check if the regions are overlapping. Set `end_eq` to prevent the overlap
 347     let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
 348     if (start_eq + end_eq) > min_block_size {
 349         end_eq = min_block_size - start_eq;
 350     }
 351
 352     (start_eq, end_eq, expr_eq)
 353 }
 354
 355 fn check_for_warn_of_moved_symbol(
 356     cx: &LateContext<'tcx>,
 357     symbols: &FxHashSet<Symbol>,
 358     if_expr: &'tcx Expr<'_>,
 359 ) -> bool {
 360     get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
 361         let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
 362
 363         symbols
 364             .iter()
 365             .filter(|sym| !sym.as_str().starts_with('_'))
 366             .any(move |sym| {
 367                 let mut walker = ContainsName {
 368                     name: *sym,
 369                     result: false,
 370                 };
 371
 372                 // Scan block
 373                 block
 374                     .stmts
 375                     .iter()
 376                     .filter(|stmt| !ignore_span.overlaps(stmt.span))
 377                     .for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
 378
 379                 if let Some(expr) = block.expr {
 380                     intravisit::walk_expr(&mut walker, expr);
 381                 }
 382
 383                 walker.result
 384             })
 385     })
 386 }
 387
 388 fn emit_shared_code_in_if_blocks_lint(
 389     cx: &LateContext<'tcx>,
 390     start_stmts: usize,
 391     end_stmts: usize,
 392     lint_end: bool,
 393     warn_about_moved_symbol: bool,
 394     blocks: &[&Block<'tcx>],
 395     if_expr: &'tcx Expr<'_>,
 396 ) {
 397     if start_stmts == 0 && !lint_end {
 398         return;
 399     }
 400
 401     // (help, span, suggestion)
 402     let mut suggestions: Vec<(&str, Span, String)> = vec![];
 403     let mut add_expr_note = false;
 404
 405     // Construct suggestions
 406     if start_stmts > 0 {
 407         let block = blocks[0];
 408         let span_start = first_line_of_span(cx, if_expr.span).shrink_to_lo();
 409         let span_end = block.stmts[start_stmts - 1].span.source_callsite();
 410
 411         let cond_span = first_line_of_span(cx, if_expr.span).until(block.span);
 412         let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
 413         let cond_indent = indent_of(cx, cond_span);
 414         let moved_span = block.stmts[0].span.source_callsite().to(span_end);
 415         let moved_snippet = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
 416         let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
 417         let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
 418
 419         let span = span_start.to(span_end);
 420         suggestions.push(("start", span, suggestion.to_string()));
 421     }
 422
 423     if lint_end {
 424         let block = blocks[blocks.len() - 1];
 425         let span_end = block.span.shrink_to_hi();
 426
 427         let moved_start = if end_stmts == 0 && block.expr.is_some() {
 428             block.expr.unwrap().span
 429         } else {
 430             block.stmts[block.stmts.len() - end_stmts].span
 431         }
 432         .source_callsite();
 433         let moved_end = block
 434             .expr
 435             .map_or_else(|| block.stmts[block.stmts.len() - 1].span, |expr| expr.span)
 436             .source_callsite();
 437
 438         let moved_span = moved_start.to(moved_end);
 439         let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
 440         let indent = indent_of(cx, if_expr.span.shrink_to_hi());
 441         let suggestion = "}\n".to_string() + &moved_snipped;
 442         let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
 443
 444         let mut span = moved_start.to(span_end);
 445         // Improve formatting if the inner block has indention (i.e. normal Rust formatting)
 446         let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt());
 447         if snippet_opt(cx, test_span)
 448             .map(|snip| snip == "    ")
 449             .unwrap_or_default()
 450         {
 451             span = span.with_lo(test_span.lo());
 452         }
 453
 454         suggestions.push(("end", span, suggestion.to_string()));
 455         add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit()
 456     }
 457
 458     let add_optional_msgs = |diag: &mut DiagnosticBuilder<'_>| {
 459         if add_expr_note {
 460             diag.note("The end suggestion probably needs some adjustments to use the expression result correctly");
 461         }
 462
 463         if warn_about_moved_symbol {
 464             diag.warn("Some moved values might need to be renamed to avoid wrong references");
 465         }
 466     };
 467
 468     // Emit lint
 469     if suggestions.len() == 1 {
 470         let (place_str, span, sugg) = suggestions.pop().unwrap();
 471         let msg = format!("all if blocks contain the same code at the {}", place_str);
 472         let help = format!("consider moving the {} statements out like this", place_str);
 473         span_lint_and_then(cx, SHARED_CODE_IN_IF_BLOCKS, span, msg.as_str(), |diag| {
 474             diag.span_suggestion(span, help.as_str(), sugg, Applicability::Unspecified);
 475
 476             add_optional_msgs(diag);
 477         });
 478     } else if suggestions.len() == 2 {
 479         let (_, end_span, end_sugg) = suggestions.pop().unwrap();
 480         let (_, start_span, start_sugg) = suggestions.pop().unwrap();
 481         span_lint_and_then(
 482             cx,
 483             SHARED_CODE_IN_IF_BLOCKS,
 484             start_span,
 485             "all if blocks contain the same code at the start and the end. Here at the start",
 486             move |diag| {
 487                 diag.span_note(end_span, "and here at the end");
 488
 489                 diag.span_suggestion(
 490                     start_span,
 491                     "consider moving the start statements out like this",
 492                     start_sugg,
 493                     Applicability::Unspecified,
 494                 );
 495
 496                 diag.span_suggestion(
 497                     end_span,
 498                     "and consider moving the end statements out like this",
 499                     end_sugg,
 500                     Applicability::Unspecified,
 501                 );
 502
 503                 add_optional_msgs(diag);
 504             },
 505         );
 506     }
 507 }
 508
 509 /// This visitor collects `HirId`s and Symbols of defined symbols and `HirId`s of used values.
 510 struct UsedValueFinderVisitor<'a, 'tcx> {
 511     cx: &'a LateContext<'tcx>,
 512
 513     /// The `HirId`s of defined values in the scanned statements
 514     defs: FxHashSet<HirId>,
 515
 516     /// The Symbols of the defined symbols in the scanned statements
 517     def_symbols: FxHashSet<Symbol>,
 518
 519     /// The `HirId`s of the used values
 520     uses: FxHashSet<HirId>,
 521 }
 522
 523 impl<'a, 'tcx> UsedValueFinderVisitor<'a, 'tcx> {
 524     fn new(cx: &'a LateContext<'tcx>) -> Self {
 525         UsedValueFinderVisitor {
 526             cx,
 527             defs: FxHashSet::default(),
 528             def_symbols: FxHashSet::default(),
 529             uses: FxHashSet::default(),
 530         }
 531     }
 532 }
 533
 534 impl<'a, 'tcx> Visitor<'tcx> for UsedValueFinderVisitor<'a, 'tcx> {
 535     type Map = Map<'tcx>;
 536
 537     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 538         NestedVisitorMap::All(self.cx.tcx.hir())
 539     }
 540
 541     fn visit_local(&mut self, l: &'tcx rustc_hir::Local<'tcx>) {
 542         let local_id = l.pat.hir_id;
 543         self.defs.insert(local_id);
 544
 545         if let Some(sym) = l.pat.simple_ident() {
 546             self.def_symbols.insert(sym.name);
 547         }
 548
 549         if let Some(expr) = l.init {
 550             intravisit::walk_expr(self, expr);
 551         }
 552     }
 553
 554     fn visit_qpath(&mut self, qpath: &'tcx rustc_hir::QPath<'tcx>, id: HirId, _span: rustc_span::Span) {
 555         if let rustc_hir::QPath::Resolved(_, ref path) = *qpath {
 556             if path.segments.len() == 1 {
 557                 if let rustc_hir::def::Res::Local(var) = self.cx.qpath_res(qpath, id) {
 558                     self.uses.insert(var);
 559                 }
 560             }
 561         }
 562     }
 563 }
 564
 565 /// Implementation of `IFS_SAME_COND`.
 566 fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 567     let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
 568         let mut h = SpanlessHash::new(cx);
 569         h.hash_expr(expr);
 570         h.finish()
 571     };
 572
 573     let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };
 574
 575     for (i, j) in search_same(conds, hash, eq) {
 576         span_lint_and_note(
 577             cx,
 578             IFS_SAME_COND,
 579             j.span,
 580             "this `if` has the same condition as a previous `if`",
 581             Some(i.span),
 582             "same as this",
 583         );
 584     }
 585 }
 586
 587 /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
 588 fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 589     let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
 590         let mut h = SpanlessHash::new(cx);
 591         h.hash_expr(expr);
 592         h.finish()
 593     };
 594
 595     let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
 596         // Do not lint if any expr originates from a macro
 597         if in_macro(lhs.span) || in_macro(rhs.span) {
 598             return false;
 599         }
 600         // Do not spawn warning if `IFS_SAME_COND` already produced it.
 601         if eq_expr_value(cx, lhs, rhs) {
 602             return false;
 603         }
 604         SpanlessEq::new(cx).eq_expr(lhs, rhs)
 605     };
 606
 607     for (i, j) in search_same(conds, hash, eq) {
 608         span_lint_and_note(
 609             cx,
 610             SAME_FUNCTIONS_IN_IF_CONDITION,
 611             j.span,
 612             "this `if` has the same function call as a previous `if`",
 613             Some(i.span),
 614             "same as this",
 615         );
 616     }
 617 }