diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index c9210bf73f8..4638af8da9f 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -935,7 +935,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));
+ store.register_late_pass(|_| Box::new(redundant_async_block::RedundantAsyncBlock));
store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped));
store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));
store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(msrv())));
diff --git a/clippy_lints/src/redundant_async_block.rs b/clippy_lints/src/redundant_async_block.rs
index 2d30e77d55d..a0f831764d0 100644
--- a/clippy_lints/src/redundant_async_block.rs
+++ b/clippy_lints/src/redundant_async_block.rs
@@ -1,8 +1,15 @@
-use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet};
-use rustc_ast::ast::{Expr, ExprKind, Stmt, StmtKind};
-use rustc_ast::visit::Visitor as AstVisitor;
+use std::ops::ControlFlow;
+
+use clippy_utils::{
+ diagnostics::span_lint_and_sugg,
+ peel_blocks,
+ source::{snippet, walk_span_to_context},
+ visitors::for_each_expr,
+};
use rustc_errors::Applicability;
-use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_hir::{AsyncGeneratorKind, Closure, Expr, ExprKind, GeneratorKind, MatchSource};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::{lint::in_external_macro, ty::UpvarCapture};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
@@ -14,106 +21,88 @@ declare_clippy_lint! {
///
/// ### Example
/// ```rust
- /// async fn f() -> i32 {
- /// 1 + 2
- /// }
- ///
+ /// let f = async {
+ /// 1 + 2
+ /// };
/// let fut = async {
- /// f().await
+ /// f.await
/// };
/// ```
/// Use instead:
/// ```rust
- /// async fn f() -> i32 {
- /// 1 + 2
- /// }
- ///
- /// let fut = f();
+ /// let f = async {
+ /// 1 + 2
+ /// };
+ /// let fut = f;
/// ```
#[clippy::version = "1.69.0"]
pub REDUNDANT_ASYNC_BLOCK,
- nursery,
+ 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)
+impl<'tcx> LateLintPass<'tcx> for RedundantAsyncBlock {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+ let span = expr.span;
+ if !in_external_macro(cx.tcx.sess, span) &&
+ let Some(body_expr) = desugar_async_block(cx, expr) &&
+ let Some(expr) = desugar_await(peel_blocks(body_expr)) &&
+ // The await prefix must not come from a macro as its content could change in the future.
+ expr.span.ctxt() == body_expr.span.ctxt() &&
+ // An async block does not have immediate side-effects from a `.await` point-of-view.
+ (!expr.can_have_side_effects() || desugar_async_block(cx, expr).is_some()) &&
+ let Some(shortened_span) = walk_span_to_context(expr.span, span.ctxt())
{
- if captures_value(last) {
- // If the async block captures variables then there is no equivalence.
- return;
- }
-
span_lint_and_sugg(
cx,
REDUNDANT_ASYNC_BLOCK,
- expr.span,
+ span,
"this async expression only awaits a single future",
"you can reduce it to",
- snippet(cx, future.span, "..").into_owned(),
+ snippet(cx, shortened_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),
- _ => (),
- }
+/// If `expr` is a desugared `async` block, return the original expression if it does not capture
+/// any variable by ref.
+fn desugar_async_block<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+ if let ExprKind::Closure(Closure { body, def_id, .. }) = expr.kind &&
+ let body = cx.tcx.hir().body(*body) &&
+ matches!(body.generator_kind, Some(GeneratorKind::Async(AsyncGeneratorKind::Block)))
+ {
+ cx
+ .typeck_results()
+ .closure_min_captures
+ .get(def_id)
+ .map_or(true, |m| {
+ m.values().all(|places| {
+ places
+ .iter()
+ .all(|place| matches!(place.info.capture_kind, UpvarCapture::ByValue))
+ })
+ })
+ .then_some(body.value)
+ } else {
+ None
}
}
-/// Check whether an expression may have captured a local variable.
-/// This is done by looking for paths with only one segment, except as
-/// a prefix of `.await` since this would be captured by value.
-///
-/// This function will sometimes return `true` even tough there are no
-/// captures happening: at the AST level, it is impossible to
-/// dinstinguish a function call from a call to a closure which comes
-/// from the local environment.
-fn captures_value(expr: &Expr) -> bool {
- let mut detector = CaptureDetector::default();
- detector.visit_expr(expr);
- detector.capture_found
-}
-
-#[derive(Default)]
-struct CaptureDetector {
- capture_found: bool,
-}
-
-impl<'ast> AstVisitor<'ast> for CaptureDetector {
- fn visit_expr(&mut self, ex: &'ast Expr) {
- match (&ex.kind, self.capture_found) {
- (ExprKind::Await(fut), _) if matches!(fut.kind, ExprKind::Path(..)) => (),
- (ExprKind::Path(_, path), _) if path.segments.len() == 1 => self.capture_found = true,
- (_, false) => rustc_ast::visit::walk_expr(self, ex),
- _ => (),
- }
+/// If `expr` is a desugared `.await`, return the original expression if it does not come from a
+/// macro expansion.
+fn desugar_await<'tcx>(expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+ if let ExprKind::Match(match_value, _, MatchSource::AwaitDesugar) = expr.kind &&
+ let ExprKind::Call(_, [into_future_arg]) = match_value.kind &&
+ let ctxt = expr.span.ctxt() &&
+ for_each_expr(into_future_arg, |e|
+ walk_span_to_context(e.span, ctxt)
+ .map_or(ControlFlow::Break(()), |_| ControlFlow::Continue(()))).is_none()
+ {
+ Some(into_future_arg)
+ } else {
+ None
}
}
diff --git a/tests/ui/redundant_async_block.fixed b/tests/ui/redundant_async_block.fixed
index d26b7a332cb..ad96993c4a7 100644
--- a/tests/ui/redundant_async_block.fixed
+++ b/tests/ui/redundant_async_block.fixed
@@ -1,6 +1,6 @@
// run-rustfix
-#![allow(unused)]
+#![allow(unused, clippy::manual_async_fn)]
#![warn(clippy::redundant_async_block)]
use std::future::Future;
@@ -16,40 +16,16 @@ async fn func2() -> String {
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 };
+ // Lint
let fut2 = fut1;
let fut1 = async { 25 };
+ // Lint
let fut2 = fut1;
+ // Lint
let fut = async { 42 };
// Do not lint: not a single expression
@@ -60,15 +36,12 @@ fn main() {
// 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);
}
#[allow(clippy::let_and_return)]
fn capture_local() -> impl Future<Output = i32> {
- // Lint
let fut = async { 17 };
+ // Lint
fut
}
@@ -80,11 +53,39 @@ fn capture_local_closure(s: &str) -> impl Future<Output = &str> {
#[allow(clippy::let_and_return)]
fn capture_arg(s: &str) -> impl Future<Output = &str> {
- // Lint
let fut = async move { s };
+ // Lint
fut
}
+fn capture_future_arg<T>(f: impl Future<Output = T>) -> impl Future<Output = T> {
+ // Lint
+ f
+}
+
+fn capture_func_result<FN, F, T>(f: FN) -> impl Future<Output = T>
+where
+ F: Future<Output = T>,
+ FN: FnOnce() -> F,
+{
+ // Do not lint, as f() would be evaluated prematurely
+ async { f().await }
+}
+
+fn double_future(f: impl Future<Output = impl Future<Output = u32>>) -> impl Future<Output = u32> {
+ // Do not lint, we will get a `.await` outside a `.async`
+ async { f.await.await }
+}
+
+fn await_in_async<F, R>(f: F) -> impl Future<Output = u32>
+where
+ F: FnOnce() -> R,
+ R: Future<Output = u32>,
+{
+ // Lint
+ async { f().await + 1 }
+}
+
#[derive(Debug, Clone)]
struct F {}
@@ -109,3 +110,84 @@ fn capture() {
// Do not lint: `val` would not live long enough
spawn(async { work(&{ val }).await });
}
+
+fn await_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e:expr) => {
+ $e.await
+ };
+ }
+ // Do not lint: the macro may change in the future
+ // or return different things depending on its argument
+ async { mac!(async { 42 }) }
+}
+
+fn async_expr_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ () => {
+ async { 42 }
+ };
+ }
+ // Do not lint: the macro may change in the future
+ async { mac!().await }
+}
+
+fn async_expr_from_macro_deep() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ () => {
+ async { 42 }
+ };
+ }
+ // Do not lint: the macro may change in the future
+ async { ({ mac!() }).await }
+}
+
+fn all_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ () => {
+ // Lint
+ async { 42 }
+ };
+ }
+ mac!()
+}
+
+fn parts_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e: expr) => {
+ // Do not lint: `$e` might not always be side-effect free
+ async { $e.await }
+ };
+ }
+ mac!(async { 42 })
+}
+
+fn safe_parts_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e: expr) => {
+ // Lint
+ async { $e }
+ };
+ }
+ mac!(42)
+}
+
+fn parts_from_macro_deep() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e: expr) => {
+ // Do not lint: `$e` might not always be side-effect free
+ async { ($e,).0.await }
+ };
+ }
+ let f = std::future::ready(42);
+ mac!(f)
+}
+
+fn await_from_macro_deep() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e:expr) => {{ $e }.await};
+ }
+ // Do not lint: the macro may change in the future
+ // or return different things depending on its argument
+ async { mac!(async { 42 }) }
+}
diff --git a/tests/ui/redundant_async_block.rs b/tests/ui/redundant_async_block.rs
index 04726e62805..7ae23558369 100644
--- a/tests/ui/redundant_async_block.rs
+++ b/tests/ui/redundant_async_block.rs
@@ -1,6 +1,6 @@
// run-rustfix
-#![allow(unused)]
+#![allow(unused, clippy::manual_async_fn)]
#![warn(clippy::redundant_async_block)]
use std::future::Future;
@@ -16,40 +16,16 @@ async fn func2() -> String {
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 };
+ // Lint
let fut2 = async { fut1.await };
let fut1 = async { 25 };
+ // Lint
let fut2 = async move { fut1.await };
+ // Lint
let fut = async { async { 42 }.await };
// Do not lint: not a single expression
@@ -60,15 +36,12 @@ fn main() {
// 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);
}
#[allow(clippy::let_and_return)]
fn capture_local() -> impl Future<Output = i32> {
- // Lint
let fut = async { 17 };
+ // Lint
async move { fut.await }
}
@@ -80,11 +53,39 @@ fn capture_local_closure(s: &str) -> impl Future<Output = &str> {
#[allow(clippy::let_and_return)]
fn capture_arg(s: &str) -> impl Future<Output = &str> {
- // Lint
let fut = async move { s };
+ // Lint
async move { fut.await }
}
+fn capture_future_arg<T>(f: impl Future<Output = T>) -> impl Future<Output = T> {
+ // Lint
+ async { f.await }
+}
+
+fn capture_func_result<FN, F, T>(f: FN) -> impl Future<Output = T>
+where
+ F: Future<Output = T>,
+ FN: FnOnce() -> F,
+{
+ // Do not lint, as f() would be evaluated prematurely
+ async { f().await }
+}
+
+fn double_future(f: impl Future<Output = impl Future<Output = u32>>) -> impl Future<Output = u32> {
+ // Do not lint, we will get a `.await` outside a `.async`
+ async { f.await.await }
+}
+
+fn await_in_async<F, R>(f: F) -> impl Future<Output = u32>
+where
+ F: FnOnce() -> R,
+ R: Future<Output = u32>,
+{
+ // Lint
+ async { async { f().await + 1 }.await }
+}
+
#[derive(Debug, Clone)]
struct F {}
@@ -109,3 +110,84 @@ fn capture() {
// Do not lint: `val` would not live long enough
spawn(async { work(&{ val }).await });
}
+
+fn await_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e:expr) => {
+ $e.await
+ };
+ }
+ // Do not lint: the macro may change in the future
+ // or return different things depending on its argument
+ async { mac!(async { 42 }) }
+}
+
+fn async_expr_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ () => {
+ async { 42 }
+ };
+ }
+ // Do not lint: the macro may change in the future
+ async { mac!().await }
+}
+
+fn async_expr_from_macro_deep() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ () => {
+ async { 42 }
+ };
+ }
+ // Do not lint: the macro may change in the future
+ async { ({ mac!() }).await }
+}
+
+fn all_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ () => {
+ // Lint
+ async { async { 42 }.await }
+ };
+ }
+ mac!()
+}
+
+fn parts_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e: expr) => {
+ // Do not lint: `$e` might not always be side-effect free
+ async { $e.await }
+ };
+ }
+ mac!(async { 42 })
+}
+
+fn safe_parts_from_macro() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e: expr) => {
+ // Lint
+ async { async { $e }.await }
+ };
+ }
+ mac!(42)
+}
+
+fn parts_from_macro_deep() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e: expr) => {
+ // Do not lint: `$e` might not always be side-effect free
+ async { ($e,).0.await }
+ };
+ }
+ let f = std::future::ready(42);
+ mac!(f)
+}
+
+fn await_from_macro_deep() -> impl Future<Output = u32> {
+ macro_rules! mac {
+ ($e:expr) => {{ $e }.await};
+ }
+ // Do not lint: the macro may change in the future
+ // or return different things depending on its argument
+ async { mac!(async { 42 }) }
+}
diff --git a/tests/ui/redundant_async_block.stderr b/tests/ui/redundant_async_block.stderr
index 1a1c1603e08..f3dcb09b444 100644
--- a/tests/ui/redundant_async_block.stderr
+++ b/tests/ui/redundant_async_block.stderr
@@ -7,34 +7,68 @@ LL | let x = async { f.await };
= note: `-D clippy::redundant-async-block` implied by `-D warnings`
error: this async expression only awaits a single future
- --> $DIR/redundant_async_block.rs:48:16
+ --> $DIR/redundant_async_block.rs:22: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:51:16
+ --> $DIR/redundant_async_block.rs:26: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:53:15
+ --> $DIR/redundant_async_block.rs:29:15
|
LL | let fut = async { async { 42 }.await };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `async { 42 }`
error: this async expression only awaits a single future
- --> $DIR/redundant_async_block.rs:72:5
+ --> $DIR/redundant_async_block.rs:45:5
|
LL | async move { fut.await }
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `fut`
error: this async expression only awaits a single future
- --> $DIR/redundant_async_block.rs:85:5
+ --> $DIR/redundant_async_block.rs:58:5
|
LL | async move { fut.await }
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `fut`
-error: aborting due to 6 previous errors
+error: this async expression only awaits a single future
+ --> $DIR/redundant_async_block.rs:63:5
+ |
+LL | async { f.await }
+ | ^^^^^^^^^^^^^^^^^ help: you can reduce it to: `f`
+
+error: this async expression only awaits a single future
+ --> $DIR/redundant_async_block.rs:86:5
+ |
+LL | async { async { f().await + 1 }.await }
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `async { f().await + 1 }`
+
+error: this async expression only awaits a single future
+ --> $DIR/redundant_async_block.rs:149:13
+ |
+LL | async { async { 42 }.await }
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `async { 42 }`
+...
+LL | mac!()
+ | ------ in this macro invocation
+ |
+ = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: this async expression only awaits a single future
+ --> $DIR/redundant_async_block.rs:169:13
+ |
+LL | async { async { $e }.await }
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `async { $e }`
+...
+LL | mac!(42)
+ | -------- in this macro invocation
+ |
+ = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 10 previous errors