]> git.proxmox.com Git - rustc.git/blob - src/tools/clippy/clippy_lints/src/copies.rs
New upstream version 1.63.0+dfsg1
[rustc.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 eq_expr_value, get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause, is_lint_allowed,
5 search_same, ContainsName, HirEqInterExpr, SpanlessEq,
6 };
7 use core::iter;
8 use rustc_errors::Applicability;
9 use rustc_hir::intravisit;
10 use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Stmt, StmtKind};
11 use rustc_lint::{LateContext, LateLintPass};
12 use rustc_session::{declare_lint_pass, declare_tool_lint};
13 use rustc_span::hygiene::walk_chain;
14 use rustc_span::source_map::SourceMap;
15 use rustc_span::{BytePos, Span, Symbol};
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 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)
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 /// Use instead:
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() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
169 let (conds, blocks) = if_sequence(expr);
170 lint_same_cond(cx, &conds);
171 lint_same_fns_in_if_cond(cx, &conds);
172 let all_same =
173 !is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks);
174 if !all_same && conds.len() != blocks.len() {
175 lint_branches_sharing_code(cx, &conds, &blocks, expr);
176 }
177 }
178 }
179 }
180
181 /// Checks if the given expression is a let chain.
182 fn contains_let(e: &Expr<'_>) -> bool {
183 match e.kind {
184 ExprKind::Let(..) => true,
185 ExprKind::Binary(op, lhs, rhs) if op.node == BinOpKind::And => {
186 matches!(lhs.kind, ExprKind::Let(..)) || contains_let(rhs)
187 },
188 _ => false,
189 }
190 }
191
192 fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool {
193 let mut eq = SpanlessEq::new(cx);
194 blocks
195 .array_windows::<2>()
196 .enumerate()
197 .fold(true, |all_eq, (i, &[lhs, rhs])| {
198 if eq.eq_block(lhs, rhs) && !contains_let(conds[i]) && conds.get(i + 1).map_or(true, |e| !contains_let(e)) {
199 span_lint_and_note(
200 cx,
201 IF_SAME_THEN_ELSE,
202 lhs.span,
203 "this `if` has identical blocks",
204 Some(rhs.span),
205 "same as this",
206 );
207 all_eq
208 } else {
209 false
210 }
211 })
212 }
213
214 fn lint_branches_sharing_code<'tcx>(
215 cx: &LateContext<'tcx>,
216 conds: &[&'tcx Expr<'_>],
217 blocks: &[&Block<'tcx>],
218 expr: &'tcx Expr<'_>,
219 ) {
220 // We only lint ifs with multiple blocks
221 let &[first_block, ref blocks @ ..] = blocks else {
222 return;
223 };
224 let &[.., last_block] = blocks else {
225 return;
226 };
227
228 let res = scan_block_for_eq(cx, conds, first_block, blocks);
229 let sm = cx.tcx.sess.source_map();
230 let start_suggestion = res.start_span(first_block, sm).map(|span| {
231 let first_line_span = first_line_of_span(cx, expr.span);
232 let replace_span = first_line_span.with_hi(span.hi());
233 let cond_span = first_line_span.until(first_block.span);
234 let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
235 let cond_indent = indent_of(cx, cond_span);
236 let moved_snippet = reindent_multiline(snippet(cx, span, "_"), true, None);
237 let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
238 let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
239 (replace_span, suggestion.to_string())
240 });
241 let end_suggestion = res.end_span(last_block, sm).map(|span| {
242 let moved_snipped = reindent_multiline(snippet(cx, span, "_"), true, None);
243 let indent = indent_of(cx, expr.span.shrink_to_hi());
244 let suggestion = "}\n".to_string() + &moved_snipped;
245 let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
246
247 let span = span.with_hi(last_block.span.hi());
248 // Improve formatting if the inner block has indention (i.e. normal Rust formatting)
249 let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent());
250 let span = if snippet_opt(cx, test_span).map_or(false, |snip| snip == " ") {
251 span.with_lo(test_span.lo())
252 } else {
253 span
254 };
255 (span, suggestion.to_string())
256 });
257
258 let (span, msg, end_span) = match (&start_suggestion, &end_suggestion) {
259 (&Some((span, _)), &Some((end_span, _))) => (
260 span,
261 "all if blocks contain the same code at both the start and the end",
262 Some(end_span),
263 ),
264 (&Some((span, _)), None) => (span, "all if blocks contain the same code at the start", None),
265 (None, &Some((span, _))) => (span, "all if blocks contain the same code at the end", None),
266 (None, None) => return,
267 };
268 span_lint_and_then(cx, BRANCHES_SHARING_CODE, span, msg, |diag| {
269 if let Some(span) = end_span {
270 diag.span_note(span, "this code is shared at the end");
271 }
272 if let Some((span, sugg)) = start_suggestion {
273 diag.span_suggestion(
274 span,
275 "consider moving these statements before the if",
276 sugg,
277 Applicability::Unspecified,
278 );
279 }
280 if let Some((span, sugg)) = end_suggestion {
281 diag.span_suggestion(
282 span,
283 "consider moving these statements after the if",
284 sugg,
285 Applicability::Unspecified,
286 );
287 if !cx.typeck_results().expr_ty(expr).is_unit() {
288 diag.note("the end suggestion probably needs some adjustments to use the expression result correctly");
289 }
290 }
291 if check_for_warn_of_moved_symbol(cx, &res.moved_locals, expr) {
292 diag.warn("some moved values might need to be renamed to avoid wrong references");
293 }
294 });
295 }
296
297 struct BlockEq {
298 /// The end of the range of equal stmts at the start.
299 start_end_eq: usize,
300 /// The start of the range of equal stmts at the end.
301 end_begin_eq: Option<usize>,
302 /// The name and id of every local which can be moved at the beginning and the end.
303 moved_locals: Vec<(HirId, Symbol)>,
304 }
305 impl BlockEq {
306 fn start_span(&self, b: &Block<'_>, sm: &SourceMap) -> Option<Span> {
307 match &b.stmts[..self.start_end_eq] {
308 [first, .., last] => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))),
309 [s] => Some(sm.stmt_span(s.span, b.span)),
310 [] => None,
311 }
312 }
313
314 fn end_span(&self, b: &Block<'_>, sm: &SourceMap) -> Option<Span> {
315 match (&b.stmts[b.stmts.len() - self.end_begin_eq?..], b.expr) {
316 ([first, .., last], None) => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))),
317 ([first, ..], Some(last)) => Some(sm.stmt_span(first.span, b.span).to(sm.stmt_span(last.span, b.span))),
318 ([s], None) => Some(sm.stmt_span(s.span, b.span)),
319 ([], Some(e)) => Some(walk_chain(e.span, b.span.ctxt())),
320 ([], None) => None,
321 }
322 }
323 }
324
325 /// If the statement is a local, checks if the bound names match the expected list of names.
326 fn eq_binding_names(s: &Stmt<'_>, names: &[(HirId, Symbol)]) -> bool {
327 if let StmtKind::Local(l) = s.kind {
328 let mut i = 0usize;
329 let mut res = true;
330 l.pat.each_binding_or_first(&mut |_, _, _, name| {
331 if names.get(i).map_or(false, |&(_, n)| n == name.name) {
332 i += 1;
333 } else {
334 res = false;
335 }
336 });
337 res && i == names.len()
338 } else {
339 false
340 }
341 }
342
343 /// Checks if the given statement should be considered equal to the statement in the same position
344 /// for each block.
345 fn eq_stmts(
346 stmt: &Stmt<'_>,
347 blocks: &[&Block<'_>],
348 get_stmt: impl for<'a> Fn(&'a Block<'a>) -> Option<&'a Stmt<'a>>,
349 eq: &mut HirEqInterExpr<'_, '_, '_>,
350 moved_bindings: &mut Vec<(HirId, Symbol)>,
351 ) -> bool {
352 (if let StmtKind::Local(l) = stmt.kind {
353 let old_count = moved_bindings.len();
354 l.pat.each_binding_or_first(&mut |_, id, _, name| {
355 moved_bindings.push((id, name.name));
356 });
357 let new_bindings = &moved_bindings[old_count..];
358 blocks
359 .iter()
360 .all(|b| get_stmt(b).map_or(false, |s| eq_binding_names(s, new_bindings)))
361 } else {
362 true
363 }) && blocks
364 .iter()
365 .all(|b| get_stmt(b).map_or(false, |s| eq.eq_stmt(s, stmt)))
366 }
367
368 fn scan_block_for_eq(cx: &LateContext<'_>, _conds: &[&Expr<'_>], block: &Block<'_>, blocks: &[&Block<'_>]) -> BlockEq {
369 let mut eq = SpanlessEq::new(cx);
370 let mut eq = eq.inter_expr();
371 let mut moved_locals = Vec::new();
372
373 let start_end_eq = block
374 .stmts
375 .iter()
376 .enumerate()
377 .find(|&(i, stmt)| !eq_stmts(stmt, blocks, |b| b.stmts.get(i), &mut eq, &mut moved_locals))
378 .map_or(block.stmts.len(), |(i, _)| i);
379
380 // Walk backwards through the final expression/statements so long as their hashes are equal. Note
381 // `SpanlessHash` treats all local references as equal allowing locals declared earlier in the block
382 // to match those in other blocks. e.g. If each block ends with the following the hash value will be
383 // the same even though each `x` binding will have a different `HirId`:
384 // let x = foo();
385 // x + 50
386 let expr_hash_eq = if let Some(e) = block.expr {
387 let hash = hash_expr(cx, e);
388 blocks
389 .iter()
390 .all(|b| b.expr.map_or(false, |e| hash_expr(cx, e) == hash))
391 } else {
392 blocks.iter().all(|b| b.expr.is_none())
393 };
394 if !expr_hash_eq {
395 return BlockEq {
396 start_end_eq,
397 end_begin_eq: None,
398 moved_locals,
399 };
400 }
401 let end_search_start = block.stmts[start_end_eq..]
402 .iter()
403 .rev()
404 .enumerate()
405 .find(|&(offset, stmt)| {
406 let hash = hash_stmt(cx, stmt);
407 blocks.iter().any(|b| {
408 b.stmts
409 // the bounds check will catch the underflow
410 .get(b.stmts.len().wrapping_sub(offset + 1))
411 .map_or(true, |s| hash != hash_stmt(cx, s))
412 })
413 })
414 .map_or(block.stmts.len() - start_end_eq, |(i, _)| i);
415
416 let moved_locals_at_start = moved_locals.len();
417 let mut i = end_search_start;
418 let end_begin_eq = block.stmts[block.stmts.len() - end_search_start..]
419 .iter()
420 .zip(iter::repeat_with(move || {
421 let x = i;
422 i -= 1;
423 x
424 }))
425 .fold(end_search_start, |init, (stmt, offset)| {
426 if eq_stmts(
427 stmt,
428 blocks,
429 |b| b.stmts.get(b.stmts.len() - offset),
430 &mut eq,
431 &mut moved_locals,
432 ) {
433 init
434 } else {
435 // Clear out all locals seen at the end so far. None of them can be moved.
436 let stmts = &blocks[0].stmts;
437 for stmt in &stmts[stmts.len() - init..=stmts.len() - offset] {
438 if let StmtKind::Local(l) = stmt.kind {
439 l.pat.each_binding_or_first(&mut |_, id, _, _| {
440 eq.locals.remove(&id);
441 });
442 }
443 }
444 moved_locals.truncate(moved_locals_at_start);
445 offset - 1
446 }
447 });
448 if let Some(e) = block.expr {
449 for block in blocks {
450 if block.expr.map_or(false, |expr| !eq.eq_expr(expr, e)) {
451 moved_locals.truncate(moved_locals_at_start);
452 return BlockEq {
453 start_end_eq,
454 end_begin_eq: None,
455 moved_locals,
456 };
457 }
458 }
459 }
460
461 BlockEq {
462 start_end_eq,
463 end_begin_eq: Some(end_begin_eq),
464 moved_locals,
465 }
466 }
467
468 fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbol)], if_expr: &Expr<'_>) -> bool {
469 get_enclosing_block(cx, if_expr.hir_id).map_or(false, |block| {
470 let ignore_span = block.span.shrink_to_lo().to(if_expr.span);
471
472 symbols
473 .iter()
474 .filter(|&&(_, name)| !name.as_str().starts_with('_'))
475 .any(|&(_, name)| {
476 let mut walker = ContainsName { name, result: false };
477
478 // Scan block
479 block
480 .stmts
481 .iter()
482 .filter(|stmt| !ignore_span.overlaps(stmt.span))
483 .for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
484
485 if let Some(expr) = block.expr {
486 intravisit::walk_expr(&mut walker, expr);
487 }
488
489 walker.result
490 })
491 })
492 }
493
494 /// Implementation of `IFS_SAME_COND`.
495 fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
496 for (i, j) in search_same(conds, |e| hash_expr(cx, e), |lhs, rhs| eq_expr_value(cx, lhs, rhs)) {
497 span_lint_and_note(
498 cx,
499 IFS_SAME_COND,
500 j.span,
501 "this `if` has the same condition as a previous `if`",
502 Some(i.span),
503 "same as this",
504 );
505 }
506 }
507
508 /// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
509 fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
510 let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
511 // Do not lint if any expr originates from a macro
512 if lhs.span.from_expansion() || rhs.span.from_expansion() {
513 return false;
514 }
515 // Do not spawn warning if `IFS_SAME_COND` already produced it.
516 if eq_expr_value(cx, lhs, rhs) {
517 return false;
518 }
519 SpanlessEq::new(cx).eq_expr(lhs, rhs)
520 };
521
522 for (i, j) in search_same(conds, |e| hash_expr(cx, e), eq) {
523 span_lint_and_note(
524 cx,
525 SAME_FUNCTIONS_IN_IF_CONDITION,
526 j.span,
527 "this `if` has the same function call as a previous `if`",
528 Some(i.span),
529 "same as this",
530 );
531 }
532 }