make [`ifs_same_cond`] use `ignore_interior_mutablility` configuration

6 files changed, 91 insertions, 17 deletions

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() {}