Skip to content

Commit aa1fa10

Browse files
dswijdswij
authored
split copies lints each into its own module (rust-lang#15721)
changelog: none
2 parents 6cc8f8d + 95f5a00 commit aa1fa10

File tree

7 files changed

+306
-277
lines changed

7 files changed

+306
-277
lines changed

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
8484
crate::collapsible_if::COLLAPSIBLE_IF_INFO,
8585
crate::collection_is_never_read::COLLECTION_IS_NEVER_READ_INFO,
8686
crate::comparison_chain::COMPARISON_CHAIN_INFO,
87-
crate::copies::BRANCHES_SHARING_CODE_INFO,
88-
crate::copies::IFS_SAME_COND_INFO,
89-
crate::copies::IF_SAME_THEN_ELSE_INFO,
90-
crate::copies::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
9187
crate::copy_iterator::COPY_ITERATOR_INFO,
9288
crate::crate_in_macro_def::CRATE_IN_MACRO_DEF_INFO,
9389
crate::create_dir::CREATE_DIR_INFO,
@@ -204,6 +200,10 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
204200
crate::if_let_mutex::IF_LET_MUTEX_INFO,
205201
crate::if_not_else::IF_NOT_ELSE_INFO,
206202
crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO,
203+
crate::ifs::BRANCHES_SHARING_CODE_INFO,
204+
crate::ifs::IFS_SAME_COND_INFO,
205+
crate::ifs::IF_SAME_THEN_ELSE_INFO,
206+
crate::ifs::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
207207
crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO,
208208
crate::impl_hash_with_borrow_str_and_bytes::IMPL_HASH_BORROW_WITH_STR_AND_BYTES_INFO,
209209
crate::implicit_hasher::IMPLICIT_HASHER_INFO,

‎clippy_lints/src/copies.rs‎ renamed to ‎clippy_lints/src/ifs/branches_sharing_code.rs‎

Lines changed: 12 additions & 271 deletions
Original file line numberDiff line numberDiff line change
@@ -1,218 +1,23 @@
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;
42
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;
64
use clippy_utils::visitors::for_each_expr_without_closures;
75
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,
118
};
129
use core::iter;
1310
use core::ops::ControlFlow;
1411
use rustc_errors::Applicability;
1512
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;
1914
use rustc_span::hygiene::walk_chain;
2015
use rustc_span::source_map::SourceMap;
2116
use rustc_span::{Span, Symbol};
2217

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;
21419

215-
fn lint_branches_sharing_code<'tcx>(
20+
pub(super) fn check<'tcx>(
21621
cx: &LateContext<'tcx>,
21722
conds: &[&'tcx Expr<'_>],
21823
blocks: &[&'tcx Block<'_>],
@@ -356,8 +161,8 @@ fn modifies_any_local<'tcx>(cx: &LateContext<'tcx>, s: &'tcx Stmt<'_>, locals: &
356161
.is_some()
357162
}
358163

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.
361166
fn eq_stmts(
362167
stmt: &Stmt<'_>,
363168
blocks: &[&Block<'_>],
@@ -516,9 +321,9 @@ fn scan_block_for_eq<'tcx>(
516321
}
517322
}
518323

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.
522327
///
523328
/// For example, for the following macro:
524329
/// ```rust,ignore
@@ -587,70 +392,6 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
587392
})
588393
}
589394

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-
654395
fn is_expr_parent_assignment(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
655396
let parent = cx.tcx.parent_hir_node(expr.hir_id);
656397
if let Node::LetStmt(LetStmt { init: Some(e), .. })

‎clippy_lints/src/ifs/if_same_then_else.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
use clippy_utils::SpanlessEq;
2+
use clippy_utils::diagnostics::span_lint_and_note;
3+
use clippy_utils::higher::has_let_expr;
4+
use rustc_hir::{Block, Expr};
5+
use rustc_lint::LateContext;
6+
7+
use super::IF_SAME_THEN_ELSE;
8+
9+
pub(super) fn check(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool {
10+
let mut eq = SpanlessEq::new(cx);
11+
blocks
12+
.array_windows::<2>()
13+
.enumerate()
14+
.fold(true, |all_eq, (i, &[lhs, rhs])| {
15+
if eq.eq_block(lhs, rhs) && !has_let_expr(conds[i]) && conds.get(i + 1).is_none_or(|e| !has_let_expr(e)) {
16+
span_lint_and_note(
17+
cx,
18+
IF_SAME_THEN_ELSE,
19+
lhs.span,
20+
"this `if` has identical blocks",
21+
Some(rhs.span),
22+
"same as this",
23+
);
24+
all_eq
25+
} else {
26+
false
27+
}
28+
})
29+
}

0 commit comments

Comments
 (0)