Skip to content

implicit_return: better handling of asynchronous code #14446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion clippy_lints/src/implicit_return.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::source::{snippet_with_applicability, snippet_with_context, walk_span_to_context};
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{get_async_fn_body, is_async_fn, is_from_proc_macro};
use clippy_utils::{desugar_await, get_async_closure_expr, get_async_fn_body, is_async_fn, is_from_proc_macro};
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
Expand Down Expand Up @@ -134,6 +134,10 @@ fn lint_implicit_returns(
},

ExprKind::Match(_, arms, _) => {
if let Some(await_expr) = desugar_await(expr) {
lint_return(cx, await_expr.hir_id, await_expr.span);
return LintLocation::Inner;
}
for arm in arms {
let res = lint_implicit_returns(
cx,
Expand Down Expand Up @@ -241,6 +245,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitReturn {
Some(e) => e,
None => return,
}
} else if let Some(expr) = get_async_closure_expr(cx.tcx, body.value) {
expr
} else {
body.value
};
Expand Down
26 changes: 2 additions & 24 deletions clippy_lints/src/redundant_async_block.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
use std::ops::ControlFlow;

use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::peel_blocks;
use clippy_utils::source::{snippet, walk_span_to_context};
use clippy_utils::ty::implements_trait;
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{desugar_await, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{
Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, MatchSource,
};
use rustc_hir::{Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::UpvarCapture;
use rustc_session::declare_lint_pass;
Expand Down Expand Up @@ -99,20 +94,3 @@ fn desugar_async_block<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Op
None
}
}

/// 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_without_closures(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
}
}
49 changes: 38 additions & 11 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ use rustc_hir::hir_id::{HirIdMap, HirIdSet};
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr};
use rustc_hir::{
self as hir, Arm, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, ConstArgKind, ConstContext,
Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArg, GenericArgs, HirId, Impl, ImplItem,
ImplItemKind, ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, OwnerNode,
Param, Pat, PatExpr, PatExprKind, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitFn, TraitItem,
TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp, def,
CoroutineDesugaring, CoroutineKind, Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArg,
GenericArgs, HirId, Impl, ImplItem, ImplItemKind, ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource,
Mutability, Node, OwnerId, OwnerNode, Param, Pat, PatExpr, PatExprKind, PatKind, Path, PathSegment, PrimTy, QPath,
Stmt, StmtKind, TraitFn, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp, def,
};
use rustc_lexer::{TokenKind, tokenize};
use rustc_lint::{LateContext, Level, Lint, LintContext};
Expand All @@ -126,6 +126,7 @@ use rustc_span::hygiene::{ExpnKind, MacroKind};
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::{Ident, Symbol, kw};
use rustc_span::{InnerSpan, Span, sym};
use source::walk_span_to_context;
use visitors::{Visitable, for_each_unconsumed_temporary};

use crate::consts::{ConstEvalCtxt, Constant, mir_to_const};
Expand Down Expand Up @@ -2133,25 +2134,34 @@ pub fn is_async_fn(kind: FnKind<'_>) -> bool {
}
}

/// Peels away all the compiler generated code surrounding the body of an async function,
pub fn get_async_fn_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'_>) -> Option<&'tcx Expr<'tcx>> {
if let ExprKind::Closure(&Closure { body, .. }) = body.value.kind
/// Peels away all the compiler generated code surrounding the body of an async closure.
pub fn get_async_closure_expr<'tcx>(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
if let ExprKind::Closure(&Closure {
body,
kind: hir::ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)),
..
}) = expr.kind
&& let ExprKind::Block(
Block {
stmts: [],
expr:
Some(Expr {
kind: ExprKind::DropTemps(expr),
kind: ExprKind::DropTemps(inner_expr),
..
}),
..
},
_,
) = tcx.hir_body(body).value.kind
{
return Some(expr);
Some(inner_expr)
} else {
None
}
None
}

/// Peels away all the compiler generated code surrounding the body of an async function,
pub fn get_async_fn_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'_>) -> Option<&'tcx Expr<'tcx>> {
get_async_closure_expr(tcx, body.value)
}

