Auto merge of rust-lang#10448 - samueltardieu:issue-10444, r=llogiq

Add new `redundant_async_block` lint

Fixes rust-lang#10444

changelog: [`redundant_async_block`]: new lint to detect `async { future.await }`

Author: bors

CHANGELOG.md

@@ -4836,6 +4836,7 @@ Released 2018-09-13
 [`read_zero_byte_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_zero_byte_vec
 [`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl
 [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation
+[`redundant_async_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_async_block
 [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
 [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
 [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call

clippy_lints/src/declared_lints.rs

@@ -519,6 +519,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::ranges::REVERSED_EMPTY_RANGES_INFO,
     crate::rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT_INFO,
     crate::read_zero_byte_vec::READ_ZERO_BYTE_VEC_INFO,
+    crate::redundant_async_block::REDUNDANT_ASYNC_BLOCK_INFO,
     crate::redundant_clone::REDUNDANT_CLONE_INFO,
     crate::redundant_closure_call::REDUNDANT_CLOSURE_CALL_INFO,
     crate::redundant_else::REDUNDANT_ELSE_INFO,

clippy_lints/src/lib.rs

@@ -251,6 +251,7 @@ mod question_mark_used;
 mod ranges;
 mod rc_clone_in_vec_init;
 mod read_zero_byte_vec;
+mod redundant_async_block;
 mod redundant_clone;
 mod redundant_closure_call;
 mod redundant_else;
@@ -928,6 +929,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
     store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead));
     store.register_late_pass(|_| Box::new(missing_assert_message::MissingAssertMessage));
+    store.register_early_pass(|| Box::new(redundant_async_block::RedundantAsyncBlock));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/redundant_async_block.rs

@@ -0,0 +1,84 @@
+use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet};
+use rustc_ast::ast::*;
+use rustc_ast::visit::Visitor as AstVisitor;
+use rustc_errors::Applicability;
+use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `async` block that only returns `await` on a future.
+    ///
+    /// ### Why is this bad?
+    /// It is simpler and more efficient to use the future directly.
+    ///
+    /// ### Example
+    /// ```rust
+    /// async fn f() -> i32 {
+    ///     1 + 2
+    /// }
+    ///
+    /// let fut = async {
+    ///     f().await
+    /// };
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// async fn f() -> i32 {
+    ///     1 + 2
+    /// }
+    ///
+    /// let fut = f();
+    /// ```
+    #[clippy::version = "1.69.0"]
+    pub REDUNDANT_ASYNC_BLOCK,
+    complexity,
+    "`async { future.await }` can be replaced by `future`"
+}
+declare_lint_pass!(RedundantAsyncBlock => [REDUNDANT_ASYNC_BLOCK]);
+
+impl EarlyLintPass for RedundantAsyncBlock {
+    fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
+        if expr.span.from_expansion() {
+            return;
+        }
+        if let ExprKind::Async(_, _, block) = &expr.kind && block.stmts.len() == 1 &&
+            let Some(Stmt { kind: StmtKind::Expr(last), .. }) = block.stmts.last() &&
+            let ExprKind::Await(future) = &last.kind &&
+            !future.span.from_expansion() &&
+            !await_in_expr(future)
+        {
+            span_lint_and_sugg(
+                cx,
+                REDUNDANT_ASYNC_BLOCK,
+                expr.span,
+                "this async expression only awaits a single future",
+                "you can reduce it to",
+                snippet(cx, future.span, "..").into_owned(),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
+/// Check whether an expression contains `.await`
+fn await_in_expr(expr: &Expr) -> bool {
+    let mut detector = AwaitDetector::default();
+    detector.visit_expr(expr);
+    detector.await_found
+}
+
+#[derive(Default)]
+struct AwaitDetector {
+    await_found: bool,
+}
+
+impl<'ast> AstVisitor<'ast> for AwaitDetector {
+    fn visit_expr(&mut self, ex: &'ast Expr) {
+        match (&ex.kind, self.await_found) {
+            (ExprKind::Await(_), _) => self.await_found = true,
+            (_, false) => rustc_ast::visit::walk_expr(self, ex),
+            _ => (),
+        }
+    }
+}

tests/ui/async_yields_async.fixed

@@ -2,6 +2,7 @@
 #![feature(lint_reasons)]
 #![feature(async_closure)]
 #![warn(clippy::async_yields_async)]
+#![allow(clippy::redundant_async_block)]
 
 use core::future::Future;
 use core::pin::Pin;

tests/ui/async_yields_async.rs

@@ -2,6 +2,7 @@
 #![feature(lint_reasons)]
 #![feature(async_closure)]
 #![warn(clippy::async_yields_async)]
+#![allow(clippy::redundant_async_block)]
 
 use core::future::Future;
 use core::pin::Pin;

tests/ui/async_yields_async.stderr

@@ -1,5 +1,5 @@
 error: an async construct yields a type which is itself awaitable
-  --> $DIR/async_yields_async.rs:39:9
+  --> $DIR/async_yields_async.rs:40:9
    |
 LL |        let _h = async {
    |  _____________________-
@@ -19,7 +19,7 @@ LL +         }.await
    |
 
 error: an async construct yields a type which is itself awaitable
-  --> $DIR/async_yields_async.rs:44:9
+  --> $DIR/async_yields_async.rs:45:9
    |
 LL |       let _i = async {
    |  ____________________-
@@ -32,7 +32,7 @@ LL | |     };
    | |_____- outer async construct
 
 error: an async construct yields a type which is itself awaitable
-  --> $DIR/async_yields_async.rs:50:9
+  --> $DIR/async_yields_async.rs:51:9
    |
 LL |        let _j = async || {
    |  ________________________-
@@ -51,7 +51,7 @@ LL +         }.await
    |
 
 error: an async construct yields a type which is itself awaitable
-  --> $DIR/async_yields_async.rs:55:9
+  --> $DIR/async_yields_async.rs:56:9
    |
 LL |       let _k = async || {
    |  _______________________-
@@ -64,7 +64,7 @@ LL | |     };
    | |_____- outer async construct
 
 error: an async construct yields a type which is itself awaitable
-  --> $DIR/async_yields_async.rs:57:23
+  --> $DIR/async_yields_async.rs:58:23
    |
 LL |     let _l = async || CustomFutureType;
    |                       ^^^^^^^^^^^^^^^^
@@ -74,7 +74,7 @@ LL |     let _l = async || CustomFutureType;
    |                       help: consider awaiting this value: `CustomFutureType.await`
 
 error: an async construct yields a type which is itself awaitable
-  --> $DIR/async_yields_async.rs:63:9
+  --> $DIR/async_yields_async.rs:64:9
    |
 LL |       let _m = async || {
    |  _______________________-

tests/ui/redundant_async_block.fixed

@@ -0,0 +1,64 @@
+// run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::redundant_async_block)]
+
+async fn func1(n: usize) -> usize {
+    n + 1
+}
+
+async fn func2() -> String {
+    let s = String::from("some string");
+    let f = async { (*s).to_owned() };
+    let x = f;
+    x.await
+}
+
+macro_rules! await_in_macro {
+    ($e:expr) => {
+        std::convert::identity($e).await
+    };
+}
+
+async fn func3(n: usize) -> usize {
+    // Do not lint (suggestion would be `std::convert::identity(func1(n))`
+    // which copies code from inside the macro)
+    async move { await_in_macro!(func1(n)) }.await
+}
+
+// This macro should never be linted as `$e` might contain `.await`
+macro_rules! async_await_parameter_in_macro {
+    ($e:expr) => {
+        async { $e.await }
+    };
+}
+
+// MISSED OPPORTUNITY: this macro could be linted as the `async` block does not
+// contain code coming from the parameters
+macro_rules! async_await_in_macro {
+    ($f:expr) => {
+        ($f)(async { func2().await })
+    };
+}
+
+fn main() {
+    let fut1 = async { 17 };
+    let fut2 = fut1;
+
+    let fut1 = async { 25 };
+    let fut2 = fut1;
+
+    let fut = async { 42 };
+
+    // Do not lint: not a single expression
+    let fut = async {
+        func1(10).await;
+        func2().await
+    };
+
+    // Do not lint: expression contains `.await`
+    let fut = async { func1(func2().await.len()).await };
+
+    let fut = async_await_parameter_in_macro!(func2());
+    let fut = async_await_in_macro!(std::convert::identity);
+}

tests/ui/redundant_async_block.rs

@@ -0,0 +1,64 @@
+// run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::redundant_async_block)]
+
+async fn func1(n: usize) -> usize {
+    n + 1
+}
+
+async fn func2() -> String {
+    let s = String::from("some string");
+    let f = async { (*s).to_owned() };
+    let x = async { f.await };
+    x.await
+}
+
+macro_rules! await_in_macro {
+    ($e:expr) => {
+        std::convert::identity($e).await
+    };
+}
+
+async fn func3(n: usize) -> usize {
+    // Do not lint (suggestion would be `std::convert::identity(func1(n))`
+    // which copies code from inside the macro)
+    async move { await_in_macro!(func1(n)) }.await
+}
+
+// This macro should never be linted as `$e` might contain `.await`
+macro_rules! async_await_parameter_in_macro {
+    ($e:expr) => {
+        async { $e.await }
+    };
+}
+
+// MISSED OPPORTUNITY: this macro could be linted as the `async` block does not
+// contain code coming from the parameters
+macro_rules! async_await_in_macro {
+    ($f:expr) => {
+        ($f)(async { func2().await })
+    };
+}
+
+fn main() {
+    let fut1 = async { 17 };
+    let fut2 = async { fut1.await };
+
+    let fut1 = async { 25 };
+    let fut2 = async move { fut1.await };
+
+    let fut = async { async { 42 }.await };
+
+    // Do not lint: not a single expression
+    let fut = async {
+        func1(10).await;
+        func2().await
+    };
+
+    // Do not lint: expression contains `.await`
+    let fut = async { func1(func2().await.len()).await };
+
+    let fut = async_await_parameter_in_macro!(func2());
+    let fut = async_await_in_macro!(std::convert::identity);
+}

tests/ui/redundant_async_block.stderr

@@ -0,0 +1,28 @@
+error: this async expression only awaits a single future
+  --> $DIR/redundant_async_block.rs:13:13
+   |
+LL |     let x = async { f.await };
+   |             ^^^^^^^^^^^^^^^^^ help: you can reduce it to: `f`
+   |
+   = note: `-D clippy::redundant-async-block` implied by `-D warnings`
+
+error: this async expression only awaits a single future
+  --> $DIR/redundant_async_block.rs:46:16
+   |
+LL |     let fut2 = async { fut1.await };
+   |                ^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `fut1`
+
+error: this async expression only awaits a single future
+  --> $DIR/redundant_async_block.rs:49:16
+   |
+LL |     let fut2 = async move { fut1.await };
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `fut1`
+
+error: this async expression only awaits a single future
+  --> $DIR/redundant_async_block.rs:51:15
+   |
+LL |     let fut = async { async { 42 }.await };
+   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `async { 42 }`
+
+error: aborting due to 4 previous errors
+

tests/ui/redundant_closure_call_fixable.fixed

@@ -2,6 +2,7 @@
 
 #![feature(async_closure)]
 #![warn(clippy::redundant_closure_call)]
+#![allow(clippy::redundant_async_block)]
 #![allow(unused)]
 
 async fn something() -> u32 {

tests/ui/redundant_closure_call_fixable.rs

@@ -2,6 +2,7 @@
 
 #![feature(async_closure)]
 #![warn(clippy::redundant_closure_call)]
+#![allow(clippy::redundant_async_block)]
 #![allow(unused)]
 
 async fn something() -> u32 {