arc_with_non_send_sync: reword and move to suspicious

clippy_lints/src/arc_with_non_send_sync.rs

@@ -1,12 +1,11 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_then;
 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_middle::ty;
+use rustc_middle::ty::print::with_forced_trimmed_paths;
 use rustc_middle::ty::GenericArgKind;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::symbol::sym;
@@ -16,61 +15,65 @@ declare_clippy_lint! {
     /// 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.
+    /// `Arc<T>` is only `Send`/`Sync` when `T` is [both `Send` and `Sync`](https://doc.rust-lang.org/std/sync/struct.Arc.html#impl-Send-for-Arc%3CT%3E),
+    /// either `T` should be made `Send + Sync` or an `Rc` should be used instead of an `Arc`
     ///
     /// ### Example
     /// ```rust
     /// # use std::cell::RefCell;
     /// # use std::sync::Arc;
     ///
     /// fn main() {
-    ///     // This is safe, as `i32` implements `Send` and `Sync`.
+    ///     // This is fine, as `i32` implements `Send` and `Sync`.
     ///     let a = Arc::new(42);
     ///
-    ///     // This is not safe, as `RefCell` does not implement `Sync`.
+    ///     // `RefCell` is `!Sync`, so either the `Arc` should be replaced with an `Rc`
+    ///     // or the `RefCell` replaced with something like a `RwLock`
     ///     let b = Arc::new(RefCell::new(42));
     /// }
     /// ```
     #[clippy::version = "1.72.0"]
     pub ARC_WITH_NON_SEND_SYNC,
-    correctness,
+    suspicious,
     "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 is_type_diagnostic_item(cx, ty, sym::Arc)
+            && let ExprKind::Call(func, [arg]) = expr.kind
+            && let ExprKind::Path(func_path) = func.kind
+            && last_path_segment(&func_path).ident.name == sym::new
+            && let arg_ty = cx.typeck_results().expr_ty(arg)
             // make sure that the type is not and does not contain any type parameters
-            if arg_ty.walk().all(|arg| {
+            && arg_ty.walk().all(|arg| {
                 !matches!(arg.unpack(), GenericArgKind::Type(ty) if matches!(ty.kind(), ty::Param(_)))
-            });
-            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, &[]));
+            })
+            && let Some(send) = cx.tcx.get_diagnostic_item(sym::Send)
+            && let Some(sync) = cx.tcx.lang_items().sync_trait()
+            && let [is_send, is_sync] = [send, sync].map(|id| implements_trait(cx, arg_ty, id, &[]))
+            && !(is_send && is_sync)
+        {
+            span_lint_and_then(
+                cx,
+                ARC_WITH_NON_SEND_SYNC,
+                expr.span,
+                "usage of an `Arc` that is not `Send` or `Sync`",
+                |diag| with_forced_trimmed_paths!({
+                    if !is_send {
+                        diag.note(format!("the trait `Send` is not implemented for `{arg_ty}`"));
+                    }
+                    if !is_sync {
+                        diag.note(format!("the trait `Sync` is not implemented for `{arg_ty}`"));
+                    }
+
+                    diag.note(format!("required for `{ty}` to implement `Send` and `Sync`"));
 
-            then {
-                span_lint_and_help(
-                    cx,
-                    ARC_WITH_NON_SEND_SYNC,
-                    expr.span,
-                    "usage of `Arc<T>` where `T` is not `Send` or `Sync`",
-                    None,
-                    "consider using `Rc<T>` instead or wrapping `T` in a std::sync type like \
-                    `Mutex<T>`",
-                );
-            }
+                    diag.help("consider using an `Rc` instead or wrapping the inner type with a `Mutex`");
+                }
+            ));
         }
     }
 }

tests/ui/arc_with_non_send_sync.rs

@@ -12,9 +12,13 @@ fn issue11076<T>() {
 }
 
 fn main() {
-    // This is safe, as `i32` implements `Send` and `Sync`.
-    let a = Arc::new(42);
+    let _ = Arc::new(42);
 
-    // This is not safe, as `RefCell` does not implement `Sync`.
-    let b = Arc::new(RefCell::new(42));
+    // !Sync
+    let _ = Arc::new(RefCell::new(42));
+    let mutex = Mutex::new(1);
+    // !Send
+    let _ = Arc::new(mutex.lock().unwrap());
+    // !Send + !Sync
+    let _ = Arc::new(&42 as *const i32);
 }

tests/ui/arc_with_non_send_sync.stderr

@@ -1,11 +1,34 @@
-error: usage of `Arc<T>` where `T` is not `Send` or `Sync`
-  --> $DIR/arc_with_non_send_sync.rs:19:13
+error: usage of an `Arc` that is not `Send` or `Sync`
+  --> $DIR/arc_with_non_send_sync.rs:18:13
    |
-LL |     let b = Arc::new(RefCell::new(42));
+LL |     let _ = Arc::new(RefCell::new(42));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = help: consider using `Rc<T>` instead or wrapping `T` in a std::sync type like `Mutex<T>`
+   = note: the trait `Sync` is not implemented for `RefCell<i32>`
+   = note: required for `Arc<RefCell<i32>>` to implement `Send` and `Sync`
+   = help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
    = note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings`
 
-error: aborting due to previous error
+error: usage of an `Arc` that is not `Send` or `Sync`
+  --> $DIR/arc_with_non_send_sync.rs:21:13
+   |
+LL |     let _ = Arc::new(mutex.lock().unwrap());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the trait `Send` is not implemented for `MutexGuard<'_, i32>`
+   = note: required for `Arc<MutexGuard<'_, i32>>` to implement `Send` and `Sync`
+   = help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
+
+error: usage of an `Arc` that is not `Send` or `Sync`
+  --> $DIR/arc_with_non_send_sync.rs:23:13
+   |
+LL |     let _ = Arc::new(&42 as *const i32);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the trait `Send` is not implemented for `*const i32`
+   = note: the trait `Sync` is not implemented for `*const i32`
+   = note: required for `Arc<*const i32>` to implement `Send` and `Sync`
+   = help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
+
+error: aborting due to 3 previous errors