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