Move MutMutexLock into Methods lint pass

Author: Jarcho

clippy_lints/src/lib.register_all.rs

@@ -182,6 +182,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
     LintId::of(methods::MAP_FLATTEN),
     LintId::of(methods::MAP_IDENTITY),
+    LintId::of(methods::MUT_MUTEX_LOCK),
     LintId::of(methods::NEEDLESS_OPTION_AS_DEREF),
     LintId::of(methods::NEEDLESS_OPTION_TAKE),
     LintId::of(methods::NEEDLESS_SPLITN),
@@ -226,7 +227,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(misc_early::ZERO_PREFIXED_LITERAL),
     LintId::of(mixed_read_write_in_expression::DIVERGING_SUB_EXPRESSION),
     LintId::of(mut_key::MUTABLE_KEY_TYPE),
-    LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
     LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
     LintId::of(needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
     LintId::of(needless_bool::BOOL_COMPARISON),

clippy_lints/src/lib.register_lints.rs

@@ -329,6 +329,7 @@ store.register_lints(&[
     methods::MAP_FLATTEN,
     methods::MAP_IDENTITY,
     methods::MAP_UNWRAP_OR,
+    methods::MUT_MUTEX_LOCK,
     methods::NAIVE_BYTECOUNT,
     methods::NEEDLESS_OPTION_AS_DEREF,
     methods::NEEDLESS_OPTION_TAKE,
@@ -389,7 +390,6 @@ store.register_lints(&[
     module_style::SELF_NAMED_MODULE_FILES,
     mut_key::MUTABLE_KEY_TYPE,
     mut_mut::MUT_MUT,
-    mut_mutex_lock::MUT_MUTEX_LOCK,
     mut_reference::UNNECESSARY_MUT_PASSED,
     mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL,
     mutex_atomic::MUTEX_ATOMIC,

clippy_lints/src/lib.register_style.rs

@@ -70,6 +70,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
     LintId::of(methods::MAP_CLONE),
     LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
+    LintId::of(methods::MUT_MUTEX_LOCK),
     LintId::of(methods::NEW_RET_NO_SELF),
     LintId::of(methods::OBFUSCATED_IF_ELSE),
     LintId::of(methods::OK_EXPECT),
@@ -89,7 +90,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(misc_early::DUPLICATE_UNDERSCORE_ARGUMENT),
     LintId::of(misc_early::MIXED_CASE_HEX_LITERALS),
     LintId::of(misc_early::REDUNDANT_PATTERN),
-    LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
     LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
     LintId::of(needless_late_init::NEEDLESS_LATE_INIT),
     LintId::of(needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS),

clippy_lints/src/lib.rs

@@ -291,7 +291,6 @@ mod mixed_read_write_in_expression;
 mod module_style;
 mod mut_key;
 mod mut_mut;
-mod mut_mutex_lock;
 mod mut_reference;
 mod mutable_debug_assertion;
 mod mutex_atomic;
@@ -815,7 +814,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));
     store.register_late_pass(|| Box::new(if_not_else::IfNotElse));
     store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
-    store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
     store.register_late_pass(|| Box::new(manual_async_fn::ManualAsyncFn));
     store.register_late_pass(|| Box::new(vec_resize_to_zero::VecResizeToZero));
     store.register_late_pass(|| Box::new(panic_in_result_fn::PanicInResultFn));

clippy_lints/src/methods/mod.rs

@@ -51,6 +51,7 @@ mod map_err_ignore;
 mod map_flatten;
 mod map_identity;
 mod map_unwrap_or;
+mod mut_mutex_lock;
 mod needless_option_as_deref;
 mod needless_option_take;
 mod no_effect_replace;
@@ -2642,6 +2643,42 @@ declare_clippy_lint! {
     "`map_err` should not ignore the original error"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `&mut Mutex::lock` calls
+    ///
+    /// ### Why is this bad?
+    /// `Mutex::lock` is less efficient than
+    /// calling `Mutex::get_mut`. In addition you also have a statically
+    /// guarantee that the mutex isn't locked, instead of just a runtime
+    /// guarantee.
+    ///
+    /// ### Example
+    /// ```rust
+    /// use std::sync::{Arc, Mutex};
+    ///
+    /// let mut value_rc = Arc::new(Mutex::new(42_u8));
+    /// let value_mutex = Arc::get_mut(&mut value_rc).unwrap();
+    ///
+    /// let mut value = value_mutex.lock().unwrap();
+    /// *value += 1;
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::sync::{Arc, Mutex};
+    ///
+    /// let mut value_rc = Arc::new(Mutex::new(42_u8));
+    /// let value_mutex = Arc::get_mut(&mut value_rc).unwrap();
+    ///
+    /// let value = value_mutex.get_mut().unwrap();
+    /// *value += 1;
+    /// ```
+    #[clippy::version = "1.49.0"]
+    pub MUT_MUTEX_LOCK,
+    style,
+    "`&mut Mutex::lock` does unnecessary locking"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2753,6 +2790,7 @@ impl_lint_pass!(Methods => [
     MANUAL_OK_OR,
     MAP_CLONE,
     MAP_ERR_IGNORE,
+    MUT_MUTEX_LOCK,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3081,6 +3119,9 @@ impl Methods {
                         }
                     }
                 },
+                ("lock", []) => {
+                    mut_mutex_lock::check(cx, expr, recv, span);
+                },
                 (name @ ("map" | "map_err"), [m_arg]) => {
                     if name == "map" {
                         map_clone::check(cx, expr, recv, m_arg, self.msrv);

clippy_lints/src/methods/mut_mutex_lock.rs

@@ -0,0 +1,30 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::ty::is_type_diagnostic_item;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, Mutability};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_span::{sym, Span};
+
+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_chain! {
+        if let ty::Ref(_, _, Mutability::Mut) = cx.typeck_results().expr_ty(recv).kind();
+        if let Some(method_id) = cx.typeck_results().type_dependent_def_id(ex.hir_id);
+        if let Some(impl_id) = cx.tcx.impl_of_method(method_id);
+        if is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id), sym::Mutex);
+        then {
+            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,
+            );
+        }
+    }
+}