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

Add async_fn_in_trait lint #116184

Merged
merged 6 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -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`

Expand Down
123 changes: 123 additions & 0 deletions compiler/rustc_lint/src/async_fn_in_trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
use crate::lints::AsyncFnInTraitDiag;
use crate::LateContext;
use crate::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)]
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
/// 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 {} }
/// }
/// ```
///
tmandry marked this conversation as resolved.
Show resolved Hide resolved
/// 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 [auto traits]
/// such as [`Send`], then the lint may be suppressed.
tmandry marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`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
pub ASYNC_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> for AsyncFnInTrait {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'tcx>) {
if let 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
});
}
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ extern crate rustc_session;
extern crate tracing;

mod array_into_iter;
mod async_fn_in_trait;
pub mod builtin;
mod context;
mod deref_into_dyn_supertrait;
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -234,6 +236,7 @@ late_lint_methods!(
MapUnitFn: MapUnitFn,
MissingDebugImplementations: MissingDebugImplementations,
MissingDoc: MissingDoc,
AsyncFnInTrait: AsyncFnInTrait,
]
]
);
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1818,3 +1818,24 @@ pub struct UnusedAllocationDiag;
#[derive(LintDiagnostic)]
#[diag(lint_unused_allocation_mut)]
pub struct UnusedAllocationMutDiag;

pub struct AsyncFnInTraitDiag {
pub sugg: Option<Vec<(Span, String)>>,
}

impl<'a> DecorateLint<'a, ()> for AsyncFnInTraitDiag {
fn decorate_lint<'b>(
self,
diag: &'b mut rustc_errors::DiagnosticBuilder<'a, ()>,
) -> &'b mut rustc_errors::DiagnosticBuilder<'a, ()> {
diag.note(fluent::lint_note);
if let Some(sugg) = self.sugg {
diag.multipart_suggestion(fluent::lint_suggestion, sugg, Applicability::MaybeIncorrect);
}
diag
}

fn msg(&self) -> rustc_errors::DiagnosticMessage {
fluent::lint_async_fn_in_trait
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.asyncness else {
return;
};
let Ok(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 {
Expand All @@ -4021,55 +4013,17 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
return;
}

let future = self.tcx.hir().item(*def).expect_opaque_ty();
let Some(hir::GenericBound::LangItemTrait(_, _, _, generics)) = future.bounds.get(0) else {
// `async fn` should always lower to a lang item bound... but don't ICE.
return;
};
let Some(hir::TypeBindingKind::Equality { term: hir::Term::Ty(future_output_ty) }) =
generics.bindings.get(0).map(|binding| binding.kind)
else {
// Also should never happen.
let Some(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);

let mut 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 {}`
if let 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 \
Expand Down Expand Up @@ -4321,3 +4275,65 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceImplTraitFolder<'tcx> {
self.tcx
}
}

pub fn suggest_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.asyncness else {
return None;
};
let Ok(async_span) = tcx.sess.source_map().span_extend_while(async_span, |c| c.is_whitespace())
else {
return None;
};

let future = tcx.hir().get_by_def_id(opaque_def_id).expect_item().expect_opaque_ty();
let Some(hir::GenericBound::LangItemTrait(_, _, _, generics)) = future.bounds.get(0) else {
// `async fn` should always lower to a lang item bound... but don't ICE.
return None;
};
let Some(hir::TypeBindingKind::Equality { term: hir::Term::Ty(future_output_ty) }) =
generics.bindings.get(0).map(|binding| binding.kind)
else {
// Also should never happen.
return None;
};

let mut 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 {}`
if let 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)
}
2 changes: 2 additions & 0 deletions tests/ui/async-await/in-trait/async-associated-types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ use std::fmt::Debug;
trait MyTrait<'a, 'b, T> where Self: 'a, T: Debug + Sized + 'b {
type MyAssoc;

#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> Self::MyAssoc;
}

impl<'a, 'b, T: Debug + Sized + 'b, U: 'a> MyTrait<'a, 'b, T> for U {
type MyAssoc = (&'a U, &'b T);

#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> (&'a U, &'b T) {
(self, key)
}
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/async-await/in-trait/async-default-fn-overridden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
use std::future::Future;

trait AsyncTrait {
#[allow(async_fn_in_trait)]
async fn default_impl() {
assert!(false);
}

#[allow(async_fn_in_trait)]
async fn call_default_impl() {
Self::default_impl().await
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::pin::Pin;
use std::task::Poll;

pub trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;
}

Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/in-trait/async-example-desugared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use std::future::Future;

trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;
}

Expand Down
3 changes: 3 additions & 0 deletions tests/ui/async-await/in-trait/async-example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
#![allow(incomplete_features)]

trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;

#[allow(async_fn_in_trait)]
async fn bar(&self) -> i32;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use std::fmt::Debug;

trait MyTrait<'a, 'b, T> {
#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> (&'a Self, &'b T) where T: Debug + Sized;
}

Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/in-trait/async-lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![allow(incomplete_features)]

trait MyTrait<'a, 'b, T> {
#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> (&'a Self, &'b T);
}

Expand Down
Loading
Loading