From 8a9492aa03792f17c7b63b15d10ae9813daa4c4a Mon Sep 17 00:00:00 2001
From: J-ZhengLi <lizheng135@huawei.com>
Date: Thu, 16 Feb 2023 17:47:28 +0800
Subject: [PATCH 1/4] enhance [`ifs_same_cond`] to lint same immutable method
calls as well
---
clippy_lints/src/copies.rs | 26 +++++++++++++++++++++++---
tests/ui/ifs_same_cond.rs | 10 ++++++++++
tests/ui/ifs_same_cond.stderr | 14 +++++++++++++-
3 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
index f10c35cde52..d729a5430c7 100644
--- a/clippy_lints/src/copies.rs
+++ b/clippy_lints/src/copies.rs
@@ -3,8 +3,8 @@ 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, 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, 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;
@@ -549,7 +549,27 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
/// Implementation of `IFS_SAME_COND`.
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
- for (i, j) in search_same(conds, |e| hash_expr(cx, e), |lhs, rhs| eq_expr_value(cx, lhs, rhs)) {
+ 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 {
+ false
+ }
+ } else {
+ eq_expr_value(cx, lhs, rhs)
+ }
+ },
+ ) {
span_lint_and_note(
cx,
IFS_SAME_COND,
diff --git a/tests/ui/ifs_same_cond.rs b/tests/ui/ifs_same_cond.rs
index 9850fc0919e..e68ec6a8573 100644
--- a/tests/ui/ifs_same_cond.rs
+++ b/tests/ui/ifs_same_cond.rs
@@ -43,4 +43,14 @@ fn ifs_same_cond() {
}
}
+fn issue10272() {
+ let a = String::from("ha");
+ if a.contains("ah") {
+ } else if a.contains("ah") {
+ // Trigger this lint
+ } else if a.contains("ha") {
+ } else if a == "wow" {
+ }
+}
+
fn main() {}
diff --git a/tests/ui/ifs_same_cond.stderr b/tests/ui/ifs_same_cond.stderr
index 4113087327a..9519f6904cb 100644
--- a/tests/ui/ifs_same_cond.stderr
+++ b/tests/ui/ifs_same_cond.stderr
@@ -35,5 +35,17 @@ note: same as this
LL | if 2 * a == 1 {
| ^^^^^^^^^^
-error: aborting due to 3 previous errors
+error: this `if` has the same condition as a previous `if`
+ --> $DIR/ifs_same_cond.rs:49:15
+ |
+LL | } else if a.contains("ah") {
+ | ^^^^^^^^^^^^^^^^
+ |
+note: same as this
+ --> $DIR/ifs_same_cond.rs:48:8
+ |
+LL | if a.contains("ah") {
+ | ^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
From f0ae2b71ca4053db3b86c20e7d612ecc11d50ee2 Mon Sep 17 00:00:00 2001
From: J-ZhengLi <lizheng135@huawei.com>
Date: Fri, 24 Feb 2023 10:46:07 +0800
Subject: [PATCH 2/4] make [`ifs_same_cond`] use `ignore_interior_mutablility`
configuration
---
clippy_lints/src/copies.rs | 70 +++++++++++++++-----
clippy_lints/src/lib.rs | 3 +-
clippy_lints/src/utils/conf.rs | 2 +-
tests/ui-toml/ifs_same_cond/clippy.toml | 1 +
tests/ui-toml/ifs_same_cond/ifs_same_cond.rs | 24 +++++++
tests/ui/ifs_same_cond.rs | 8 +++
6 files changed, 91 insertions(+), 17 deletions(-)
create mode 100644 tests/ui-toml/ifs_same_cond/clippy.toml
create mode 100644 tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
index d729a5430c7..39f8f7220f1 100644
--- a/clippy_lints/src/copies.rs
+++ b/clippy_lints/src/copies.rs
@@ -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)
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 491732be208..bde84686cc1 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -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));
diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
index 1c7f3e96db8..8ba252425a3 100644
--- a/clippy_lints/src/utils/conf.rs
+++ b/clippy_lints/src/utils/conf.rs
@@ -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
diff --git a/tests/ui-toml/ifs_same_cond/clippy.toml b/tests/ui-toml/ifs_same_cond/clippy.toml
new file mode 100644
index 00000000000..1615d970c68
--- /dev/null
+++ b/tests/ui-toml/ifs_same_cond/clippy.toml
@@ -0,0 +1 @@
+ignore-interior-mutability = ["std::cell::Cell"]
\ No newline at end of file
diff --git a/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs b/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
new file mode 100644
index 00000000000..92438e7d1f2
--- /dev/null
+++ b/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
@@ -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 {
+ }
+}
diff --git a/tests/ui/ifs_same_cond.rs b/tests/ui/ifs_same_cond.rs
index e68ec6a8573..ae91611c472 100644
--- a/tests/ui/ifs_same_cond.rs
+++ b/tests/ui/ifs_same_cond.rs
@@ -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() {}
From f4ccb06d699b91b8de8b797a584ce185d6856ec2 Mon Sep 17 00:00:00 2001
From: J-ZhengLi <lizheng135@huawei.com>
Date: Mon, 13 Mar 2023 20:17:30 +0800
Subject: [PATCH 3/4] extract `is_interior_mutable_type` from [`mut_key`] to
`clippy_utils::ty`;
fix configuration of [`ifs_same_cond`];
add some style improvement for [`ifs_same_cond`];
---
clippy_lints/src/copies.rs | 52 +++++++---------
clippy_lints/src/mut_key.rs | 60 ++++---------------
clippy_utils/src/ty.rs | 44 ++++++++++++++
tests/ui-toml/ifs_same_cond/clippy.toml | 2 +-
tests/ui-toml/ifs_same_cond/ifs_same_cond.rs | 12 +---
.../ifs_same_cond/ifs_same_cond.stderr | 15 +++++
tests/ui/ifs_same_cond.rs | 8 +++
7 files changed, 107 insertions(+), 86 deletions(-)
create mode 100644 tests/ui-toml/ifs_same_cond/ifs_same_cond.stderr
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
index 39f8f7220f1..970f5004993 100644
--- a/clippy_lints/src/copies.rs
+++ b/clippy_lints/src/copies.rs
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
-use clippy_utils::ty::needs_ordered_drop;
+use clippy_utils::ty::{is_interior_mut_ty, needs_ordered_drop};
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{
capture_local_usage, def_path_def_ids, eq_expr_value, find_binding_init, get_enclosing_block, hash_expr, hash_stmt,
@@ -8,9 +8,8 @@ use clippy_utils::{
};
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::def_id::DefIdSet;
use rustc_hir::intravisit;
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, HirIdSet, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
@@ -164,12 +163,14 @@ declare_clippy_lint! {
pub struct CopyAndPaste {
ignore_interior_mutability: Vec<String>,
+ ignored_ty_ids: DefIdSet,
}
impl CopyAndPaste {
pub fn new(ignore_interior_mutability: Vec<String>) -> Self {
Self {
ignore_interior_mutability,
+ ignored_ty_ids: DefIdSet::new(),
}
}
}
@@ -182,17 +183,18 @@ impl_lint_pass!(CopyAndPaste => [
]);
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
+ fn check_crate(&mut self, cx: &LateContext<'tcx>) {
+ 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()) {
+ self.ignored_ty_ids.insert(id);
+ }
+ }
+ }
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);
- 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_cond(cx, &conds, &self.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);
@@ -569,38 +571,30 @@ 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 {
+fn method_caller_is_mutable(cx: &LateContext<'_>, caller_expr: &Expr<'_>, ignored_ty_ids: &DefIdSet) -> 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
- };
+ // Check if given type has inner mutability and was not set to ignored by the configuration
+ let is_inner_mut_ty = is_interior_mut_ty(cx, caller_ty)
+ && !matches!(caller_ty.ty_adt_id(), Some(adt_id) if ignored_ty_ids.contains(&adt_id));
- if is_ignored_ty
+ is_inner_mut_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()
- {
- return true;
- }
-
- false
}
/// Implementation of `IFS_SAME_COND`.
-fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>], ignored_ty_ids: &FxHashSet<DefId>) {
+fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>], ignored_ty_ids: &DefIdSet) {
for (i, j) in search_same(
conds,
|e| hash_expr(cx, e),
|lhs, rhs| {
+ // Ignore eq_expr side effects iff one of the expressin 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_ignored_or_mutable(cx, caller, ignored_ty_ids) {
+ if method_caller_is_mutable(cx, caller, ignored_ty_ids) {
false
} else {
SpanlessEq::new(cx).eq_expr(lhs, rhs)
diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs
index 8aa814b7405..309f67521a3 100644
--- a/clippy_lints/src/mut_key.rs
+++ b/clippy_lints/src/mut_key.rs
@@ -1,10 +1,11 @@
use clippy_utils::diagnostics::span_lint;
+use clippy_utils::ty::is_interior_mut_ty;
use clippy_utils::{def_path_def_ids, trait_ref_of_method};
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::TypeVisitableExt;
-use rustc_middle::ty::{Adt, Array, Ref, Slice, Tuple, Ty};
+use rustc_middle::query::Key;
+use rustc_middle::ty::{Adt, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::LocalDefId;
use rustc_span::source_map::Span;
@@ -153,53 +154,18 @@ impl MutableKeyType {
let is_keyed_type = [sym::HashMap, sym::BTreeMap, sym::HashSet, sym::BTreeSet]
.iter()
.any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did()));
- if is_keyed_type && self.is_interior_mutable_type(cx, substs.type_at(0)) {
+ if !is_keyed_type {
+ return;
+ }
+
+ let subst_ty = substs.type_at(0);
+ // Determines if a type contains interior mutability which would affect its implementation of
+ // [`Hash`] or [`Ord`].
+ if is_interior_mut_ty(cx, subst_ty)
+ && !matches!(subst_ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id))
+ {
span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
}
}
}
-
- /// Determines if a type contains interior mutability which would affect its implementation of
- /// [`Hash`] or [`Ord`].
- fn is_interior_mutable_type<'tcx>(&self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
- match *ty.kind() {
- Ref(_, inner_ty, mutbl) => mutbl == hir::Mutability::Mut || self.is_interior_mutable_type(cx, inner_ty),
- Slice(inner_ty) => self.is_interior_mutable_type(cx, inner_ty),
- Array(inner_ty, size) => {
- size.try_eval_target_usize(cx.tcx, cx.param_env)
- .map_or(true, |u| u != 0)
- && self.is_interior_mutable_type(cx, inner_ty)
- },
- Tuple(fields) => fields.iter().any(|ty| self.is_interior_mutable_type(cx, ty)),
- Adt(def, substs) => {
- // Special case for collections in `std` who's impl of `Hash` or `Ord` delegates to
- // that of their type parameters. Note: we don't include `HashSet` and `HashMap`
- // because they have no impl for `Hash` or `Ord`.
- let def_id = def.did();
- let is_std_collection = [
- sym::Option,
- sym::Result,
- sym::LinkedList,
- sym::Vec,
- sym::VecDeque,
- sym::BTreeMap,
- sym::BTreeSet,
- sym::Rc,
- sym::Arc,
- ]
- .iter()
- .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def_id));
- let is_box = Some(def_id) == cx.tcx.lang_items().owned_box();
- if is_std_collection || is_box || self.ignore_mut_def_ids.contains(&def_id) {
- // The type is mutable if any of its type parameters are
- substs.types().any(|ty| self.is_interior_mutable_type(cx, ty))
- } else {
- !ty.has_escaping_bound_vars()
- && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok()
- && !ty.is_freeze(cx.tcx, cx.param_env)
- }
- },
- _ => false,
- }
- }
}
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
index e0ea3952785..f1c6f1dddd8 100644
--- a/clippy_utils/src/ty.rs
+++ b/clippy_utils/src/ty.rs
@@ -1121,3 +1121,47 @@ pub fn make_normalized_projection<'tcx>(
}
helper(tcx, param_env, make_projection(tcx, container_id, assoc_ty, substs)?)
}
+
+/// Check if given type has inner mutability such as [`std::cell::Cell`] or [`std::cell::RefCell`]
+/// etc.
+pub fn is_interior_mut_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+ match *ty.kind() {
+ ty::Ref(_, inner_ty, mutbl) => mutbl == Mutability::Mut || is_interior_mut_ty(cx, inner_ty),
+ ty::Slice(inner_ty) => is_interior_mut_ty(cx, inner_ty),
+ ty::Array(inner_ty, size) => {
+ size.try_eval_target_usize(cx.tcx, cx.param_env)
+ .map_or(true, |u| u != 0)
+ && is_interior_mut_ty(cx, inner_ty)
+ },
+ ty::Tuple(fields) => fields.iter().any(|ty| is_interior_mut_ty(cx, ty)),
+ ty::Adt(def, substs) => {
+ // Special case for collections in `std` who's impl of `Hash` or `Ord` delegates to
+ // that of their type parameters. Note: we don't include `HashSet` and `HashMap`
+ // because they have no impl for `Hash` or `Ord`.
+ let def_id = def.did();
+ let is_std_collection = [
+ sym::Option,
+ sym::Result,
+ sym::LinkedList,
+ sym::Vec,
+ sym::VecDeque,
+ sym::BTreeMap,
+ sym::BTreeSet,
+ sym::Rc,
+ sym::Arc,
+ ]
+ .iter()
+ .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def_id));
+ let is_box = Some(def_id) == cx.tcx.lang_items().owned_box();
+ if is_std_collection || is_box {
+ // The type is mutable if any of its type parameters are
+ substs.types().any(|ty| is_interior_mut_ty(cx, ty))
+ } else {
+ !ty.has_escaping_bound_vars()
+ && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok()
+ && !ty.is_freeze(cx.tcx, cx.param_env)
+ }
+ },
+ _ => false,
+ }
+}
diff --git a/tests/ui-toml/ifs_same_cond/clippy.toml b/tests/ui-toml/ifs_same_cond/clippy.toml
index 1615d970c68..90a36ecd920 100644
--- a/tests/ui-toml/ifs_same_cond/clippy.toml
+++ b/tests/ui-toml/ifs_same_cond/clippy.toml
@@ -1 +1 @@
-ignore-interior-mutability = ["std::cell::Cell"]
\ No newline at end of file
+ignore-interior-mutability = ["std::cell::Cell"]
diff --git a/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs b/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
index 92438e7d1f2..d623ac7e020 100644
--- a/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
+++ b/tests/ui-toml/ifs_same_cond/ifs_same_cond.rs
@@ -6,19 +6,13 @@ fn main() {}
fn issue10272() {
use std::cell::Cell;
+ // Because the `ignore-interior-mutability` configuration
+ // is set to ignore for `std::cell::Cell`, the following `get()` calls
+ // should trigger warning
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 {
}
}
diff --git a/tests/ui-toml/ifs_same_cond/ifs_same_cond.stderr b/tests/ui-toml/ifs_same_cond/ifs_same_cond.stderr
new file mode 100644
index 00000000000..2841f62bc94
--- /dev/null
+++ b/tests/ui-toml/ifs_same_cond/ifs_same_cond.stderr
@@ -0,0 +1,15 @@
+error: this `if` has the same condition as a previous `if`
+ --> $DIR/ifs_same_cond.rs:15:15
+ |
+LL | } else if x.get() {
+ | ^^^^^^^
+ |
+note: same as this
+ --> $DIR/ifs_same_cond.rs:13:8
+ |
+LL | if x.get() {
+ | ^^^^^^^
+ = note: `-D clippy::ifs-same-cond` implied by `-D warnings`
+
+error: aborting due to previous error
+
diff --git a/tests/ui/ifs_same_cond.rs b/tests/ui/ifs_same_cond.rs
index ae91611c472..9ce9a87626a 100644
--- a/tests/ui/ifs_same_cond.rs
+++ b/tests/ui/ifs_same_cond.rs
@@ -59,6 +59,14 @@ fn issue10272() {
// ok, p is mutable pointer
} else {
}
+
+ let x = std::cell::Cell::new(true);
+ if x.get() {
+ } else if !x.take() {
+ } else if x.get() {
+ // ok, x is interior mutable type
+ } else {
+ }
}
fn main() {}
From 011bb463370aceee33d7f00a39d87ae8fc856a1c Mon Sep 17 00:00:00 2001
From: J-ZhengLi <lizheng135@huawei.com>
Date: Tue, 14 Mar 2023 10:24:28 +0800
Subject: [PATCH 4/4] update lint configuration doc for [`ifs_same_cond`]
---
book/src/lint_configuration.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md
index 995dd2f04b1..9ed6627b741 100644
--- a/book/src/lint_configuration.md
+++ b/book/src/lint_configuration.md
@@ -519,6 +519,7 @@ for the generic parameters for determining interior mutability
**Default Value:** `["bytes::Bytes"]` (`Vec<String>`)
* [mutable_key_type](https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type)
+* [ifs_same_cond](https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond)
### allow-mixed-uninlined-format-args