Auto merge of #116184 - compiler-errors:afit-lint, r=tmandry

Add `async_fn_in_trait` lint
cc #115822 (comment)
Mostly unsure what the messaging should be. Feedback required.
r? `@tmandry`

Author: bors

compiler/rustc_lint/messages.ftl

@@ -5,6 +5,10 @@ lint_array_into_iter =
 .use_explicit_into_iter_suggestion =
  or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value
 
+lint_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified
+.note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future`
+.suggestion = you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`
+
 lint_atomic_ordering_fence = memory fences cannot have `Relaxed` ordering
 .help = consider using ordering modes `Acquire`, `Release`, `AcqRel` or `SeqCst`
 

compiler/rustc_lint/src/async_fn_in_trait.rs

@@ -0,0 +1,128 @@
+usecrate::lints::AsyncFnInTraitDiag;
+usecrate::LateContext;
+usecrate::LateLintPass;
+use rustc_hir as hir;
+use rustc_trait_selection::traits::error_reporting::suggestions::suggest_desugaring_async_fn_to_impl_future_in_trait;
+
+declare_lint!{
+/// The `async_fn_in_trait` lint detects use of `async fn` in the
+/// definition of a publicly-reachable trait.
+///
+/// ### Example
+///
+/// ```rust
+/// # #![feature(async_fn_in_trait)]
+/// pub trait Trait {
+/// async fn method(&self);
+/// }
+/// # fn main() {}
+/// ```
+///
+/// {{produces}}
+///
+/// ### Explanation
+///
+/// When `async fn` is used in a trait definition, the trait does not
+/// promise that the opaque [`Future`] returned by the associated function
+/// or method will implement any [auto traits] such as [`Send`]. This may
+/// be surprising and may make the associated functions or methods on the
+/// trait less useful than intended. On traits exposed publicly from a
+/// crate, this may affect downstream crates whose authors cannot alter
+/// the trait definition.
+///
+/// For example, this code is invalid:
+///
+/// ```rust,compile_fail
+/// # #![feature(async_fn_in_trait)]
+/// pub trait Trait {
+/// async fn method(&self) {}
+/// }
+///
+/// fn test<T: Trait>(x: T) {
+/// fn spawn<T: Send>(_: T) {}
+/// spawn(x.method()); // Not OK.
+/// }
+/// ```
+///
+/// This lint exists to warn authors of publicly-reachable traits that
+/// they may want to consider desugaring the `async fn` to a normal `fn`
+/// that returns an opaque `impl Future<..> + Send` type.
+///
+/// For example, instead of:
+///
+/// ```rust
+/// # #![feature(async_fn_in_trait)]
+/// pub trait Trait {
+/// async fn method(&self) {}
+/// }
+/// ```
+///
+/// The author of the trait may want to write:
+///
+///
+/// ```rust
+/// # #![feature(return_position_impl_trait_in_trait)]
+/// use core::future::Future;
+/// pub trait Trait {
+/// fn method(&self) -> impl Future<Output = ()> + Send { async {} }
+/// }
+/// ```
+///
+/// This still allows the use of `async fn` within impls of the trait.
+/// However, it also means that the trait will never be compatible with
+/// impls where the returned [`Future`] of the method does not implement
+/// `Send`.
+///
+/// Conversely, if the trait is used only locally, if it is never used in
+/// generic functions, or if it is only used in single-threaded contexts
+/// that do not care whether the returned [`Future`] implements [`Send`],
+/// then the lint may be suppressed.
+///
+/// [`Future`]: https://doc.rust-lang.org/core/future/trait.Future.html
+/// [`Send`]: https://doc.rust-lang.org/core/marker/trait.Send.html
+/// [auto traits]: https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits
+pubASYNC_FN_IN_TRAIT,
+Warn,
+"use of `async fn` in definition of a publicly-reachable trait"
+}
+
+declare_lint_pass!(
+/// Lint for use of `async fn` in the definition of a publicly-reachable
+/// trait.
+AsyncFnInTrait => [ASYNC_FN_IN_TRAIT]
+);
+
+impl<'tcx>LateLintPass<'tcx>forAsyncFnInTrait{
+fncheck_trait_item(&mutself,cx:&LateContext<'tcx>,item:&'tcx hir::TraitItem<'tcx>){
+iflet hir::TraitItemKind::Fn(sig, body) = item.kind
+ && let hir::IsAsync::Async(async_span) = sig.header.asyncness
+{
+// RTN can be used to bound `async fn` in traits in a better way than "always"
+if cx.tcx.features().return_type_notation{
+return;
+}
+
+// Only need to think about library implications of reachable traits
+if !cx.tcx.effective_visibilities(()).is_reachable(item.owner_id.def_id){
+return;
+}
+
+let hir::FnRetTy::Return(hir::Ty{kind: hir::TyKind::OpaqueDef(def, ..), .. }) =
+ sig.decl.output
+else{
+// This should never happen, but let's not ICE.
+return;
+};
+let sugg = suggest_desugaring_async_fn_to_impl_future_in_trait(
+ cx.tcx,
+ sig,
+ body,
+ def.owner_id.def_id,
+" + Send",
+);
+ cx.tcx.emit_spanned_lint(ASYNC_FN_IN_TRAIT, item.hir_id(), async_span,AsyncFnInTraitDiag{
+ sugg
+});
+}
+}
+}

