fix(mut_mutex_lock): don't lint on indexing and field accesses

clippy_lints/src/methods/mod.rs

@@ -2519,6 +2519,10 @@ declare_clippy_lint! {
     /// guarantee that the mutex isn't locked, instead of just a runtime
     /// guarantee.
     ///
+    /// ### Known problems
+    ///
+    /// This lint doesn't take lifetimes into account.
+    ///
     /// ### Example
     /// ```no_run
     /// use std::sync::{Arc, Mutex};
@@ -5592,7 +5596,7 @@ impl Methods {
                     }
                 },
                 (sym::lock, []) => {
-                    mut_mutex_lock::check(cx, expr, recv, span);
+                    mut_mutex_lock::check(cx, recv, span);
                 },
                 (name @ (sym::map | sym::map_err), [m_arg]) => {
                     if name == sym::map {

clippy_lints/src/methods/mut_mutex_lock.rs

@@ -1,21 +1,41 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::expr_custom_deref_adjustment;
 use clippy_utils::res::MaybeDef as _;
-use clippy_utils::ty::peel_and_count_ty_refs;
+use clippy_utils::ty::{implements_trait, peel_and_count_ty_refs};
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, Mutability};
+use rustc_hir::{Expr, ExprKind, Mutability, UnOp};
 use rustc_lint::LateContext;
+use rustc_middle::ty::adjustment::{Adjust, DerefAdjustKind};
 use rustc_span::{Span, sym};
 
 use super::MUT_MUTEX_LOCK;
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>, recv: &'tcx Expr<'tcx>, name_span: Span) {
-    if matches!(expr_custom_deref_adjustment(cx, recv), None | Some(Mutability::Mut))
-        && let (_, _, Some(Mutability::Mut)) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(recv))
-        && let Some(method_id) = cx.typeck_results().type_dependent_def_id(ex.hir_id)
-        && let Some(impl_id) = cx.tcx.impl_of_assoc(method_id)
-        && cx.tcx.type_of(impl_id).is_diag_item(cx, sym::Mutex)
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, recv: &'tcx Expr<'tcx>, name_span: Span) {
+    // Make sure that we have a mutable access to `Mutex`:
+    // 1. Check that the receiver is behind one or more `&mut`s -- we want to have mutable access, while
+    //    not outright owning it -- if the latter were the case, then changing `.lock()` to
+    //    `.get_mut()`, could easily result in a conflict with other existing immutable borrows.
+    //
+    // NOTE: `mutbl` being `Some` is enough to determine that there was at least one layer of
+    // references, so no need to check `n`
+    // NOTE: the reason we don't use `expr_ty_adjusted` here is that a call
+    // to `Mutex::lock` by itself adjusts the receiver to be `&Mutex`
+    if let (recv_ty, _, Some(Mutability::Mut)) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(recv))
+        && recv_ty.is_diag_item(cx, sym::Mutex)
+        // 2. The mutex was accessed either directly (`mutex.lock()`), or through a series of
+        // deref/field/indexing projections. Since the final `.lock()` call only requires `&Mutex`,
+        // those might be immutable, and so we need to manually check whether mutable projections
+        // would've been possible. For that, we'll repeatedly peel off projections and check each
+        // intermediary receiver.
+        && let Some(recv) = check_projections(cx, recv)
     {
+        if let ExprKind::Path(ref p) = recv.kind
+            && let Some(did) = cx.qpath_res(p, recv.hir_id).opt_def_id()
+            && cx.tcx.is_static(did)
+        {
+            // Don't lint on statics
+            return;
+        }
+
         span_lint_and_sugg(
             cx,
             MUT_MUTEX_LOCK,
@@ -27,3 +47,60 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>, recv: &'
         );
     }
 }
+
+/// See the comment at the callsite.
+///
+/// Returns the innermost receiver if all the projections could be made mutable.
+fn check_projections<'tcx>(cx: &LateContext<'tcx>, mut recv: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    let deref_mut_trait = cx.tcx.lang_items().deref_mut_trait();
+    let impls_deref_mut = |ty| deref_mut_trait.is_some_and(|trait_id| implements_trait(cx, ty, trait_id, &[]));
+    let typeck = cx.typeck_results();
+
+    loop {
+        // Check that the (deref) adjustments could be made mutable
+        #[expect(clippy::question_mark, reason = "the suggested form is confusing")]
+        if (typeck.expr_adjustments(recv))
+            .iter()
+            .map_while(|a| match a.kind {
+                Adjust::Deref(x) => Some((a.target, x)),
+                _ => None,
+            })
+            .try_fold(typeck.expr_ty(recv), |ty, (target, deref)| match deref {
+                // There has been an overloaded deref, most likely an immutable one, as `.lock()` didn't require a
+                // mutable one -- we need to check if a mutable deref would've been possible, i.e. if
+                // `ty: DerefMut<Target = target>` (we don't need to check the `Target` part, as `Deref` and
+                // `DerefMut` impls necessarily have the same one)
+                DerefAdjustKind::Overloaded(_) => impls_deref_mut(ty).then_some(target),
+                // There has been a simple deref; if it happened on a `&T`, then we know it will can't be changed to
+                // provide mutable access
+                DerefAdjustKind::Builtin => (ty.ref_mutability() != Some(Mutability::Not)).then_some(target),
+                DerefAdjustKind::Pin => Some(target),
+            })
+            .is_none()
+        {
+            return None;
+        }
+
+        // Peel off one projection
+        match recv.kind {
+            ExprKind::Unary(UnOp::Deref, base) => {
+                if impls_deref_mut(typeck.expr_ty_adjusted(base)) {
+                    recv = base;
+                } else {
+                    return None;
+                }
+            },
+            ExprKind::Index(..) | ExprKind::Field(..) => {
+                // We don't want to lint on indexing and field accesses, as both of those would take exclusive
+                // access to only part of a value -- which would conflict with any immutable reborrow over
+                // the whole value
+                return None;
+            },
+            _ => {
+                // No more projections to check
+                break;
+            },
+        }
+    }
+    Some(recv)
+}

