Improve lint message of await_holding_*

Improves the message of the lints await_holding_lock and await_holding_refcell_ref. Now also actually tests RwLock.
rust-lang · Feb 17, 2022 · cdf9a28 · cdf9a28
1 parent 668b3e4
commit cdf9a28
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 37 deletions.
diff --git a/clippy_lints/src/await_holding_invalid.rs b/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};
@@ -118,23 +118,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",
+                        );
+                    },
                 );
             }
         }

diff --git a/tests/ui/await_holding_lock.rs b/tests/ui/await_holding_lock.rs
@@ -1,6 +1,6 @@
 #![warn(clippy::await_holding_lock)]
 
-use std::sync::Mutex;
+use std::sync::{Mutex, RwLock};
 
 async fn bad(x: &Mutex<u32>) -> u32 {
     let guard = x.lock().unwrap();
@@ -17,6 +17,30 @@ async fn good(x: &Mutex<u32>) -> u32 {
     47
 }
 
+pub async fn bad_rw(x: &RwLock<u32>) -> u32 {
+    let guard = x.read().unwrap();
+    baz().await
+}
+
+pub async fn bad_rw_write(x: &RwLock<u32>) -> u32 {
+    let mut guard = x.write().unwrap();
+    baz().await
+}
+
+pub async fn good_rw(x: &RwLock<u32>) -> u32 {
+    {
+        let guard = x.read().unwrap();
+        let y = *guard + 1;
+    }
+    {
+        let mut guard = x.write().unwrap();
+        *guard += 1;
+    }
+    baz().await;
+    let guard = x.read().unwrap();
+    47
+}
+
 async fn baz() -> u32 {
     42
 }

diff --git a/tests/ui/await_holding_lock.stderr b/tests/ui/await_holding_lock.stderr
@@ -1,26 +1,58 @@
-error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await
+error: this `MutexGuard` is held across an `await` point
   --> $DIR/await_holding_lock.rs:6:9
    |
 LL |     let guard = x.lock().unwrap();
    |         ^^^^^
    |
    = note: `-D clippy::await-holding-lock` implied by `-D warnings`
-note: these are all the await points this lock is held through
+   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
+note: these are all the `await` points this lock is held through
   --> $DIR/await_holding_lock.rs:6:5
    |
 LL | /     let guard = x.lock().unwrap();
 LL | |     baz().await
 LL | | }
    | |_^
 
-error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await
-  --> $DIR/await_holding_lock.rs:27:9
+error: this `MutexGuard` is held across an `await` point
+  --> $DIR/await_holding_lock.rs:21:9
+   |
+LL |     let guard = x.read().unwrap();
+   |         ^^^^^
+   |
+   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
+note: these are all the `await` points this lock is held through
+  --> $DIR/await_holding_lock.rs:21:5
+   |
+LL | /     let guard = x.read().unwrap();
+LL | |     baz().await
+LL | | }
+   | |_^
+
+error: this `MutexGuard` is held across an `await` point
+  --> $DIR/await_holding_lock.rs:26:9
+   |
+LL |     let mut guard = x.write().unwrap();
+   |         ^^^^^^^^^
+   |
+   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
+note: these are all the `await` points this lock is held through
+  --> $DIR/await_holding_lock.rs:26:5
+   |
+LL | /     let mut guard = x.write().unwrap();
+LL | |     baz().await
+LL | | }
+   | |_^
+
+error: this `MutexGuard` is held across an `await` point
+  --> $DIR/await_holding_lock.rs:51:9
    |
 LL |     let guard = x.lock().unwrap();
    |         ^^^^^
    |
-note: these are all the await points this lock is held through
-  --> $DIR/await_holding_lock.rs:27:5
+   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
+note: these are all the `await` points this lock is held through
+  --> $DIR/await_holding_lock.rs:51:5
    |
 LL | /     let guard = x.lock().unwrap();
 LL | |
@@ -31,33 +63,35 @@ LL | |     first + second + third
 LL | | }
    | |_^
 
-error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await
-  --> $DIR/await_holding_lock.rs:40:13
+error: this `MutexGuard` is held across an `await` point
+  --> $DIR/await_holding_lock.rs:64:13
    |
 LL |         let guard = x.lock().unwrap();
    |             ^^^^^
    |
