From 18bc948ee777a9f43f6c4e4773fa54ef7882fb05 Mon Sep 17 00:00:00 2001 From: Francis Murillo Date: Sat, 10 Oct 2020 18:07:47 +0800 Subject: [PATCH] Use `sugg_lint_and_help` --- clippy_lints/src/mut_mutex_lock.rs | 18 +++++++++++------- tests/ui/mut_mutex_lock.fixed | 21 +++++++++++++++++++++ tests/ui/mut_mutex_lock.rs | 2 ++ tests/ui/mut_mutex_lock.stderr | 5 ++--- 4 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 tests/ui/mut_mutex_lock.fixed diff --git a/clippy_lints/src/mut_mutex_lock.rs b/clippy_lints/src/mut_mutex_lock.rs index ca3371a5d75c..82ed2d6d69c3 100644 --- a/clippy_lints/src/mut_mutex_lock.rs +++ b/clippy_lints/src/mut_mutex_lock.rs @@ -1,5 +1,6 @@ -use crate::utils::{is_type_diagnostic_item, span_lint_and_help}; +use crate::utils::{is_type_diagnostic_item, span_lint_and_sugg}; use if_chain::if_chain; +use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Mutability}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; @@ -9,7 +10,9 @@ 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` + /// calling `Mutex::get_mut`. In addition you also have a statically + /// guarantee that the mutex isn't locked, instead of just a runtime + /// guarantee. /// /// **Known problems:** None. /// @@ -44,19 +47,20 @@ declare_lint_pass!(MutMutexLock => [MUT_MUTEX_LOCK]); impl<'tcx> LateLintPass<'tcx> for MutMutexLock { fn check_expr(&mut self, cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>) { if_chain! { - if let ExprKind::MethodCall(path, _span, args, _) = &ex.kind; + if let ExprKind::MethodCall(path, method_span, args, _) = &ex.kind; if path.ident.name == sym!(lock); let ty = cx.typeck_results().expr_ty(&args[0]); if let ty::Ref(_, inner_ty, Mutability::Mut) = ty.kind(); if is_type_diagnostic_item(cx, inner_ty, sym!(mutex_type)); then { - span_lint_and_help( + span_lint_and_sugg( cx, MUT_MUTEX_LOCK, - ex.span, + *method_span, "calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference", - None, - "use `&mut Mutex::get_mut` instead", + "change this to", + "get_mut".to_owned(), + Applicability::MachineApplicable, ); } } diff --git a/tests/ui/mut_mutex_lock.fixed b/tests/ui/mut_mutex_lock.fixed new file mode 100644 index 000000000000..36bc52e3374e --- /dev/null +++ b/tests/ui/mut_mutex_lock.fixed @@ -0,0 +1,21 @@ +// run-rustfix +#![allow(dead_code, unused_mut)] +#![warn(clippy::mut_mutex_lock)] + +use std::sync::{Arc, Mutex}; + +fn mut_mutex_lock() { + 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.get_mut().unwrap(); + *value += 1; +} + +fn no_owned_mutex_lock() { + let mut value_rc = Arc::new(Mutex::new(42_u8)); + let mut value = value_rc.lock().unwrap(); + *value += 1; +} + +fn main() {} diff --git a/tests/ui/mut_mutex_lock.rs b/tests/ui/mut_mutex_lock.rs index 9cd98e90c29d..ea60df5ae1bb 100644 --- a/tests/ui/mut_mutex_lock.rs +++ b/tests/ui/mut_mutex_lock.rs @@ -1,3 +1,5 @@ +// run-rustfix +#![allow(dead_code, unused_mut)] #![warn(clippy::mut_mutex_lock)] use std::sync::{Arc, Mutex}; diff --git a/tests/ui/mut_mutex_lock.stderr b/tests/ui/mut_mutex_lock.stderr index d521ebb56c43..21c1b3486cac 100644 --- a/tests/ui/mut_mutex_lock.stderr +++ b/tests/ui/mut_mutex_lock.stderr @@ -1,11 +1,10 @@ error: calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference - --> $DIR/mut_mutex_lock.rs:9:21 + --> $DIR/mut_mutex_lock.rs:11:33 | LL | let mut value = value_mutex.lock().unwrap(); - | ^^^^^^^^^^^^^^^^^^ + | ^^^^ help: change this to: `get_mut` | = note: `-D clippy::mut-mutex-lock` implied by `-D warnings` - = help: use `&mut Mutex::get_mut` instead error: aborting due to previous error