clippy_lints/src/copies.rs

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