Improve lint message of await_holding_*

flip1995 · flip1995 · commit cdf9a28006b1 · 2022-02-17T18:00:40.000+01:00
Improves the message of the lints await_holding_lock and
await_holding_refcell_ref. Now also actually tests RwLock.
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();

Original file line number	Diff line number	Diff line change
`@@ -1,39 +1,42 @@`
`1`		`-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await`
	`1`	+error: this `RefCell` reference is held across an `await` point
`2`	`2`	`--> $DIR/await_holding_refcell_ref.rs:6:9`
`3`	`3`	`\|`
`4`	`4`	`LL \| let b = x.borrow();`
`5`	`5`	`\| ^`
`6`	`6`	`\|`
`7`	`7`	= note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings`
`8`		`-note: these are all the await points this ref is held through`
	`8`	+ = help: ensure the reference is dropped before calling `await`
	`9`	+note: these are all the `await` points this reference is held through
`9`	`10`	`--> $DIR/await_holding_refcell_ref.rs:6:5`
`10`	`11`	`\|`
`11`	`12`	`LL \| / let b = x.borrow();`
`12`	`13`	`LL \| \| baz().await`
`13`	`14`	`LL \| \| }`
`14`	`15`	`\| \|_^`
`15`	`16`
`16`		`-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await`
	`17`	+error: this `RefCell` reference is held across an `await` point
`17`	`18`	`--> $DIR/await_holding_refcell_ref.rs:11:9`
`18`	`19`	`\|`
`19`	`20`	`LL \| let b = x.borrow_mut();`
`20`	`21`	`\| ^`
`21`	`22`	`\|`
`22`		`-note: these are all the await points this ref is held through`
	`23`	+ = help: ensure the reference is dropped before calling `await`
	`24`	+note: these are all the `await` points this reference is held through
`23`	`25`	`--> $DIR/await_holding_refcell_ref.rs:11:5`
`24`	`26`	`\|`
`25`	`27`	`LL \| / let b = x.borrow_mut();`
`26`	`28`	`LL \| \| baz().await`
`27`	`29`	`LL \| \| }`
`28`	`30`	`\| \|_^`
`29`	`31`
`30`		`-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await`
	`32`	+error: this `RefCell` reference is held across an `await` point
`31`	`33`	`--> $DIR/await_holding_refcell_ref.rs:32:9`
`32`	`34`	`\|`
`33`	`35`	`LL \| let b = x.borrow_mut();`
`34`	`36`	`\| ^`
`35`	`37`	`\|`
`36`		`-note: these are all the await points this ref is held through`
	`38`	+ = help: ensure the reference is dropped before calling `await`
	`39`	+note: these are all the `await` points this reference is held through
`37`	`40`	`--> $DIR/await_holding_refcell_ref.rs:32:5`
`38`	`41`	`\|`
`39`	`42`	`LL \| / let b = x.borrow_mut();`
`@@ -45,13 +48,14 @@ LL \| \| first + second + third`
`45`	`48`	`LL \| \| }`
`46`	`49`	`\| \|_^`
`47`	`50`
`48`		`-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await`
	`51`	+error: this `RefCell` reference is held across an `await` point
`49`	`52`	`--> $DIR/await_holding_refcell_ref.rs:44:9`
`50`	`53`	`\|`
`51`	`54`	`LL \| let b = x.borrow_mut();`
`52`	`55`	`\| ^`
`53`	`56`	`\|`
`54`		`-note: these are all the await points this ref is held through`
	`57`	+ = help: ensure the reference is dropped before calling `await`
	`58`	+note: these are all the `await` points this reference is held through
`55`	`59`	`--> $DIR/await_holding_refcell_ref.rs:44:5`
`56`	`60`	`\|`
`57`	`61`	`LL \| / let b = x.borrow_mut();`
`@@ -63,27 +67,29 @@ LL \| \| first + second + third`
`63`	`67`	`LL \| \| }`
`64`	`68`	`\| \|_^`
`65`	`69`
`66`		`-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await`
	`70`	+error: this `RefCell` reference is held across an `await` point
`67`	`71`	`--> $DIR/await_holding_refcell_ref.rs:59:13`
`68`	`72`	`\|`
`69`	`73`	`LL \| let b = x.borrow_mut();`
`70`	`74`	`\| ^`
`71`	`75`	`\|`
`72`		`-note: these are all the await points this ref is held through`
	`76`	+ = help: ensure the reference is dropped before calling `await`
	`77`	+note: these are all the `await` points this reference is held through
`73`	`78`	`--> $DIR/await_holding_refcell_ref.rs:59:9`
`74`	`79`	`\|`
`75`	`80`	`LL \| / let b = x.borrow_mut();`
`76`	`81`	`LL \| \| baz().await`
`77`	`82`	`LL \| \| };`
`78`	`83`	`\| \|_____^`
`79`	`84`
`80`		`-error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await`
	`85`	+error: this `RefCell` reference is held across an `await` point
`81`	`86`	`--> $DIR/await_holding_refcell_ref.rs:71:13`
`82`	`87`	`\|`
`83`	`88`	`LL \| let b = x.borrow_mut();`
`84`	`89`	`\| ^`
`85`	`90`	`\|`
`86`		`-note: these are all the await points this ref is held through`
	`91`	+ = help: ensure the reference is dropped before calling `await`
	`92`	+note: these are all the `await` points this reference is held through
`87`	`93`	`--> $DIR/await_holding_refcell_ref.rs:71:9`
`88`	`94`	`\|`
`89`	`95`	`LL \| / let b = x.borrow_mut();`