clippy_utils/src/lib.rs

@@ -467,10 +467,9 @@ pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, owner: OwnerId) -> Opti
 /// This method will return tuple of projection stack and root of the expression,
 /// used in `can_mut_borrow_both`.
 ///
-/// For example, if `e` represents the `v[0].a.b[x]`
-/// this method will return a tuple, composed of a `Vec`
-/// containing the `Expr`s for `v[0], v[0].a, v[0].a.b, v[0].a.b[x]`
-/// and an `Expr` for root of them, `v`
+/// For example, if `e` represents the `v[0].a.b[x]` this method will return a tuple, composed of:
+/// - a `Vec` containing the `Expr`s for `v[0], v[0].a, v[0].a.b, v[0].a.b[x]`
+/// - and an `Expr` for root of them, `v`
 fn projection_stack<'a, 'hir>(
     mut e: &'a Expr<'hir>,
     ctxt: SyntaxContext,
@@ -500,7 +499,8 @@ pub fn expr_custom_deref_adjustment(cx: &LateContext<'_>, e: &Expr<'_>) -> Optio
             Adjust::Deref(DerefAdjustKind::Builtin) => None,
             _ => Some(None),
         })
-        .and_then(|x| x)
+        // if there were no adjustments to begin with, trivially none of them are of the custom-deref kind
+        .unwrap_or(None)
 }
 
 /// Checks if two expressions can be mutably borrowed simultaneously

tests/ui/mut_mutex_lock.fixed

@@ -37,4 +37,47 @@ fn mut_ref_ref_mutex_lock() {
     *guard += 1;
 }
 
+mod issue16253 {
+    use std::sync::{Arc, Mutex};
+
+    // Do not lint on an owned value since it has a good chance of already having another active borrow.
+    fn dont_lint_owned(m: Mutex<i32>) {
+        m.lock();
+    }
+
+    struct Wrapper {
+        arc: Arc<Mutex<i32>>,
+        ref_: &'static Mutex<i32>,
+        ref_mut: &'static mut Mutex<i32>,
+        owned: Mutex<i32>,
+    }
+
+    // Do not lint, even if the projection chain would theoretically allow mutable access
+    fn field(w: Wrapper, ref_w: &Wrapper, refmut_w: &mut Wrapper) {
+        w.arc.lock();
+        w.ref_.lock();
+        w.ref_mut.lock();
+        w.owned.lock();
+        ref_w.arc.lock();
+        ref_w.ref_.lock();
+        ref_w.ref_mut.lock();
+        ref_w.owned.lock();
+        refmut_w.arc.lock();
+        refmut_w.ref_.lock();
+        refmut_w.ref_mut.lock();
+        refmut_w.owned.lock();
+    }
+
+    // Do not lint, even if the `.index()` could be turned into `.index_mut()`
+    fn index(mutexes: &mut [Mutex<u32>]) {
+        // even though `[0]` is _currently_ an `.index(0)`, it can be turned into `.index_mut()` to
+        // enable mutable access: `&mut [Mutex] -> &mut Mutex`
+        mutexes[0].lock().unwrap();
+
+        // `exes` is `&[Mutex] = &Mutex`, so we can't get to `&mut Mutex` no matter what
+        let exes: &_ = mutexes;
+        exes[0].lock().unwrap();
+    }
+}
+
 fn main() {}

tests/ui/mut_mutex_lock.rs

@@ -37,4 +37,47 @@ fn mut_ref_ref_mutex_lock() {
     *guard += 1;
 }
 
+mod issue16253 {
+    use std::sync::{Arc, Mutex};
+
+    // Do not lint on an owned value since it has a good chance of already having another active borrow.
+    fn dont_lint_owned(m: Mutex<i32>) {
+        m.lock();
+    }
+
+    struct Wrapper {
+        arc: Arc<Mutex<i32>>,
+        ref_: &'static Mutex<i32>,
+        ref_mut: &'static mut Mutex<i32>,
+        owned: Mutex<i32>,
+    }
+
+    // Do not lint, even if the projection chain would theoretically allow mutable access
+    fn field(w: Wrapper, ref_w: &Wrapper, refmut_w: &mut Wrapper) {
+        w.arc.lock();
+        w.ref_.lock();
+        w.ref_mut.lock();
+        w.owned.lock();
+        ref_w.arc.lock();
+        ref_w.ref_.lock();
+        ref_w.ref_mut.lock();
+        ref_w.owned.lock();
+        refmut_w.arc.lock();
+        refmut_w.ref_.lock();
+        refmut_w.ref_mut.lock();
+        refmut_w.owned.lock();
+    }
+
+    // Do not lint, even if the `.index()` could be turned into `.index_mut()`
+    fn index(mutexes: &mut [Mutex<u32>]) {
+        // even though `[0]` is _currently_ an `.index(0)`, it can be turned into `.index_mut()` to
+        // enable mutable access: `&mut [Mutex] -> &mut Mutex`
+        mutexes[0].lock().unwrap();
+
+        // `exes` is `&[Mutex] = &Mutex`, so we can't get to `&mut Mutex` no matter what
+        let exes: &_ = mutexes;
+        exes[0].lock().unwrap();
+    }
+}
+
 fn main() {}