Skip to content

Commit 3f1d5ee

Browse files
committed
Add a lint for an async block/closure that yields a type that is itself awaitable.
This catches bugs of the form tokio::spawn(async move { let f = some_async_thing(); f // Oh no I forgot to await f so that work will never complete. });
1 parent 8332fe8 commit 3f1d5ee

File tree

7 files changed

+319
-0
lines changed

7 files changed

+319
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,7 @@ Released 2018-09-13
14101410
[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
14111411
[`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
14121412
[`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops
1413+
[`async_yields_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#async_yields_async
14131414
[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
14141415
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
14151416
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
use crate::utils::{implements_trait, snippet, span_lint_and_then};
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{AsyncGeneratorKind, Body, BodyId, ExprKind, GeneratorKind, QPath};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
7+
declare_clippy_lint! {
8+
/// **What it does:**
9+
/// Checks for async blocks that yield values of types that can themselves
10+
/// be awaited.
11+
///
12+
/// **Why is this bad?**
13+
/// An await is likely missing.
14+
///
15+
/// **Known problems:** None.
16+
///
17+
/// **Example:**
18+
///
19+
/// ```rust
20+
/// async fn foo() {}
21+
///
22+
/// fn bar() {
23+
/// let x = async {
24+
/// foo()
25+
/// };
26+
/// }
27+
/// ```
28+
/// Use instead:
29+
/// ```rust
30+
/// async fn foo() {}
31+
///
32+
/// fn bar() {
33+
/// let x = async {
34+
/// foo().await
35+
/// };
36+
/// }
37+
/// ```
38+
pub ASYNC_YIELDS_ASYNC,
39+
correctness,
40+
"async blocks that return a type that can be awaited"
41+
}
42+
43+
declare_lint_pass!(AsyncYieldsAsync => [ASYNC_YIELDS_ASYNC]);
44+
45+
impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync {
46+
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
47+
use AsyncGeneratorKind::{Block, Closure};
48+
// For functions, with explicitly defined types, don't warn.
49+
// XXXkhuey maybe we should?
50+
if let Some(GeneratorKind::Async(Block | Closure)) = body.generator_kind {
51+
if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() {
52+
let body_id = BodyId {
53+
hir_id: body.value.hir_id,
54+
};
55+
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
56+
let typeck_results = cx.tcx.typeck(def_id);
57+
let expr_ty = typeck_results.expr_ty(&body.value);
58+
59+
if implements_trait(cx, expr_ty, future_trait_def_id, &[]) {
60+
let return_expr_span = match &body.value.kind {
61+
// XXXkhuey there has to be a better way.
62+
ExprKind::Block(block, _) => block.expr.map(|e| e.span),
63+
ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span),
64+
_ => None,
65+
};
66+
if let Some(return_expr_span) = return_expr_span {
67+
span_lint_and_then(
68+
cx,
69+
ASYNC_YIELDS_ASYNC,
70+
return_expr_span,
71+
"an async construct yields a type which is itself awaitable",
72+
|db| {
73+
db.span_label(body.value.span, "outer async construct");
74+
db.span_label(return_expr_span, "awaitable value not awaited");
75+
db.span_suggestion(
76+
return_expr_span,
77+
"consider awaiting this value",
78+
format!("{}.await", snippet(cx, return_expr_span, "..")),
79+
Applicability::MaybeIncorrect,
80+
);
81+
},
82+
);
83+
}
84+
}
85+
}
86+
}
87+
}
88+
}

clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ mod arithmetic;
154154
mod as_conversions;
155155
mod assertions_on_constants;
156156
mod assign_ops;
157+
mod async_yields_async;
157158
mod atomic_ordering;
158159
mod attrs;
159160
mod await_holding_lock;
@@ -481,6 +482,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
481482
&assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
482483
&assign_ops::ASSIGN_OP_PATTERN,
483484
&assign_ops::MISREFACTORED_ASSIGN_OP,
485+
&async_yields_async::ASYNC_YIELDS_ASYNC,
484486
&atomic_ordering::INVALID_ATOMIC_ORDERING,
485487
&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS,
486488
&attrs::DEPRECATED_CFG_ATTR,
@@ -1093,6 +1095,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10931095
store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);
10941096
store.register_late_pass(|| box repeat_once::RepeatOnce);
10951097
store.register_late_pass(|| box self_assignment::SelfAssignment);
1098+
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
10961099

10971100
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10981101
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1225,6 +1228,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12251228
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
12261229
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
12271230
LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP),
1231+
LintId::of(&async_yields_async::ASYNC_YIELDS_ASYNC),
12281232
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
12291233
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
12301234
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
@@ -1667,6 +1671,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16671671

