Skip to content
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

Implement simple, unstable lint to suggest turning closure-of-async-block into async-closure #127097

Merged
merged 3 commits into from
Jul 11, 2024
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
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ lint_cfg_attr_no_attributes =

lint_check_name_unknown_tool = unknown lint tool: `{$tool_name}`

lint_closure_returning_async_block = closure returning async block can be made into an async closure
.label = this async block can be removed, and the closure can be turned into an async closure
.suggestion = turn this into an async closure

lint_command_line_source = `forbid` lint level was set on command line

lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as identifiers, which look alike
Expand Down
129 changes: 129 additions & 0 deletions compiler/rustc_lint/src/async_closures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
use rustc_hir as hir;
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::Span;

use crate::{LateContext, LateLintPass};

declare_lint! {
/// The `closure_returning_async_block` lint detects cases where users
/// write a closure that returns an async block.
///
/// ### Example
///
/// ```rust
/// #![warn(closure_returning_async_block)]
/// let c = |x: &str| async {};
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Using an async closure is preferable over a closure that returns an
/// async block, since async closures are less restrictive in how its
/// captures are allowed to be used.
///
/// For example, this code does not work with a closure returning an async
/// block:
///
/// ```rust,compile_fail
/// async fn callback(x: &str) {}
///
/// let captured_str = String::new();
/// let c = move || async {
/// callback(&captured_str).await;
/// };
/// ```
///
/// But it does work with async closures:
///
/// ```rust
/// #![feature(async_closure)]
///
/// async fn callback(x: &str) {}
///
/// let captured_str = String::new();
/// let c = async move || {
/// callback(&captured_str).await;
/// };
/// ```
pub CLOSURE_RETURNING_ASYNC_BLOCK,
Allow,
"closure that returns `async {}` could be rewritten as an async closure",
@feature_gate = async_closure;
}

declare_lint_pass!(
/// Lint for potential usages of async closures and async fn trait bounds.
AsyncClosureUsage => [CLOSURE_RETURNING_ASYNC_BLOCK]
);

impl<'tcx> LateLintPass<'tcx> for AsyncClosureUsage {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
let hir::ExprKind::Closure(&hir::Closure {
body,
kind: hir::ClosureKind::Closure,
fn_decl_span,
..
}) = expr.kind
else {
return;
};

let mut body = cx.tcx.hir().body(body).value;

// Only peel blocks that have no expressions.
while let hir::ExprKind::Block(&hir::Block { stmts: [], expr: Some(tail), .. }, None) =
body.kind
{
body = tail;
}

let hir::ExprKind::Closure(&hir::Closure {
kind:
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Async,
hir::CoroutineSource::Block,
)),
fn_decl_span: async_decl_span,
..
}) = body.kind
else {
return;
};

let deletion_span = cx.tcx.sess.source_map().span_extend_while_whitespace(async_decl_span);

cx.tcx.emit_node_span_lint(
CLOSURE_RETURNING_ASYNC_BLOCK,
expr.hir_id,
fn_decl_span,
ClosureReturningAsyncBlock {
async_decl_span,
sugg: AsyncClosureSugg {
deletion_span,
insertion_span: fn_decl_span.shrink_to_lo(),
},
},
);
}
}

