|
1 |
| -use clippy_config::Conf; |
2 |
| -use clippy_utils::diagnostics::{span_lint, span_lint_and_note, span_lint_and_then}; |
3 |
| -use clippy_utils::higher::has_let_expr; |
| 1 | +use clippy_utils::diagnostics::span_lint_and_then; |
4 | 2 | use clippy_utils::source::{IntoSpan, SpanRangeExt, first_line_of_span, indent_of, reindent_multiline, snippet};
|
5 |
| -use clippy_utils::ty::{InteriorMut, needs_ordered_drop}; |
| 3 | +use clippy_utils::ty::needs_ordered_drop; |
6 | 4 | use clippy_utils::visitors::for_each_expr_without_closures;
|
7 | 5 | use clippy_utils::{
|
8 |
| - ContainsName, HirEqInterExpr, SpanlessEq, capture_local_usage, eq_expr_value, find_binding_init, |
9 |
| - get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause, is_lint_allowed, path_to_local, |
10 |
| - search_same, |
| 6 | + ContainsName, HirEqInterExpr, SpanlessEq, capture_local_usage, get_enclosing_block, hash_expr, hash_stmt, |
| 7 | + path_to_local, |
11 | 8 | };
|
12 | 9 | use core::iter;
|
13 | 10 | use core::ops::ControlFlow;
|
14 | 11 | use rustc_errors::Applicability;
|
15 | 12 | use rustc_hir::{Block, Expr, ExprKind, HirId, HirIdSet, LetStmt, Node, Stmt, StmtKind, intravisit};
|
16 |
| -use rustc_lint::{LateContext, LateLintPass}; |
17 |
| -use rustc_middle::ty::TyCtxt; |
18 |
| -use rustc_session::impl_lint_pass; |
| 13 | +use rustc_lint::LateContext; |
19 | 14 | use rustc_span::hygiene::walk_chain;
|
20 | 15 | use rustc_span::source_map::SourceMap;
|
21 | 16 | use rustc_span::{Span, Symbol};
|
22 | 17 |
|
23 |
| -declare_clippy_lint! { |
24 |
| - /// ### What it does |
25 |
| - /// Checks for consecutive `if`s with the same condition. |
26 |
| - /// |
27 |
| - /// ### Why is this bad? |
28 |
| - /// This is probably a copy & paste error. |
29 |
| - /// |
30 |
| - /// ### Example |
31 |
| - /// ```ignore |
32 |
| - /// if a == b { |
33 |
| - /// … |
34 |
| - /// } else if a == b { |
35 |
| - /// … |
36 |
| - /// } |
37 |
| - /// ``` |
38 |
| - /// |
39 |
| - /// Note that this lint ignores all conditions with a function call as it could |
40 |
| - /// have side effects: |
41 |
| - /// |
42 |
| - /// ```ignore |
43 |
| - /// if foo() { |
44 |
| - /// … |
45 |
| - /// } else if foo() { // not linted |
46 |
| - /// … |
47 |
| - /// } |
48 |
| - /// ``` |
49 |
| - #[clippy::version = "pre 1.29.0"] |
50 |
| - pub IFS_SAME_COND, |
51 |
| - correctness, |
52 |
| - "consecutive `if`s with the same condition" |
53 |
| -} |
54 |
| - |
55 |
| -declare_clippy_lint! { |
56 |
| - /// ### What it does |
57 |
| - /// Checks for consecutive `if`s with the same function call. |
58 |
| - /// |
59 |
| - /// ### Why is this bad? |
60 |
| - /// This is probably a copy & paste error. |
61 |
| - /// Despite the fact that function can have side effects and `if` works as |
62 |
| - /// intended, such an approach is implicit and can be considered a "code smell". |
63 |
| - /// |
64 |
| - /// ### Example |
65 |
| - /// ```ignore |
66 |
| - /// if foo() == bar { |
67 |
| - /// … |
68 |
| - /// } else if foo() == bar { |
69 |
| - /// … |
70 |
| - /// } |
71 |
| - /// ``` |
72 |
| - /// |
73 |
| - /// This probably should be: |
74 |
| - /// ```ignore |
75 |
| - /// if foo() == bar { |
76 |
| - /// … |
77 |
| - /// } else if foo() == baz { |
78 |
| - /// … |
79 |
| - /// } |
80 |
| - /// ``` |
81 |
| - /// |
82 |
| - /// or if the original code was not a typo and called function mutates a state, |
83 |
| - /// consider move the mutation out of the `if` condition to avoid similarity to |
84 |
| - /// a copy & paste error: |
85 |
| - /// |
86 |
| - /// ```ignore |
87 |
| - /// let first = foo(); |
88 |
| - /// if first == bar { |
89 |
| - /// … |
90 |
| - /// } else { |
91 |
| - /// let second = foo(); |
92 |
| - /// if second == bar { |
93 |
| - /// … |
94 |
| - /// } |
95 |
| - /// } |
96 |
| - /// ``` |
97 |
| - #[clippy::version = "1.41.0"] |
98 |
| - pub SAME_FUNCTIONS_IN_IF_CONDITION, |
99 |
| - pedantic, |
100 |
| - "consecutive `if`s with the same function call" |
101 |
| -} |
102 |
| - |
103 |
| -declare_clippy_lint! { |
104 |
| - /// ### What it does |
105 |
| - /// Checks for `if/else` with the same body as the *then* part |
106 |
| - /// and the *else* part. |
107 |
| - /// |
108 |
| - /// ### Why is this bad? |
109 |
| - /// This is probably a copy & paste error. |
110 |
| - /// |
111 |
| - /// ### Example |
112 |
| - /// ```ignore |
113 |
| - /// let foo = if … { |
114 |
| - /// 42 |
115 |
| - /// } else { |
116 |
| - /// 42 |
117 |
| - /// }; |
118 |
| - /// ``` |
119 |
| - #[clippy::version = "pre 1.29.0"] |
120 |
| - pub IF_SAME_THEN_ELSE, |
121 |
| - style, |
122 |
| - "`if` with the same `then` and `else` blocks" |
123 |
| -} |
124 |
| - |
125 |
| -declare_clippy_lint! { |
126 |
| - /// ### What it does |
127 |
| - /// Checks if the `if` and `else` block contain shared code that can be |
128 |
| - /// moved out of the blocks. |
129 |
| - /// |
130 |
| - /// ### Why is this bad? |
131 |
| - /// Duplicate code is less maintainable. |
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 |
| -pub struct CopyAndPaste<'tcx> { |
160 |
| - interior_mut: InteriorMut<'tcx>, |
161 |
| -} |
162 |
| - |
163 |
| -impl<'tcx> CopyAndPaste<'tcx> { |
164 |
| - pub fn new(tcx: TyCtxt<'tcx>, conf: &'static Conf) -> Self { |
165 |
| - Self { |
166 |
| - interior_mut: InteriorMut::new(tcx, &conf.ignore_interior_mutability), |
167 |
| - } |
168 |
| - } |
169 |
| -} |
170 |
| - |
171 |
| -impl_lint_pass!(CopyAndPaste<'_> => [ |
172 |
| - IFS_SAME_COND, |
173 |
| - SAME_FUNCTIONS_IN_IF_CONDITION, |
174 |
| - IF_SAME_THEN_ELSE, |
175 |
| - BRANCHES_SHARING_CODE |
176 |
| -]); |
177 |
| - |
178 |
| -impl<'tcx> LateLintPass<'tcx> for CopyAndPaste<'tcx> { |
179 |
| - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { |
180 |
| - if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) { |
181 |
| - let (conds, blocks) = if_sequence(expr); |
182 |
| - lint_same_cond(cx, &conds, &mut self.interior_mut); |
183 |
| - lint_same_fns_in_if_cond(cx, &conds); |
184 |
| - let all_same = |
185 |
| - !is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks); |
186 |
| - if !all_same && conds.len() != blocks.len() { |
187 |
| - lint_branches_sharing_code(cx, &conds, &blocks, expr); |
188 |
| - } |
189 |
| - } |
190 |
| - } |
191 |
| -} |
192 |
| - |
193 |
| -fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool { |
194 |
| - let mut eq = SpanlessEq::new(cx); |
195 |
| - blocks |
196 |
| - .array_windows::<2>() |
197 |
| - .enumerate() |
198 |
| - .fold(true, |all_eq, (i, &[lhs, rhs])| { |
199 |
| - if eq.eq_block(lhs, rhs) && !has_let_expr(conds[i]) && conds.get(i + 1).is_none_or(|e| !has_let_expr(e)) { |
200 |
| - span_lint_and_note( |
201 |
| - cx, |
202 |
| - IF_SAME_THEN_ELSE, |
203 |
| - lhs.span, |
204 |
| - "this `if` has identical blocks", |
205 |
| - Some(rhs.span), |
206 |
| - "same as this", |
207 |
| - ); |
208 |
| - all_eq |
209 |
| - } else { |
210 |
| - false |
211 |
| - } |
212 |
| - }) |
213 |
| -} |
| 18 | +use super::BRANCHES_SHARING_CODE; |
214 | 19 |
|
215 |
| -fn lint_branches_sharing_code<'tcx>( |
| 20 | +pub(super) fn check<'tcx>( |
216 | 21 | cx: &LateContext<'tcx>,
|
217 | 22 | conds: &[&'tcx Expr<'_>],
|
218 | 23 | blocks: &[&'tcx Block<'_>],
|
@@ -356,8 +161,8 @@ fn modifies_any_local<'tcx>(cx: &LateContext<'tcx>, s: &'tcx Stmt<'_>, locals: &
|
356 | 161 | .is_some()
|
357 | 162 | }
|
358 | 163 |
|
359 |
| -/// Checks if the given statement should be considered equal to the statement in the same position |
360 |
| -/// for each block. |
| 164 | +/// Checks if the given statement should be considered equal to the statement in the same |
| 165 | +/// position for each block. |
361 | 166 | fn eq_stmts(
|
362 | 167 | stmt: &Stmt<'_>,
|
363 | 168 | blocks: &[&Block<'_>],
|
@@ -516,9 +321,9 @@ fn scan_block_for_eq<'tcx>(
|
516 | 321 | }
|
517 | 322 | }
|
518 | 323 |
|
519 |
| -/// Adjusts the index for which the statements begin to differ to the closest macro callsite. This |
520 |
| -/// avoids giving suggestions that requires splitting a macro call in half, when only a part of the |
521 |
| -/// macro expansion is equal. |
| 324 | +/// Adjusts the index for which the statements begin to differ to the closest macro callsite. |
| 325 | +/// This avoids giving suggestions that requires splitting a macro call in half, when only a |
| 326 | +/// part of the macro expansion is equal. |
522 | 327 | ///
|
523 | 328 | /// For example, for the following macro:
|
524 | 329 | /// ```rust,ignore
|
@@ -587,70 +392,6 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
|
587 | 392 | })
|
588 | 393 | }
|
589 | 394 |
|
590 |
| -fn method_caller_is_mutable<'tcx>( |
591 |
| - cx: &LateContext<'tcx>, |
592 |
| - caller_expr: &Expr<'_>, |
593 |
| - interior_mut: &mut InteriorMut<'tcx>, |
594 |
| -) -> bool { |
595 |
| - let caller_ty = cx.typeck_results().expr_ty(caller_expr); |
596 |
| - |
597 |
| - interior_mut.is_interior_mut_ty(cx, caller_ty) |
598 |
| - || caller_ty.is_mutable_ptr() |
599 |
| - // `find_binding_init` will return the binding iff its not mutable |
600 |
| - || path_to_local(caller_expr) |
601 |
| - .and_then(|hid| find_binding_init(cx, hid)) |
602 |
| - .is_none() |
603 |
| -} |
604 |
| - |
605 |
| -/// Implementation of `IFS_SAME_COND`. |
606 |
| -fn lint_same_cond<'tcx>(cx: &LateContext<'tcx>, conds: &[&Expr<'_>], interior_mut: &mut InteriorMut<'tcx>) { |
607 |
| - for group in search_same( |
608 |
| - conds, |
609 |
| - |e| hash_expr(cx, e), |
610 |
| - |lhs, rhs| { |
611 |
| - // Ignore eq_expr side effects iff one of the expression kind is a method call |
612 |
| - // and the caller is not a mutable, including inner mutable type. |
613 |
| - if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind { |
614 |
| - if method_caller_is_mutable(cx, caller, interior_mut) { |
615 |
| - false |
616 |
| - } else { |
617 |
| - SpanlessEq::new(cx).eq_expr(lhs, rhs) |
618 |
| - } |
619 |
| - } else { |
620 |
| - eq_expr_value(cx, lhs, rhs) |
621 |
| - } |
622 |
| - }, |
623 |
| - ) { |
624 |
| - let spans: Vec<_> = group.into_iter().map(|expr| expr.span).collect(); |
625 |
| - span_lint(cx, IFS_SAME_COND, spans, "these `if` branches have the same condition"); |
626 |
| - } |
627 |
| -} |
628 |
| - |
629 |
| -/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`. |
630 |
| -fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) { |
631 |
| - let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { |
632 |
| - // Do not lint if any expr originates from a macro |
633 |
| - if lhs.span.from_expansion() || rhs.span.from_expansion() { |
634 |
| - return false; |
635 |
| - } |
636 |
| - // Do not spawn warning if `IFS_SAME_COND` already produced it. |
637 |
| - if eq_expr_value(cx, lhs, rhs) { |
638 |
| - return false; |
639 |
| - } |
640 |
| - SpanlessEq::new(cx).eq_expr(lhs, rhs) |
641 |
| - }; |
642 |
| - |
643 |
| - for group in search_same(conds, |e| hash_expr(cx, e), eq) { |
644 |
| - let spans: Vec<_> = group.into_iter().map(|expr| expr.span).collect(); |
645 |
| - span_lint( |
646 |
| - cx, |
647 |
| - SAME_FUNCTIONS_IN_IF_CONDITION, |
648 |
| - spans, |
649 |
| - "these `if` branches have the same function call", |
650 |
| - ); |
651 |
| - } |
652 |
| -} |
653 |
| - |
654 | 395 | fn is_expr_parent_assignment(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
655 | 396 | let parent = cx.tcx.parent_hir_node(expr.hir_id);
|
656 | 397 | if let Node::LetStmt(LetStmt { init: Some(e), .. })
|
|
0 commit comments