From 339cd14f27e9db4f7b040068abad8a20a0034fa8 Mon Sep 17 00:00:00 2001 From: avborhanian Date: Sun, 4 Jun 2023 10:33:13 -0700 Subject: [PATCH] Adds new lint `arc_with_non_send_or_sync` --- CHANGELOG.md | 1 + clippy_lints/src/arc_with_non_send_sync.rs | 71 ++++++++++++++++++++++ clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + tests/ui/arc_with_non_send_sync.rs | 12 ++++ tests/ui/arc_with_non_send_sync.stderr | 11 ++++ 6 files changed, 98 insertions(+) create mode 100644 clippy_lints/src/arc_with_non_send_sync.rs create mode 100644 tests/ui/arc_with_non_send_sync.rs create mode 100644 tests/ui/arc_with_non_send_sync.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index a65ac4d46de..a53567f25c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4628,6 +4628,7 @@ Released 2018-09-13 [`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant +[`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync [`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions [`as_ptr_cast_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut diff --git a/clippy_lints/src/arc_with_non_send_sync.rs b/clippy_lints/src/arc_with_non_send_sync.rs new file mode 100644 index 00000000000..2e14f81f8e4 --- /dev/null +++ b/clippy_lints/src/arc_with_non_send_sync.rs @@ -0,0 +1,71 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::last_path_segment; +use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; +use if_chain::if_chain; + +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_lint::LateLintPass; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; + +declare_clippy_lint! { + /// ### What it does. + /// This lint warns when you use `Arc` with a type that does not implement `Send` or `Sync`. + /// + /// ### Why is this bad? + /// Wrapping a type in Arc doesn't add thread safety to the underlying data, so data races + /// could occur when touching the underlying data. + /// + /// ### Example. + /// ```rust + /// use std::cell::RefCell; + /// use std::sync::Arc; + /// + /// fn main() { + /// // This is safe, as `i32` implements `Send` and `Sync`. + /// let a = Arc::new(42); + /// + /// // This is not safe, as `RefCell` does not implement `Sync`. + /// let b = Arc::new(RefCell::new(42)); + /// } + /// ``` + /// ``` + #[clippy::version = "1.72.0"] + pub ARC_WITH_NON_SEND_SYNC, + correctness, + "using `Arc` with a type that does not implement `Send` or `Sync`" +} +declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]); + +impl LateLintPass<'_> for ArcWithNonSendSync { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + let ty = cx.typeck_results().expr_ty(expr); + if_chain! { + if is_type_diagnostic_item(cx, ty, sym::Arc); + if let ExprKind::Call(func, [arg]) = expr.kind; + if let ExprKind::Path(func_path) = func.kind; + if last_path_segment(&func_path).ident.name == sym::new; + if let arg_ty = cx.typeck_results().expr_ty(arg); + if !cx.tcx + .lang_items() + .sync_trait() + .map_or(false, |id| implements_trait(cx, arg_ty, id, &[])) || + !cx.tcx + .get_diagnostic_item(sym::Send) + .map_or(false, |id| implements_trait(cx, arg_ty, id, &[])); + + then { + span_lint_and_help( + cx, + ARC_WITH_NON_SEND_SYNC, + expr.span, + "usage of `Arc` where `T` is not `Send` or `Sync`", + None, + "consider using `Rc` instead or wrapping `T` in a std::sync type like \ + Mutex", + ); + } + } + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index d014534cee9..f82118e569f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -40,6 +40,7 @@ crate::allow_attributes::ALLOW_ATTRIBUTES_INFO, crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO, crate::approx_const::APPROX_CONSTANT_INFO, + crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO, crate::as_conversions::AS_CONVERSIONS_INFO, crate::asm_syntax::INLINE_ASM_X86_ATT_SYNTAX_INFO, crate::asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d53c6de5451..2f5932104c1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -68,6 +68,7 @@ mod allow_attributes; mod almost_complete_range; mod approx_const; +mod arc_with_non_send_sync; mod as_conversions; mod asm_syntax; mod assertions_on_constants; @@ -1014,6 +1015,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(missing_fields_in_debug::MissingFieldsInDebug)); store.register_late_pass(|_| Box::new(endian_bytes::EndianBytes)); store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations)); + store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/arc_with_non_send_sync.rs b/tests/ui/arc_with_non_send_sync.rs new file mode 100644 index 00000000000..4c2d84be5db --- /dev/null +++ b/tests/ui/arc_with_non_send_sync.rs @@ -0,0 +1,12 @@ +#![warn(clippy::arc_with_non_send_sync)] +#![allow(unused_variables)] +use std::cell::RefCell; +use std::sync::{Arc, Mutex}; + +fn main() { + // This is safe, as `i32` implements `Send` and `Sync`. + let a = Arc::new(42); + + // This is not safe, as `RefCell` does not implement `Sync`. + let b = Arc::new(RefCell::new(42)); +} diff --git a/tests/ui/arc_with_non_send_sync.stderr b/tests/ui/arc_with_non_send_sync.stderr new file mode 100644 index 00000000000..99e38bcc1a2 --- /dev/null +++ b/tests/ui/arc_with_non_send_sync.stderr @@ -0,0 +1,11 @@ +error: usage of `Arc` where `T` is not `Send` or `Sync` + --> $DIR/arc_with_non_send_sync.rs:11:13 + | +LL | let b = Arc::new(RefCell::new(42)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using `Rc` instead or wrapping `T` in a std::sync type like Mutex + = note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings` + +error: aborting due to previous error +