From d5429eab8aee46fd2148f99b8f9ba394addb5ba7 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sat, 4 Mar 2023 17:28:53 +0100 Subject: [PATCH] Add new `redundant_async_block` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/redundant_async_block.rs | 84 +++++++++++++++++++ tests/ui/async_yields_async.fixed | 1 + tests/ui/async_yields_async.rs | 1 + tests/ui/async_yields_async.stderr | 12 +-- tests/ui/redundant_async_block.fixed | 64 ++++++++++++++ tests/ui/redundant_async_block.rs | 64 ++++++++++++++ tests/ui/redundant_async_block.stderr | 28 +++++++ tests/ui/redundant_closure_call_fixable.fixed | 1 + tests/ui/redundant_closure_call_fixable.rs | 1 + .../ui/redundant_closure_call_fixable.stderr | 12 +-- 13 files changed, 260 insertions(+), 12 deletions(-) create mode 100644 clippy_lints/src/redundant_async_block.rs create mode 100644 tests/ui/redundant_async_block.fixed create mode 100644 tests/ui/redundant_async_block.rs create mode 100644 tests/ui/redundant_async_block.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 62cc7437b8240..d797b30f47bb3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 462fcb6483de7..d3b9cc675795d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6d4ec5277567f..ea1e0ab506ccf 100644 --- a/clippy_lints/src/lib.rs +++ b/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` } diff --git a/clippy_lints/src/redundant_async_block.rs b/clippy_lints/src/redundant_async_block.rs new file mode 100644 index 0000000000000..27ad430863743 --- /dev/null +++ b/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), + _ => (), + } + } +} diff --git a/tests/ui/async_yields_async.fixed b/tests/ui/async_yields_async.fixed index 3cf380d2b954b..579a63ea47722 100644 --- a/tests/ui/async_yields_async.fixed +++ b/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; diff --git a/tests/ui/async_yields_async.rs b/tests/ui/async_yields_async.rs index dd4131b60ab3a..5aec2fb50f6a1 100644 --- a/tests/ui/async_yields_async.rs +++ b/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; diff --git a/tests/ui/async_yields_async.stderr b/tests/ui/async_yields_async.stderr index 22ce1c6f6471b..7f72534832b4e 100644 --- a/tests/ui/async_yields_async.stderr +++ b/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 || { | _______________________- diff --git a/tests/ui/redundant_async_block.fixed b/tests/ui/redundant_async_block.fixed new file mode 100644 index 0000000000000..5f9931df45e92 --- /dev/null +++ b/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); +} diff --git a/tests/ui/redundant_async_block.rs b/tests/ui/redundant_async_block.rs new file mode 100644 index 0000000000000..de3c9970c65f2 --- /dev/null +++ b/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); +} diff --git a/tests/ui/redundant_async_block.stderr b/tests/ui/redundant_async_block.stderr new file mode 100644 index 0000000000000..b16d96dce84eb --- /dev/null +++ b/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 + diff --git a/tests/ui/redundant_closure_call_fixable.fixed b/tests/ui/redundant_closure_call_fixable.fixed index c0e49ff4caa74..b987fd2ce6f08 100644 --- a/tests/ui/redundant_closure_call_fixable.fixed +++ b/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 { diff --git a/tests/ui/redundant_closure_call_fixable.rs b/tests/ui/redundant_closure_call_fixable.rs index 9e6e54348a8c2..633a2979d5da3 100644 --- a/tests/ui/redundant_closure_call_fixable.rs +++ b/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 { diff --git a/tests/ui/redundant_closure_call_fixable.stderr b/tests/ui/redundant_closure_call_fixable.stderr index d71bcba2a8200..8a1f0771659b1 100644 --- a/tests/ui/redundant_closure_call_fixable.stderr +++ b/tests/ui/redundant_closure_call_fixable.stderr @@ -1,5 +1,5 @@ error: try not to call a closure in the expression where it is declared - --> $DIR/redundant_closure_call_fixable.rs:16:13 + --> $DIR/redundant_closure_call_fixable.rs:17:13 | LL | let a = (|| 42)(); | ^^^^^^^^^ help: try doing something like: `42` @@ -7,7 +7,7 @@ LL | let a = (|| 42)(); = note: `-D clippy::redundant-closure-call` implied by `-D warnings` error: try not to call a closure in the expression where it is declared - --> $DIR/redundant_closure_call_fixable.rs:17:13 + --> $DIR/redundant_closure_call_fixable.rs:18:13 | LL | let b = (async || { | _____________^ @@ -27,7 +27,7 @@ LL ~ }; | error: try not to call a closure in the expression where it is declared - --> $DIR/redundant_closure_call_fixable.rs:22:13 + --> $DIR/redundant_closure_call_fixable.rs:23:13 | LL | let c = (|| { | _____________^ @@ -47,13 +47,13 @@ LL ~ }; | error: try not to call a closure in the expression where it is declared - --> $DIR/redundant_closure_call_fixable.rs:27:13 + --> $DIR/redundant_closure_call_fixable.rs:28:13 | LL | let d = (async || something().await)(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try doing something like: `async { something().await }` error: try not to call a closure in the expression where it is declared - --> $DIR/redundant_closure_call_fixable.rs:36:13 + --> $DIR/redundant_closure_call_fixable.rs:37:13 | LL | (|| m!())() | ^^^^^^^^^^^ help: try doing something like: `m!()` @@ -64,7 +64,7 @@ LL | m2!(); = note: this error originates in the macro `m2` (in Nightly builds, run with -Z macro-backtrace for more info) error: try not to call a closure in the expression where it is declared - --> $DIR/redundant_closure_call_fixable.rs:31:13 + --> $DIR/redundant_closure_call_fixable.rs:32:13 | LL | (|| 0)() | ^^^^^^^^ help: try doing something like: `0`