src/tools/clippy/clippy_lints/src/copies.rs

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