mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 23:24:24 +00:00
make [ifs_same_cond
] use ignore_interior_mutablility
configuration
This commit is contained in:
parent
8a9492aa03
commit
f0ae2b71ca
6 changed files with 91 additions and 17 deletions
|
@ -3,16 +3,19 @@ use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, sn
|
|||
use clippy_utils::ty::needs_ordered_drop;
|
||||
use clippy_utils::visitors::for_each_expr;
|
||||
use clippy_utils::{
|
||||
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, def_path_def_ids, 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,
|
||||
};
|
||||
use core::iter;
|
||||
use core::ops::ControlFlow;
|
||||
use rustc_data_structures::fx::FxHashSet;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::def_id::DefId;
|
||||
use rustc_hir::intravisit;
|
||||
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, HirIdSet, Stmt, StmtKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_middle::query::Key;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::hygiene::walk_chain;
|
||||
use rustc_span::source_map::SourceMap;
|
||||
use rustc_span::{BytePos, Span, Symbol};
|
||||
|
@ -159,7 +162,19 @@ declare_clippy_lint! {
|
|||
"`if` statement with shared code in all blocks"
|
||||
}
|
||||
|
||||
declare_lint_pass!(CopyAndPaste => [
|
||||
pub struct CopyAndPaste {
|
||||
ignore_interior_mutability: Vec<String>,
|
||||
}
|
||||
|
||||
impl CopyAndPaste {
|
||||
pub fn new(ignore_interior_mutability: Vec<String>) -> Self {
|
||||
Self {
|
||||
ignore_interior_mutability,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl_lint_pass!(CopyAndPaste => [
|
||||
IFS_SAME_COND,
|
||||
SAME_FUNCTIONS_IN_IF_CONDITION,
|
||||
IF_SAME_THEN_ELSE,
|
||||
|
@ -170,7 +185,14 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
|
|||
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);
|
||||
let mut ignored_ty_ids = FxHashSet::default();
|
||||
for ignored_ty in &self.ignore_interior_mutability {
|
||||
let path: Vec<&str> = ignored_ty.split("::").collect();
|
||||
for id in def_path_def_ids(cx, path.as_slice()) {
|
||||
ignored_ty_ids.insert(id);
|
||||
}
|
||||
}
|
||||
lint_same_cond(cx, &conds, &ignored_ty_ids);
|
||||
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);
|
||||
|
@ -547,23 +569,41 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
|
|||
})
|
||||
}
|
||||
|
||||
fn method_caller_is_ignored_or_mutable(
|
||||
cx: &LateContext<'_>,
|
||||
caller_expr: &Expr<'_>,
|
||||
ignored_ty_ids: &FxHashSet<DefId>,
|
||||
) -> bool {
|
||||
let caller_ty = cx.typeck_results().expr_ty(caller_expr);
|
||||
let is_ignored_ty = if let Some(adt_id) = caller_ty.ty_adt_id() && ignored_ty_ids.contains(&adt_id) {
|
||||
true
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if is_ignored_ty
|
||||
|| caller_ty.is_mutable_ptr()
|
||||
|| path_to_local(caller_expr)
|
||||
.and_then(|hid| find_binding_init(cx, hid))
|
||||
.is_none()
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
/// Implementation of `IFS_SAME_COND`.
|
||||
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
|
||||
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>], ignored_ty_ids: &FxHashSet<DefId>) {
|
||||
for (i, j) in search_same(
|
||||
conds,
|
||||
|e| hash_expr(cx, e),
|
||||
|lhs, rhs| {
|
||||
// If any side (ex. lhs) is a method call, and the caller is not mutable,
|
||||
// then we can ignore side effects?
|
||||
if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind {
|
||||
if path_to_local(caller)
|
||||
.and_then(|hir_id| find_binding_init(cx, hir_id))
|
||||
.is_some()
|
||||
{
|
||||
// caller is not declared as mutable
|
||||
SpanlessEq::new(cx).eq_expr(lhs, rhs)
|
||||
} else {
|
||||
if method_caller_is_ignored_or_mutable(cx, caller, ignored_ty_ids) {
|
||||
false
|
||||
} else {
|
||||
SpanlessEq::new(cx).eq_expr(lhs, rhs)
|
||||
}
|
||||
} else {
|
||||
eq_expr_value(cx, lhs, rhs)
|
||||
|
|
|
@ -656,7 +656,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(|_| Box::new(empty_enum::EmptyEnum));
|
||||
store.register_late_pass(|_| Box::new(invalid_upcast_comparisons::InvalidUpcastComparisons));
|
||||
store.register_late_pass(|_| Box::new(regex::Regex));
|
||||
store.register_late_pass(|_| Box::new(copies::CopyAndPaste));
|
||||
let ignore_interior_mutability = conf.ignore_interior_mutability.clone();
|
||||
store.register_late_pass(move |_| Box::new(copies::CopyAndPaste::new(ignore_interior_mutability.clone())));
|
||||
store.register_late_pass(|_| Box::new(copy_iterator::CopyIterator));
|
||||
store.register_late_pass(|_| Box::new(format::UselessFormat));
|
||||
store.register_late_pass(|_| Box::new(swap::Swap));
|
||||
|
|
|
@ -437,7 +437,7 @@ define_Conf! {
|
|||
///
|
||||
/// The maximum size of the `Err`-variant in a `Result` returned from a function
|
||||
(large_error_threshold: u64 = 128),
|
||||
/// Lint: MUTABLE_KEY_TYPE.
|
||||
/// Lint: MUTABLE_KEY_TYPE, IFS_SAME_COND.
|
||||
///
|
||||
/// A list of paths to types that should be treated like `Arc`, i.e. ignored but
|
||||
/// for the generic parameters for determining interior mutability
|
||||
|
|
1
tests/ui-toml/ifs_same_cond/clippy.toml
Normal file
1
tests/ui-toml/ifs_same_cond/clippy.toml
Normal file
|
@ -0,0 +1 @@
|
|||
ignore-interior-mutability = ["std::cell::Cell"]
|
24
tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
Normal file
24
tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
Normal file
|
@ -0,0 +1,24 @@
|
|||
#![warn(clippy::ifs_same_cond)]
|
||||
#![allow(clippy::if_same_then_else, clippy::comparison_chain)]
|
||||
|
||||
fn main() {}
|
||||
|
||||
fn issue10272() {
|
||||
use std::cell::Cell;
|
||||
|
||||
let x = Cell::new(true);
|
||||
if x.get() {
|
||||
} else if !x.take() {
|
||||
} else if x.get() {
|
||||
// ok, x is interior mutable type
|
||||
} else {
|
||||
}
|
||||
|
||||
let a = [Cell::new(true)];
|
||||
if a[0].get() {
|
||||
} else if a[0].take() {
|
||||
} else if a[0].get() {
|
||||
// ok, a contains interior mutable type
|
||||
} else {
|
||||
}
|
||||
}
|
|
@ -51,6 +51,14 @@ fn issue10272() {
|
|||
} else if a.contains("ha") {
|
||||
} else if a == "wow" {
|
||||
}
|
||||
|
||||
let p: *mut i8 = std::ptr::null_mut();
|
||||
if p.is_null() {
|
||||
} else if p.align_offset(0) == 0 {
|
||||
} else if p.is_null() {
|
||||
// ok, p is mutable pointer
|
||||
} else {
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
Loading…
Reference in a new issue