Incorrect add bounds suggestion when given type parameter like <T:>

[compiler-errors]

Given the following code:

fn foo<T:>(t: T) {
    t.clone();
}

The current output is:

error[E0599]: no method named `clone` found for type parameter `T` in the current scope
 --> src/lib.rs:2:7
  |
2 |     t.clone();
  |       ^^^^^ method not found in `T`
  |
  = help: items from traits can only be used if the type parameter is bounded by the trait
help: the following trait defines an item `clone`, perhaps you need to restrict type parameter `T` with it:
  |
1 | fn foo<T: Clone:>(t: T) {
  |        ~~~~~~~~

Ideally the output should look like:

error[E0599]: no method named `clone` found for type parameter `T` in the current scope
 --> src/lib.rs:2:7
  |
2 |     t.clone();
  |       ^^^^^ method not found in `T`
  |
  = help: items from traits can only be used if the type parameter is bounded by the trait
help: the following trait defines an item `clone`, perhaps you need to restrict type parameter `T` with it:
  |
1 | fn foo<T: Clone>(t: T) {
  |          ++++++

Note: The T: bound is treated incorrectly.

[PoorlyDefinedBehaviour]

@rustbot claim

[PoorlyDefinedBehaviour]

Note: The T: bound is treated incorrectly.

You mean that suggest_traits_to_import does not handle T: correctly, right?

[compiler-errors]

Correct, the suggestion reads T: Clone: which is not syntactical. The fix would be to detect this case and correctly suggest T: Clone

[PoorlyDefinedBehaviour]

It's my first time contributing and i was taking a look at suggest_traits_to_import and didn't find a way to detect T:.

So i ended up adding a flag to GenericParam, setting it in parse_ty_param and checking the flag in suggest_traits_to_import to handle the T: case.

Does that seem reasonable?

[compiler-errors]

Adding a new flag to hir::GenericParam seems a bit heavy. I was thinking you could one of the methods on Span to detect if there is a colon following the generic param.

Also hmm... I think @WaffleLapkin was also looking at trying to solve this... I opened up this issue before he said that he was trying to fix this issue, though I'm not sure if in general or just a specific case in something he was working on (i.e. another suggestion). Perhaps he could comment to see if he's already solved this, in order to deduplicate work?

[WaffleLapkin]

So. I was fixing the same problem in rustc_middle::ty::suggest_constraining_type_params and I just got a working fix!

My solution is to use SourceMap in order to find the span of :. It may be hacky, but I think it's good enough for diagnostics.

It seems like my fix doesn't fix this issue, because this suggestion is created using different API (suggest_traits_to_import), but it should be easy to extend it to this case too. For example suggest_traits_to_import could use suggest_constraining_type_params internally (suggest_constraining_type_params produces better suggestions anyway :D).

I'll try to clean my fix & create a PR today.

[PoorlyDefinedBehaviour]

So. I was fixing the same problem in rustc_middle::ty::suggest_constraining_type_params and I just got a working fix!

My solution is to use SourceMap in order to find the span of :. It may be hacky, but I think it's good enough for diagnostics.

It seems like my fix doesn't fix this issue, because this suggestion is created using different API (suggest_traits_to_import), but it should be easy to extend it to this case too. For example suggest_traits_to_import could use suggest_constraining_type_params internally (suggest_constraining_type_params produces better suggestions anyway :D).

I'll try to clean my fix & create a PR today.

I took a look at your PR, that's way better than my solution.

I will use the method you added to GenericParam to fix this issue if you aren't working on it as well.