user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

split copies lints each into its own module (#15721)

Browse source 
changelog: none
This commit is contained in:
dswij 2025-10-02 18:12:24 +00:00 • committed by GitHub
commit aa1fa10458
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 306 additions and 277 deletions
Download patch file Download diff file Expand all files Collapse all files

8
clippy_lints/src/declared_lints.rs
View file

@ -84,10 +84,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::collapsible_if::COLLAPSIBLE_IF_INFO,
crate::collection_is_never_read::COLLECTION_IS_NEVER_READ_INFO,
crate::comparison_chain::COMPARISON_CHAIN_INFO,
crate::copies::BRANCHES_SHARING_CODE_INFO,
crate::copies::IFS_SAME_COND_INFO,
crate::copies::IF_SAME_THEN_ELSE_INFO,
crate::copies::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
crate::copy_iterator::COPY_ITERATOR_INFO,
crate::crate_in_macro_def::CRATE_IN_MACRO_DEF_INFO,
crate::create_dir::CREATE_DIR_INFO,
@ -204,6 +200,10 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::if_let_mutex::IF_LET_MUTEX_INFO,
crate::if_not_else::IF_NOT_ELSE_INFO,
crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO,
crate::ifs::BRANCHES_SHARING_CODE_INFO,
crate::ifs::IFS_SAME_COND_INFO,
crate::ifs::IF_SAME_THEN_ELSE_INFO,
crate::ifs::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO,
crate::impl_hash_with_borrow_str_and_bytes::IMPL_HASH_BORROW_WITH_STR_AND_BYTES_INFO,
crate::implicit_hasher::IMPLICIT_HASHER_INFO,

283
clippy_lints/src/copies.rs → clippy_lints/src/ifs/branches_sharing_code.rs
View file

@ -1,218 +1,23 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::{span_lint, span_lint_and_note, span_lint_and_then};
use clippy_utils::higher::has_let_expr;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{IntoSpan, SpanRangeExt, first_line_of_span, indent_of, reindent_multiline, snippet};
use clippy_utils::ty::{InteriorMut, needs_ordered_drop};
use clippy_utils::ty::needs_ordered_drop;
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{
ContainsName, HirEqInterExpr, SpanlessEq, capture_local_usage, eq_expr_value, find_binding_init,
get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause, is_lint_allowed, path_to_local,
search_same,
ContainsName, HirEqInterExpr, SpanlessEq, capture_local_usage, get_enclosing_block, hash_expr, hash_stmt,
path_to_local,
};
use core::iter;
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir::{Block, Expr, ExprKind, HirId, HirIdSet, LetStmt, Node, Stmt, StmtKind, intravisit};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass;
use rustc_lint::LateContext;
use rustc_span::hygiene::walk_chain;
use rustc_span::source_map::SourceMap;
use rustc_span::{Span, Symbol};
declare_clippy_lint! {
/// ### What it does
/// Checks for consecutive `if`s with the same condition.
///
/// ### Why is this bad?
/// This is probably a copy & paste error.
///
/// ### Example
/// ```ignore
/// if a == b {
/// …
/// } else if a == b {
/// …
/// }
/// ```
///
/// Note that this lint ignores all conditions with a function call as it could
/// have side effects:
///
/// ```ignore
/// if foo() {
/// …
/// } else if foo() { // not linted
/// …
/// }
/// ```
#[clippy::version = "pre 1.29.0"]
pub IFS_SAME_COND,
correctness,
"consecutive `if`s with the same condition"
}
use super::BRANCHES_SHARING_CODE;
declare_clippy_lint! {
/// ### What it does
/// Checks for consecutive `if`s with the same function call.
///
/// ### Why is this bad?
/// This is probably a copy & paste error.
/// Despite the fact that function can have side effects and `if` works as
/// intended, such an approach is implicit and can be considered a "code smell".
///
/// ### Example
/// ```ignore
/// if foo() == bar {
/// …
/// } else if foo() == bar {
/// …
/// }
/// ```
///
/// This probably should be:
/// ```ignore
/// if foo() == bar {
/// …
/// } else if foo() == baz {
/// …
/// }
/// ```
///
/// or if the original code was not a typo and called function mutates a state,
/// consider move the mutation out of the `if` condition to avoid similarity to
/// a copy & paste error:
///
/// ```ignore
/// let first = foo();
/// if first == bar {
/// …
/// } else {
/// let second = foo();
/// if second == bar {
/// …
/// }
/// }
/// ```
#[clippy::version = "1.41.0"]
pub SAME_FUNCTIONS_IN_IF_CONDITION,
pedantic,
"consecutive `if`s with the same function call"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `if/else` with the same body as the *then* part
/// and the *else* part.
///
/// ### Why is this bad?
/// This is probably a copy & paste error.
///
/// ### Example
/// ```ignore
/// let foo = if … {
/// 42
/// } else {
/// 42
/// };
/// ```
#[clippy::version = "pre 1.29.0"]
pub IF_SAME_THEN_ELSE,
style,
"`if` with the same `then` and `else` blocks"
}
declare_clippy_lint! {
/// ### What it does
/// Checks if the `if` and `else` block contain shared code that can be
/// moved out of the blocks.
///
/// ### Why is this bad?
/// Duplicate code is less maintainable.
///
/// ### Example
/// ```ignore
/// let foo = if … {
/// println!("Hello World");
/// 13
/// } else {
/// println!("Hello World");
/// 42
/// };
/// ```
///
/// Use instead:
/// ```ignore
/// println!("Hello World");
/// let foo = if … {
/// 13
/// } else {
/// 42
/// };
/// ```
#[clippy::version = "1.53.0"]
pub BRANCHES_SHARING_CODE,
nursery,
"`if` statement with shared code in all blocks"
}
pub struct CopyAndPaste<'tcx> {
interior_mut: InteriorMut<'tcx>,
}
impl<'tcx> CopyAndPaste<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, conf: &'static Conf) -> Self {
Self {
interior_mut: InteriorMut::new(tcx, &conf.ignore_interior_mutability),
}
}
}
impl_lint_pass!(CopyAndPaste<'_> => [
IFS_SAME_COND,
SAME_FUNCTIONS_IN_IF_CONDITION,
IF_SAME_THEN_ELSE,
BRANCHES_SHARING_CODE
]);
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste<'tcx> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
let (conds, blocks) = if_sequence(expr);
lint_same_cond(cx, &conds, &mut self.interior_mut);
lint_same_fns_in_if_cond(cx, &conds);
let all_same =
!is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks);
if !all_same && conds.len() != blocks.len() {
lint_branches_sharing_code(cx, &conds, &blocks, expr);
}
}
}
}
fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool {
let mut eq = SpanlessEq::new(cx);
blocks
.array_windows::<2>()
.enumerate()
.fold(true, |all_eq, (i, &[lhs, rhs])| {
if eq.eq_block(lhs, rhs) && !has_let_expr(conds[i]) && conds.get(i + 1).is_none_or(|e| !has_let_expr(e)) {
span_lint_and_note(
cx,
IF_SAME_THEN_ELSE,
lhs.span,
"this `if` has identical blocks",
Some(rhs.span),
"same as this",
);
all_eq
} else {
false
}
})
}
fn lint_branches_sharing_code<'tcx>(
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
conds: &[&'tcx Expr<'_>],
blocks: &[&'tcx Block<'_>],
@ -356,8 +161,8 @@ fn modifies_any_local<'tcx>(cx: &LateContext<'tcx>, s: &'tcx Stmt<'_>, locals: &
.is_some()
}
/// Checks if the given statement should be considered equal to the statement in the same position
/// for each block.
/// Checks if the given statement should be considered equal to the statement in the same
/// position for each block.
fn eq_stmts(
stmt: &Stmt<'_>,
blocks: &[&Block<'_>],
@ -516,9 +321,9 @@ fn scan_block_for_eq<'tcx>(
}
}
/// Adjusts the index for which the statements begin to differ to the closest macro callsite. This
/// avoids giving suggestions that requires splitting a macro call in half, when only a part of the
/// macro expansion is equal.
/// Adjusts the index for which the statements begin to differ to the closest macro callsite.
/// This avoids giving suggestions that requires splitting a macro call in half, when only a
/// part of the macro expansion is equal.
///
/// For example, for the following macro:
/// ```rust,ignore
@ -587,70 +392,6 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
})
}
fn method_caller_is_mutable<'tcx>(
cx: &LateContext<'tcx>,
caller_expr: &Expr<'_>,
interior_mut: &mut InteriorMut<'tcx>,
) -> bool {
let caller_ty = cx.typeck_results().expr_ty(caller_expr);
interior_mut.is_interior_mut_ty(cx, caller_ty)
|| caller_ty.is_mutable_ptr()
// `find_binding_init` will return the binding iff its not mutable
|| path_to_local(caller_expr)
.and_then(|hid| find_binding_init(cx, hid))
.is_none()
}
/// Implementation of `IFS_SAME_COND`.
fn lint_same_cond<'tcx>(cx: &LateContext<'tcx>, conds: &[&Expr<'_>], interior_mut: &mut InteriorMut<'tcx>) {
for group in search_same(
conds,
|e| hash_expr(cx, e),
|lhs, rhs| {
// Ignore eq_expr side effects iff one of the expression kind is a method call
// and the caller is not a mutable, including inner mutable type.
if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind {
if method_caller_is_mutable(cx, caller, interior_mut) {
false
} else {
SpanlessEq::new(cx).eq_expr(lhs, rhs)
}
} else {
eq_expr_value(cx, lhs, rhs)
}
},
) {
let spans: Vec<_> = group.into_iter().map(|expr| expr.span).collect();
span_lint(cx, IFS_SAME_COND, spans, "these `if` branches have the same condition");
}
}
/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
// Do not lint if any expr originates from a macro
if lhs.span.from_expansion() || rhs.span.from_expansion() {
return false;
}
// Do not spawn warning if `IFS_SAME_COND` already produced it.
if eq_expr_value(cx, lhs, rhs) {
return false;
}
SpanlessEq::new(cx).eq_expr(lhs, rhs)
};
for group in search_same(conds, |e| hash_expr(cx, e), eq) {
let spans: Vec<_> = group.into_iter().map(|expr| expr.span).collect();
span_lint(
cx,
SAME_FUNCTIONS_IN_IF_CONDITION,
spans,
"these `if` branches have the same function call",
);
}
}
fn is_expr_parent_assignment(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let parent = cx.tcx.parent_hir_node(expr.hir_id);
if let Node::LetStmt(LetStmt { init: Some(e), .. })

29
clippy_lints/src/ifs/if_same_then_else.rs Normal file
View file

@ -0,0 +1,29 @@
use clippy_utils::SpanlessEq;
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::higher::has_let_expr;
use rustc_hir::{Block, Expr};
use rustc_lint::LateContext;
use super::IF_SAME_THEN_ELSE;
pub(super) fn check(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> bool {
let mut eq = SpanlessEq::new(cx);
blocks
.array_windows::<2>()
.enumerate()
.fold(true, |all_eq, (i, &[lhs, rhs])| {
if eq.eq_block(lhs, rhs) && !has_let_expr(conds[i]) && conds.get(i + 1).is_none_or(|e| !has_let_expr(e)) {
span_lint_and_note(
cx,
IF_SAME_THEN_ELSE,
lhs.span,
"this `if` has identical blocks",
Some(rhs.span),
"same as this",
);
all_eq
} else {
false
}
})
}

46
clippy_lints/src/ifs/ifs_same_cond.rs Normal file
View file

@ -0,0 +1,46 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::InteriorMut;
use clippy_utils::{SpanlessEq, eq_expr_value, find_binding_init, hash_expr, path_to_local, search_same};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use super::IFS_SAME_COND;
fn method_caller_is_mutable<'tcx>(
cx: &LateContext<'tcx>,
caller_expr: &Expr<'_>,
interior_mut: &mut InteriorMut<'tcx>,
) -> bool {
let caller_ty = cx.typeck_results().expr_ty(caller_expr);
interior_mut.is_interior_mut_ty(cx, caller_ty)
|| caller_ty.is_mutable_ptr()
// `find_binding_init` will return the binding iff its not mutable
|| path_to_local(caller_expr)
.and_then(|hid| find_binding_init(cx, hid))
.is_none()
}
/// Implementation of `IFS_SAME_COND`.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, conds: &[&Expr<'_>], interior_mut: &mut InteriorMut<'tcx>) {
for group in search_same(
conds,
|e| hash_expr(cx, e),
|lhs, rhs| {
// Ignore eq_expr side effects iff one of the expression kind is a method call
// and the caller is not a mutable, including inner mutable type.
if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind {
if method_caller_is_mutable(cx, caller, interior_mut) {
false
} else {
SpanlessEq::new(cx).eq_expr(lhs, rhs)
}
} else {
eq_expr_value(cx, lhs, rhs)
}
},
) {
let spans: Vec<_> = group.into_iter().map(|expr| expr.span).collect();
span_lint(cx, IFS_SAME_COND, spans, "these `if` branches have the same condition");
}
}

