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};
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,
7 use if_chain::if_chain;
8 use rustc_data_structures::fx::FxHashSet;
9 use rustc_errors::{Applicability, Diagnostic};
10 use rustc_hir::intravisit::{self, Visitor};
11 use rustc_hir::{Block, Expr, ExprKind, HirId};
12 use rustc_lint::{LateContext, LateLintPass, LintContext};
13 use rustc_middle::hir::nested_filter;
14 use rustc_session::{declare_lint_pass, declare_tool_lint};
15 use rustc_span::{source_map::Span, symbol::Symbol, BytePos};
18 declare_clippy_lint! {
20 /// Checks for consecutive `if`s with the same condition.
22 /// ### Why is this bad?
23 /// This is probably a copy & paste error.
29 /// } else if a == b {
34 /// Note that this lint ignores all conditions with a function call as it could
35 /// have side effects:
40 /// } else if foo() { // not linted
44 #[clippy::version = "pre 1.29.0"]
47 "consecutive `if`s with the same condition"
50 declare_clippy_lint! {
52 /// Checks for consecutive `if`s with the same function call.
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".
63 /// } else if foo() == bar {
68 /// This probably should be:
72 /// } else if foo() == baz {
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:
82 /// let first = foo();
86 /// let second = foo();
87 /// if second == bar {
92 #[clippy::version = "1.41.0"]
93 pub SAME_FUNCTIONS_IN_IF_CONDITION,
95 "consecutive `if`s with the same function call"
98 declare_clippy_lint! {
100 /// Checks for `if/else` with the same body as the *then* part
101 /// and the *else* part.
103 /// ### Why is this bad?
104 /// This is probably a copy & paste error.
114 #[clippy::version = "pre 1.29.0"]
115 pub IF_SAME_THEN_ELSE,
117 "`if` with the same `then` and `else` blocks"
120 declare_clippy_lint! {
122 /// Checks if the `if` and `else` block contain shared code that can be
123 /// moved out of the blocks.
125 /// ### Why is this bad?
126 /// Duplicate code is less maintainable.
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)
136 /// println!("Hello World");
139 /// println!("Hello World");
144 /// Could be written as:
146 /// println!("Hello World");
153 #[clippy::version = "1.53.0"]
154 pub BRANCHES_SHARING_CODE,
156 "`if` statement with shared code in all blocks"
159 declare_lint_pass!(CopyAndPaste => [
161 SAME_FUNCTIONS_IN_IF_CONDITION,
163 BRANCHES_SHARING_CODE
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
172 kind: ExprKind::If(_, _, Some(else_expr)),
174 }) = get_parent_expr(cx, expr)
176 if else_expr.hir_id == expr.hir_id {
181 let (conds, blocks) = if_sequence(expr);
183 lint_same_cond(cx, &conds);
184 lint_same_fns_in_if_cond(cx, &conds);
186 lint_same_then_else(cx, &conds, &blocks, conds.len() == blocks.len(), expr);
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 conds: &[&'tcx Expr<'_>],
196 blocks: &[&Block<'tcx>],
197 has_conditional_else: bool,
198 expr: &'tcx Expr<'_>,
200 // We only lint ifs with multiple blocks
201 if blocks.len() < 2 || is_else_clause(cx.tcx, expr) {
205 // Check if each block has shared code
206 let has_expr = blocks[0].expr.is_some();
208 let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, conds, blocks) {
209 (block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq)
214 // BRANCHES_SHARING_CODE prerequisites
215 if has_conditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
219 // Only the start is the same
220 if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
221 let block = blocks[0];
222 let start_stmts = block.stmts.split_at(start_eq).0;
224 let mut start_walker = UsedValueFinderVisitor::new(cx);
225 for stmt in start_stmts {
226 intravisit::walk_stmt(&mut start_walker, stmt);
229 emit_branches_sharing_code_lint(
234 check_for_warn_of_moved_symbol(cx, &start_walker.def_symbols, expr),
238 } else if end_eq != 0 || (has_expr && expr_eq) {
239 let block = blocks[blocks.len() - 1];
240 let (start_stmts, block_stmts) = block.stmts.split_at(start_eq);
241 let (block_stmts, end_stmts) = block_stmts.split_at(block_stmts.len() - end_eq);
244 let mut start_walker = UsedValueFinderVisitor::new(cx);
245 for stmt in start_stmts {
246 intravisit::walk_stmt(&mut start_walker, stmt);
248 let mut moved_syms = start_walker.def_symbols;
251 let mut block_walker = UsedValueFinderVisitor::new(cx);
252 for stmt in block_stmts {
253 intravisit::walk_stmt(&mut block_walker, stmt);
255 let mut block_defs = block_walker.defs;
258 let mut moved_start: Option<usize> = None;
259 let mut end_walker = UsedValueFinderVisitor::new(cx);
260 for (index, stmt) in end_stmts.iter().enumerate() {
261 intravisit::walk_stmt(&mut end_walker, stmt);
263 for value in &end_walker.uses {
264 // Well we can't move this and all prev statements. So reset
265 if block_defs.contains(value) {
266 moved_start = Some(index + 1);
267 end_walker.defs.drain().for_each(|x| {
268 block_defs.insert(x);
271 end_walker.def_symbols.clear();
275 end_walker.uses.clear();
278 if let Some(moved_start) = moved_start {
279 end_eq -= moved_start;
282 let end_linable = block.expr.map_or_else(
285 intravisit::walk_expr(&mut end_walker, expr);
286 end_walker.uses.iter().any(|x| !block_defs.contains(x))
291 end_walker.def_symbols.drain().for_each(|x| {
292 moved_syms.insert(x);
296 emit_branches_sharing_code_lint(
301 check_for_warn_of_moved_symbol(cx, &moved_syms, expr),
309 /// The amount statements that are equal from the start
311 /// The amount statements that are equal from the end
313 /// An indication if the block expressions are the same. This will also be true if both are
318 /// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to
319 /// abort any further processing and avoid duplicate lint triggers.
320 fn scan_block_for_eq(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> Option<BlockEqual> {
321 let mut start_eq = usize::MAX;
322 let mut end_eq = usize::MAX;
323 let mut expr_eq = true;
324 let mut iter = blocks.windows(2).enumerate();
325 while let Some((i, &[block0, block1])) = iter.next() {
326 let l_stmts = block0.stmts;
327 let r_stmts = block1.stmts;
329 // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
330 // The comparison therefore needs to be done in a way that builds the correct context.
331 let mut evaluator = SpanlessEq::new(cx);
332 let mut evaluator = evaluator.inter_expr();
334 let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
336 let current_end_eq = {
337 // We skip the middle statements which can't be equal
338 let end_comparison_count = l_stmts.len().min(r_stmts.len()) - current_start_eq;
339 let it1 = l_stmts.iter().skip(l_stmts.len() - end_comparison_count);
340 let it2 = r_stmts.iter().skip(r_stmts.len() - end_comparison_count);
342 .fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
344 let block_expr_eq = both(&block0.expr, &block1.expr, |l, r| evaluator.eq_expr(l, r));
349 if l_stmts.len() == r_stmts.len();
350 if l_stmts.len() == current_start_eq;
351 // `conds` may have one last item than `blocks`.
352 // Any `i` from `blocks.windows(2)` will exist in `conds`, but `i+1` may not exist on the last iteration.
353 if !matches!(conds[i].kind, ExprKind::Let(..));
354 if !matches!(conds.get(i + 1).map(|e| &e.kind), Some(ExprKind::Let(..)));
355 if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block0.hir_id);
356 if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block1.hir_id);
362 "this `if` has identical blocks",
371 start_eq = start_eq.min(current_start_eq);
372 end_eq = end_eq.min(current_end_eq);
373 expr_eq &= block_expr_eq;
380 // Check if the regions are overlapping. Set `end_eq` to prevent the overlap
381 let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
382 if (start_eq + end_eq) > min_block_size {
383 end_eq = min_block_size - start_eq;
393 fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &FxHashSet<Symbol>, if_expr: &Expr<'_>) -> bool {
394 get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
395 let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
399 .filter(|sym| !sym.as_str().starts_with('_'))
401 let mut walker = ContainsName {
410 .filter(|stmt| !ignore_span.overlaps(stmt.span))
411 .for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
413 if let Some(expr) = block.expr {
414 intravisit::walk_expr(&mut walker, expr);
422 fn emit_branches_sharing_code_lint(
423 cx: &LateContext<'_>,
427 warn_about_moved_symbol: bool,
428 blocks: &[&Block<'_>],
431 if start_stmts == 0 && !lint_end {
435 // (help, span, suggestion)
436 let mut suggestions: Vec<(&str, Span, String)> = vec![];
437 let mut add_expr_note = false;
439 // Construct suggestions
440 let sm = cx.sess().source_map();
442 let block = blocks[0];
443 let span_start = first_line_of_span(cx, if_expr.span).shrink_to_lo();
444 let span_end = sm.stmt_span(block.stmts[start_stmts - 1].span, block.span);
446 let cond_span = first_line_of_span(cx, if_expr.span).until(block.span);
447 let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
448 let cond_indent = indent_of(cx, cond_span);
449 let moved_span = block.stmts[0].span.source_callsite().to(span_end);
450 let moved_snippet = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
451 let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
452 let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
454 let span = span_start.to(span_end);
455 suggestions.push(("start", span, suggestion.to_string()));
459 let block = blocks[blocks.len() - 1];
460 let span_end = block.span.shrink_to_hi();
462 let moved_start = if end_stmts == 0 && block.expr.is_some() {
463 block.expr.unwrap().span.source_callsite()
465 sm.stmt_span(block.stmts[block.stmts.len() - end_stmts].span, block.span)
467 let moved_end = block.expr.map_or_else(
468 || sm.stmt_span(block.stmts[block.stmts.len() - 1].span, block.span),
469 |expr| expr.span.source_callsite(),
472 let moved_span = moved_start.to(moved_end);
473 let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
474 let indent = indent_of(cx, if_expr.span.shrink_to_hi());
475 let suggestion = "}\n".to_string() + &moved_snipped;
476 let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
478 let mut span = moved_start.to(span_end);
479 // Improve formatting if the inner block has indention (i.e. normal Rust formatting)
480 let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent());
481 if snippet_opt(cx, test_span)
482 .map(|snip| snip == " ")
485 span = span.with_lo(test_span.lo());
488 suggestions.push(("end", span, suggestion.to_string()));
489 add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit();
492 let add_optional_msgs = |diag: &mut Diagnostic| {
494 diag.note("The end suggestion probably needs some adjustments to use the expression result correctly");
497 if warn_about_moved_symbol {
498 diag.warn("Some moved values might need to be renamed to avoid wrong references");
503 if suggestions.len() == 1 {
504 let (place_str, span, sugg) = suggestions.pop().unwrap();
505 let msg = format!("all if blocks contain the same code at the {}", place_str);
506 let help = format!("consider moving the {} statements out like this", place_str);
507 span_lint_and_then(cx, BRANCHES_SHARING_CODE, span, msg.as_str(), |diag| {
508 diag.span_suggestion(span, help.as_str(), sugg, Applicability::Unspecified);
510 add_optional_msgs(diag);
512 } else if suggestions.len() == 2 {
513 let (_, end_span, end_sugg) = suggestions.pop().unwrap();
514 let (_, start_span, start_sugg) = suggestions.pop().unwrap();
517 BRANCHES_SHARING_CODE,
519 "all if blocks contain the same code at the start and the end. Here at the start",
521 diag.span_note(end_span, "and here at the end");
523 diag.span_suggestion(
525 "consider moving the start statements out like this",
527 Applicability::Unspecified,
530 diag.span_suggestion(
532 "and consider moving the end statements out like this",
534 Applicability::Unspecified,
537 add_optional_msgs(diag);
543 /// This visitor collects `HirId`s and Symbols of defined symbols and `HirId`s of used values.
544 struct UsedValueFinderVisitor<'a, 'tcx> {
545 cx: &'a LateContext<'tcx>,
547 /// The `HirId`s of defined values in the scanned statements
548 defs: FxHashSet<HirId>,
550 /// The Symbols of the defined symbols in the scanned statements
551 def_symbols: FxHashSet<Symbol>,
553 /// The `HirId`s of the used values
554 uses: FxHashSet<HirId>,
557 impl<'a, 'tcx> UsedValueFinderVisitor<'a, 'tcx> {
558 fn new(cx: &'a LateContext<'tcx>) -> Self {
559 UsedValueFinderVisitor {
561 defs: FxHashSet::default(),
562 def_symbols: FxHashSet::default(),
563 uses: FxHashSet::default(),
568 impl<'a, 'tcx> Visitor<'tcx> for UsedValueFinderVisitor<'a, 'tcx> {
569 type NestedFilter = nested_filter::All;
571 fn nested_visit_map(&mut self) -> Self::Map {
575 fn visit_local(&mut self, l: &'tcx rustc_hir::Local<'tcx>) {
576 let local_id = l.pat.hir_id;
577 self.defs.insert(local_id);
579 if let Some(sym) = l.pat.simple_ident() {
580 self.def_symbols.insert(sym.name);
583 if let Some(expr) = l.init {
584 intravisit::walk_expr(self, expr);
588 fn visit_qpath(&mut self, qpath: &'tcx rustc_hir::QPath<'tcx>, id: HirId, _span: rustc_span::Span) {
589 if let rustc_hir::QPath::Resolved(_, path) = *qpath {
590 if path.segments.len() == 1 {
591 if let rustc_hir::def::Res::Local(var) = self.cx.qpath_res(qpath, id) {
592 self.uses.insert(var);
599 /// Implementation of `IFS_SAME_COND`.
600 fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
601 let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
602 let mut h = SpanlessHash::new(cx);
607 let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };
609 for (i, j) in search_same(conds, hash, eq) {
614 "this `if` has the same condition as a previous `if`",
621 /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
622 fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
623 let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
624 let mut h = SpanlessHash::new(cx);
629 let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
630 // Do not lint if any expr originates from a macro
631 if lhs.span.from_expansion() || rhs.span.from_expansion() {
634 // Do not spawn warning if `IFS_SAME_COND` already produced it.
635 if eq_expr_value(cx, lhs, rhs) {
638 SpanlessEq::new(cx).eq_expr(lhs, rhs)
641 for (i, j) in search_same(conds, hash, eq) {
644 SAME_FUNCTIONS_IN_IF_CONDITION,
646 "this `if` has the same function call as a previous `if`",