about summary refs log tree commit diff
diff options
context:
space:
mode:
authorflip1995 <philipp.krones@embecosm.com>2022-02-17 17:57:39 +0100
committerflip1995 <philipp.krones@embecosm.com>2022-02-17 18:03:17 +0100
commitea0ce7bb76bb01741fc3ba2ca32a80c75a380d0f (patch)
tree381f20ed7273c44f9a573c40a270216226d7132f
parentc5709419b1b31c8d4c5e369c7b9adbf592c599ca (diff)
downloadrust-ea0ce7bb76bb01741fc3ba2ca32a80c75a380d0f.tar.gz
rust-ea0ce7bb76bb01741fc3ba2ca32a80c75a380d0f.zip
Move await_holding_* lints to suspicious and improve doc
Even though the FP for that the lints were moved to pedantic isn't fixed
yet, running the lintcheck tool over the most popular 279 crates didn't
trigger this lint once. I would say that this lint is valuable enough,
despite the known FP, to be warn-by-default. Especially since a pretty
nice workaround exists.
-rw-r--r--clippy_lints/src/await_holding_invalid.rs88
-rw-r--r--clippy_lints/src/lib.register_all.rs2
-rw-r--r--clippy_lints/src/lib.register_pedantic.rs2
-rw-r--r--clippy_lints/src/lib.register_suspicious.rs2
4 files changed, 64 insertions, 30 deletions
diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs
index f7bc8395c82..f0979840ff8 100644
--- a/clippy_lints/src/await_holding_invalid.rs
+++ b/clippy_lints/src/await_holding_invalid.rs
@@ -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,41 +21,57 @@ 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
@@ -64,35 +79,52 @@ declare_clippy_lint! {
     /// 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]);
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index de0ff536907..a94f6b528b4 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/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),
diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs
index 1292675f4a9..00d30513181 100644
--- a/clippy_lints/src/lib.register_pedantic.rs
+++ b/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),
diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs
index 10f8ae4b7f7..da56f800804 100644
--- a/clippy_lints/src/lib.register_suspicious.rs
+++ b/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),