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

@@ -666,6 +666,8 @@ 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_atomic::RWLOCK_ATOMIC_INFO,
+    crate::rwlock_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

@@ -317,6 +317,7 @@ mod replace_box;
 mod reserve_after_initialization;
 mod return_self_not_must_use;
 mod returns;
+mod rwlock_atomic;
 mod same_name_method;
 mod self_named_constructors;
 mod semicolon_block;
@@ -850,6 +851,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
         Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
         Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
+        Box::new(|_| Box::new(rwlock_atomic::RwLock)),
         // add late passes here, used by `cargo dev new_lint`
     ];
     store.late_passes.extend(late_lints);

clippy_lints/src/rwlock_atomic.rs

@@ -0,0 +1,176 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::res::MaybeDef;
+use clippy_utils::source::{IntoSpan, SpanRangeExt};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::ty_from_hir_ty;
+use rustc_errors::{Applicability, Diag};
+use rustc_hir::{self as hir, Expr, ExprKind, Item, ItemKind, LetStmt, QPath};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::mir::Mutability;
+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
+    /// verify correctness. An atomic does not behave the same 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
+    /// verify correctness. An atomic does not behave the same 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_lint_pass!(RwLock => [RWLOCK_ATOMIC, RWLOCK_INTEGER]);
+
+impl<'tcx> LateLintPass<'tcx> for RwLock {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
+        if !item.span.from_expansion()
+            && let ItemKind::Static(_, _, ty, body_id) = item.kind
+        {
+            let body = cx.tcx.hir_body(body_id);
+            let mid_ty = ty_from_hir_ty(cx, ty);
+            check_expr(cx, body.value.peel_blocks(), &TypeAscriptionKind::Required(ty), mid_ty);
+        }
+    }
+    fn check_local(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx LetStmt<'_>) {
+        if !stmt.span.from_expansion()
+            && let Some(init) = stmt.init
+        {
+            let mid_ty = cx.typeck_results().expr_ty(init);
+            check_expr(cx, init.peel_blocks(), &TypeAscriptionKind::Optional(stmt.ty), mid_ty);
+        }
+    }
+}
+
+enum TypeAscriptionKind<'tcx> {
+    Required(&'tcx hir::Ty<'tcx>),
+    Optional(Option<&'tcx hir::Ty<'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::RwLock)
+        && let rwlock_param = subst.type_at(0)
+        && let Some(atomic_name) = get_atomic_name(rwlock_param)
+    {
+        let msg = "using a `RwLock` where an atomic would do";
+        let diag = |diag: &mut Diag<'_, _>| {
+            if let ExprKind::Call(qpath, [arg]) = expr.kind
+                && let ExprKind::Path(QPath::TypeRelative(_rwlock, new)) = qpath.kind
+                && new.ident.name == sym::new
+            {
+                let mut applicability = Applicability::MaybeIncorrect;
+                let arg = Sugg::hir_with_applicability(cx, arg, "_", &mut applicability);
+                let mut suggs = vec![(expr.span, format!("std::sync::atomic::{atomic_name}::new({arg})"))];
+                match ty_ascription {
+                    TypeAscriptionKind::Required(ty_ascription) => {
+                        suggs.push((ty_ascription.span, format!("std::sync::atomic::{atomic_name}")));
+                    },
+                    TypeAscriptionKind::Optional(Some(ty_ascription)) => {
+                        let colon_ascription = (cx.sess().source_map())
+                            .span_extend_to_prev_char_before(ty_ascription.span, ':', true)
+                            .with_leading_whitespace(cx)
+                            .into_span();
+                        suggs.push((colon_ascription, String::new()));
+                    },
+                    TypeAscriptionKind::Optional(None) => {},
+                }
+                diag.multipart_suggestion("try", suggs, applicability);
+            } 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 `RwLock<()>`");
+        };
+        match *rwlock_param.kind() {
+            ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, RWLOCK_INTEGER, expr.span, msg, diag),
+            ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, RWLOCK_INTEGER, expr.span, msg, diag),
+            _ => span_lint_and_then(cx, RWLOCK_ATOMIC, expr.span, msg, diag),
+        }
+    }
+}
+
+fn get_atomic_name(ty: Ty<'_>) -> Option<&'static str> {
+    match ty.kind() {
+        ty::Bool => Some("AtomicBool"),
+        ty::Uint(uint_ty) => {
+            match uint_ty {
+                UintTy::U8 => Some("AtomicU8"),
+                UintTy::U16 => Some("AtomicU16"),
+                UintTy::U32 => Some("AtomicU32"),
+                UintTy::U64 => Some("AtomicU64"),
+                UintTy::Usize => Some("AtomicUsize"),
+                // `AtomicU128` is unstable and only available on a few platforms: https://github.com/rust-lang/rust/issues/99069
+                UintTy::U128 => None,
+            }
+        },
+        ty::Int(int_ty) => {
+            match int_ty {
+                IntTy::I8 => Some("AtomicI8"),
+                IntTy::I16 => Some("AtomicI16"),
+                IntTy::I32 => Some("AtomicI32"),
+                IntTy::I64 => Some("AtomicI64"),
+                IntTy::Isize => Some("AtomicIsize"),
+                // `AtomicU128` is unstable and only available on a few platforms: https://github.com/rust-lang/rust/issues/99069
+                IntTy::I128 => None,
+            }
+        },
+        // `AtomicPtr` only accepts `*mut T`
+        ty::RawPtr(_, Mutability::Mut) => Some("AtomicPtr"),
+        _ => None,
+    }
+}

tests/ui/rwlock_atomic.fixed

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

tests/ui/rwlock_atomic.rs

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