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.
This commit is contained in:
flip1995 2022-02-17 17:57:39 +01:00
parent c5709419b1
commit ea0ce7bb76
No known key found for this signature in database
GPG Key ID: 1CA0DF2AF59D68A5
4 changed files with 64 additions and 30 deletions

View File

@ -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]);

View File

@ -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),

View File

@ -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),

View File

@ -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),