Move MutMutexLock into Methods lint pass

This commit is contained in:
Jason Newcomb 2022-06-05 19:57:58 -04:00
parent 2f0ed0a0b1
commit 508cf6bdbc
7 changed files with 74 additions and 75 deletions

View File

@ -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),

View File

@ -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,

View File

@ -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),

View File

@ -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));

View File

@ -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);

View File

@ -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,
);
}
}
}

View File

@ -1,70 +0,0 @@
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, ExprKind, Mutability};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
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"
}
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, [self_arg, ..], _) = &ex.kind;
if path.ident.name == sym!(lock);
let ty = cx.typeck_results().expr_ty(self_arg);
if let ty::Ref(_, inner_ty, Mutability::Mut) = ty.kind();
if is_type_diagnostic_item(cx, *inner_ty, sym::Mutex);
then {
span_lint_and_sugg(
cx,
MUT_MUTEX_LOCK,
path.ident.span,
"calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference",
"change this to",
"get_mut".to_owned(),
Applicability::MaybeIncorrect,
);
}
}
}
}