Auto merge of #127097 - compiler-errors:async-closure-lint, r=oli-obk

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

We want to eventually suggest people to turn `|| async {}` to `async || {}`. This begins doing that. It's a pretty rudimentary lint, but I wanted to get something down so I wouldn't lose the code.

Tracking:
* #62290

Author: bors

compiler/rustc_lint/messages.ftl

@@ -185,6 +185,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

compiler/rustc_lint/src/async_closures.rs

@@ -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,
+}

compiler/rustc_lint/src/impl_trait_overcaptures.rs

@@ -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};
@@ -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>",
@@ -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!(

compiler/rustc_lint/src/lib.rs

@@ -41,6 +41,7 @@
 #![feature(trait_upcasting)]
 // tidy-alphabetical-end
 
+mod async_closures;
 mod async_fn_in_trait;
 pub mod builtin;
 mod context;
@@ -87,6 +88,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::*;
@@ -229,6 +231,7 @@ late_lint_methods!(
             MapUnitFn: MapUnitFn,
             MissingDebugImplementations: MissingDebugImplementations,
             MissingDoc: MissingDoc,
+            AsyncClosureUsage: AsyncClosureUsage,
             AsyncFnInTrait: AsyncFnInTrait,
             NonLocalDefinitions: NonLocalDefinitions::default(),
             ImplTraitOvercaptures: ImplTraitOvercaptures,

compiler/rustc_lint/src/multiple_supertrait_upcastable.rs

@@ -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
@@ -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]);

compiler/rustc_lint_defs/src/builtin.rs

@@ -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
@@ -463,7 +462,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! {
@@ -1647,7 +1646,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),
@@ -2695,7 +2694,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! {
@@ -2741,7 +2740,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! {
@@ -3925,7 +3924,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! {
@@ -4045,7 +4044,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! {

compiler/rustc_lint_defs/src/lib.rs

@@ -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),* $(,)*
@@ -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,)*
@@ -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 {
@@ -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()
         };

src/librustdoc/lint.rs

@@ -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
@@ -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! {

tests/ui/async-await/async-closures/lint-closure-returning-async-block.rs

@@ -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
+}