-note: these are all the await points this lock is held through
-  --> $DIR/await_holding_lock.rs:40:9
+   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
+note: these are all the `await` points this lock is held through
+  --> $DIR/await_holding_lock.rs:64:9
    |
 LL | /         let guard = x.lock().unwrap();
 LL | |         baz().await
 LL | |     };
    | |_____^
 
-error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await
-  --> $DIR/await_holding_lock.rs:52:13
+error: this `MutexGuard` is held across an `await` point
+  --> $DIR/await_holding_lock.rs:76:13
    |
 LL |         let guard = x.lock().unwrap();
    |             ^^^^^
    |
-note: these are all the await points this lock is held through
-  --> $DIR/await_holding_lock.rs:52:9
+   = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await
+note: these are all the `await` points this lock is held through
+  --> $DIR/await_holding_lock.rs:76:9
    |
 LL | /         let guard = x.lock().unwrap();
 LL | |         baz().await
 LL | |     }
    | |_____^
 
-error: aborting due to 4 previous errors
+error: aborting due to 6 previous errors
 
diff --git a/tests/ui/await_holding_refcell_ref.stderr b/tests/ui/await_holding_refcell_ref.stderr
@@ -1,39 +1,42 @@
-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await
+error: this `RefCell` reference is held across an `await` point
   --> $DIR/await_holding_refcell_ref.rs:6:9
    |
 LL |     let b = x.borrow();
    |         ^
    |
    = note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings`
-note: these are all the await points this ref is held through
+   = help: ensure the reference is dropped before calling `await`
+note: these are all the `await` points this reference is held through
   --> $DIR/await_holding_refcell_ref.rs:6:5
    |
 LL | /     let b = x.borrow();
 LL | |     baz().await
 LL | | }
    | |_^
 
-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await
+error: this `RefCell` reference is held across an `await` point
   --> $DIR/await_holding_refcell_ref.rs:11:9
    |
 LL |     let b = x.borrow_mut();
    |         ^
    |
-note: these are all the await points this ref is held through
+   = help: ensure the reference is dropped before calling `await`
+note: these are all the `await` points this reference is held through
   --> $DIR/await_holding_refcell_ref.rs:11:5
    |
 LL | /     let b = x.borrow_mut();
 LL | |     baz().await
 LL | | }
    | |_^
 
-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await
+error: this `RefCell` reference is held across an `await` point
   --> $DIR/await_holding_refcell_ref.rs:32:9
    |
 LL |     let b = x.borrow_mut();
    |         ^
    |
-note: these are all the await points this ref is held through
+   = help: ensure the reference is dropped before calling `await`
+note: these are all the `await` points this reference is held through
   --> $DIR/await_holding_refcell_ref.rs:32:5
    |
 LL | /     let b = x.borrow_mut();
@@ -45,13 +48,14 @@ LL | |     first + second + third
 LL | | }
    | |_^
 
-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await
+error: this `RefCell` reference is held across an `await` point
   --> $DIR/await_holding_refcell_ref.rs:44:9
    |
 LL |     let b = x.borrow_mut();
    |         ^
    |
-note: these are all the await points this ref is held through
+   = help: ensure the reference is dropped before calling `await`
+note: these are all the `await` points this reference is held through
   --> $DIR/await_holding_refcell_ref.rs:44:5
    |
 LL | /     let b = x.borrow_mut();
@@ -63,27 +67,29 @@ LL | |     first + second + third
 LL | | }
    | |_^
 
-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await
+error: this `RefCell` reference is held across an `await` point
   --> $DIR/await_holding_refcell_ref.rs:59:13
    |
 LL |         let b = x.borrow_mut();
    |             ^
    |
-note: these are all the await points this ref is held through
+   = help: ensure the reference is dropped before calling `await`
+note: these are all the `await` points this reference is held through
   --> $DIR/await_holding_refcell_ref.rs:59:9
    |
 LL | /         let b = x.borrow_mut();
 LL | |         baz().await
 LL | |     };
    | |_____^
 
-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await
+error: this `RefCell` reference is held across an `await` point
   --> $DIR/await_holding_refcell_ref.rs:71:13
    |
 LL |         let b = x.borrow_mut();
    |             ^
    |
-note: these are all the await points this ref is held through
+   = help: ensure the reference is dropped before calling `await`
+note: these are all the `await` points this reference is held through
   --> $DIR/await_holding_refcell_ref.rs:71:9
    |
 LL | /         let b = x.borrow_mut();