Fix attribute handling

3 files changed, 197 insertions, 40 deletions

diff --git a/clippy_lints/src/non_send_field_in_send_ty.rs b/clippy_lints/src/non_send_field_in_send_ty.rs
index 5d6501196cd..07b50cb8274 100644
--- a/clippy_lints/src/non_send_field_in_send_ty.rs
+++ b/clippy_lints/src/non_send_field_in_send_ty.rs
@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
+use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::ty::{implements_trait, is_copy};
 use rustc_ast::ImplPolarity;
 use rustc_hir::{Item, ItemKind};
@@ -39,17 +39,19 @@ declare_clippy_lint! {
     /// ```
     pub NON_SEND_FIELD_IN_SEND_TY,
     nursery,
-    "a field in a `Send` struct does not implement `Send`"
+    "there is field that does not implement `Send` in a `Send` struct"
 }
 
 declare_lint_pass!(NonSendFieldInSendTy => [NON_SEND_FIELD_IN_SEND_TY]);
 
 impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
-        let send_trait = cx.tcx.get_diagnostic_item(sym::send_trait).unwrap();
-
-        // Check if we are in `Send` impl item
+        // Checks if we are in `Send` impl item.
+        // We start from `Send` impl instead of `check_field_def()` because
+        // single `AdtDef` may have multiple `Send` impls due to generic
+        // parameters, and the lint is much easier to implement in this way.
         if_chain! {
+            if let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::send_trait);
             if let ItemKind::Impl(hir_impl) = &item.kind;
             if let Some(trait_ref) = &hir_impl.of_trait;
             if let Some(trait_id) = trait_ref.trait_def_id();
@@ -63,8 +65,6 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
                     for field in &variant.fields {
                         let field_ty = field.ty(cx.tcx, impl_trait_substs);
 
-                        // TODO: substs rebase_onto
-
                         if raw_pointer_in_ty_def(cx, field_ty)
                             || implements_trait(cx, field_ty, send_trait, &[])
                             || is_copy(cx, field_ty)
@@ -72,28 +72,31 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
                             continue;
                         }
 
-                        if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) {
-                            if is_ty_param(field_ty) {
-                                span_lint_and_help(
-                                    cx,
-                                    NON_SEND_FIELD_IN_SEND_TY,
-                                    field_span,
-                                    "a field in a `Send` struct does not implement `Send`",
-                                    Some(item.span),
-                                    &format!("add `{}: Send` in `Send` impl for `{}`", field_ty, self_ty),
-                                )
-                            } else {
-                                span_lint_and_note(
+                        if let Some(field_hir_id) = field
+                            .did
+                            .as_local()
+                            .map(|local_def_id| cx.tcx.hir().local_def_id_to_hir_id(local_def_id))
+                        {
+                            if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) {
+                                span_lint_hir_and_then(
                                     cx,
                                     NON_SEND_FIELD_IN_SEND_TY,
+                                    field_hir_id,
                                     field_span,
-                                    "a field in a `Send` struct does not implement `Send`",
-                                    Some(item.span),
-                                    &format!(
-                                        "type `{}` doesn't implement `Send` when `{}` is `Send`",
-                                        field_ty, self_ty
-                                    ),
-                                )
+                                    "non-`Send` field found in a `Send` struct",
+                                    |diag| {
+                                        diag.span_note(
+                                            item.span,
+                                            &format!(
+                                                "type `{}` doesn't implement `Send` when `{}` is `Send`",
+                                                field_ty, self_ty
+                                            ),
+                                        );
+                                        if is_ty_param(field_ty) {
+                                            diag.help(&format!("add `{}: Send` bound", field_ty));
+                                        }
+                                    },
+                                );
                             }
                         }
                     }
@@ -121,6 +124,6 @@ fn raw_pointer_in_ty_def<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> b
 }
 
 /// Returns `true` if the type is a type parameter such as `T`.
