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

Tweak suggestions for bare trait used as a type #119148

Merged
merged 6 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Provide better suggestions when encountering a bare trait as a type
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 Jan 3, 2024
commit 048750077676190a0733112d8f700e76a0553f60
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(
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
&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])
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
}

/// 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.dcx(), 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