about summary refs log tree commit diff
path: root/compiler/rustc_parse/src
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-02-14 18:02:50 +0100
committerGitHub <noreply@github.com>2023-02-14 18:02:50 +0100
commit202c70666f4fa0a5d823169620487fd685555f75 (patch)
tree4d8cec0c78764b2f6ef85a8d4c4f9c07e72e3d3a /compiler/rustc_parse/src
parent9bb6e60d1f1360234aae90c97964c0fa5524f141 (diff)
parenta3d32bbbbe06ffe42edbc4905e964d394de5ee02 (diff)
downloadrust-202c70666f4fa0a5d823169620487fd685555f75.tar.gz
rust-202c70666f4fa0a5d823169620487fd685555f75.zip
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
Diffstat (limited to 'compiler/rustc_parse/src')
-rw-r--r--compiler/rustc_parse/src/parser/diagnostics.rs58
1 files changed, 56 insertions, 2 deletions
diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs
index cd9d85b1d91..49eff41329c 100644
--- a/compiler/rustc_parse/src/parser/diagnostics.rs
+++ b/compiler/rustc_parse/src/parser/diagnostics.rs
@@ -284,7 +284,7 @@ impl<'a> Parser<'a> {
         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 @@ impl<'a> Parser<'a> {
             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(