Auto merge of #5909 - khuey:async_yields_async, r=yaahc
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. }); See the two XXXkhuey comments and the unfixed `_l` structure for things that need more thought. *Please keep the line below* changelog: none
This commit is contained in:
commit
8334a58c2f
@ -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
|
||||
|
86
clippy_lints/src/async_yields_async.rs
Normal file
86
clippy_lints/src/async_yields_async.rs
Normal file
@ -0,0 +1,86 @@
|
||||
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,
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
@ -154,6 +154,7 @@ macro_rules! declare_clippy_lint {
|
||||
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),
|
||||
|
@ -52,6 +52,13 @@
|
||||
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",
|
||||
|
68
tests/ui/async_yields_async.fixed
Normal file
68
tests/ui/async_yields_async.fixed
Normal file
@ -0,0 +1,68 @@
|
||||
// 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
|
||||
}
|
||||
|
||||
async fn f() -> CustomFutureType {
|
||||
// Don't warn for functions since you have to explicitly declare their
|
||||
// return types.
|
||||
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();
|
||||
let _o = async || f();
|
||||
}
|
68
tests/ui/async_yields_async.rs
Normal file
68
tests/ui/async_yields_async.rs
Normal file
@ -0,0 +1,68 @@
|
||||
// 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
|
||||
}
|
||||
|
||||
async fn f() -> CustomFutureType {
|
||||
// Don't warn for functions since you have to explicitly declare their
|
||||
// return types.
|
||||
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();
|
||||
let _o = async || f();
|
||||
}
|
96
tests/ui/async_yields_async.stderr
Normal file
96
tests/ui/async_yields_async.stderr
Normal file
@ -0,0 +1,96 @@
|
||||
error: an async construct yields a type which is itself awaitable
|
||||
--> $DIR/async_yields_async.rs:40: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:45: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:51: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:56: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:58: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:64: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
|
||||
|
Loading…
Reference in New Issue
Block a user