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.
});
This commit is contained in:
Kyle Huey 2020-08-14 17:30:48 -07:00
parent c88c614941
commit 4972989b61
7 changed files with 319 additions and 0 deletions

View file

@ -1512,6 +1512,7 @@ Released 2018-09-13
[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
[`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
[`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops
[`async_yields_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#async_yields_async
[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map

View file

@ -0,0 +1,88 @@
use crate::utils::{implements_trait, snippet, span_lint_and_then};
use rustc_errors::Applicability;
use rustc_hir::{AsyncGeneratorKind, Body, BodyId, ExprKind, GeneratorKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// **What it does:**
/// Checks for async blocks that yield values of types that can themselves
/// be awaited.
///
/// **Why is this bad?**
/// An await is likely missing.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// async fn foo() {}
///
/// fn bar() {
/// let x = async {
/// foo()
/// };
/// }
/// ```
/// Use instead:
/// ```rust
/// async fn foo() {}
///
/// fn bar() {
/// let x = async {
/// foo().await
/// };
/// }
/// ```
pub ASYNC_YIELDS_ASYNC,
correctness,
"async blocks that return a type that can be awaited"
}
declare_lint_pass!(AsyncYieldsAsync => [ASYNC_YIELDS_ASYNC]);
impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync {
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
use AsyncGeneratorKind::{Block, Closure};
// For functions, with explicitly defined types, don't warn.
// XXXkhuey maybe we should?
if let Some(GeneratorKind::Async(Block | Closure)) = body.generator_kind {
if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() {
let body_id = BodyId {
hir_id: body.value.hir_id,
};
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
let typeck_results = cx.tcx.typeck(def_id);
let expr_ty = typeck_results.expr_ty(&body.value);
if implements_trait(cx, expr_ty, future_trait_def_id, &[]) {
let return_expr_span = match &body.value.kind {
// XXXkhuey there has to be a better way.
ExprKind::Block(block, _) => block.expr.map(|e| e.span),
ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span),
_ => None,
};
if let Some(return_expr_span) = return_expr_span {
span_lint_and_then(
cx,
ASYNC_YIELDS_ASYNC,
return_expr_span,
"an async construct yields a type which is itself awaitable",
|db| {
db.span_label(body.value.span, "outer async construct");
db.span_label(return_expr_span, "awaitable value not awaited");
db.span_suggestion(
return_expr_span,
"consider awaiting this value",
format!("{}.await", snippet(cx, return_expr_span, "..")),
Applicability::MaybeIncorrect,
);
},
);
}
}
}
}
}
}

View file

