Skip to content

Commit

Permalink
Add new redundant_async_block lint
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Mar 8, 2023
1 parent 5eefbb3 commit d5429ea
Show file tree
Hide file tree
Showing 13 changed files with 260 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
84 changes: 84 additions & 0 deletions clippy_lints/src/redundant_async_block.rs
Original file line number Diff line number Diff line change
@@ -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),
_ => (),
}
}
}
1 change: 1 addition & 0 deletions tests/ui/async_yields_async.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions tests/ui/async_yields_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/async_yields_async.stderr
Original file line number Diff line number Diff line change
@@ -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 {
| _____________________-
Expand All @@ -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 {
| ____________________-
Expand All @@ -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 || {
| ________________________-
Expand All @@ -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 || {
| _______________________-
Expand All @@ -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;
| ^^^^^^^^^^^^^^^^
Expand All @@ -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 || {
| _______________________-
Expand Down
64 changes: 64 additions & 0 deletions tests/ui/redundant_async_block.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
64 changes: 64 additions & 0 deletions tests/ui/redundant_async_block.rs
Original file line number Diff line number Diff line change
@@ -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);
}
28 changes: 28 additions & 0 deletions tests/ui/redundant_async_block.stderr
Original file line number Diff line number Diff line change
@@ -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

1 change: 1 addition & 0 deletions tests/ui/redundant_closure_call_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![feature(async_closure)]
#![warn(clippy::redundant_closure_call)]
#![allow(clippy::redundant_async_block)]
#![allow(unused)]

async fn something() -> u32 {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/redundant_closure_call_fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![feature(async_closure)]
#![warn(clippy::redundant_closure_call)]
#![allow(clippy::redundant_async_block)]
#![allow(unused)]

async fn something() -> u32 {
Expand Down
Loading

0 comments on commit d5429ea

Please sign in to comment.