Rollup merge of #103478 - SpanishPear:spanishpear/issue_103366_fix, r=TaKO8Ki

Suggest fix for misplaced generic params on fn item #103366

fixes #103366

This still has some work to go, but works for 2/3 of the initial base cases described in #1033366

simple fn:
```
error: expected identifier, found `<`
 --> shreys/test_1.rs:1:3
  |
1 | fn<T> id(x: T) -> T { x }
  |   ^ expected identifier
  |
help: help: place the generic parameter list after the function name:
  |
1 | fn id<T>(x: T) -> T { x }
  |    ~~~~

```

Complicated bounds
```
error: expected identifier, found `<`
 --> spanishpear/test_2.rs:1:3
  |
1 | fn<'a, B: 'a + std::ops::Add<Output = u32>> f(_x: B) { }
  |   ^ expected identifier
  |
help: help: place the generic parameter list after the function name:
  |
1 | fn f<'a, B: 'a + std::ops::Add<Output = u32>>(_x: B) { }
  |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Opening a draft PR for comments on approach, particularly I have the following questions:
 -  [x]  Is it okay to be using `err.span_suggestion` over struct derives? I struggled to get the initial implementation (particularly the correct suggestion message) on struct derives, although I think given what I've learned since starting, I could attempt re-doing it with that approach.
  -  [x] in the case where the snippet cannot be obtained from a span, is the `help` but no suggestion okay? I think yes (also, when does this case occur?)
  -  [x] are there any red flags for the generalisation of this work for relevant item kinds (i.e. `struct`, `enum`, `trait`, and `union`). My basic testing indicates it does work for those types except the help tip is currently hardcoded to `after the function name` - which should change dependent on the item.
  - [x] I am planning to not show the suggestion if there is already a `<` after the item identifier, (i.e. if there are already generics, as after a function name per the original issue). Any major objections?
  - [x] Is the style of error okay? I wasn't sure if there was a way to make it display nicer, or if thats handled by span_suggestion

These aren't blocking questions, and I will keep working on:
  - check if there is a `<` after the ident (and if so, not showing the suggestion)
  - generalize the help message
  - figuring out how to write/run/etc ui tests (including reading the docs for them)
  - logic cleanups
This commit is contained in:
Matthias Krüger 2023-02-14 18:02:50 +01:00 committed by GitHub
commit 202c70666f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 287 additions and 2 deletions

View File

@ -284,7 +284,7 @@ pub(super) fn span_to_snippet(&self, span: Span) -> Result<String, SpanSnippetEr
self.sess.source_map().span_to_snippet(span)
}
pub(super) fn expected_ident_found(&self) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
pub(super) fn expected_ident_found(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
let valid_follow = &[
TokenKind::Eq,
TokenKind::Colon,
@ -324,7 +324,61 @@ pub(super) fn expected_ident_found(&self) -> DiagnosticBuilder<'a, ErrorGuarante
suggest_raw,
suggest_remove_comma,
};
err.into_diagnostic(&self.sess.span_diagnostic)
let mut err = err.into_diagnostic(&self.sess.span_diagnostic);
// if the token we have is a `<`
// it *might* be a misplaced generic
if self.token == token::Lt {
// all keywords that could have generic applied
let valid_prev_keywords =
[kw::Fn, kw::Type, kw::Struct, kw::Enum, kw::Union, kw::Trait];
// If we've expected an identifier,
// and the current token is a '<'
// if the previous token is a valid keyword
// that might use a generic, then suggest a correct
// generic placement (later on)
let maybe_keyword = self.prev_token.clone();
if valid_prev_keywords.into_iter().any(|x| maybe_keyword.is_keyword(x)) {
// if we have a valid keyword, attempt to parse generics
// also obtain the keywords symbol
match self.parse_generics() {
Ok(generic) => {
if let TokenKind::Ident(symbol, _) = maybe_keyword.kind {
let ident_name = symbol;
// at this point, we've found something like
// `fn <T>id`
// and current token should be Ident with the item name (i.e. the function name)
// if there is a `<` after the fn name, then don't show a suggestion, show help
if !self.look_ahead(1, |t| *t == token::Lt) &&
let Ok(snippet) = self.sess.source_map().span_to_snippet(generic.span) {
err.multipart_suggestion_verbose(
format!("place the generic parameter name after the {ident_name} name"),
vec![
(self.token.span.shrink_to_hi(), snippet),
(generic.span, String::new())
],
Applicability::MaybeIncorrect,
);
} else {
err.help(format!(
"place the generic parameter name after the {ident_name} name"
));
}
}
}
Err(err) => {
// if there's an error parsing the generics,
// then don't do a misplaced generics suggestion
// and emit the expected ident error instead;
err.cancel();
}
}
}
}
err
}
pub(super) fn expected_one_of_not_found(

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
enum Foo<T> { Variant(T) }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the enum name
fn main() {}

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
enum<T> Foo { Variant(T) }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the enum name
fn main() {}

View File

@ -0,0 +1,14 @@
error: expected identifier, found `<`
--> $DIR/enum.rs:5:5
|
LL | enum<T> Foo { Variant(T) }
| ^ expected identifier
|
help: place the generic parameter name after the enum name
|
LL - enum<T> Foo { Variant(T) }
LL + enum Foo<T> { Variant(T) }
|
error: aborting due to previous error

View File

@ -0,0 +1,9 @@
// Issue: 103366
// there is already an existing generic on f, so don't show a suggestion
#[allow(unused)]
fn<'a, B: 'a + std::ops::Add<Output = u32>> f<T>(_x: B) { }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the fn name
fn main() {}

View File

@ -0,0 +1,10 @@
error: expected identifier, found `<`
--> $DIR/existing_generics.rs:5:3
|
LL | fn<'a, B: 'a + std::ops::Add<Output = u32>> f<T>(_x: B) { }
| ^ expected identifier
|
= help: place the generic parameter name after the fn name
error: aborting due to previous error

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
fn f<'a, B: 'a + std::ops::Add<Output = u32>>(_x: B) { }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the fn name
fn main() {}

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
fn<'a, B: 'a + std::ops::Add<Output = u32>> f(_x: B) { }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the fn name
fn main() {}

View File

@ -0,0 +1,14 @@
error: expected identifier, found `<`
--> $DIR/fn-complex-generics.rs:5:3
|
LL | fn<'a, B: 'a + std::ops::Add<Output = u32>> f(_x: B) { }
| ^ expected identifier
|
help: place the generic parameter name after the fn name
|
LL - fn<'a, B: 'a + std::ops::Add<Output = u32>> f(_x: B) { }
LL + fn f<'a, B: 'a + std::ops::Add<Output = u32>>(_x: B) { }
|
error: aborting due to previous error

View File

@ -0,0 +1,8 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// The generics fail to parse here, so don't make any suggestions/help
#[allow(unused)]
fn<~>()> id(x: T) -> T { x }
//~^ ERROR expected identifier, found `<`
fn main() {}

View File

@ -0,0 +1,8 @@
error: expected identifier, found `<`
--> $DIR/fn-invalid-generics.rs:5:3
|
LL | fn<~>()> id(x: T) -> T { x }
| ^ expected identifier
error: aborting due to previous error

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
fn id<T>(x: T) -> T { x }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the fn name
fn main() {}

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
fn<T> id(x: T) -> T { x }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the fn name
fn main() {}

View File

@ -0,0 +1,14 @@
error: expected identifier, found `<`
--> $DIR/fn-simple.rs:5:3
|
LL | fn<T> id(x: T) -> T { x }
| ^ expected identifier
|
help: place the generic parameter name after the fn name
|
LL - fn<T> id(x: T) -> T { x }
LL + fn id<T>(x: T) -> T { x }
|
error: aborting due to previous error

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
struct Foo<T> { x: T }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the struct name
fn main() {}

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
struct<T> Foo { x: T }
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the struct name
fn main() {}

View File

@ -0,0 +1,14 @@
error: expected identifier, found `<`
--> $DIR/struct.rs:5:7
|
LL | struct<T> Foo { x: T }
| ^ expected identifier
|
help: place the generic parameter name after the struct name
|
LL - struct<T> Foo { x: T }
LL + struct Foo<T> { x: T }
|
error: aborting due to previous error

View File

@ -0,0 +1,11 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
trait Foo<T> {
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the trait name
}
fn main() {}

View File

@ -0,0 +1,11 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
trait<T> Foo {
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the trait name
}
fn main() {}

View File

@ -0,0 +1,14 @@
error: expected identifier, found `<`
--> $DIR/trait.rs:5:6
|
LL | trait<T> Foo {
| ^ expected identifier
|
help: place the generic parameter name after the trait name
|
LL - trait<T> Foo {
LL + trait Foo<T> {
|
error: aborting due to previous error

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
type Foo<T> = T;
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the type name
fn main() {}

View File

@ -0,0 +1,9 @@
// Issue: 103366 , Suggest fix for misplaced generic params
// run-rustfix
#[allow(unused)]
type<T> Foo = T;
//~^ ERROR expected identifier, found `<`
//~| HELP place the generic parameter name after the type name
fn main() {}

View File

@ -0,0 +1,14 @@
error: expected identifier, found `<`
--> $DIR/type.rs:5:5
|
LL | type<T> Foo = T;
| ^ expected identifier
|
help: place the generic parameter name after the type name
|
LL - type<T> Foo = T;
LL + type Foo<T> = T;
|
error: aborting due to previous error