about summary refs log tree commit diff
diff options
context:
space:
mode:
authorKyle Huey <khuey@kylehuey.com>2020-08-14 17:30:48 -0700
committerKyle Huey <khuey@kylehuey.com>2020-08-29 15:33:54 -0700
commit4972989b616cbf96c015cd9fdf1f4b4464ecaace (patch)
tree08b2ca3af70dc01dad8d4b173ae76ee79f0b3554
parentc88c6149415dd47b5f05e69d7307e0a1967c33f2 (diff)
downloadrust-4972989b616cbf96c015cd9fdf1f4b4464ecaace.tar.gz
rust-4972989b616cbf96c015cd9fdf1f4b4464ecaace.zip
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.
});
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/async_yields_async.rs88
-rw-r--r--clippy_lints/src/lib.rs5
-rw-r--r--src/lintlist/mod.rs7
-rw-r--r--tests/ui/async_yields_async.fixed61
-rw-r--r--tests/ui/async_yields_async.rs61
-rw-r--r--tests/ui/async_yields_async.stderr96
7 files changed, 319 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 34d48821023..99a8b1a6293 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/clippy_lints/src/async_yields_async.rs b/clippy_lints/src/async_yields_async.rs
new file mode 100644
index 00000000000..ae347fcd3e8
--- /dev/null
+++ b/clippy_lints/src/async_yields_async.rs
@@ -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,
+                                );
+                            },
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 577ce6523b4..0eb1d331366 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -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),
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 687fac7baa8..dff19ef440f 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -53,6 +53,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         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",
         desc: "Inside an async function, holding a MutexGuard while calling await",
diff --git a/tests/ui/async_yields_async.fixed b/tests/ui/async_yields_async.fixed
new file mode 100644
index 00000000000..cadc6494c76
--- /dev/null
+++ b/tests/ui/async_yields_async.fixed
@@ -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();
+}
diff --git a/tests/ui/async_yields_async.rs b/tests/ui/async_yields_async.rs
new file mode 100644
index 00000000000..898fe1a9561
--- /dev/null
+++ b/tests/ui/async_yields_async.rs
@@ -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();
+}
diff --git a/tests/ui/async_yields_async.stderr b/tests/ui/async_yields_async.stderr
new file mode 100644
index 00000000000..112984cdccb
--- /dev/null
+++ b/tests/ui/async_yields_async.stderr
@@ -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
+