diff options
Diffstat (limited to 'clippy_lints/src/methods')
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 41 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mut_mutex_lock.rs | 30 |
2 files changed, 71 insertions, 0 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index e327bdcf578..6ad32ae09d7 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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); diff --git a/clippy_lints/src/methods/mut_mutex_lock.rs b/clippy_lints/src/methods/mut_mutex_lock.rs new file mode 100644 index 00000000000..bd8458a222e --- /dev/null +++ b/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, + ); + } + } +} |
