Auto merge of #10898 - avborhanian:master, r=Manishearth,Centri3

Adds new lint `arc_with_non_send_or_sync`

Fixes #653

Adds a new lint to check for uses of non-Send/Sync types within Arc.

```
changelog: [`arc_with_non_send_sync`]: Added a lint to detect uses of non-Send/Sync types within Arc.
```

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

clippy_lints/src/arc_with_non_send_sync.rs

@@ -0,0 +1,72 @@
+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_middle::ty;
+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 !matches!(arg_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, &[]));
+
+            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>`",
+                );
+            }
+        }
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -40,6 +40,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     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,

clippy_lints/src/lib.rs

@@ -68,6 +68,7 @@ mod renamed_lints;
 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`
 }
 

tests/ui/arc_with_non_send_sync.rs

@@ -0,0 +1,17 @@
+#![warn(clippy::arc_with_non_send_sync)]
+#![allow(unused_variables)]
+use std::cell::RefCell;
+use std::sync::{Arc, Mutex};
+
+fn foo<T>(x: T) {
+    // Should not lint - purposefully ignoring generic args.
+    let a = Arc::new(x);
+}
+
+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));
+}

tests/ui/arc_with_non_send_sync.stderr

@@ -0,0 +1,11 @@
+error: usage of `Arc<T>` where `T` is not `Send` or `Sync`
+  --> $DIR/arc_with_non_send_sync.rs:16:13
+   |
+LL |     let b = Arc::new(RefCell::new(42));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using `Rc<T>` instead or wrapping `T` in a std::sync type like `Mutex<T>`
+   = note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings`
+
+error: aborting due to previous error
+