fix(mut_mutex_lock): FP in projection chains

clippy_lints/src/methods/mod.rs

@@ -5360,7 +5360,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,29 +1,104 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::expr_custom_deref_adjustment;
 use clippy_utils::res::MaybeDef;
-use clippy_utils::ty::peel_and_count_ty_refs;
+use clippy_utils::ty::implements_trait;
 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;
 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)
-    {
-        span_lint_and_sugg(
-            cx,
-            MUT_MUTEX_LOCK,
-            name_span,
-            "calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference",
-            "change this to",
-            "get_mut".to_owned(),
-            Applicability::MaybeIncorrect,
-        );
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, recv: &'tcx Expr<'tcx>, name_span: Span) {
+    let typeck = cx.typeck_results();
+    if !typeck.expr_ty_adjusted(recv).peel_refs().is_diag_item(cx, sym::Mutex) {
+        return;
     }
+
+    let deref_mut_trait = cx.tcx.lang_items().deref_mut_trait();
+    let index_mut_trait = cx.tcx.lang_items().index_mut_trait();
+    let impls_deref_mut = |ty| deref_mut_trait.is_some_and(|trait_id| implements_trait(cx, ty, trait_id, &[]));
+    let impls_index_mut = |ty, idx| index_mut_trait.is_some_and(|trait_id| implements_trait(cx, ty, trait_id, &[idx]));
+
+    // 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 mut r = recv;
+    loop {
+        // Check that the (deref) adjustments could be made mutable
+        if (typeck.expr_adjustments(r))
+            .iter()
+            .map_while(|a| match a.kind {
+                Adjust::Deref(x) => Some((a.target, x)),
+                _ => None,
+            })
+            .try_fold(typeck.expr_ty(r), |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)
+                Some(_) => 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
+                None => (ty.ref_mutability() != Some(Mutability::Not)).then_some(target),
+            })
+            .is_none()
+        {
+            return;
+        }
+
+        // Peel off one projection
+        match r.kind {
+            // In order to be able to make this `*` mean `.deref_mut()`, `base: DerefMut` needs to hold
+            ExprKind::Unary(UnOp::Deref, base) => {
+                if impls_deref_mut(typeck.expr_ty_adjusted(base)) {
+                    r = base;
+                } else {
+                    return;
+                }
+            },
+            // In order to be able to make this `[idx]` mean `.index_mut(idx)`, `base: IndexMut<idx>` needs to hold
+            ExprKind::Index(base, idx, _) => {
+                // NOTE: the reason we do need to take into account `idx` here is that it's a _generic_ of
+                // `IndexMut`, not an associated type of the impl
+                if impls_index_mut(typeck.expr_ty_adjusted(base), typeck.expr_ty_adjusted(idx).into()) {
+                    r = base;
+                } else {
+                    return;
+                }
+            },
+            // A field projection by itself can't be mutable/immutable -- we'll only need to check
+            // that the field type is not a `&T`, and we'll do that in the next iteration of the
+            // loop, during adjustment checking
+            ExprKind::Field(base, _) => r = base,
+            // We arrived at the innermost receiver
+            _ => {
+                if let ExprKind::Path(ref p) = r.kind
+                    && cx
+                        .qpath_res(p, r.hir_id)
+                        .opt_def_id()
+                        .and_then(|id| cx.tcx.static_mutability(id))
+                        == Some(Mutability::Not)
+                {
+                    // The mutex is stored in a `static`, and we don't want to suggest making that
+                    // mutable
+                    return;
+                }
+                // No more projections to check
+                break;
+            },
+        }
+    }
+
+    span_lint_and_sugg(
+        cx,
+        MUT_MUTEX_LOCK,
+        name_span,
+        "calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference",
+        "change this to",
+        "get_mut".to_owned(),
+        Applicability::MaybeIncorrect,
+    );
 }

clippy_utils/src/lib.rs

