Auto merge of #8419 - flip1995:await_parking_alot, r=llogiq

Fix `await_holding_lock` not linting `parking_lot` Mutex/RwLock

This adds tests for `RwLock` and `parking_lot::{Mutex, RwLock}`, which were added before in 2dc8c08, but never tested in UI tests. I noticed this while reading [fasterthanli.me](https://fasterthanli.me/articles/a-rust-match-made-in-hell) latest blog post, complaining that Clippy doesn't catch this for `parking_lot`. (Too many people read his blog, he's too powerful)

Some more things:
- Adds a test for #6446
- Improves the lint message

changelog: [`await_holding_lock`]: Now also lints for `parking_lot::{Mutex, RwLock}`

clippy_lints/src/await_holding_invalid.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{match_def_path, paths};
 use rustc_hir::def_id::DefId;
 use rustc_hir::{AsyncGeneratorKind, Body, BodyId, GeneratorKind};
@@ -9,8 +9,7 @@ use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for calls to await while holding a
-    /// non-async-aware MutexGuard.
+    /// Checks for calls to await while holding a non-async-aware MutexGuard.
     ///
     /// ### Why is this bad?
     /// The Mutex types found in std::sync and parking_lot
@@ -22,77 +21,110 @@ declare_clippy_lint! {
     /// either by introducing a scope or an explicit call to Drop::drop.
     ///
     /// ### Known problems
-    /// Will report false positive for explicitly dropped guards ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)).
+    /// Will report false positive for explicitly dropped guards
+    /// ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)). A workaround for this is
+    /// to wrap the `.lock()` call in a block instead of explicitly dropping the guard.
     ///
     /// ### Example
-    /// ```rust,ignore
-    /// use std::sync::Mutex;
-    ///
+    /// ```rust
+    /// # use std::sync::Mutex;
+    /// # async fn baz() {}
     /// async fn foo(x: &Mutex<u32>) {
-    ///   let guard = x.lock().unwrap();
+    ///   let mut guard = x.lock().unwrap();
     ///   *guard += 1;
-    ///   bar.await;
+    ///   baz().await;
+    /// }
+    ///
+    /// async fn bar(x: &Mutex<u32>) {
+    ///   let mut guard = x.lock().unwrap();
+    ///   *guard += 1;
+    ///   drop(guard); // explicit drop
+    ///   baz().await;
     /// }
     /// ```
     ///
     /// Use instead:
-    /// ```rust,ignore
-    /// use std::sync::Mutex;
-    ///
+    /// ```rust
+    /// # use std::sync::Mutex;
+    /// # async fn baz() {}
     /// async fn foo(x: &Mutex<u32>) {
     ///   {
-    ///     let guard = x.lock().unwrap();
+    ///     let mut guard = x.lock().unwrap();
     ///     *guard += 1;
     ///   }
-    ///   bar.await;
+    ///   baz().await;
+    /// }
+    ///
+    /// async fn bar(x: &Mutex<u32>) {
+    ///   {
+    ///     let mut guard = x.lock().unwrap();
+    ///     *guard += 1;
+    ///   } // guard dropped here at end of scope
+    ///   baz().await;
     /// }
     /// ```
     #[clippy::version = "1.45.0"]
     pub AWAIT_HOLDING_LOCK,
-    pedantic,
-    "Inside an async function, holding a MutexGuard while calling await"
+    suspicious,
+    "inside an async function, holding a `MutexGuard` while calling `await`"
 }
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for calls to await while holding a
-    /// `RefCell` `Ref` or `RefMut`.
+    /// Checks for calls to await while holding a `RefCell` `Ref` or `RefMut`.
     ///
     /// ### Why is this bad?
     /// `RefCell` refs only check for exclusive mutable access
     /// at runtime. Holding onto a `RefCell` ref across an `await` suspension point
     /// risks panics from a mutable ref shared while other refs are outstanding.
     ///
     /// ### Known problems
-    /// Will report false positive for explicitly dropped refs ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)).
+    /// Will report false positive for explicitly dropped refs
+    /// ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)). A workaround for this is
+    /// to wrap the `.borrow[_mut]()` call in a block instead of explicitly dropping the ref.
     ///
     /// ### Example
-    /// ```rust,ignore
-    /// use std::cell::RefCell;
-    ///
+    /// ```rust
+    /// # use std::cell::RefCell;
+    /// # async fn baz() {}
     /// async fn foo(x: &RefCell<u32>) {
     ///   let mut y = x.borrow_mut();
     ///   *y += 1;
-    ///   bar.await;
+    ///   baz().await;
+    /// }
+    ///
+    /// async fn bar(x: &RefCell<u32>) {
+    ///   let mut y = x.borrow_mut();
+    ///   *y += 1;
+    ///   drop(y); // explicit drop
+    ///   baz().await;
     /// }
     /// ```
     ///
     /// Use instead:
-    /// ```rust,ignore
-    /// use std::cell::RefCell;
-    ///
+    /// ```rust
+    /// # use std::cell::RefCell;
+    /// # async fn baz() {}
     /// async fn foo(x: &RefCell<u32>) {
     ///   {
     ///      let mut y = x.borrow_mut();
     ///      *y += 1;
     ///   }
-    ///   bar.await;
+    ///   baz().await;
+    /// }
+    ///
+    /// async fn bar(x: &RefCell<u32>) {
+    ///   {
+    ///     let mut y = x.borrow_mut();
+    ///     *y += 1;
+    ///   } // y dropped here at end of scope
+    ///   baz().await;
     /// }
     /// ```
     #[clippy::version = "1.49.0"]
     pub AWAIT_HOLDING_REFCELL_REF,
-    pedantic,
-    "Inside an async function, holding a RefCell ref while calling await"
+    suspicious,
+    "inside an async function, holding a `RefCell` ref while calling `await`"
 }
 
 declare_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF]);