@ -154,6 +154,7 @@ mod arithmetic;
mod as_conversions;
mod assertions_on_constants;
mod assign_ops;
mod async_yields_async;
mod atomic_ordering;
mod attrs;
mod await_holding_lock;
@ -483,6 +484,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
&assign_ops::ASSIGN_OP_PATTERN,
&assign_ops::MISREFACTORED_ASSIGN_OP,
&async_yields_async::ASYNC_YIELDS_ASYNC,
&atomic_ordering::INVALID_ATOMIC_ORDERING,
&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS,
&attrs::DEPRECATED_CFG_ATTR,
@ -1099,6 +1101,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box unwrap_in_result::UnwrapInResult);
store.register_late_pass(|| box self_assignment::SelfAssignment);
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1232,6 +1235,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP),
LintId::of(&async_yields_async::ASYNC_YIELDS_ASYNC),
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
@ -1675,6 +1679,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_group(true, "clippy::correctness", Some("clippy_correctness"), vec![
LintId::of(&approx_const::APPROX_CONSTANT),
LintId::of(&async_yields_async::ASYNC_YIELDS_ASYNC),
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
LintId::of(&attrs::DEPRECATED_SEMVER),
LintId::of(&attrs::MISMATCHED_TARGET_OS),

View file

@ -52,6 +52,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "assign_ops",
},
Lint {
name: "async_yields_async",
group: "correctness",
desc: "async blocks that return a type that can be awaited",
deprecation: None,
module: "async_yields_async",
},
Lint {
name: "await_holding_lock",
group: "pedantic",

View file

@ -0,0 +1,61 @@
// run-rustfix
// edition:2018
#![feature(async_closure)]
#![warn(clippy::async_yields_async)]
use core::future::Future;
use core::pin::Pin;
use core::task::{Context, Poll};
struct CustomFutureType;
impl Future for CustomFutureType {
type Output = u8;
fn poll(self: Pin<&mut Self>, _: &mut Context) -> Poll<Self::Output> {
Poll::Ready(3)
}
}
fn custom_future_type_ctor() -> CustomFutureType {
CustomFutureType
}
#[rustfmt::skip]
fn main() {
let _f = {
3
};
let _g = async {
3
};
let _h = async {
async {
3
}.await
};
let _i = async {
CustomFutureType.await
};
let _i = async || {
3
};
let _j = async || {
async {
3
}.await
};
let _k = async || {
CustomFutureType.await
};
let _l = async || CustomFutureType.await;
let _m = async || {
println!("I'm bored");
// Some more stuff
// Finally something to await
CustomFutureType.await
};
let _n = async || custom_future_type_ctor();
}

View file

@ -0,0 +1,61 @@
// run-rustfix
// edition:2018
#![feature(async_closure)]
#![warn(clippy::async_yields_async)]
use core::future::Future;
use core::pin::Pin;
use core::task::{Context, Poll};
struct CustomFutureType;
impl Future for CustomFutureType {
type Output = u8;
fn poll(self: Pin<&mut Self>, _: &mut Context) -> Poll<Self::Output> {
Poll::Ready(3)
}
}
fn custom_future_type_ctor() -> CustomFutureType {
CustomFutureType
}
#[rustfmt::skip]
fn main() {
let _f = {
3
};
let _g = async {
3
};
let _h = async {
async {
3
}
};
let _i = async {
CustomFutureType
};
let _i = async || {
3
};
let _j = async || {
async {
3
}
};
let _k = async || {
CustomFutureType
};
let _l = async || CustomFutureType;
let _m = async || {
println!("I'm bored");
// Some more stuff
// Finally something to await
CustomFutureType
};
let _n = async || custom_future_type_ctor();
}

View file

@ -0,0 +1,96 @@
error: an async construct yields a type which is itself awaitable
--> $DIR/async_yields_async.rs:34:9
|
LL | let _h = async {
| ____________________-
LL | | async {
| |_________^
LL | || 3
LL | || }
| ||_________^ awaitable value not awaited
LL | | };
| |_____- outer async construct
|
= note: `-D clippy::async-yields-async` implied by `-D warnings`
help: consider awaiting this value
|
LL | async {
LL | 3
LL | }.await
|
error: an async construct yields a type which is itself awaitable
--> $DIR/async_yields_async.rs:39:9
|
LL | let _i = async {
| ____________________-
LL | | CustomFutureType
| | ^^^^^^^^^^^^^^^^
| | |
| | awaitable value not awaited
| | help: consider awaiting this value: `CustomFutureType.await`
LL | | };
| |_____- outer async construct
error: an async construct yields a type which is itself awaitable
--> $DIR/async_yields_async.rs:45:9
|
LL | let _j = async || {
| _______________________-
LL | | async {
| |_________^
LL | || 3
LL | || }
| ||_________^ awaitable value not awaited
LL | | };
| |_____- outer async construct
|
help: consider awaiting this value
|
LL | async {
LL | 3
LL | }.await
|
error: an async construct yields a type which is itself awaitable
--> $DIR/async_yields_async.rs:50:9
|
LL | let _k = async || {
| _______________________-
LL | | CustomFutureType
| | ^^^^^^^^^^^^^^^^
| | |
| | awaitable value not awaited
| | help: consider awaiting this value: `CustomFutureType.await`
LL | | };
| |_____- outer async construct
error: an async construct yields a type which is itself awaitable
--> $DIR/async_yields_async.rs:52:23
|
LL | let _l = async || CustomFutureType;
| ^^^^^^^^^^^^^^^^
| |
| outer async construct
| awaitable value not awaited
| help: consider awaiting this value: `CustomFutureType.await`
error: an async construct yields a type which is itself awaitable
--> $DIR/async_yields_async.rs:58:9
|
LL | let _m = async || {
| _______________________-
LL | | println!("I'm bored");
LL | | // Some more stuff
LL | |
LL | | // Finally something to await
LL | | CustomFutureType
| | ^^^^^^^^^^^^^^^^
| | |
| | awaitable value not awaited
| | help: consider awaiting this value: `CustomFutureType.await`
LL | | };
| |_____- outer async construct
error: aborting due to 6 previous errors