compiler/rustc_lint/src/lib.rs

@@ -50,6 +50,7 @@ extern crate rustc_session;
 externcrate tracing;
 
 mod array_into_iter;
+mod async_fn_in_trait;
 pubmod builtin;
 mod context;
 mod deref_into_dyn_supertrait;
@@ -96,6 +97,7 @@ use rustc_session::lint::builtin::{
 };
 
 use array_into_iter::ArrayIntoIter;
+use async_fn_in_trait::AsyncFnInTrait;
 use builtin::*;
 use deref_into_dyn_supertrait::*;
 use drop_forget_useless::*;
@@ -234,6 +236,7 @@ late_lint_methods!(
 MapUnitFn:MapUnitFn,
 MissingDebugImplementations:MissingDebugImplementations,
 MissingDoc:MissingDoc,
+AsyncFnInTrait:AsyncFnInTrait,
 ]
 ]
 );

compiler/rustc_lint/src/lints.rs

@@ -1818,3 +1818,24 @@ pub struct UnusedAllocationDiag;
 #[derive(LintDiagnostic)]
 #[diag(lint_unused_allocation_mut)]
 pubstructUnusedAllocationMutDiag;
+
+pubstructAsyncFnInTraitDiag{
+pubsugg:Option<Vec<(Span,String)>>,
+}
+
+impl<'a>DecorateLint<'a,()>forAsyncFnInTraitDiag{
+fndecorate_lint<'b>(
+self,
+diag:&'bmut rustc_errors::DiagnosticBuilder<'a,()>,
+) -> &'bmut rustc_errors::DiagnosticBuilder<'a,()>{
+ diag.note(fluent::lint_note);
+ifletSome(sugg) = self.sugg{
+ diag.multipart_suggestion(fluent::lint_suggestion, sugg,Applicability::MaybeIncorrect);
+}
+ diag
+}
+
+fnmsg(&self) -> rustc_errors::DiagnosticMessage{
+ fluent::lint_async_fn_in_trait
+}
+}

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -4000,14 +4000,6 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 
 // ... whose signature is `async` (i.e. this is an AFIT)
 let(sig, body) = item.expect_fn();
-let hir::IsAsync::Async(async_span) = sig.header.asyncnesselse{
-return;
-};
-letOk(async_span) =
-self.tcx.sess.source_map().span_extend_while(async_span, |c| c.is_whitespace())
-else{
-return;
-};
 let hir::FnRetTy::Return(hir::Ty{kind: hir::TyKind::OpaqueDef(def, ..), .. }) =
  sig.decl.output
 else{
@@ -4021,55 +4013,17 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 return;
 }
 