-fn is_ty_param<'tcx>(target_ty: Ty<'tcx>) -> bool {
+fn is_ty_param(target_ty: Ty<'_>) -> bool {
     matches!(target_ty.kind(), ty::Param(_))
 }
diff --git a/tests/ui/non_send_field_in_send_ty.rs b/tests/ui/non_send_field_in_send_ty.rs
index a0c574f8e59..b97501aa457 100644
--- a/tests/ui/non_send_field_in_send_ty.rs
+++ b/tests/ui/non_send_field_in_send_ty.rs
@@ -3,7 +3,7 @@
 
 use std::cell::UnsafeCell;
 use std::ptr::NonNull;
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, MutexGuard};
 
 // disrustor / RUSTSEC-2020-0150
 pub struct RingBuffer<T> {
@@ -46,6 +46,22 @@ pub struct DeviceHandle<T: UsbContext> {
 
 unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
 
+// Other basic tests
+pub struct MultiField<T> {
+    field1: T,
+    field2: T,
+    field3: T,
+}
+
+unsafe impl<T> Send for MultiField<T> {}
+
+pub enum MyOption<T> {
+    MySome(T),
+    MyNone,
+}
+
+unsafe impl<T> Send for MyOption<T> {}
+
 // Raw pointers are allowed
 extern "C" {
     type SomeFfiType;
@@ -57,7 +73,7 @@ pub struct FpTest {
 
 unsafe impl Send for FpTest {}
 
-// Check raw pointer false positive
+// Test attributes
 #[allow(clippy::non_send_field_in_send_ty)]
 pub struct AttrTest1<T>(T);
 
@@ -76,19 +92,15 @@ unsafe impl<T> Send for AttrTest1<T> {}
 unsafe impl<T> Send for AttrTest2<T> {}
 unsafe impl<T> Send for AttrTest3<T> {}
 
-pub struct MultiField<T> {
-    field1: T,
-    field2: T,
-    field3: T,
+// Multiple non-overlapping `Send` for a single type
+pub struct Complex<A, B> {
+    field1: A,
+    field2: B,
 }
 
-unsafe impl<T> Send for MultiField<T> {}
+unsafe impl<P> Send for Complex<P, u32> {}
 
-pub enum MyOption<T> {
-    MySome(T),
-    MyNone,
-}
-
-unsafe impl<T> Send for MyOption<T> {}
+// `MutexGuard` is non-Send
+unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
 
 fn main() {}
diff --git a/tests/ui/non_send_field_in_send_ty.stderr b/tests/ui/non_send_field_in_send_ty.stderr
new file mode 100644
index 00000000000..3a82828e0fe
--- /dev/null
+++ b/tests/ui/non_send_field_in_send_ty.stderr
@@ -0,0 +1,142 @@
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:10:5
+   |
+LL |     data: Vec<UnsafeCell<T>>,
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::non-send-field-in-send-ty` implied by `-D warnings`
+note: type `std::vec::Vec<std::cell::UnsafeCell<T>>` doesn't implement `Send` when `RingBuffer<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:15:1
+   |
+LL | unsafe impl<T> Send for RingBuffer<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:20:5
+   |
+LL |     lock: Mutex<Box<T>>,
+   |     ^^^^^^^^^^^^^^^^^^^
+   |
+note: type `std::sync::Mutex<std::boxed::Box<T>>` doesn't implement `Send` when `MvccRwLock<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:23:1
+   |
+LL | unsafe impl<T> Send for MvccRwLock<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:28:5
+   |
+LL |     head: Arc<RC>,
+   |     ^^^^^^^^^^^^^
+   |
+note: type `std::sync::Arc<RC>` doesn't implement `Send` when `ArcGuard<RC, T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:31:1
+   |
+LL | unsafe impl<RC, T: Send> Send for ArcGuard<RC, T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:43:5
+   |
+LL |     context: T,
+   |     ^^^^^^^^^^
+   |
+note: type `T` doesn't implement `Send` when `DeviceHandle<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:47:1
+   |
+LL | unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `T: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:51:5
+   |
+LL |     field1: T,
+   |     ^^^^^^^^^
+   |
+note: type `T` doesn't implement `Send` when `MultiField<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:56:1
+   |
+LL | unsafe impl<T> Send for MultiField<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `T: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:52:5
+   |
+LL |     field2: T,
+   |     ^^^^^^^^^
+   |
+note: type `T` doesn't implement `Send` when `MultiField<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:56:1
+   |
+LL | unsafe impl<T> Send for MultiField<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `T: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:53:5
+   |
+LL |     field3: T,
+   |     ^^^^^^^^^
+   |
+note: type `T` doesn't implement `Send` when `MultiField<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:56:1
+   |
+LL | unsafe impl<T> Send for MultiField<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `T: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:59:12
+   |
+LL |     MySome(T),
+   |            ^
+   |
+note: type `T` doesn't implement `Send` when `MyOption<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:63:1
+   |
+LL | unsafe impl<T> Send for MyOption<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `T: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:88:11
+   |
+LL |     Enum2(T),
+   |           ^
+   |
+note: type `T` doesn't implement `Send` when `AttrTest3<T>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:93:1
+   |
+LL | unsafe impl<T> Send for AttrTest3<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `T: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:97:5
+   |
+LL |     field1: A,
+   |     ^^^^^^^^^
+   |
+note: type `P` doesn't implement `Send` when `Complex<P, u32>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:101:1
+   |
+LL | unsafe impl<P> Send for Complex<P, u32> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: add `P: Send` bound
+
+error: non-`Send` field found in a `Send` struct
+  --> $DIR/non_send_field_in_send_ty.rs:98:5
+   |
+LL |     field2: B,
+   |     ^^^^^^^^^
+   |
+note: type `std::sync::MutexGuard<'static, bool>` doesn't implement `Send` when `Complex<Q, std::sync::MutexGuard<'static, bool>>` is `Send`
+  --> $DIR/non_send_field_in_send_ty.rs:104:1
+   |
+LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 11 previous errors
+