Skip to content

Commit

Permalink
Provide better suggestions when encountering a bare trait as a type
Browse files Browse the repository at this point in the history
Add the following suggestions:

```
error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:11
   |
LL | fn bar(x: Foo) -> Foo {
   |           ^^^
   |
help: use a generic type parameter, constrained by the trait `Foo`
   |
LL | fn bar<T: Foo>(x: T) -> Foo {
   |       ++++++++    ~
help: you can also use `impl Foo`, but users won't be able to specify the type paramer when calling the `fn`, having to rely exclusively on type inference
   |
LL | fn bar(x: impl Foo) -> Foo {
   |           ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
   |
LL | fn bar(x: &dyn Foo) -> Foo {
   |           ++++

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:11:19
   |
LL | fn bar(x: Foo) -> Foo {
   |                   ^^^
   |
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
   |
LL | fn bar(x: Foo) -> impl Foo {
   |                   ++++
help: alternatively, you can return an owned trait object
   |
LL | fn bar(x: Foo) -> Box<dyn Foo> {
   |                   +++++++    +
```
  • Loading branch information
estebank committed Dec 20, 2023
1 parent 57ad505 commit eb4b2e9
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 21 deletions.
131 changes: 113 additions & 18 deletions compiler/rustc_hir_analysis/src/astconv/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use rustc_ast::TraitObjectSyntax;
use rustc_errors::{Diagnostic, StashKey};
use rustc_hir as hir;
use rustc_lint_defs::{builtin::BARE_TRAIT_OBJECTS, Applicability};
use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamName;

use super::AstConv;
Expand Down Expand Up @@ -32,32 +33,120 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
let of_trait_span = of_trait_ref.path.span;
// make sure that we are not calling unwrap to abort during the compilation
let Ok(impl_trait_name) = tcx.sess.source_map().span_to_snippet(self_ty.span) else {
return;
};
let Ok(of_trait_name) = tcx.sess.source_map().span_to_snippet(of_trait_span) else {
return;
};
// check if the trait has generics, to make a correct suggestion
let param_name = generics.params.next_type_param_name(None);

let add_generic_sugg = if let Some(span) = generics.span_for_param_suggestion() {
(span, format!(", {param_name}: {impl_trait_name}"))
} else {
(generics.span, format!("<{param_name}: {impl_trait_name}>"))
let Ok(impl_trait_name) = self.tcx().sess.source_map().span_to_snippet(self_ty.span)
else {
return;
};
let Some(sugg) = self.generics_suggestion(generics, self_ty.span, &impl_trait_name)
else {
return;
};
diag.multipart_suggestion(
format!(
"alternatively use a blanket \
implementation to implement `{of_trait_name}` for \
"alternatively use a blanket implementation to implement `{of_trait_name}` for \
all types that also implement `{impl_trait_name}`"
),
vec![(self_ty.span, param_name), add_generic_sugg],
sugg,
Applicability::MaybeIncorrect,
);
}
}

fn generics_suggestion(
&self,
generics: &hir::Generics<'_>,
self_ty_span: Span,
impl_trait_name: &str,
) -> Option<Vec<(Span, String)>> {
// check if the trait has generics, to make a correct suggestion
let param_name = generics.params.next_type_param_name(None);

let add_generic_sugg = if let Some(span) = generics.span_for_param_suggestion() {
(span, format!(", {param_name}: {impl_trait_name}"))
} else {
(generics.span, format!("<{param_name}: {impl_trait_name}>"))
};
Some(vec![(self_ty_span, param_name), add_generic_sugg])
}

/// Make sure that we are in the condition to suggest `impl Trait`.
fn maybe_lint_impl_trait(&self, self_ty: &hir::Ty<'_>, diag: &mut Diagnostic) -> bool {
let tcx = self.tcx();
let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id;
let (hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. })
| hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(sig, _),
generics,
..
})) = tcx.hir_node_by_def_id(parent_id)
else {
return false;
};
let Ok(trait_name) = self.tcx().sess.source_map().span_to_snippet(self_ty.span) else {
return false;
};
let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())];
if let hir::FnRetTy::Return(ty) = sig.decl.output
&& ty.hir_id == self_ty.hir_id
{
diag.multipart_suggestion_verbose(
format!("use `impl {trait_name}` to return an opaque type, as long as you return a single underlying type"),
impl_sugg,
Applicability::MachineApplicable,
);
diag.multipart_suggestion_verbose(
"alternatively, you can return an owned trait object",
vec![
(ty.span.shrink_to_lo(), "Box<dyn ".to_string()),
(ty.span.shrink_to_hi(), ">".to_string()),
],
Applicability::MachineApplicable,
);
return true;
}
for ty in sig.decl.inputs {
if ty.hir_id == self_ty.hir_id {
if let Some(sugg) = self.generics_suggestion(generics, self_ty.span, &trait_name) {
diag.multipart_suggestion_verbose(
format!("use a new generic type parameter, constrained by `{trait_name}`"),
sugg,
Applicability::MachineApplicable,
);
diag.multipart_suggestion_verbose(
"you can also use an opaque type, but users won't be able to specify the \
type parameter when calling the `fn`, having to rely exclusively on type \
inference",
impl_sugg,
Applicability::MachineApplicable,
);
}
let sugg = if let hir::TyKind::TraitObject([_, _, ..], _, _) = self_ty.kind {
// There are more than one trait bound, we need surrounding parentheses.
vec![
(self_ty.span.shrink_to_lo(), "&(dyn ".to_string()),
(self_ty.span.shrink_to_hi(), ")".to_string()),
]
} else {
vec![(self_ty.span.shrink_to_lo(), "&dyn ".to_string())]
};
diag.multipart_suggestion_verbose(
format!(
"alternatively, use a trait object to accept any type that implements \
`{trait_name}`, accessing its methods at runtime using dynamic dispatch",
),
sugg,
Applicability::MachineApplicable,
);
return true;
}
}
false
}

