clippy_lints/src/copies.rs

   1 use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
   2 use clippy_utils::macros::macro_backtrace;
   3 use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
   4 use clippy_utils::{
   5     eq_expr_value, get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause, is_lint_allowed,
   6     search_same, ContainsName, HirEqInterExpr, SpanlessEq,
   7 };
   8 use core::iter;
   9 use rustc_errors::Applicability;
  10 use rustc_hir::intravisit;
  11 use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, QPath, Stmt, StmtKind};
  12 use rustc_lint::{LateContext, LateLintPass};
  13 use rustc_session::{declare_lint_pass, declare_tool_lint};
  14 use rustc_span::hygiene::walk_chain;
  15 use rustc_span::source_map::SourceMap;
  16 use rustc_span::{sym, BytePos, Span, Symbol};
  17 use std::borrow::Cow;
  18
  19 declare_clippy_lint! {
  20     /// ### What it does
  21     /// Checks for consecutive `if`s with the same condition.
  22     ///
  23     /// ### Why is this bad?
  24     /// This is probably a copy & paste error.
  25     ///
  26     /// ### Example
  27     /// ```ignore
  28     /// if a == b {
  29     ///     …
  30     /// } else if a == b {
  31     ///     …
  32     /// }
  33     /// ```
  34     ///
  35     /// Note that this lint ignores all conditions with a function call as it could
  36     /// have side effects:
  37     ///
  38     /// ```ignore
  39     /// if foo() {
  40     ///     …
  41     /// } else if foo() { // not linted
  42     ///     …
  43     /// }
  44     /// ```
  45     #[clippy::version = "pre 1.29.0"]
  46     pub IFS_SAME_COND,
  47     correctness,
  48     "consecutive `if`s with the same condition"
  49 }
  50
  51 declare_clippy_lint! {
  52     /// ### What it does
  53     /// Checks for consecutive `if`s with the same function call.
  54     ///
  55     /// ### Why is this bad?
  56     /// This is probably a copy & paste error.
  57     /// Despite the fact that function can have side effects and `if` works as
  58     /// intended, such an approach is implicit and can be considered a "code smell".
  59     ///
  60     /// ### Example
  61     /// ```ignore
  62     /// if foo() == bar {
  63     ///     …
  64     /// } else if foo() == bar {
  65     ///     …
  66     /// }
  67     /// ```
  68     ///
  69     /// This probably should be:
  70     /// ```ignore
  71     /// if foo() == bar {
  72     ///     …
  73     /// } else if foo() == baz {
  74     ///     …
  75     /// }
  76     /// ```
  77     ///
  78     /// or if the original code was not a typo and called function mutates a state,
  79     /// consider move the mutation out of the `if` condition to avoid similarity to
  80     /// a copy & paste error:
  81     ///
  82     /// ```ignore
  83     /// let first = foo();
  84     /// if first == bar {
  85     ///     …
  86     /// } else {
  87     ///     let second = foo();
  88     ///     if second == bar {
  89     ///     …
  90     ///     }
  91     /// }
  92     /// ```
  93     #[clippy::version = "1.41.0"]
  94     pub SAME_FUNCTIONS_IN_IF_CONDITION,
  95     pedantic,
  96     "consecutive `if`s with the same function call"
  97 }
  98
  99 declare_clippy_lint! {
 100     /// ### What it does
 101     /// Checks for `if/else` with the same body as the *then* part
 102     /// and the *else* part.
 103     ///
 104     /// ### Why is this bad?
 105     /// This is probably a copy & paste error.
 106     ///
 107     /// ### Example
 108     /// ```ignore
 109     /// let foo = if … {
 110     ///     42
 111     /// } else {
 112     ///     42
 113     /// };
 114     /// ```
 115     #[clippy::version = "pre 1.29.0"]
 116     pub IF_SAME_THEN_ELSE,
 117     correctness,
 118     "`if` with the same `then` and `else` blocks"
 119 }
 120
 121 declare_clippy_lint! {
 122     /// ### What it does
 123     /// Checks if the `if` and `else` block contain shared code that can be
 124     /// moved out of the blocks.
 125     ///
 126     /// ### Why is this bad?
 127     /// Duplicate code is less maintainable.
 128     ///
 129     /// ### Known problems
 130     /// * The lint doesn't check if the moved expressions modify values that are being used in
 131     ///   the if condition. The suggestion can in that case modify the behavior of the program.
 132     ///   See [rust-clippy#7452](https://github.com/rust-lang/rust-clippy/issues/7452)
 133     ///
 134     /// ### Example
 135     /// ```ignore
 136     /// let foo = if … {
 137     ///     println!("Hello World");
 138     ///     13
 139     /// } else {
 140     ///     println!("Hello World");
 141     ///     42
 142     /// };
 143     /// ```
 144     ///
 145     /// Use instead:
 146     /// ```ignore
 147     /// println!("Hello World");
 148     /// let foo = if … {
 149     ///     13
 150     /// } else {
 151     ///     42
 152     /// };
 153     /// ```
 154     #[clippy::version = "1.53.0"]
 155     pub BRANCHES_SHARING_CODE,
 156     nursery,
 157     "`if` statement with shared code in all blocks"
 158 }
 159
 160 declare_lint_pass!(CopyAndPaste => [
 161     IFS_SAME_COND,
 162     SAME_FUNCTIONS_IN_IF_CONDITION,
 163     IF_SAME_THEN_ELSE,
 164     BRANCHES_SHARING_CODE
 165 ]);
 166
 167 impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
 168     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 169         if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
 170             let (conds, blocks) = if_sequence(expr);
 171             lint_same_cond(cx, &conds);
 172             lint_same_fns_in_if_cond(cx, &conds);
 173             let all_same =
 174                 !is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks);
 175             if !all_same && conds.len() != blocks.len() {
 176                 lint_branches_sharing_code(cx, &conds, &blocks, expr);
 177             }
 178         }
 179     }
 180 }
 181
 182 /// Checks if the given expression is a let chain.
 183 fn contains_let(e: &Expr<'_>) -> bool {
 184     match e.kind {
 185         ExprKind::Let(..) => true,
 186         ExprKind::Binary(op, lhs, rhs) if op.node == BinOpKind::And => {
 187             matches!(lhs.kind, ExprKind::Let(..)) || contains_let(rhs)
 188         },
 189         _ => false,
 190     }
 191 }
 192
 193 fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool {
 194     let mut eq = SpanlessEq::new(cx);
 195     blocks
 196         .array_windows::<2>()
 197         .enumerate()
 198         .fold(true, |all_eq, (i, &[lhs, rhs])| {
 199             if eq.eq_block(lhs, rhs)
 200                 && !contains_acceptable_macro(cx, lhs)
 201                 && !contains_acceptable_macro(cx, rhs)
 202                 && !contains_let(conds[i])
 203                 && conds.get(i + 1).map_or(true, |e| !contains_let(e))
 204             {
 205                 span_lint_and_note(
 206                     cx,
 207                     IF_SAME_THEN_ELSE,
 208                     lhs.span,
 209                     "this `if` has identical blocks",
 210                     Some(rhs.span),
 211                     "same as this",
 212                 );
 213                 all_eq
 214             } else {
 215                 false
 216             }
 217         })
 218 }
 219
 220 fn lint_branches_sharing_code<'tcx>(
 221     cx: &LateContext<'tcx>,
 222     conds: &[&'tcx Expr<'_>],
 223     blocks: &[&Block<'tcx>],
 224     expr: &'tcx Expr<'_>,
 225 ) {
 226     // We only lint ifs with multiple blocks
 227     let &[first_block, ref blocks @ ..] = blocks else {
 228         return;
 229     };
 230     let &[.., last_block] = blocks else {
 231         return;
 232     };
 233
 234     let res = scan_block_for_eq(cx, conds, first_block, blocks);
 235     let sm = cx.tcx.sess.source_map();
 236     let start_suggestion = res.start_span(first_block, sm).map(|span| {
 237         let first_line_span = first_line_of_span(cx, expr.span);
 238         let replace_span = first_line_span.with_hi(span.hi());
 239         let cond_span = first_line_span.until(first_block.span);
 240         let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
 241         let cond_indent = indent_of(cx, cond_span);
 242         let moved_snippet = reindent_multiline(snippet(cx, span, "_"), true, None);
 243         let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
 244         let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
 245         (replace_span, suggestion.to_string())
 246     });
 247     let end_suggestion = res.end_span(last_block, sm).map(|span| {
 248         let moved_snipped = reindent_multiline(snippet(cx, span, "_"), true, None);
 249         let indent = indent_of(cx, expr.span.shrink_to_hi());
 250         let suggestion = "}\n".to_string() + &moved_snipped;
 251         let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
 252
 253         let span = span.with_hi(last_block.span.hi());
 254         // Improve formatting if the inner block has indention (i.e. normal Rust formatting)
 255         let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent());
 256         let span = if snippet_opt(cx, test_span).map_or(false, |snip| snip == "    ") {
 257             span.with_lo(test_span.lo())
 258         } else {
 259             span
 260         };
 261         (span, suggestion.to_string())
 262     });
 263
 264     let (span, msg, end_span) = match (&start_suggestion, &end_suggestion) {
 265         (&Some((span, _)), &Some((end_span, _))) => (
 266             span,
 267             "all if blocks contain the same code at both the start and the end",
 268             Some(end_span),
 269         ),
 270         (&Some((span, _)), None) => (span, "all if blocks contain the same code at the start", None),
 271         (None, &Some((span, _))) => (span, "all if blocks contain the same code at the end", None),
 272         (None, None) => return,
 273     };
 274     span_lint_and_then(cx, BRANCHES_SHARING_CODE, span, msg, |diag| {
 275         if let Some(span) = end_span {
 276             diag.span_note(span, "this code is shared at the end");
 277         }
 278         if let Some((span, sugg)) = start_suggestion {
 279             diag.span_suggestion(
 280                 span,
 281                 "consider moving these statements before the if",
 282                 sugg,
 283                 Applicability::Unspecified,
 284             );
 285         }
 286         if let Some((span, sugg)) = end_suggestion {
 287             diag.span_suggestion(
 288                 span,
 289                 "consider moving these statements after the if",
 290                 sugg,
 291                 Applicability::Unspecified,
 292             );
 293             if !cx.typeck_results().expr_ty(expr).is_unit() {
 294                 diag.note("the end suggestion probably needs some adjustments to use the expression result correctly");
 295             }
 296         }
 297         if check_for_warn_of_moved_symbol(cx, &res.moved_locals, expr) {
 298             diag.warn("some moved values might need to be renamed to avoid wrong references");
 299         }
 300     });
 301 }
 302
 303 struct BlockEq {
 304     /// The end of the range of equal stmts at the start.
 305     start_end_eq: usize,
 306     /// The start of the range of equal stmts at the end.
 307     end_begin_eq: Option<usize>,
 308     /// The name and id of every local which can be moved at the beginning and the end.
 309     moved_locals: Vec<(HirId, Symbol)>,
 310 }
 311 impl BlockEq {
 312     fn start_span(&self, b: &Block<'_>, sm: &SourceMap) -> Option<Span> {
 313         match &b.stmts[..self.start_end_eq] {
 314             [first, .., last] => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))),
 315             [s] => Some(sm.stmt_span(s.span, b.span)),
 316             [] => None,
 317         }
 318     }
 319
 320     fn end_span(&self, b: &Block<'_>, sm: &SourceMap) -> Option<Span> {
 321         match (&b.stmts[b.stmts.len() - self.end_begin_eq?..], b.expr) {
 322             ([first, .., last], None) => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))),
 323             ([first, ..], Some(last)) => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))),
 324             ([s], None) => Some(sm.stmt_span(s.span, b.span)),
 325             ([], Some(e)) => Some(walk_chain(e.span, b.span.ctxt())),
 326             ([], None) => None,
 327         }
 328     }
 329 }
 330
 331 /// If the statement is a local, checks if the bound names match the expected list of names.
 332 fn eq_binding_names(s: &Stmt<'_>, names: &[(HirId, Symbol)]) -> bool {
 333     if let StmtKind::Local(l) = s.kind {
 334         let mut i = 0usize;
 335         let mut res = true;
 336         l.pat.each_binding_or_first(&mut |_, _, _, name| {
 337             if names.get(i).map_or(false, |&(_, n)| n == name.name) {
 338                 i += 1;
 339             } else {
 340                 res = false;
 341             }
 342         });
 343         res && i == names.len()
 344     } else {
 345         false
 346     }
 347 }
 348
 349 /// Checks if the given statement should be considered equal to the statement in the same position
 350 /// for each block.
 351 fn eq_stmts(
 352     stmt: &Stmt<'_>,
 353     blocks: &[&Block<'_>],
 354     get_stmt: impl for<'a> Fn(&'a Block<'a>) -> Option<&'a Stmt<'a>>,
 355     eq: &mut HirEqInterExpr<'_, '_, '_>,
 356     moved_bindings: &mut Vec<(HirId, Symbol)>,
 357 ) -> bool {
 358     (if let StmtKind::Local(l) = stmt.kind {
 359         let old_count = moved_bindings.len();
 360         l.pat.each_binding_or_first(&mut |_, id, _, name| {
 361             moved_bindings.push((id, name.name));
 362         });
 363         let new_bindings = &moved_bindings[old_count..];
 364         blocks
 365             .iter()
 366             .all(|b| get_stmt(b).map_or(false, |s| eq_binding_names(s, new_bindings)))
 367     } else {
 368         true
 369     }) && blocks
 370         .iter()
 371         .all(|b| get_stmt(b).map_or(false, |s| eq.eq_stmt(s, stmt)))
 372 }
 373
 374 fn contains_acceptable_macro(cx: &LateContext<'_>, block: &Block<'_>) -> bool {
 375     for stmt in block.stmts {
 376         match stmt.kind {
 377             StmtKind::Semi(semi_expr) if acceptable_macro(cx, semi_expr) => return true,
 378             _ => {},
 379         }
 380     }
 381
 382     if let Some(block_expr) = block.expr
 383         && acceptable_macro(cx, block_expr)
 384     {
 385         return true
 386     }
 387
 388     false
 389 }
 390
 391 fn acceptable_macro(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 392     if let ExprKind::Call(call_expr, _)  = expr.kind
 393         && let ExprKind::Path(QPath::Resolved(None, path)) = call_expr.kind
 394         && macro_backtrace(path.span).any(|macro_call| {
 395             matches!(
 396                 &cx.tcx.get_diagnostic_name(macro_call.def_id),
 397                 Some(sym::todo_macro | sym::unimplemented_macro)
 398             )
 399     }) {
 400         return true;
 401     }
 402
 403     false
 404 }
 405
 406 fn scan_block_for_eq(cx: &LateContext<'_>, _conds: &[&Expr<'_>], block: &Block<'_>, blocks: &[&Block<'_>]) -> BlockEq {
 407     let mut eq = SpanlessEq::new(cx);
 408     let mut eq = eq.inter_expr();
 409     let mut moved_locals = Vec::new();
 410
 411     let start_end_eq = block
 412         .stmts
 413         .iter()
 414         .enumerate()
 415         .find(|&(i, stmt)| !eq_stmts(stmt, blocks, |b| b.stmts.get(i), &mut eq, &mut moved_locals))
 416         .map_or(block.stmts.len(), |(i, _)| i);
 417
 418     // Walk backwards through the final expression/statements so long as their hashes are equal. Note
 419     // `SpanlessHash` treats all local references as equal allowing locals declared earlier in the block
 420     // to match those in other blocks. e.g. If each block ends with the following the hash value will be
 421     // the same even though each `x` binding will have a different `HirId`:
 422     //     let x = foo();
 423     //     x + 50
 424     let expr_hash_eq = if let Some(e) = block.expr {
 425         let hash = hash_expr(cx, e);
 426         blocks
 427             .iter()
 428             .all(|b| b.expr.map_or(false, |e| hash_expr(cx, e) == hash))
 429     } else {
 430         blocks.iter().all(|b| b.expr.is_none())
 431     };
 432     if !expr_hash_eq {
 433         return BlockEq {
 434             start_end_eq,
 435             end_begin_eq: None,
 436             moved_locals,
 437         };
 438     }
 439     let end_search_start = block.stmts[start_end_eq..]
 440         .iter()
 441         .rev()
 442         .enumerate()
 443         .find(|&(offset, stmt)| {
 444             let hash = hash_stmt(cx, stmt);
 445             blocks.iter().any(|b| {
 446                 b.stmts
 447                     // the bounds check will catch the underflow
 448                     .get(b.stmts.len().wrapping_sub(offset + 1))
 449                     .map_or(true, |s| hash != hash_stmt(cx, s))
 450             })
 451         })
 452         .map_or(block.stmts.len() - start_end_eq, |(i, _)| i);
 453
 454     let moved_locals_at_start = moved_locals.len();
 455     let mut i = end_search_start;
 456     let end_begin_eq = block.stmts[block.stmts.len() - end_search_start..]
 457         .iter()
 458         .zip(iter::repeat_with(move || {
 459             let x = i;
 460             i -= 1;
 461             x
 462         }))
 463         .fold(end_search_start, |init, (stmt, offset)| {
 464             if eq_stmts(
 465                 stmt,
 466                 blocks,
 467                 |b| b.stmts.get(b.stmts.len() - offset),
 468                 &mut eq,
 469                 &mut moved_locals,
 470             ) {
 471                 init
 472             } else {
 473                 // Clear out all locals seen at the end so far. None of them can be moved.
 474                 let stmts = &blocks[0].stmts;
 475                 for stmt in &stmts[stmts.len() - init..=stmts.len() - offset] {
 476                     if let StmtKind::Local(l) = stmt.kind {
 477                         l.pat.each_binding_or_first(&mut |_, id, _, _| {
 478                             eq.locals.remove(&id);
 479                         });
 480                     }
 481                 }
 482                 moved_locals.truncate(moved_locals_at_start);
 483                 offset - 1
 484             }
 485         });
 486     if let Some(e) = block.expr {
 487         for block in blocks {
 488             if block.expr.map_or(false, |expr| !eq.eq_expr(expr, e)) {
 489                 moved_locals.truncate(moved_locals_at_start);
 490                 return BlockEq {
 491                     start_end_eq,
 492                     end_begin_eq: None,
 493                     moved_locals,
 494                 };
 495             }
 496         }
 497     }
 498
 499     BlockEq {
 500         start_end_eq,
 501         end_begin_eq: Some(end_begin_eq),
 502         moved_locals,
 503     }
 504 }
 505
 506 fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbol)], if_expr: &Expr<'_>) -> bool {
 507     get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
 508         let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
 509
 510         symbols
 511             .iter()
 512             .filter(|&&(_, name)| !name.as_str().starts_with('_'))
 513             .any(|&(_, name)| {
 514                 let mut walker = ContainsName { name, result: false };
 515
 516                 // Scan block
 517                 block
 518                     .stmts
 519                     .iter()
 520                     .filter(|stmt| !ignore_span.overlaps(stmt.span))
 521                     .for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
 522
 523                 if let Some(expr) = block.expr {
 524                     intravisit::walk_expr(&mut walker, expr);
 525                 }
 526
 527                 walker.result
 528             })
 529     })
 530 }
 531
 532 /// Implementation of `IFS_SAME_COND`.
 533 fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 534     for (i, j) in search_same(conds, |e| hash_expr(cx, e), |lhs, rhs| eq_expr_value(cx, lhs, rhs)) {
 535         span_lint_and_note(
 536             cx,
 537             IFS_SAME_COND,
 538             j.span,
 539             "this `if` has the same condition as a previous `if`",
 540             Some(i.span),
 541             "same as this",
 542         );
 543     }
 544 }
 545
 546 /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
 547 fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 548     let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
 549         // Do not lint if any expr originates from a macro
 550         if lhs.span.from_expansion() || rhs.span.from_expansion() {
 551             return false;
 552         }
 553         // Do not spawn warning if `IFS_SAME_COND` already produced it.
 554         if eq_expr_value(cx, lhs, rhs) {
 555             return false;
 556         }
 557         SpanlessEq::new(cx).eq_expr(lhs, rhs)
 558     };
 559
 560     for (i, j) in search_same(conds, |e| hash_expr(cx, e), eq) {
 561         span_lint_and_note(
 562             cx,
 563             SAME_FUNCTIONS_IN_IF_CONDITION,
 564             j.span,
 565             "this `if` has the same function call as a previous `if`",
 566             Some(i.span),
 567             "same as this",
 568         );
 569     }
 570 }