@@ -459,10 +459,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>) -> (Vec<&'a Expr<'hir>>, &'a Expr<'hir>) {
     let mut result = vec![];
     let root = loop {
@@ -488,7 +487,8 @@ pub fn expr_custom_deref_adjustment(cx: &LateContext<'_>, e: &Expr<'_>) -> Optio
             Adjust::Deref(None) => 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/arc_with_non_send_sync.rs

@@ -1,6 +1,5 @@
 //@aux-build:proc_macros.rs
 #![warn(clippy::arc_with_non_send_sync)]
-#![allow(unused_variables)]
 
 #[macro_use]
 extern crate proc_macros;
@@ -35,7 +34,7 @@ fn main() {
     let _ = Arc::new(RefCell::new(42));
     //~^ arc_with_non_send_sync
 
-    let mutex = Mutex::new(1);
+    let mutex = &Mutex::new(1);
     let _ = Arc::new(mutex.lock().unwrap());
     //~^ arc_with_non_send_sync
 

tests/ui/arc_with_non_send_sync.stderr

@@ -1,5 +1,5 @@
 error: usage of an `Arc` that is not `Send` and `Sync`
-  --> tests/ui/arc_with_non_send_sync.rs:35:13
+  --> tests/ui/arc_with_non_send_sync.rs:34:13
    |
 LL |     let _ = Arc::new(RefCell::new(42));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -11,7 +11,7 @@ LL |     let _ = Arc::new(RefCell::new(42));
    = help: to override `-D warnings` add `#[allow(clippy::arc_with_non_send_sync)]`
 
 error: usage of an `Arc` that is not `Send` and `Sync`
-  --> tests/ui/arc_with_non_send_sync.rs:39:13
+  --> tests/ui/arc_with_non_send_sync.rs:38:13
    |
 LL |     let _ = Arc::new(mutex.lock().unwrap());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -21,7 +21,7 @@ LL |     let _ = Arc::new(mutex.lock().unwrap());
    = help: otherwise make `MutexGuard<'_, i32>` `Send` and `Sync` or consider a wrapper type such as `Mutex`
 
 error: usage of an `Arc` that is not `Send` and `Sync`
-  --> tests/ui/arc_with_non_send_sync.rs:42:13
+  --> tests/ui/arc_with_non_send_sync.rs:41:13
    |
 LL |     let _ = Arc::new(&42 as *const i32);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^

tests/ui/await_holding_lock.rs

@@ -173,7 +173,7 @@ async fn baz() -> u32 {
     42
 }
 
-async fn no_await(x: std::sync::Mutex<u32>) {
+async fn no_await(x: &std::sync::Mutex<u32>) {
     let mut guard = x.lock().unwrap();
     *guard += 1;
 }
@@ -182,7 +182,7 @@ async fn no_await(x: std::sync::Mutex<u32>) {
 // something the needs to be fixed in rustc. There's already drop-tracking, but this is currently
 // disabled, see rust-lang/rust#93751. This case isn't picked up by drop-tracking though. If the
 // `*guard += 1` is removed it is picked up.
-async fn dropped_before_await(x: std::sync::Mutex<u32>) {
+async fn dropped_before_await(x: &std::sync::Mutex<u32>) {
     let mut guard = x.lock().unwrap();
     //~^ ERROR: this `MutexGuard` is held across an await point
     *guard += 1;

tests/ui/branches_sharing_code/false_positives.rs

@@ -1,6 +1,6 @@
 //@ check-pass
 
-#![allow(dead_code)]
+#![allow(clippy::mut_mutex_lock)]
 #![deny(clippy::if_same_then_else, clippy::branches_sharing_code)]
 
 use std::sync::Mutex;

tests/ui/entry.fixed

@@ -255,8 +255,7 @@ mod issue_16173 {
 
     async fn f() {}
 
-    async fn foo() {
-        let mu_map = Mutex::new(HashMap::new());
+    async fn foo(mu_map: &Mutex<HashMap<i32, i32>>) {
         if !mu_map.lock().unwrap().contains_key(&0) {
             f().await;
             mu_map.lock().unwrap().insert(0, 0);

tests/ui/entry.rs

@@ -261,8 +261,7 @@ mod issue_16173 {
 
     async fn f() {}
 
-    async fn foo() {
-        let mu_map = Mutex::new(HashMap::new());
+    async fn foo(mu_map: &Mutex<HashMap<i32, i32>>) {
         if !mu_map.lock().unwrap().contains_key(&0) {
             f().await;
             mu_map.lock().unwrap().insert(0, 0);

tests/ui/entry.stderr

@@ -247,7 +247,7 @@ LL +     }
    |
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> tests/ui/entry.rs:285:5
+  --> tests/ui/entry.rs:284:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |

tests/ui/if_let_mutex.rs

@@ -5,7 +5,7 @@
 //@[edition2024] edition:2024
 //@[edition2024] check-pass
 #![warn(clippy::if_let_mutex)]
-#![allow(clippy::redundant_pattern_matching)]
+#![allow(clippy::redundant_pattern_matching, clippy::mut_mutex_lock)]
 
 use std::ops::Deref;
 use std::sync::Mutex;

tests/ui/let_underscore_lock.rs

@@ -1,4 +1,5 @@
 #![warn(clippy::let_underscore_lock)]
+#![allow(clippy::mut_mutex_lock)]
 
 extern crate parking_lot;
 

tests/ui/let_underscore_lock.stderr

@@ -1,5 +1,5 @@
 error: non-binding `let` on a synchronization lock
-  --> tests/ui/let_underscore_lock.rs:10:5
+  --> tests/ui/let_underscore_lock.rs:11:5
    |
 LL |     let _ = p_m.lock();
    |     ^^^^^^^^^^^^^^^^^^^
@@ -9,23 +9,23 @@ LL |     let _ = p_m.lock();
    = help: to override `-D warnings` add `#[allow(clippy::let_underscore_lock)]`
 
 error: non-binding `let` on a synchronization lock
-  --> tests/ui/let_underscore_lock.rs:14:5
+  --> tests/ui/let_underscore_lock.rs:15:5
    |
 LL |     let _ = p_m1.lock();
    |     ^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
 
 error: non-binding `let` on a synchronization lock
-  --> tests/ui/let_underscore_lock.rs:18:5
+  --> tests/ui/let_underscore_lock.rs:19:5
    |
 LL |     let _ = p_rw.read();
    |     ^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
 
 error: non-binding `let` on a synchronization lock
-  --> tests/ui/let_underscore_lock.rs:20:5
+  --> tests/ui/let_underscore_lock.rs:21:5
    |
 LL |     let _ = p_rw.write();
    |     ^^^^^^^^^^^^^^^^^^^^^