pub(super) fn maybe_lint_bare_trait(&self, self_ty: &hir::Ty<'_>, in_path: bool) {
let tcx = self.tcx();
if let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
Expand Down Expand Up @@ -98,7 +187,9 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let label = "add `dyn` keyword before this trait";
let mut diag =
rustc_errors::struct_span_err!(tcx.sess, self_ty.span, E0782, "{}", msg);
if self_ty.span.can_be_used_for_suggestions() {
if self_ty.span.can_be_used_for_suggestions()
&& !self.maybe_lint_impl_trait(self_ty, &mut diag)
{
diag.multipart_suggestion_verbose(
label,
sugg,
Expand All @@ -116,11 +207,15 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
self_ty.span,
msg,
|lint| {
lint.multipart_suggestion_verbose(
"use `dyn`",
sugg,
Applicability::MachineApplicable,
);
if self_ty.span.can_be_used_for_suggestions()
&& !self.maybe_lint_impl_trait(self_ty, lint)
{
lint.multipart_suggestion_verbose(
"use `dyn`",
sugg,
Applicability::MachineApplicable,
);
}
self.maybe_lint_blanket_trait_impl(self_ty, lint);
},
);
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/traits/bound/not-on-bare-trait-2021.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// edition:2021
trait Foo {
fn dummy(&self) {}
}

// This should emit the less confusing error, not the more confusing one.

fn foo(_x: Foo + Send) {
//~^ ERROR trait objects must include the `dyn` keyword
}
fn bar(x: Foo) -> Foo {
//~^ ERROR trait objects must include the `dyn` keyword
//~| ERROR trait objects must include the `dyn` keyword
x
}

fn main() {}
56 changes: 56 additions & 0 deletions tests/ui/traits/bound/not-on-bare-trait-2021.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/not-on-bare-trait-2021.rs:8:12
|
LL | fn foo(_x: Foo + Send) {
| ^^^^^^^^^^
|
help: use a new generic type parameter, constrained by `Foo + Send`
|
LL | fn foo<T: Foo + Send>(_x: T) {
| +++++++++++++++ ~
help: you can also use an opaque type, but users won't be able to specify the type parameter when calling the `fn`, having to rely exclusively on type inference
|
LL | fn foo(_x: impl Foo + Send) {
| ++++
help: alternatively, use a trait object to accept any type that implements `Foo + Send`, accessing its methods at runtime using dynamic dispatch
|
LL | fn foo(_x: &(dyn Foo + Send)) {
| +++++ +

error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/not-on-bare-trait-2021.rs:11:11
|
LL | fn bar(x: Foo) -> Foo {
| ^^^
|
help: use a new generic type parameter, constrained by `Foo`
|
LL | fn bar<T: Foo>(x: T) -> Foo {
| ++++++++ ~
help: you can also use an opaque type, but users won't be able to specify the type parameter when calling the `fn`, having to rely exclusively on type inference
|
LL | fn bar(x: impl Foo) -> Foo {
| ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
|
LL | fn bar(x: &dyn Foo) -> Foo {
| ++++

error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/not-on-bare-trait-2021.rs:11:19
|
LL | fn bar(x: Foo) -> Foo {
| ^^^
|
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
|
LL | fn bar(x: Foo) -> impl Foo {
| ++++
help: alternatively, you can return an owned trait object
|
LL | fn bar(x: Foo) -> Box<dyn Foo> {
| +++++++ +

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0782`.
14 changes: 11 additions & 3 deletions tests/ui/traits/bound/not-on-bare-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@ LL | fn foo(_x: Foo + Send) {
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: `#[warn(bare_trait_objects)]` on by default
help: use `dyn`
help: use a new generic type parameter, constrained by `Foo + Send`
|
LL | fn foo(_x: dyn Foo + Send) {
| +++
LL | fn foo<T: Foo + Send>(_x: T) {
| +++++++++++++++ ~
help: you can also use an opaque type, but users won't be able to specify the type parameter when calling the `fn`, having to rely exclusively on type inference
|
LL | fn foo(_x: impl Foo + Send) {
| ++++
help: alternatively, use a trait object to accept any type that implements `Foo + Send`, accessing its methods at runtime using dynamic dispatch
|
LL | fn foo(_x: &(dyn Foo + Send)) {
| +++++ +

error[E0277]: the size for values of type `(dyn Foo + Send + 'static)` cannot be known at compilation time
--> $DIR/not-on-bare-trait.rs:7:8
Expand Down

0 comments on commit eb4b2e9

Please sign in to comment.