182
clippy_lints/src/ifs/mod.rs Normal file
View file

@ -0,0 +1,182 @@
use clippy_config::Conf;
use clippy_utils::ty::InteriorMut;
use clippy_utils::{if_sequence, is_else_clause, is_lint_allowed};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass;
mod branches_sharing_code;
mod if_same_then_else;
mod ifs_same_cond;
mod same_functions_in_if_cond;
declare_clippy_lint! {
/// ### What it does
/// Checks for consecutive `if`s with the same condition.
///
/// ### Why is this bad?
/// This is probably a copy & paste error.
///
/// ### Example
/// ```ignore
/// if a == b {
/// …
/// } else if a == b {
/// …
/// }
/// ```
///
/// Note that this lint ignores all conditions with a function call as it could
/// have side effects:
///
/// ```ignore
/// if foo() {
/// …
/// } else if foo() { // not linted
/// …
/// }
/// ```
#[clippy::version = "pre 1.29.0"]
pub IFS_SAME_COND,
correctness,
"consecutive `if`s with the same condition"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for consecutive `if`s with the same function call.
///
/// ### Why is this bad?
/// This is probably a copy & paste error.
/// Despite the fact that function can have side effects and `if` works as
/// intended, such an approach is implicit and can be considered a "code smell".
///
/// ### Example
/// ```ignore
/// if foo() == bar {
/// …
/// } else if foo() == bar {
/// …
/// }
/// ```
///
/// This probably should be:
/// ```ignore
/// if foo() == bar {
/// …
/// } else if foo() == baz {
/// …
/// }
/// ```
///
/// or if the original code was not a typo and called function mutates a state,
/// consider move the mutation out of the `if` condition to avoid similarity to
/// a copy & paste error:
///
/// ```ignore
/// let first = foo();
/// if first == bar {
/// …
/// } else {
/// let second = foo();
/// if second == bar {
/// …
/// }
/// }
/// ```
#[clippy::version = "1.41.0"]
pub SAME_FUNCTIONS_IN_IF_CONDITION,
pedantic,
"consecutive `if`s with the same function call"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `if/else` with the same body as the *then* part
/// and the *else* part.
///
/// ### Why is this bad?
/// This is probably a copy & paste error.
///
/// ### Example
/// ```ignore
/// let foo = if … {
/// 42
/// } else {
/// 42
/// };
/// ```
#[clippy::version = "pre 1.29.0"]
pub IF_SAME_THEN_ELSE,
style,
"`if` with the same `then` and `else` blocks"
}
declare_clippy_lint! {
/// ### What it does
/// Checks if the `if` and `else` block contain shared code that can be
/// moved out of the blocks.
///
/// ### Why is this bad?
/// Duplicate code is less maintainable.
///
/// ### Example
/// ```ignore
/// let foo = if … {
/// println!("Hello World");
/// 13
/// } else {
/// println!("Hello World");
/// 42
/// };
/// ```
///
/// Use instead:
/// ```ignore
/// println!("Hello World");
/// let foo = if … {
/// 13
/// } else {
/// 42
/// };
/// ```
#[clippy::version = "1.53.0"]
pub BRANCHES_SHARING_CODE,
nursery,
"`if` statement with shared code in all blocks"
}
pub struct CopyAndPaste<'tcx> {
interior_mut: InteriorMut<'tcx>,
}
impl<'tcx> CopyAndPaste<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, conf: &'static Conf) -> Self {
Self {
interior_mut: InteriorMut::new(tcx, &conf.ignore_interior_mutability),
}
}
}
impl_lint_pass!(CopyAndPaste<'_> => [
IFS_SAME_COND,
SAME_FUNCTIONS_IN_IF_CONDITION,
IF_SAME_THEN_ELSE,
BRANCHES_SHARING_CODE
]);
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste<'tcx> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
let (conds, blocks) = if_sequence(expr);
ifs_same_cond::check(cx, &conds, &mut self.interior_mut);
same_functions_in_if_cond::check(cx, &conds);
let all_same =
!is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && if_same_then_else::check(cx, &conds, &blocks);
if !all_same && conds.len() != blocks.len() {
branches_sharing_code::check(cx, &conds, &blocks, expr);
}
}
}
}

