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