feat(rwlock_atomic): add new lints for using RwLock with atomic types

CHANGELOG.md

@@ -6837,6 +6837,8 @@ Released 2018-09-13
 [`return_self_not_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use
 [`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop
 [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
+[`rwlock_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#rwlock_atomic
+[`rwlock_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rwlock_integer
 [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
 [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
 [`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method

README.md

@@ -4,7 +4,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

@@ -5,7 +5,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

clippy_lints/src/declared_lints.rs

@@ -535,8 +535,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::mut_key::MUTABLE_KEY_TYPE_INFO,
     crate::mut_mut::MUT_MUT_INFO,
     crate::mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL_INFO,
-    crate::mutex_atomic::MUTEX_ATOMIC_INFO,
-    crate::mutex_atomic::MUTEX_INTEGER_INFO,
     crate::needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE_INFO,
     crate::needless_bool::NEEDLESS_BOOL_INFO,
     crate::needless_bool::NEEDLESS_BOOL_ASSIGN_INFO,
@@ -666,6 +664,10 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::returns::LET_AND_RETURN_INFO,
     crate::returns::NEEDLESS_RETURN_INFO,
     crate::returns::NEEDLESS_RETURN_WITH_QUESTION_MARK_INFO,
+    crate::rwlock_mutex_atomic::MUTEX_ATOMIC_INFO,
+    crate::rwlock_mutex_atomic::MUTEX_INTEGER_INFO,
+    crate::rwlock_mutex_atomic::RWLOCK_ATOMIC_INFO,
+    crate::rwlock_mutex_atomic::RWLOCK_INTEGER_INFO,
     crate::same_name_method::SAME_NAME_METHOD_INFO,
     crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO,
     crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO,

clippy_lints/src/lib.rs

@@ -242,7 +242,6 @@ mod multiple_unsafe_ops_per_block;
 mod mut_key;
 mod mut_mut;
 mod mutable_debug_assertion;
-mod mutex_atomic;
 mod needless_arbitrary_self_type;
 mod needless_bool;
 mod needless_borrowed_ref;
@@ -317,6 +316,7 @@ mod replace_box;
 mod reserve_after_initialization;
 mod return_self_not_must_use;
 mod returns;
+mod rwlock_mutex_atomic;
 mod same_name_method;
 mod self_named_constructors;
 mod semicolon_block;
@@ -584,7 +584,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::new(entry::HashMapPass)),
         Box::new(|_| Box::new(minmax::MinMaxPass)),
         Box::new(|_| Box::new(zero_div_zero::ZeroDiv)),
-        Box::new(|_| Box::new(mutex_atomic::Mutex)),
+        Box::new(|_| Box::new(rwlock_mutex_atomic::SyncGuard)),
         Box::new(|_| Box::new(needless_update::NeedlessUpdate)),
         Box::new(|_| Box::new(needless_borrowed_ref::NeedlessBorrowedRef)),
         Box::new(|_| Box::new(borrow_deref_ref::BorrowDerefRef)),

clippy_lints/src/mutex_atomic.rs → clippy_lints/src/rwlock_mutex_atomic.rs

@@ -11,6 +11,70 @@ use rustc_middle::ty::{self, IntTy, Ty, UintTy};
 use rustc_session::declare_lint_pass;
 use rustc_span::sym;
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `RwLock<X>` where an atomic will do.
+    ///
+    /// ### Why is this bad?
+    /// Using a RwLock just to make access to a plain bool or
+    /// reference sequential is shooting flies with cannons.
+    /// `std::sync::atomic::AtomicBool` and `std::sync::atomic::AtomicPtr` are leaner and
+    /// faster.
+    ///
+    /// On the other hand, `RwLock`s are, in general, easier to reason about
+    /// and to verify for correctness. Atomics do not provide the same
+    /// synchronization semantics as an equivalent `RwLock`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let y = true;
+    /// # use std::sync::RwLock;
+    /// let x = RwLock::new(&y);
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// # let y = true;
+    /// # use std::sync::atomic::AtomicBool;
+    /// let x = AtomicBool::new(y);
+    /// ```
+    #[clippy::version = "1.93.0"]
+    pub RWLOCK_ATOMIC,
+    restriction,
+    "using a RwLock where an atomic value could be used instead."
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `RwLock<X>` where `X` is an integral
+    /// type.
+    ///
+    /// ### Why is this bad?
+    /// Using a RwLock just to make access to a plain integer
+    /// sequential is
+    /// shooting flies with cannons. `std::sync::atomic::AtomicUsize` is leaner and faster.
+    ///
+    /// On the other hand, `RwLock`s are, in general, easier to reason about
+    /// and to verify for correctness. Atomics do not provide the same
+    /// synchronization semantics as an equivalent `RwLock`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # use std::sync::RwLock;
+    /// let x = RwLock::new(0usize);
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// # use std::sync::atomic::AtomicUsize;
+    /// let x = AtomicUsize::new(0usize);
+    /// ```
+    #[clippy::version = "1.93.0"]
+    pub RWLOCK_INTEGER,
+    restriction,
+    "using a RwLock for an integer type"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of `Mutex<X>` where an atomic will do.
@@ -91,11 +155,16 @@ declare_clippy_lint! {
     "using a mutex for an integer type"
 }
 
-declare_lint_pass!(Mutex => [MUTEX_ATOMIC, MUTEX_INTEGER]);
+declare_lint_pass!(SyncGuard => [
+    RWLOCK_ATOMIC,
+    RWLOCK_INTEGER,
+    MUTEX_ATOMIC,
+    MUTEX_INTEGER
+]);
 
 // NOTE: we don't use `check_expr` because that would make us lint every _use_ of such mutexes, not
 // just their definitions
-impl<'tcx> LateLintPass<'tcx> for Mutex {
+impl<'tcx> LateLintPass<'tcx> for SyncGuard {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
         if !item.span.from_expansion()
             && let ItemKind::Static(_, _, ty, body_id) = item.kind
@@ -123,6 +192,8 @@ enum TypeAscriptionKind<'tcx> {
     /// No; the ascription might've been necessary in an expression like:
     /// ```ignore
     /// let mutex: Mutex<u64> = Mutex::new(0);
+    /// // Or
+    /// let rwlock: RwLock<u64> = RwLock::new(0);
     /// ```
     /// to specify the type of `0`, but since `AtomicX` already refers to a concrete type, we won't
     /// need this ascription anymore.
@@ -131,13 +202,19 @@ enum TypeAscriptionKind<'tcx> {
 
 fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty_ascription: &TypeAscriptionKind<'tcx>, ty: Ty<'tcx>) {
     if let ty::Adt(_, subst) = ty.kind()
-        && ty.is_diag_item(cx, sym::Mutex)
+        && let Some((lock_name, lint_integer, lint_atomic)) = if ty.is_diag_item(cx, sym::Mutex) {
+            Some(("Mutex", MUTEX_INTEGER, MUTEX_ATOMIC))
+        } else if ty.is_diag_item(cx, sym::RwLock) {
+            Some(("RwLock", RWLOCK_INTEGER, RWLOCK_ATOMIC))
+        } else {
+            None
+        }
         && let mutex_param = subst.type_at(0)
         && let Some(atomic_name) = get_atomic_name(mutex_param)
     {
-        let msg = "using a `Mutex` where an atomic would do";
+        let msg = format!("using a `{lock_name}` where an atomic would do");
         let diag = |diag: &mut Diag<'_, _>| {
-            // if `expr = Mutex::new(arg)`, we can try emitting a suggestion
+            // if `expr = Mutex::new(arg)` or `expr = RwLock::new(arg)`, we can try emitting a suggestion
             if let ExprKind::Call(qpath, [arg]) = expr.kind
                 && let ExprKind::Path(QPath::TypeRelative(_mutex, new)) = qpath.kind
                 && new.ident.name == sym::new
@@ -164,12 +241,14 @@ fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty_ascription: &T
             } else {
                 diag.help(format!("consider using an `{atomic_name}` instead"));
             }
-            diag.help("if you just want the locking behavior and not the internal type, consider using `Mutex<()>`");
+            diag.help(format!(
+                "if you just want the locking behavior and not the internal type, consider using `{lock_name}<()>`"
+            ));
         };
         match *mutex_param.kind() {
-            ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag),
-            ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag),
-            _ => span_lint_and_then(cx, MUTEX_ATOMIC, expr.span, msg, diag),
+            ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, lint_integer, expr.span, msg, diag),
+            ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, lint_integer, expr.span, msg, diag),
+            _ => span_lint_and_then(cx, lint_atomic, expr.span, msg, diag),
         }
     }
 }

tests/ui/mutex_atomic.fixed → tests/ui/rwlock_mutex_atomic.fixed

@@ -1,49 +1,75 @@
+#![warn(clippy::rwlock_integer)]
+#![warn(clippy::rwlock_atomic)]
 #![warn(clippy::mutex_integer)]
 #![warn(clippy::mutex_atomic)]
 #![allow(clippy::borrow_as_ptr)]
 
-use std::sync::Mutex;
+use std::sync::{Mutex, RwLock};
 
 fn main() {
+    let _ = std::sync::atomic::AtomicBool::new(true);
+    //~^ rwlock_atomic
     let _ = std::sync::atomic::AtomicBool::new(true);
     //~^ mutex_atomic
 
+    let _ = std::sync::atomic::AtomicUsize::new(5usize);
+    //~^ rwlock_atomic
     let _ = std::sync::atomic::AtomicUsize::new(5usize);
     //~^ mutex_atomic
 
+    let _ = std::sync::atomic::AtomicIsize::new(9isize);
+    //~^ rwlock_atomic
     let _ = std::sync::atomic::AtomicIsize::new(9isize);
     //~^ mutex_atomic
 
     let mut x = 4u32;
-    // `AtomicPtr` only accepts `*mut T`, so this should not lint
+    // `AtomicPtr` only accepts `*mut T`, so these should not lint
+    let _ = RwLock::new(&x as *const u32);
     let _ = Mutex::new(&x as *const u32);
 
+    let _ = std::sync::atomic::AtomicPtr::new(&mut x as *mut u32);
+    //~^ rwlock_atomic
     let _ = std::sync::atomic::AtomicPtr::new(&mut x as *mut u32);
     //~^ mutex_atomic
 
+    let _ = std::sync::atomic::AtomicU32::new(0u32);
+    //~^ rwlock_integer
     let _ = std::sync::atomic::AtomicU32::new(0u32);
     //~^ mutex_integer
 
+    let _ = std::sync::atomic::AtomicI32::new(0i32);
+    //~^ rwlock_integer
     let _ = std::sync::atomic::AtomicI32::new(0i32);
     //~^ mutex_integer
 
+    let _ = RwLock::new(0f32); // there are no float atomics, so this should not lint
+    let _ = std::sync::atomic::AtomicU8::new(0u8);
+    //~^ rwlock_integer
     let _ = Mutex::new(0f32); // there are no float atomics, so this should not lint
     let _ = std::sync::atomic::AtomicU8::new(0u8);
     //~^ mutex_integer
 
+    let _ = std::sync::atomic::AtomicI16::new(0i16);
+    //~^ rwlock_integer
     let _ = std::sync::atomic::AtomicI16::new(0i16);
     //~^ mutex_integer
 
+    let _x = std::sync::atomic::AtomicI8::new(0);
+    //~^ rwlock_integer
     let _x = std::sync::atomic::AtomicI8::new(0);
     //~^ mutex_integer
 
     const X: i64 = 0;
     let _ = std::sync::atomic::AtomicI64::new(X);
+    //~^ rwlock_integer
+    let _ = std::sync::atomic::AtomicI64::new(X);
     //~^ mutex_integer
 
-    // there are no 128 atomics, so these two should not lint
+    // there are no 128 atomics, so these four should not lint
     {
+        let _ = RwLock::new(0u128);
         let _ = Mutex::new(0u128);
+        let _x: RwLock<i128> = RwLock::new(0);
         let _x: Mutex<i128> = Mutex::new(0);
     }
 }

tests/ui/mutex_atomic.rs → tests/ui/rwlock_mutex_atomic.rs

@@ -1,49 +1,75 @@
+#![warn(clippy::rwlock_integer)]
+#![warn(clippy::rwlock_atomic)]
 #![warn(clippy::mutex_integer)]
 #![warn(clippy::mutex_atomic)]
 #![allow(clippy::borrow_as_ptr)]
 
-use std::sync::Mutex;
+use std::sync::{Mutex, RwLock};
 
 fn main() {
+    let _ = RwLock::new(true);
+    //~^ rwlock_atomic
     let _ = Mutex::new(true);
     //~^ mutex_atomic
 
+    let _ = RwLock::new(5usize);
+    //~^ rwlock_atomic
     let _ = Mutex::new(5usize);
     //~^ mutex_atomic
 
+    let _ = RwLock::new(9isize);
+    //~^ rwlock_atomic
     let _ = Mutex::new(9isize);
     //~^ mutex_atomic
 
     let mut x = 4u32;
-    // `AtomicPtr` only accepts `*mut T`, so this should not lint
+    // `AtomicPtr` only accepts `*mut T`, so these should not lint
+    let _ = RwLock::new(&x as *const u32);
     let _ = Mutex::new(&x as *const u32);
 
+    let _ = RwLock::new(&mut x as *mut u32);
+    //~^ rwlock_atomic
     let _ = Mutex::new(&mut x as *mut u32);
     //~^ mutex_atomic
 
+    let _ = RwLock::new(0u32);
+    //~^ rwlock_integer
     let _ = Mutex::new(0u32);
     //~^ mutex_integer
 
+    let _ = RwLock::new(0i32);
+    //~^ rwlock_integer
     let _ = Mutex::new(0i32);
     //~^ mutex_integer
 
+    let _ = RwLock::new(0f32); // there are no float atomics, so this should not lint
+    let _ = RwLock::new(0u8);
+    //~^ rwlock_integer
     let _ = Mutex::new(0f32); // there are no float atomics, so this should not lint
     let _ = Mutex::new(0u8);
     //~^ mutex_integer
 
+    let _ = RwLock::new(0i16);
+    //~^ rwlock_integer
     let _ = Mutex::new(0i16);
     //~^ mutex_integer
 
+    let _x: RwLock<i8> = RwLock::new(0);
+    //~^ rwlock_integer
     let _x: Mutex<i8> = Mutex::new(0);
     //~^ mutex_integer
 
     const X: i64 = 0;
+    let _ = RwLock::new(X);
+    //~^ rwlock_integer
     let _ = Mutex::new(X);
     //~^ mutex_integer
 
-    // there are no 128 atomics, so these two should not lint
+    // there are no 128 atomics, so these four should not lint
     {
+        let _ = RwLock::new(0u128);
         let _ = Mutex::new(0u128);
+        let _x: RwLock<i128> = RwLock::new(0);
         let _x: Mutex<i128> = Mutex::new(0);
     }
 }