Auto merge of rust-lang#13122 - rshearman:fix-9854, r=blyxyas

Fix `mut_mutex_lock` when reference not ultimately mutable

When there is are multiple references where one of the references isn't mutable then this results in a false-positive for `mut_mutex_lock` as it only checks the mutability of the first reference level.

Fix this by using `peel_mid_ty_refs_is_mutable` which correctly determines whether the reference is ultimately mutable and thus whether `Mutex::get_lock()` can actually be used.

Fixes rust-lang#9854

changelog: [`mut_mutex_lock`]: No longer lints if the mutex is behind multiple references and one of those references isn't mutable

Author: bors

clippy_lints/src/methods/mut_mutex_lock.rs

@@ -1,17 +1,17 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::expr_custom_deref_adjustment;
-use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, Mutability};
 use rustc_lint::LateContext;
-use rustc_middle::ty;
 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 ty::Ref(_, _, Mutability::Mut) = cx.typeck_results().expr_ty(recv).kind()
+        && let (_, ref_depth, Mutability::Mut) = peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(recv))
+        && ref_depth >= 1
         && let Some(method_id) = cx.typeck_results().type_dependent_def_id(ex.hir_id)
         && let Some(impl_id) = cx.tcx.impl_of_method(method_id)
         && is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::Mutex)

tests/ui/mut_mutex_lock.fixed

@@ -9,6 +9,11 @@ fn mut_mutex_lock() {
 
     let mut value = value_mutex.get_mut().unwrap();
     *value += 1;
+
+    let mut value_mutex = Mutex::new(42_u8);
+    let mut_ref_mut_ref_mutex = &mut &mut value_mutex;
+    let mut value = mut_ref_mut_ref_mutex.get_mut().unwrap();
+    *value += 1;
 }
 
 fn no_owned_mutex_lock() {
@@ -24,4 +29,11 @@ fn issue9415() {
     *guard += 1;
 }
 
+fn mut_ref_ref_mutex_lock() {
+    let mutex = Mutex::new(42_u8);
+    let mut_ref_ref_mutex = &mut &mutex;
+    let mut guard = mut_ref_ref_mutex.lock().unwrap();
+    *guard += 1;
+}
+
 fn main() {}

tests/ui/mut_mutex_lock.rs

@@ -9,6 +9,11 @@ fn mut_mutex_lock() {
 
     let mut value = value_mutex.lock().unwrap();
     *value += 1;
+
+    let mut value_mutex = Mutex::new(42_u8);
+    let mut_ref_mut_ref_mutex = &mut &mut value_mutex;
+    let mut value = mut_ref_mut_ref_mutex.lock().unwrap();
+    *value += 1;
 }
 
 fn no_owned_mutex_lock() {
@@ -24,4 +29,11 @@ fn issue9415() {
     *guard += 1;
 }
 
+fn mut_ref_ref_mutex_lock() {
+    let mutex = Mutex::new(42_u8);
+    let mut_ref_ref_mutex = &mut &mutex;
+    let mut guard = mut_ref_ref_mutex.lock().unwrap();
+    *guard += 1;
+}
+
 fn main() {}

tests/ui/mut_mutex_lock.stderr

@@ -7,5 +7,11 @@ LL |     let mut value = value_mutex.lock().unwrap();
    = note: `-D clippy::mut-mutex-lock` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::mut_mutex_lock)]`
 
-error: aborting due to 1 previous error
+error: calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference
+  --> tests/ui/mut_mutex_lock.rs:15:43
+   |
+LL |     let mut value = mut_ref_mut_ref_mutex.lock().unwrap();
+   |                                           ^^^^ help: change this to: `get_mut`
+
+error: aborting due to 2 previous errors