31
clippy_lints/src/ifs/same_functions_in_if_cond.rs Normal file
View file

@ -0,0 +1,31 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::{SpanlessEq, eq_expr_value, hash_expr, search_same};
use rustc_hir::Expr;
use rustc_lint::LateContext;
use super::SAME_FUNCTIONS_IN_IF_CONDITION;
/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
pub(super) fn check(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
// Do not lint if any expr originates from a macro
if lhs.span.from_expansion() || rhs.span.from_expansion() {
return false;
}
// Do not spawn warning if `IFS_SAME_COND` already produced it.
if eq_expr_value(cx, lhs, rhs) {
return false;
}
SpanlessEq::new(cx).eq_expr(lhs, rhs)
};
for group in search_same(conds, |e| hash_expr(cx, e), eq) {
let spans: Vec<_> = group.into_iter().map(|expr| expr.span).collect();
span_lint(
cx,
SAME_FUNCTIONS_IN_IF_CONDITION,
spans,
"these `if` branches have the same function call",
);
}
}

4
clippy_lints/src/lib.rs
View file

@ -100,7 +100,6 @@ mod cognitive_complexity;
mod collapsible_if;
mod collection_is_never_read;
mod comparison_chain;
mod copies;
mod copy_iterator;
mod crate_in_macro_def;
mod create_dir;
@ -158,6 +157,7 @@ mod future_not_send;
mod if_let_mutex;
mod if_not_else;
mod if_then_some_else_none;
mod ifs;
mod ignored_unit_patterns;
mod impl_hash_with_borrow_str_and_bytes;
mod implicit_hasher;
@ -549,7 +549,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
store.register_late_pass(|_| Box::new(empty_enum::EmptyEnum));
store.register_late_pass(|_| Box::new(invalid_upcast_comparisons::InvalidUpcastComparisons));
store.register_late_pass(|_| Box::<regex::Regex>::default());
store.register_late_pass(move |tcx| Box::new(copies::CopyAndPaste::new(tcx, conf)));
store.register_late_pass(move |tcx| Box::new(ifs::CopyAndPaste::new(tcx, conf)));
store.register_late_pass(|_| Box::new(copy_iterator::CopyIterator));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(format::UselessFormat::new(format_args.clone())));