When suggesting trait bounds, use fully-qualified path

[estebank]

This side-steps issues of name clashes with imports or traits in the current scope.

error[E0277]: the trait bound `T: MyTrait` is not satisfied
  --> $DIR/bound-suggestions.rs:72:27
   |
LL |     fn method<T: MyTrait, W: super::Wrapper<T>>() {}
   |                              ^^^^^^^^^^^^^^^^^ the trait `MyTrait` is not implemented for `T`
   |
note: required by a bound in `Wrapper`
  --> $DIR/bound-suggestions.rs:66:18
   |
LL | trait Wrapper<T: MyTrait> {}
   |                  ^^^^^^^ required by this bound in `Wrapper`
help: consider further restricting type parameter `T` with trait `MyTrait`
   |
LL |     fn method<T: MyTrait + crate::MyTrait, W: super::Wrapper<T>>() {}
   |                          ++++++++++++++++

Ideally we'd lean on resolve to have the best name for a DefId on a given scope, but then we'd have to ferry the scope around for pretty much every diagnostic. This is the second best thing that does address the problem of "the suggestion is broken". This PR addresses #36184 on edition>=2018. For edition 2015 we'd have to suggest super::MyTrait.

[rustbot]

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[BoxyUwU]

This seems to make most diagnostics significantly more verbose, in some cases even suggesting full paths to Sized which is pretty yikes.

It's not clear to me that it's an urgent problem that we have unqualified paths in diagnostic suggestions. To me this feels like swapping one bad diagnostic for another bad diagnostic.

In general I'd rather we properly figure out some "minimal" path to use for diagnostics and use that instead of a binary no qualification vs overly verbose full qualification.

Can you explain a bit why you prefer this state over the previous state? And also why not do the work to figure out "minimally valid" paths to use in diagnostic suggestions?

View changes since this review

[estebank]

Can you explain a bit why you prefer this state over the previous state?

Providing verbose but correct suggestions feels like a more reasonable state than short but incorrect ones. In general I've relied on the machinery that looks for names not in scope everywhere, so that at worst a suggestion for Foo where it should have been path::to::Foo is two errors two working code instead of one. I deemed that acceptable.

To me this feels like swapping one bad diagnostic for another bad diagnostic.

But in the case that we have here not having the accurate path just gives us an error cycle, where the diagnostics never guide you to the right place :(

And also why not do the work to figure out "minimally valid" paths to use in diagnostic suggestions?

Figuring out the minimally valid accurate path requires us always having the DefId of the scope we're in first. In this case we have item_id so it should be possible with little fanfare, I just don't recall us having a proper mechanism to get that path today. I'm ok with leaving that ticket open and closing this PR until we build that functionality.