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