2 both, count_eq, eq_expr_value, first_line_of_span, get_enclosing_block, get_parent_expr, if_sequence, in_macro,
3 indent_of, parent_node_is_if_expr, reindent_multiline, run_lints, search_same, snippet, snippet_opt,
4 span_lint_and_note, span_lint_and_then, ContainsName, SpanlessEq, SpanlessHash,
6 use if_chain::if_chain;
7 use rustc_data_structures::fx::FxHashSet;
8 use rustc_errors::{Applicability, DiagnosticBuilder};
9 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
10 use rustc_hir::{Block, Expr, ExprKind, HirId};
11 use rustc_lint::{LateContext, LateLintPass};
12 use rustc_middle::hir::map::Map;
13 use rustc_session::{declare_lint_pass, declare_tool_lint};
14 use rustc_span::{source_map::Span, symbol::Symbol, BytePos};
17 declare_clippy_lint! {
18 /// **What it does:** Checks for consecutive `if`s with the same condition.
20 /// **Why is this bad?** This is probably a copy & paste error.
22 /// **Known problems:** Hopefully none.
28 /// } else if a == b {
33 /// Note that this lint ignores all conditions with a function call as it could
34 /// have side effects:
39 /// } else if foo() { // not linted
45 "consecutive `if`s with the same condition"
48 declare_clippy_lint! {
49 /// **What it does:** Checks for consecutive `if`s with the same function call.
51 /// **Why is this bad?** This is probably a copy & paste error.
52 /// Despite the fact that function can have side effects and `if` works as
53 /// intended, such an approach is implicit and can be considered a "code smell".
55 /// **Known problems:** Hopefully none.
61 /// } else if foo() == bar {
66 /// This probably should be:
70 /// } else if foo() == baz {
75 /// or if the original code was not a typo and called function mutates a state,
76 /// consider move the mutation out of the `if` condition to avoid similarity to
77 /// a copy & paste error:
80 /// let first = foo();
84 /// let second = foo();
85 /// if second == bar {
90 pub SAME_FUNCTIONS_IN_IF_CONDITION,
92 "consecutive `if`s with the same function call"
95 declare_clippy_lint! {
96 /// **What it does:** Checks for `if/else` with the same body as the *then* part
97 /// and the *else* part.
99 /// **Why is this bad?** This is probably a copy & paste error.
101 /// **Known problems:** Hopefully none.
111 pub IF_SAME_THEN_ELSE,
113 "`if` with the same `then` and `else` blocks"
116 declare_clippy_lint! {
117 /// **What it does:** Checks if the `if` and `else` block contain shared code that can be
118 /// moved out of the blocks.
120 /// **Why is this bad?** Duplicate code is less maintainable.
122 /// **Known problems:** Hopefully none.
127 /// println!("Hello World");
130 /// println!("Hello World");
135 /// Could be written as:
137 /// println!("Hello World");
144 pub SHARED_CODE_IN_IF_BLOCKS,
146 "`if` statement with shared code in all blocks"
149 declare_lint_pass!(CopyAndPaste => [
151 SAME_FUNCTIONS_IN_IF_CONDITION,
153 SHARED_CODE_IN_IF_BLOCKS
156 impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
157 fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
158 if !expr.span.from_expansion() {
159 // skip ifs directly in else, it will be checked in the parent if
161 kind: ExprKind::If(_, _, Some(ref else_expr)),
163 }) = get_parent_expr(cx, expr)
165 if else_expr.hir_id == expr.hir_id {
170 let (conds, blocks) = if_sequence(expr);
172 lint_same_cond(cx, &conds);
173 lint_same_fns_in_if_cond(cx, &conds);
175 lint_same_then_else(cx, &blocks, conds.len() != blocks.len(), expr);
180 /// Implementation of `SHARED_CODE_IN_IF_BLOCKS` and `IF_SAME_THEN_ELSE` if the blocks are equal.
181 fn lint_same_then_else<'tcx>(
182 cx: &LateContext<'tcx>,
183 blocks: &[&Block<'tcx>],
184 has_unconditional_else: bool,
185 expr: &'tcx Expr<'_>,
187 // We only lint ifs with multiple blocks
188 if blocks.len() < 2 || parent_node_is_if_expr(expr, cx) {
192 // Check if each block has shared code
193 let has_expr = blocks[0].expr.is_some();
194 let (start_eq, mut end_eq, expr_eq) = scan_block_for_eq(cx, blocks);
196 // SHARED_CODE_IN_IF_BLOCKS prerequisites
197 if !has_unconditional_else || (start_eq == 0 && end_eq == 0 && (has_expr && !expr_eq)) {
201 // Only the start is the same
202 if start_eq != 0 && end_eq == 0 && (!has_expr || !expr_eq) {
203 let block = blocks[0];
204 let start_stmts = block.stmts.split_at(start_eq).0;
206 let mut start_walker = UsedValueFinderVisitor::new(cx);
207 for stmt in start_stmts {
208 intravisit::walk_stmt(&mut start_walker, stmt);
211 emit_shared_code_in_if_blocks_lint(
216 check_for_warn_of_moved_symbol(cx, &start_walker.def_symbols, expr),
220 } else if end_eq != 0 || (has_expr && expr_eq) {
221 let block = blocks[blocks.len() - 1];
222 let (start_stmts, block_stmts) = block.stmts.split_at(start_eq);
223 let (block_stmts, end_stmts) = block_stmts.split_at(block_stmts.len() - end_eq);
226 let mut start_walker = UsedValueFinderVisitor::new(cx);
227 for stmt in start_stmts {
228 intravisit::walk_stmt(&mut start_walker, stmt);
230 let mut moved_syms = start_walker.def_symbols;
233 let mut block_walker = UsedValueFinderVisitor::new(cx);
234 for stmt in block_stmts {
235 intravisit::walk_stmt(&mut block_walker, stmt);
237 let mut block_defs = block_walker.defs;
240 let mut moved_start: Option<usize> = None;
241 let mut end_walker = UsedValueFinderVisitor::new(cx);
242 for (index, stmt) in end_stmts.iter().enumerate() {
243 intravisit::walk_stmt(&mut end_walker, stmt);
245 for value in &end_walker.uses {
246 // Well we can't move this and all prev statements. So reset
247 if block_defs.contains(&value) {
248 moved_start = Some(index + 1);
249 end_walker.defs.drain().for_each(|x| {
250 block_defs.insert(x);
253 end_walker.def_symbols.clear();
257 end_walker.uses.clear();
260 if let Some(moved_start) = moved_start {
261 end_eq -= moved_start;
264 let end_linable = block.expr.map_or_else(
267 intravisit::walk_expr(&mut end_walker, expr);
268 end_walker.uses.iter().any(|x| !block_defs.contains(x))
273 end_walker.def_symbols.drain().for_each(|x| {
274 moved_syms.insert(x);
278 emit_shared_code_in_if_blocks_lint(
283 check_for_warn_of_moved_symbol(cx, &moved_syms, expr),
290 fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> (usize, usize, bool) {
291 let mut start_eq = usize::MAX;
292 let mut end_eq = usize::MAX;
293 let mut expr_eq = true;
294 for win in blocks.windows(2) {
295 let l_stmts = win[0].stmts;
296 let r_stmts = win[1].stmts;
298 let mut evaluator = SpanlessEq::new(cx);
299 let current_start_eq = count_eq(&mut l_stmts.iter(), &mut r_stmts.iter(), |l, r| evaluator.eq_stmt(l, r));
300 let current_end_eq = count_eq(&mut l_stmts.iter().rev(), &mut r_stmts.iter().rev(), |l, r| {
301 evaluator.eq_stmt(l, r)
303 let block_expr_eq = both(&win[0].expr, &win[1].expr, |l, r| evaluator.eq_expr(l, r));
308 if l_stmts.len() == r_stmts.len();
309 if l_stmts.len() == current_start_eq;
310 if run_lints(cx, &[IF_SAME_THEN_ELSE], win[0].hir_id);
311 if run_lints(cx, &[IF_SAME_THEN_ELSE], win[1].hir_id);
317 "this `if` has identical blocks",
322 return (0, 0, false);
326 start_eq = start_eq.min(current_start_eq);
327 end_eq = end_eq.min(current_end_eq);
328 expr_eq &= block_expr_eq;
331 let has_expr = blocks[0].expr.is_some();
332 if has_expr && !expr_eq {
336 // Check if the regions are overlapping. Set `end_eq` to prevent the overlap
337 let min_block_size = blocks.iter().map(|x| x.stmts.len()).min().unwrap();
338 if (start_eq + end_eq) > min_block_size {
339 end_eq = min_block_size - start_eq;
342 (start_eq, end_eq, expr_eq)
345 fn check_for_warn_of_moved_symbol(
346 cx: &LateContext<'tcx>,
347 symbols: &FxHashSet<Symbol>,
348 if_expr: &'tcx Expr<'_>,
350 get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
351 let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
355 .filter(|sym| !sym.as_str().starts_with('_'))
357 let mut walker = ContainsName {
366 .filter(|stmt| !ignore_span.overlaps(stmt.span))
367 .for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
369 if let Some(expr) = block.expr {
370 intravisit::walk_expr(&mut walker, expr);
378 fn emit_shared_code_in_if_blocks_lint(
379 cx: &LateContext<'tcx>,
383 warn_about_moved_symbol: bool,
384 blocks: &[&Block<'tcx>],
385 if_expr: &'tcx Expr<'_>,
387 if start_stmts == 0 && !lint_end {
391 // (help, span, suggestion)
392 let mut suggestions: Vec<(&str, Span, String)> = vec![];
393 let mut add_expr_note = false;
395 // Construct suggestions
397 let block = blocks[0];
398 let span_start = first_line_of_span(cx, if_expr.span).shrink_to_lo();
399 let span_end = block.stmts[start_stmts - 1].span.source_callsite();
401 let cond_span = first_line_of_span(cx, if_expr.span).until(block.span);
402 let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
403 let cond_indent = indent_of(cx, cond_span);
404 let moved_span = block.stmts[0].span.source_callsite().to(span_end);
405 let moved_snippet = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
406 let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
407 let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
409 let span = span_start.to(span_end);
410 suggestions.push(("start", span, suggestion.to_string()));
414 let block = blocks[blocks.len() - 1];
415 let span_end = block.span.shrink_to_hi();
417 let moved_start = if end_stmts == 0 && block.expr.is_some() {
418 block.expr.unwrap().span
420 block.stmts[block.stmts.len() - end_stmts].span
423 let moved_end = block
425 .map_or_else(|| block.stmts[block.stmts.len() - 1].span, |expr| expr.span)
428 let moved_span = moved_start.to(moved_end);
429 let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None);
430 let indent = indent_of(cx, if_expr.span.shrink_to_hi());
431 let suggestion = "}\n".to_string() + &moved_snipped;
432 let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
434 let mut span = moved_start.to(span_end);
435 // Improve formatting if the inner block has indention (i.e. normal Rust formatting)
436 let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt());
437 if snippet_opt(cx, test_span)
438 .map(|snip| snip == " ")
441 span = span.with_lo(test_span.lo());
444 suggestions.push(("end", span, suggestion.to_string()));
445 add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit()
448 let add_optional_msgs = |diag: &mut DiagnosticBuilder<'_>| {
450 diag.note("The end suggestion probably needs some adjustments to use the expression result correctly.");
453 if warn_about_moved_symbol {
454 diag.warn("Some moved values might need to be renamed to avoid wrong references.");
459 if suggestions.len() == 1 {
460 let (place_str, span, sugg) = suggestions.pop().unwrap();
461 let msg = format!("All if blocks contain the same code at the {}", place_str);
462 let help = format!("Consider moving the {} statements out like this", place_str);
463 span_lint_and_then(cx, SHARED_CODE_IN_IF_BLOCKS, span, msg.as_str(), |diag| {
464 diag.span_suggestion(span, help.as_str(), sugg, Applicability::Unspecified);
466 add_optional_msgs(diag);
468 } else if suggestions.len() == 2 {
469 let (_, end_span, end_sugg) = suggestions.pop().unwrap();
470 let (_, start_span, start_sugg) = suggestions.pop().unwrap();
473 SHARED_CODE_IN_IF_BLOCKS,
475 "All if blocks contain the same code at the start and the end. Here at the start:",
477 diag.span_note(end_span, "And here at the end:");
479 diag.span_suggestion(
481 "Consider moving the start statements out like this:",
483 Applicability::Unspecified,
486 diag.span_suggestion(
488 "And consider moving the end statements out like this:",
490 Applicability::Unspecified,
493 add_optional_msgs(diag);
499 /// This visitor collects `HirId`s and Symbols of defined symbols and `HirId`s of used values.
500 struct UsedValueFinderVisitor<'a, 'tcx> {
501 cx: &'a LateContext<'tcx>,
503 /// The `HirId`s of defined values in the scanned statements
504 defs: FxHashSet<HirId>,
506 /// The Symbols of the defined symbols in the scanned statements
507 def_symbols: FxHashSet<Symbol>,
509 /// The `HirId`s of the used values
510 uses: FxHashSet<HirId>,
513 impl<'a, 'tcx> UsedValueFinderVisitor<'a, 'tcx> {
514 fn new(cx: &'a LateContext<'tcx>) -> Self {
515 UsedValueFinderVisitor {
517 defs: FxHashSet::default(),
518 def_symbols: FxHashSet::default(),
519 uses: FxHashSet::default(),
524 impl<'a, 'tcx> Visitor<'tcx> for UsedValueFinderVisitor<'a, 'tcx> {
525 type Map = Map<'tcx>;
527 fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
528 NestedVisitorMap::All(self.cx.tcx.hir())
531 fn visit_local(&mut self, l: &'tcx rustc_hir::Local<'tcx>) {
532 let local_id = l.pat.hir_id;
533 self.defs.insert(local_id);
535 if let Some(sym) = l.pat.simple_ident() {
536 self.def_symbols.insert(sym.name);
539 if let Some(expr) = l.init {
540 intravisit::walk_expr(self, expr);
544 fn visit_qpath(&mut self, qpath: &'tcx rustc_hir::QPath<'tcx>, id: HirId, _span: rustc_span::Span) {
545 if let rustc_hir::QPath::Resolved(_, ref path) = *qpath {
546 if path.segments.len() == 1 {
547 if let rustc_hir::def::Res::Local(var) = self.cx.qpath_res(qpath, id) {
548 self.uses.insert(var);
555 /// Implementation of `IFS_SAME_COND`.
556 fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
557 let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
558 let mut h = SpanlessHash::new(cx);
563 let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };
565 for (i, j) in search_same(conds, hash, eq) {
570 "this `if` has the same condition as a previous `if`",
577 /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
578 fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
579 let hash: &dyn Fn(&&Expr<'_>) -> u64 = &|expr| -> u64 {
580 let mut h = SpanlessHash::new(cx);
585 let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
586 // Do not lint if any expr originates from a macro
587 if in_macro(lhs.span) || in_macro(rhs.span) {
590 // Do not spawn warning if `IFS_SAME_COND` already produced it.
591 if eq_expr_value(cx, lhs, rhs) {
594 SpanlessEq::new(cx).eq_expr(lhs, rhs)
597 for (i, j) in search_same(conds, hash, eq) {
600 SAME_FUNCTIONS_IN_IF_CONDITION,
602 "this `if` has the same function call as a previous `if`",