#[derive(LintDiagnostic)]
#[diag(lint_closure_returning_async_block)]
struct ClosureReturningAsyncBlock {
#[label]
async_decl_span: Span,
#[subdiagnostic]
sugg: AsyncClosureSugg,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(lint_suggestion, applicability = "maybe-incorrect")]
struct AsyncClosureSugg {
#[suggestion_part(code = "")]
deletion_span: Span,
#[suggestion_part(code = "async ")]
insertion_span: Span,
}
6 changes: 3 additions & 3 deletions compiler/rustc_lint/src/impl_trait_overcaptures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::ty::{
self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::{sym, Span};
use rustc_span::Span;

use crate::fluent_generated as fluent;
use crate::{LateContext, LateLintPass};
Expand Down Expand Up @@ -57,7 +57,7 @@ declare_lint! {
pub IMPL_TRAIT_OVERCAPTURES,
Allow,
"`impl Trait` will capture more lifetimes than possibly intended in edition 2024",
@feature_gate = sym::precise_capturing;
@feature_gate = precise_capturing;
//@future_incompatible = FutureIncompatibleInfo {
// reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
// reference: "<FIXME>",
Expand Down Expand Up @@ -91,7 +91,7 @@ declare_lint! {
pub IMPL_TRAIT_REDUNDANT_CAPTURES,
Warn,
"redundant precise-capturing `use<...>` syntax on an `impl Trait`",
@feature_gate = sym::precise_capturing;
@feature_gate = precise_capturing;
}

declare_lint_pass!(
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#![feature(trait_upcasting)]
// tidy-alphabetical-end

mod async_closures;
mod async_fn_in_trait;
pub mod builtin;
mod context;
Expand Down Expand Up @@ -86,6 +87,7 @@ use rustc_hir::def_id::LocalModDefId;
use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt;

use async_closures::AsyncClosureUsage;
use async_fn_in_trait::AsyncFnInTrait;
use builtin::*;
use deref_into_dyn_supertrait::*;
Expand Down Expand Up @@ -227,6 +229,7 @@ late_lint_methods!(
MapUnitFn: MapUnitFn,
MissingDebugImplementations: MissingDebugImplementations,
MissingDoc: MissingDoc,
AsyncClosureUsage: AsyncClosureUsage,
AsyncFnInTrait: AsyncFnInTrait,
NonLocalDefinitions: NonLocalDefinitions::default(),
ImplTraitOvercaptures: ImplTraitOvercaptures,
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_lint/src/multiple_supertrait_upcastable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::{LateContext, LateLintPass, LintContext};

use rustc_hir as hir;
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::sym;

declare_lint! {
/// The `multiple_supertrait_upcastable` lint detects when an object-safe trait has multiple
Expand Down Expand Up @@ -30,7 +29,7 @@ declare_lint! {
pub MULTIPLE_SUPERTRAIT_UPCASTABLE,
Allow,
"detect when an object-safe trait has multiple supertraits",
@feature_gate = sym::multiple_supertrait_upcastable;
@feature_gate = multiple_supertrait_upcastable;
}

declare_lint_pass!(MultipleSupertraitUpcastable => [MULTIPLE_SUPERTRAIT_UPCASTABLE]);
Expand Down
13 changes: 6 additions & 7 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

use crate::{declare_lint, declare_lint_pass, FutureIncompatibilityReason};
use rustc_span::edition::Edition;
use rustc_span::symbol::sym;

declare_lint_pass! {
/// Does nothing as a lint pass, but registers some `Lint`s
Expand Down Expand Up @@ -461,7 +460,7 @@ declare_lint! {
pub MUST_NOT_SUSPEND,
Allow,
"use of a `#[must_not_suspend]` value across a yield point",
@feature_gate = rustc_span::symbol::sym::must_not_suspend;
@feature_gate = must_not_suspend;
}

declare_lint! {
Expand Down Expand Up @@ -1645,7 +1644,7 @@ declare_lint! {
pub RUST_2024_INCOMPATIBLE_PAT,
Allow,
"detects patterns whose meaning will change in Rust 2024",
@feature_gate = sym::ref_pat_eat_one_layer_2024;
@feature_gate = ref_pat_eat_one_layer_2024;
// FIXME uncomment below upon stabilization
/*@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
Expand Down Expand Up @@ -2693,7 +2692,7 @@ declare_lint! {
pub FUZZY_PROVENANCE_CASTS,
Allow,
"a fuzzy integer to pointer cast is used",
@feature_gate = sym::strict_provenance;
@feature_gate = strict_provenance;
}

declare_lint! {
Expand Down Expand Up @@ -2739,7 +2738,7 @@ declare_lint! {
pub LOSSY_PROVENANCE_CASTS,
Allow,
"a lossy pointer to integer cast is used",
@feature_gate = sym::strict_provenance;
@feature_gate = strict_provenance;
}

declare_lint! {
Expand Down Expand Up @@ -3923,7 +3922,7 @@ declare_lint! {
pub NON_EXHAUSTIVE_OMITTED_PATTERNS,
Allow,
"detect when patterns of types marked `non_exhaustive` are missed",
@feature_gate = sym::non_exhaustive_omitted_patterns_lint;
@feature_gate = non_exhaustive_omitted_patterns_lint;
}

declare_lint! {
Expand Down Expand Up @@ -4043,7 +4042,7 @@ declare_lint! {
pub TEST_UNSTABLE_LINT,
Deny,
"this unstable lint is only for testing",
@feature_gate = sym::test_unstable_lint;
@feature_gate = test_unstable_lint;
}

declare_lint! {
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ macro_rules! declare_lint {
);
);
($(#[$attr:meta])* $vis: vis $NAME: ident, $Level: ident, $desc: expr,
$(@feature_gate = $gate:expr;)?
$(@feature_gate = $gate:ident;)?
$(@future_incompatible = FutureIncompatibleInfo {
reason: $reason:expr,
$($field:ident : $val:expr),* $(,)*
Expand All @@ -879,7 +879,7 @@ macro_rules! declare_lint {
desc: $desc,
is_externally_loaded: false,
$($v: true,)*
$(feature_gate: Some($gate),)?
$(feature_gate: Some(rustc_span::symbol::sym::$gate),)?
$(future_incompatible: Some($crate::FutureIncompatibleInfo {
reason: $reason,
$($field: $val,)*
Expand All @@ -895,21 +895,21 @@ macro_rules! declare_lint {
macro_rules! declare_tool_lint {
(
$(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level: ident, $desc: expr
$(, @feature_gate = $gate:expr;)?
$(, @feature_gate = $gate:ident;)?
) => (
$crate::declare_tool_lint!{$(#[$attr])* $vis $tool::$NAME, $Level, $desc, false $(, @feature_gate = $gate;)?}
);
(
$(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level:ident, $desc:expr,
report_in_external_macro: $rep:expr
$(, @feature_gate = $gate:expr;)?
$(, @feature_gate = $gate:ident;)?
) => (
$crate::declare_tool_lint!{$(#[$attr])* $vis $tool::$NAME, $Level, $desc, $rep $(, @feature_gate = $gate;)?}
);
(
$(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level:ident, $desc:expr,
$external:expr
$(, @feature_gate = $gate:expr;)?
$(, @feature_gate = $gate:ident;)?
) => (
$(#[$attr])*
$vis static $NAME: &$crate::Lint = &$crate::Lint {
Expand All @@ -920,7 +920,7 @@ macro_rules! declare_tool_lint {
report_in_external_macro: $external,
future_incompatible: None,
is_externally_loaded: true,
$(feature_gate: Some($gate),)?
$(feature_gate: Some(rustc_span::symbol::sym::$gate),)?
crate_level_only: false,
..$crate::Lint::default_fields_for_macro()
};
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ where
macro_rules! declare_rustdoc_lint {
(
$(#[$attr:meta])* $name: ident, $level: ident, $descr: literal $(,)?
$(@feature_gate = $gate:expr;)?
$(@feature_gate = $gate:ident;)?
) => {
declare_tool_lint! {
$(#[$attr])* pub rustdoc::$name, $level, $descr
Expand Down Expand Up @@ -128,7 +128,7 @@ declare_rustdoc_lint! {
MISSING_DOC_CODE_EXAMPLES,
Allow,
"detects publicly-exported items without code samples in their documentation",
@feature_gate = rustc_span::symbol::sym::rustdoc_missing_doc_code_examples;
@feature_gate = rustdoc_missing_doc_code_examples;
}

declare_rustdoc_lint! {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//@ edition: 2021

#![feature(async_closure)]
#![deny(closure_returning_async_block)]

fn main() {
let x = || async {};
//~^ ERROR closure returning async block can be made into an async closure

let x = || async move {};
//~^ ERROR closure returning async block can be made into an async closure

let x = move || async move {};
//~^ ERROR closure returning async block can be made into an async closure

let x = move || async {};
//~^ ERROR closure returning async block can be made into an async closure

let x = || {{ async {} }};
//~^ ERROR closure returning async block can be made into an async closure
}
Loading
Loading