Invalid `dyn` suggestions

[m-ou-se]

In a small crater run for the 2021 edition migrations, a few problems turned up with the suggestions we give for missing dyn keywords:

• Invalid suggestion to change Trait::thing to <dyn Trait>::thing without generic args:
  • https://crater-reports.s3.amazonaws.com/pr-87190-2/try%23d8627d19850b63702b76089d7b055730e9ddca14/reg/biofile-1.10.2/log.txt
  • https://crater-reports.s3.amazonaws.com/pr-87190-2/try%23d8627d19850b63702b76089d7b055730e9ddca14/reg/epoxy_streams-0.3.1/log.txt
• Invalid suggestion to change <Trait>::.. to <<dyn Trait>>::..: https://crater-reports.s3.amazonaws.com/pr-87190-2/try%23d8627d19850b63702b76089d7b055730e9ddca14/reg/extended-primitives-0.3.5/log.txt

[ehuss]

There seems to be two issues here.

Duplicate diagnostic causes extra brackets.

use std::fmt;

#[derive(Debug)]
pub struct Foo;

impl fmt::Display for Foo {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        <fmt::Debug>::fmt(self, f)  // bare_trait_objects
    }
}

The above will trigger two warnings for bare_trait_objects. I thought the lint system had some deduplication support, so I'm unsure why that is happening (I also thought rustfix had some sort of overlap protection as well). When the fixes are applied, it results in <<dyn fmt::Debug>>::fmt(self, f) which is invalid syntax.

Transition from associated trait path to qualified path requires type parameters

trait Foo<T> {}

impl<T> dyn Foo<T> {
    fn hi(_x: T)  {}
}

fn main() {
    Foo::hi(123); // bare_trait_objects
}

Here, the suggestion wants to fix it to <dyn Foo>::hi(123);. However, that fails to compile because a qualified path requires the generic arguments to be specified, whereas the non-qualified path I believe uses inference?

[m-ou-se]

The first case doesn't seem to be a case of doubly-applied suggestions:

rust/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs

Lines 936 to 941 in 6cfa773

	let (sugg, app) = match self.tcx.sess.source_map().span_to_snippet(self_ty.span) {
	Ok(s) if poly_trait_ref.trait_ref.path.is_global() => {
	(format!("<dyn ({})>", s), Applicability::MachineApplicable)
	}
	Ok(s) => (format!("<dyn {}>", s), Applicability::MachineApplicable),
	Err(_) => ("<dyn <type>>".to_string(), Applicability::HasPlaceholders),

Changing the <> there to «» results in:

 <«dyn fmt::Debug»>::fmt(self, f)

[m-ou-se]

I've split the << .. >> issue into another issue: #88508

[camelid]

Transition from associated trait path to qualified path requires type parameters

trait Foo<T> {}

impl<T> dyn Foo<T> {
    fn hi(_x: T)  {}
}

fn main() {
    Foo::hi(123); // bare_trait_objects
}

Here, the suggestion wants to fix it to <dyn Foo>::hi(123);. However, that fails to compile because a qualified path requires the generic arguments to be specified, whereas the non-qualified path I believe uses inference?

I'll note that the error that is reported if you apply the <dyn Foo>::hi(123); suggestion further suggests that you add a generic parameter, so it's not great that rustc suggests invalid code, but at least rustc will then tell you how to fix it :)

   Compiling playground v0.0.1 (/playground)
error[E0107]: missing generics for trait `Foo`
 --> src/main.rs:8:10
  |
8 |     <dyn Foo>::hi(123); // bare_trait_objects
  |          ^^^ expected 1 generic argument
  |
note: trait defined here, with 1 generic parameter: `T`
 --> src/main.rs:1:7
  |
1 | trait Foo<T> {}
  |       ^^^ -
help: add missing generic argument
  |
8 |     <dyn Foo<T>>::hi(123); // bare_trait_objects
  |          ~~~~~~

For more information about this error, try `rustc --explain E0107`.
error: could not compile `playground` due to previous error

[estebank]

Triage:

warning: trait objects without an explicit `dyn` are deprecated
 --> f1000.rs:8:10
  |
8 |         <fmt::Debug>::fmt(self, f)  // bare_trait_objects
  |          ^^^^^^^^^^
  |
  = 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: if this is an object-safe trait, use `dyn`
  |
8 |         <dyn fmt::Debug>::fmt(self, f)  // bare_trait_objects
  |          +++

Consider the above solved, as the suggestion is now more targeted.

---

warning: trait objects without an explicit `dyn` are deprecated
 --> f1000.rs:8:5
  |
8 |     Foo::hi(123); // bare_trait_objects
  |     ^^^
  |
  = 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: if this is an object-safe trait, use `dyn`
  |
8 |     <dyn Foo>::hi(123); // bare_trait_objects
  |     ++++    +

Applying the suggestion results in

error[E0107]: missing generics for trait `Foo`
 --> f1000.rs:8:10
  |
8 |     <dyn Foo>::hi(123); // bare_trait_objects
  |          ^^^ expected 1 generic argument
  |
note: trait defined here, with 1 generic parameter: `T`
 --> f1000.rs:1:7
  |
1 | trait Foo<T> {}
  |       ^^^ -
help: add missing generic argument
  |
8 |     <dyn Foo<T>>::hi(123); // bare_trait_objects
  |             +++

Changing T with i32 results in successful compilation.

This is the same situation we originally had. I consider two "hops" of suggestions resulting in working code to be fairly good. Better if it is in a single "hop", but that's not always possible (otherwise we'd have to teach the parser to typeck, for example).

To fix that suggestion, we need to modify this bit of code

rust/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

Lines 48 to 57 in f617130

	if is_global || needs_bracket {
	sugg.push((
	self_ty.span.shrink_to_hi(),
	format!(
	"{}{}",
	if is_global { ")" } else { "" },
	if needs_bracket { ">" } else { "" },
	),
	));
	}

to check that poly_trait_ref.trait_ref.path has no args, and check whether the trait poly_trait_ref.trait_ref.trait_def_id() needs type params.