16681672
store.register_group(true, "clippy::correctness", Some("clippy_correctness"), vec![
16691673
LintId::of(&approx_const::APPROX_CONSTANT),
1674+
LintId::of(&async_yields_async::ASYNC_YIELDS_ASYNC),
16701675
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
16711676
LintId::of(&attrs::DEPRECATED_SEMVER),
16721677
LintId::of(&attrs::MISMATCHED_TARGET_OS),

src/lintlist/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
5252
deprecation: None,
5353
module: "assign_ops",
5454
},
55+
Lint {
56+
name: "async_yields_async",
57+
group: "correctness",
58+
desc: "async blocks that return a type that can be awaited",
59+
deprecation: None,
60+
module: "async_yields_async",
61+
},
5562
Lint {
5663
name: "await_holding_lock",
5764
group: "pedantic",

tests/ui/async_yields_async.fixed

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// run-rustfix
2+
// edition:2018
3+
4+
#![feature(async_closure)]
5+
#![warn(clippy::async_yields_async)]
6+
7+
use core::future::Future;
8+
use core::pin::Pin;
9+
use core::task::{Context, Poll};
10+
11+
struct CustomFutureType;
12+
13+
impl Future for CustomFutureType {
14+
type Output = u8;
15+
16+
fn poll(self: Pin<&mut Self>, _: &mut Context) -> Poll<Self::Output> {
17+
Poll::Ready(3)
18+
}
19+
}
20+
21+
fn custom_future_type_ctor() -> CustomFutureType {
22+
CustomFutureType
23+
}
24+
25+
#[rustfmt::skip]
26+
fn main() {
27+
let _f = {
28+
3
29+
};
30+
let _g = async {
31+
3
32+
};
33+
let _h = async {
34+
async {
35+
3
36+
}.await
37+
};
38+
let _i = async {
39+
CustomFutureType.await
40+
};
41+
let _i = async || {
42+
3
43+
};
44+
let _j = async || {
45+
async {
46+
3
47+
}.await
48+
};
49+
let _k = async || {
50+
CustomFutureType.await
51+
};
52+
let _l = async || CustomFutureType.await;
53+
let _m = async || {
54+
println!("I'm bored");
55+
// Some more stuff
56+
57+
// Finally something to await
58+
CustomFutureType.await
59+
};
60+
let _n = async || custom_future_type_ctor();
61+
}

tests/ui/async_yields_async.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// run-rustfix
2+
// edition:2018
3+
4+
#![feature(async_closure)]
5+
#![warn(clippy::async_yields_async)]
6+
7+
use core::future::Future;
8+
use core::pin::Pin;
9+
use core::task::{Context, Poll};
10+
11+
struct CustomFutureType;
12+
13+
impl Future for CustomFutureType {
14+
type Output = u8;
15+
16+
fn poll(self: Pin<&mut Self>, _: &mut Context) -> Poll<Self::Output> {
17+
Poll::Ready(3)
18+
}
19+
}
20+
21+
fn custom_future_type_ctor() -> CustomFutureType {
22+
CustomFutureType
23+
}
24+
25+
#[rustfmt::skip]
26+
fn main() {
27+
let _f = {
28+
3
29+
};
30+
let _g = async {
31+
3
32+
};
33+
let _h = async {
34+
async {
35+
3
36+
}
37+
};
38+
let _i = async {
39+
CustomFutureType
40+
};
41+
let _i = async || {
42+
3
43+
};
44+
let _j = async || {
45+
async {
46+
3
47+
}
48+
};
49+
let _k = async || {
50+
CustomFutureType
51+
};
52+
let _l = async || CustomFutureType;
53+
let _m = async || {
54+
println!("I'm bored");
55+
// Some more stuff
56+
57+
// Finally something to await
58+
CustomFutureType
59+
};
60+
let _n = async || custom_future_type_ctor();
61+
}

tests/ui/async_yields_async.stderr

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
error: an async construct yields a type which is itself awaitable
2+
--> $DIR/async_yields_async.rs:34:9
3+
|
4+
LL | let _h = async {
5+
| ____________________-
6+
LL | | async {
7+
| |_________^
8+
LL | || 3
9+
LL | || }
10+
| ||_________^ awaitable value not awaited
11+
LL | | };
12+
| |_____- outer async construct
13+
|
14+
= note: `-D clippy::async-yields-async` implied by `-D warnings`
15+
help: consider awaiting this value
16+
|
17+
LL | async {
18+
LL | 3
19+
LL | }.await
20+
|
21+
22+
error: an async construct yields a type which is itself awaitable
23+
--> $DIR/async_yields_async.rs:39:9
24+
|
25+
LL | let _i = async {
26+
| ____________________-
27+
LL | | CustomFutureType
28+
| | ^^^^^^^^^^^^^^^^
29+
| | |
30+
| | awaitable value not awaited
31+
| | help: consider awaiting this value: `CustomFutureType.await`
32+
LL | | };
33+
| |_____- outer async construct
34+
35+
error: an async construct yields a type which is itself awaitable
36+
--> $DIR/async_yields_async.rs:45:9
37+
|
38+
LL | let _j = async || {
39+
| _______________________-
40+
LL | | async {
41+
| |_________^
42+
LL | || 3
43+
LL | || }
44+
| ||_________^ awaitable value not awaited
45+
LL | | };
46+
| |_____- outer async construct
47+
|
48+
help: consider awaiting this value
49+
|
50+
LL | async {
51+
LL | 3
52+
LL | }.await
53+
|
54+
55+
error: an async construct yields a type which is itself awaitable
56+
--> $DIR/async_yields_async.rs:50:9
57+
|
58+
LL | let _k = async || {
59+
| _______________________-
60+
LL | | CustomFutureType
61+
| | ^^^^^^^^^^^^^^^^
62+
| | |
63+
| | awaitable value not awaited
64+
| | help: consider awaiting this value: `CustomFutureType.await`
65+
LL | | };
66+
| |_____- outer async construct
67+
68+
error: an async construct yields a type which is itself awaitable
69+
--> $DIR/async_yields_async.rs:52:23
70+
|
71+
LL | let _l = async || CustomFutureType;
72+
| ^^^^^^^^^^^^^^^^
73+
| |
74+
| outer async construct
75+
| awaitable value not awaited
76+
| help: consider awaiting this value: `CustomFutureType.await`
77+
78+
error: an async construct yields a type which is itself awaitable
79+
--> $DIR/async_yields_async.rs:58:9
80+
|
81+
LL | let _m = async || {
82+
| _______________________-
83+
LL | | println!("I'm bored");
84+
LL | | // Some more stuff
85+
LL | |
86+
LL | | // Finally something to await
87+
LL | | CustomFutureType
88+
| | ^^^^^^^^^^^^^^^^
89+
| | |
90+
| | awaitable value not awaited
91+
| | help: consider awaiting this value: `CustomFutureType.await`
92+
LL | | };
93+
| |_____- outer async construct
94+
95+
error: aborting due to 6 previous errors
96+

0 commit comments

Comments
 (0)