-let future = self.tcx.hir().item(*def).expect_opaque_ty();
-letSome(hir::GenericBound::LangItemTrait(_, _, _, generics)) = future.bounds.get(0)else{
-// `async fn` should always lower to a lang item bound... but don't ICE.
-return;
-};
-letSome(hir::TypeBindingKind::Equality{term: hir::Term::Ty(future_output_ty)}) =
- generics.bindings.get(0).map(|binding| binding.kind)
-else{
-// Also should never happen.
+letSome(sugg) = suggest_desugaring_async_fn_to_impl_future_in_trait(
+self.tcx,
+*sig,
+*body,
+ opaque_def_id.expect_local(),
+&format!(" + {auto_trait}"),
+)else{
 return;
 };
 
 let function_name = self.tcx.def_path_str(fn_def_id);
-
-letmut sugg = if future_output_ty.span.is_empty(){
-vec![
-(async_span,String::new()),
-(
- future_output_ty.span,
- format!(" -> impl std::future::Future<Output = ()> + {auto_trait}"),
-),
-]
-}else{
-vec![
-(
- future_output_ty.span.shrink_to_lo(),
-"impl std::future::Future<Output = ".to_owned(),
-),
-(future_output_ty.span.shrink_to_hi(), format!("> + {auto_trait}")),
-(async_span,String::new()),
-]
-};
-
-// If there's a body, we also need to wrap it in `async {}`
-iflet hir::TraitFn::Provided(body) = body {
-let body = self.tcx.hir().body(*body);
-let body_span = body.value.span;
-let body_span_without_braces =
- body_span.with_lo(body_span.lo() + BytePos(1)).with_hi(body_span.hi() - BytePos(1));
-if body_span_without_braces.is_empty(){
- sugg.push((body_span_without_braces," async {} ".to_owned()));
-}else{
- sugg.extend([
-(body_span_without_braces.shrink_to_lo(),"async {".to_owned()),
-(body_span_without_braces.shrink_to_hi(),"} ".to_owned()),
-]);
-}
-}
-
  err.multipart_suggestion(
 format!(
 "`{auto_trait}` can be made part of the associated future's \
@@ -4321,3 +4275,65 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceImplTraitFolder<'tcx> {
 self.tcx
 }
 }
+
+pubfnsuggest_desugaring_async_fn_to_impl_future_in_trait<'tcx>(
+tcx:TyCtxt<'tcx>,
+sig: hir::FnSig<'tcx>,
+body: hir::TraitFn<'tcx>,
+opaque_def_id:LocalDefId,
+add_bounds:&str,
+) -> Option<Vec<(Span,String)>>{
+let hir::IsAsync::Async(async_span) = sig.header.asyncnesselse{
+returnNone;
+};
+letOk(async_span) = tcx.sess.source_map().span_extend_while(async_span, |c| c.is_whitespace())
+else{
+returnNone;
+};
+
+let future = tcx.hir().get_by_def_id(opaque_def_id).expect_item().expect_opaque_ty();
+letSome(hir::GenericBound::LangItemTrait(_, _, _, generics)) = future.bounds.get(0)else{
+// `async fn` should always lower to a lang item bound... but don't ICE.
+returnNone;
+};
+letSome(hir::TypeBindingKind::Equality{term: hir::Term::Ty(future_output_ty)}) =
+ generics.bindings.get(0).map(|binding| binding.kind)
+else{
+// Also should never happen.
+returnNone;
+};
+
+letmut sugg = if future_output_ty.span.is_empty(){
+vec![
+(async_span,String::new()),
+(
+ future_output_ty.span,
+ format!(" -> impl std::future::Future<Output = ()>{add_bounds}"),
+),
+]
+}else{
+vec![
+(future_output_ty.span.shrink_to_lo(),"impl std::future::Future<Output = ".to_owned()),
+(future_output_ty.span.shrink_to_hi(), format!(">{add_bounds}")),
+(async_span,String::new()),
+]
+};
+
+// If there's a body, we also need to wrap it in `async {}`
+iflet hir::TraitFn::Provided(body) = body {
+let body = tcx.hir().body(body);
+let body_span = body.value.span;
+let body_span_without_braces =
+ body_span.with_lo(body_span.lo() + BytePos(1)).with_hi(body_span.hi() - BytePos(1));
+if body_span_without_braces.is_empty(){
+ sugg.push((body_span_without_braces," async {} ".to_owned()));
+}else{
+ sugg.extend([
+(body_span_without_braces.shrink_to_lo(),"async {".to_owned()),
+(body_span_without_braces.shrink_to_hi(),"} ".to_owned()),
+]);
+}
+}
+
+Some(sugg)
+}

tests/ui/async-await/in-trait/async-associated-types.rs

@@ -9,12 +9,14 @@ use std::fmt::Debug;
 traitMyTrait<'a,'b,T>whereSelf:'a,T:Debug + Sized + 'b{
 typeMyAssoc;
 
+#[allow(async_fn_in_trait)]
 asyncfnfoo(&'aself,key:&'bT) -> Self::MyAssoc;
 }
 
 impl<'a,'b,T:Debug + Sized + 'b,U:'a>MyTrait<'a,'b,T>forU{
 typeMyAssoc = (&'aU,&'bT);
 
+#[allow(async_fn_in_trait)]
 asyncfnfoo(&'aself,key:&'bT) -> (&'aU,&'bT){
 (self, key)
 }

tests/ui/async-await/in-trait/async-default-fn-overridden.rs

@@ -6,10 +6,12 @@
 use std::future::Future;
 
 traitAsyncTrait{
+#[allow(async_fn_in_trait)]
 asyncfndefault_impl(){
 assert!(false);
 }
 
+#[allow(async_fn_in_trait)]
 asyncfncall_default_impl(){
 Self::default_impl().await
 }

tests/ui/async-await/in-trait/async-example-desugared-extra.rs

@@ -10,6 +10,7 @@ use std::pin::Pin;
 use std::task::Poll;
 
 pubtraitMyTrait{
+#[allow(async_fn_in_trait)]
 asyncfnfoo(&self) -> i32;
 }
 

tests/ui/async-await/in-trait/async-example-desugared.rs

@@ -8,6 +8,7 @@
 use std::future::Future;
 
 traitMyTrait{
+#[allow(async_fn_in_trait)]
 asyncfnfoo(&self) -> i32;
 }
 

tests/ui/async-await/in-trait/async-example.rs

@@ -5,7 +5,10 @@
 #![allow(incomplete_features)]
 
 traitMyTrait{
+#[allow(async_fn_in_trait)]
 asyncfnfoo(&self) -> i32;
+
+#[allow(async_fn_in_trait)]
 asyncfnbar(&self) -> i32;
 }