@@ -118,23 +150,36 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType
     for ty_cause in ty_causes {
         if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind() {
             if is_mutex_guard(cx, adt.did) {
-                span_lint_and_note(
+                span_lint_and_then(
                     cx,
                     AWAIT_HOLDING_LOCK,
                     ty_cause.span,
-                    "this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await",
-                    ty_cause.scope_span.or(Some(span)),
-                    "these are all the await points this lock is held through",
+                    "this `MutexGuard` is held across an `await` point",
+                    |diag| {
+                        diag.help(
+                            "consider using an async-aware `Mutex` type or ensuring the \
+                                `MutexGuard` is dropped before calling await",
+                        );
+                        diag.span_note(
+                            ty_cause.scope_span.unwrap_or(span),
+                            "these are all the `await` points this lock is held through",
+                        );
+                    },
                 );
             }
             if is_refcell_ref(cx, adt.did) {
-                span_lint_and_note(
+                span_lint_and_then(
                     cx,
                     AWAIT_HOLDING_REFCELL_REF,
                     ty_cause.span,
-                    "this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await",
-                    ty_cause.scope_span.or(Some(span)),
-                    "these are all the await points this ref is held through",
+                    "this `RefCell` reference is held across an `await` point",
+                    |diag| {
+                        diag.help("ensure the reference is dropped before calling `await`");
+                        diag.span_note(
+                            ty_cause.scope_span.unwrap_or(span),
+                            "these are all the `await` points this reference is held through",
+                        );
+                    },
                 );
             }
         }

clippy_lints/src/lib.register_all.rs

@@ -14,6 +14,8 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(attrs::DEPRECATED_SEMVER),
     LintId::of(attrs::MISMATCHED_TARGET_OS),
     LintId::of(attrs::USELESS_ATTRIBUTE),
+    LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
+    LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
     LintId::of(bit_mask::BAD_BIT_MASK),
     LintId::of(bit_mask::INEFFECTIVE_BIT_MASK),
     LintId::of(blacklisted_name::BLACKLISTED_NAME),

clippy_lints/src/lib.register_pedantic.rs

@@ -4,8 +4,6 @@
 
 store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(attrs::INLINE_ALWAYS),
-    LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
-    LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
     LintId::of(bit_mask::VERBOSE_BIT_MASK),
     LintId::of(borrow_as_ptr::BORROW_AS_PTR),
     LintId::of(bytecount::NAIVE_BYTECOUNT),

clippy_lints/src/lib.register_suspicious.rs

@@ -5,6 +5,8 @@
 store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![
     LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP),
     LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
+    LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
+    LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
     LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE),
     LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
     LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),

clippy_utils/src/paths.rs

@@ -105,9 +105,9 @@ pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString",
 pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"];
 pub const PARKING_LOT_RAWMUTEX: [&str; 3] = ["parking_lot", "raw_mutex", "RawMutex"];
 pub const PARKING_LOT_RAWRWLOCK: [&str; 3] = ["parking_lot", "raw_rwlock", "RawRwLock"];
-pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"];
-pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"];
-pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"];
+pub const PARKING_LOT_MUTEX_GUARD: [&str; 3] = ["lock_api", "mutex", "MutexGuard"];
+pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwLockReadGuard"];
+pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwLockWriteGuard"];
 pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
 pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
 pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"];