// check if expr is calling method or function with #[must_use] attribute
Expand Down Expand Up @@ -3718,3 +3728,20 @@ pub fn peel_hir_ty_options<'tcx>(cx: &LateContext<'tcx>, mut hir_ty: &'tcx hir::
}
hir_ty
}

/// If `expr` is a desugared `.await`, return the original expression if it does not come from a
/// macro expansion.
pub 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_without_closures(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
}
}
43 changes: 43 additions & 0 deletions tests/ui/implicit_return.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,46 @@ with_span!(
x
}
);

fn desugared_closure_14446() {
let _ = async || return 0;
//~^ implicit_return
#[rustfmt::skip]
let _ = async || -> i32 { return 0 };
//~^ implicit_return
let _ = async |a: i32| return a;
//~^ implicit_return
#[rustfmt::skip]
let _ = async |a: i32| { return a };
//~^ implicit_return

let _ = async || return 0;
let _ = async || -> i32 { return 0 };
let _ = async |a: i32| return a;
#[rustfmt::skip]
let _ = async |a: i32| { return a; };

let _ = async || return foo().await;
//~^ implicit_return
let _ = async || {
foo().await;
return foo().await
};
//~^^ implicit_return
#[rustfmt::skip]
let _ = async || { return foo().await };
//~^ implicit_return
let _ = async || -> bool { return foo().await };
//~^ implicit_return

let _ = async || return foo().await;
let _ = async || {
foo().await;
return foo().await;
};
#[rustfmt::skip]
let _ = async || { return foo().await; };
let _ = async || -> bool {
return foo().await;
};
}
43 changes: 43 additions & 0 deletions tests/ui/implicit_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,46 @@ with_span!(
x
}
);

fn desugared_closure_14446() {
let _ = async || 0;
//~^ implicit_return
#[rustfmt::skip]
let _ = async || -> i32 { 0 };
//~^ implicit_return
let _ = async |a: i32| a;
//~^ implicit_return
#[rustfmt::skip]
let _ = async |a: i32| { a };
//~^ implicit_return

let _ = async || return 0;
let _ = async || -> i32 { return 0 };
let _ = async |a: i32| return a;
#[rustfmt::skip]
let _ = async |a: i32| { return a; };

let _ = async || foo().await;
//~^ implicit_return
let _ = async || {
foo().await;
foo().await
};
//~^^ implicit_return
#[rustfmt::skip]
let _ = async || { foo().await };
//~^ implicit_return
let _ = async || -> bool { foo().await };
//~^ implicit_return

let _ = async || return foo().await;
let _ = async || {
foo().await;
return foo().await;
};
#[rustfmt::skip]
let _ = async || { return foo().await; };
let _ = async || -> bool {
return foo().await;
};
}
90 changes: 89 additions & 1 deletion tests/ui/implicit_return.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -183,5 +183,93 @@ help: add `return` as shown
LL | return true
| ++++++

error: aborting due to 16 previous errors
error: missing `return` statement
--> tests/ui/implicit_return.rs:170:22
|
LL | let _ = async || 0;
| ^
|
help: add `return` as shown
|
LL | let _ = async || return 0;
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:173:31
|
LL | let _ = async || -> i32 { 0 };
| ^
|
help: add `return` as shown
|
LL | let _ = async || -> i32 { return 0 };
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:175:28
|
LL | let _ = async |a: i32| a;
| ^
|
help: add `return` as shown
|
LL | let _ = async |a: i32| return a;
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:178:30
|
LL | let _ = async |a: i32| { a };
| ^
|
help: add `return` as shown
|
LL | let _ = async |a: i32| { return a };
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:187:22
|
LL | let _ = async || foo().await;
| ^^^^^
|
help: add `return` as shown
|
LL | let _ = async || return foo().await;
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:191:9
|
LL | foo().await
| ^^^^^
|
help: add `return` as shown
|
LL | return foo().await
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:195:24
|
LL | let _ = async || { foo().await };
| ^^^^^
|
help: add `return` as shown
|
LL | let _ = async || { return foo().await };
| ++++++

error: missing `return` statement
--> tests/ui/implicit_return.rs:197:32
|
LL | let _ = async || -> bool { foo().await };
| ^^^^^
|
help: add `return` as shown
|
LL | let _ = async || -> bool { return foo().await };
| ++++++

error: aborting due to 24 previous errors