Better message for invalid keyword placement in fn

poliorcetics · poliorcetics · commit 690cbb79b396 · 2021-08-02T11:10:19.000+02:00
After this commit, `unsafe async fn ...` now suggests the `async unsafe` fix
instead of misunderstanding the issue.

This is not perfect for repeated keywords (`const async const`) and for
keywords that are misplaced after `extern "some abi"` because of the way
`check_fn_font_matter` works, but changing it breaks so many tests and
diagnostics it has been judged too high a cost for this PR.
diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs
@@ -1771,8 +1771,14 @@ impl<'a> Parser<'a> {
     pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
         let sp_start = self.token.span;
         let constness = self.parse_constness();
+
+        let async_start_sp = self.token.span;
         let asyncness = self.parse_asyncness();
+
+        let unsafe_start_sp = self.token.span;
         let unsafety = self.parse_unsafety();
+
+        let ext_start_sp = self.token.span;
         let ext = self.parse_extern();
 
         if let Async::Yes { span, .. } = asyncness {
@@ -1787,8 +1793,35 @@ impl<'a> Parser<'a> {
                 Ok(true) => {}
                 Ok(false) => unreachable!(),
                 Err(mut err) => {
+                    // Qualifier keywords ordering check
+
+                    // This will allow the machine fix to directly place the keyword in the correct place
+                    let current_qual_sp = if self.check_keyword(kw::Const) {
+                        Some(async_start_sp)
+                    } else if self.check_keyword(kw::Async) {
+                        Some(unsafe_start_sp)
+                    } else if self.check_keyword(kw::Unsafe) {
+                        Some(ext_start_sp)
+                    } else {
+                        None
+                    };
+
+                    if let Some(current_qual_sp) = current_qual_sp {
+                        let current_qual_sp = current_qual_sp.to(self.prev_token.span);
+                        if let Ok(current_qual) = self.span_to_snippet(current_qual_sp) {
+                            let invalid_qual_sp = self.token.uninterpolated_span();
+                            let invalid_qual = self.span_to_snippet(invalid_qual_sp).unwrap();
+
+                            err.span_suggestion(
+                                current_qual_sp.to(invalid_qual_sp),
+                                &format!("`{}` must come before `{}`", invalid_qual, current_qual),
+                                format!("{} {}", invalid_qual, current_qual),
+                                Applicability::MachineApplicable,
+                            ).note("keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`");
+                        }
+                    }
                     // Recover incorrect visibility order such as `async pub`.
-                    if self.check_keyword(kw::Pub) {
+                    else if self.check_keyword(kw::Pub) {
                         let sp = sp_start.to(self.prev_token.span);
                         if let Ok(snippet) = self.span_to_snippet(sp) {
                             let vis = match self.parse_visibility(FollowedByType::No) {
diff --git a/src/test/ui/async-await/no-async-const.stderr b/src/test/ui/async-await/no-async-const.stderr
@@ -2,7 +2,12 @@ error: expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
   --> $DIR/no-async-const.rs:4:11
    |
 LL | pub async const fn x() {}
-   |           ^^^^^ expected one of `extern`, `fn`, or `unsafe`
+   |     ------^^^^^
+   |     |     |
+   |     |     expected one of `extern`, `fn`, or `unsafe`
+   |     help: `const` must come before `async`: `const async`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/async-await/no-unsafe-async.stderr b/src/test/ui/async-await/no-unsafe-async.stderr
@@ -5,15 +5,25 @@ LL | impl S {
    |        - while parsing this item list starting here
 LL |     #[cfg(FALSE)]
 LL |     unsafe async fn g() {}
-   |            ^^^^^ expected one of `extern` or `fn`
+   |     -------^^^^^
+   |     |      |
+   |     |      expected one of `extern` or `fn`
+   |     help: `async` must come before `unsafe`: `async unsafe`
 LL | }
    | - the item list ends here
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
 
 error: expected one of `extern` or `fn`, found keyword `async`
   --> $DIR/no-unsafe-async.rs:11:8
    |
 LL | unsafe async fn f() {}
-   |        ^^^^^ expected one of `extern` or `fn`
+   | -------^^^^^
+   | |      |
+   | |      expected one of `extern` or `fn`
+   | help: `async` must come before `unsafe`: `async unsafe`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/parser/issue-87217-keyword-order/const-async-const.rs b/src/test/ui/parser/issue-87217-keyword-order/const-async-const.rs
@@ -0,0 +1,11 @@
+// edition:2018
+
+// Test that even when `const` is already present, the proposed fix is `const const async`,
+// like for `pub pub`.
+
+const async const fn test() {}
+//~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
+//~| NOTE expected one of `extern`, `fn`, or `unsafe`
+//~| HELP `const` must come before `async`
+//~| SUGGESTION const async
+//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
diff --git a/src/test/ui/parser/issue-87217-keyword-order/const-async-const.stderr b/src/test/ui/parser/issue-87217-keyword-order/const-async-const.stderr
@@ -0,0 +1,13 @@
+error: expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
+  --> $DIR/const-async-const.rs:6:13
+   |
+LL | const async const fn test() {}
+   |       ------^^^^^
+   |       |     |
+   |       |     expected one of `extern`, `fn`, or `unsafe`
+   |       help: `const` must come before `async`: `const async`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.rs b/src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.rs
@@ -0,0 +1,14 @@
+// edition:2018
+
+// There is an order to respect for keywords before a function:
+// `<visibility>, const, async, unsafe, extern, "<ABI>"`
+//
+// This test ensures the compiler is helpful about them being misplaced.
+// Visibilities are tested elsewhere.
+
+async unsafe const fn test() {}
+//~^ ERROR expected one of `extern` or `fn`, found keyword `const`
+//~| NOTE expected one of `extern` or `fn`
+//~| HELP `const` must come before `async unsafe`
+//~| SUGGESTION const async unsafe
+//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
diff --git a/src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.stderr b/src/test/ui/parser/issue-87217-keyword-order/several-kw-jump.stderr
@@ -0,0 +1,13 @@
+error: expected one of `extern` or `fn`, found keyword `const`
+  --> $DIR/several-kw-jump.rs:9:14
+   |
+LL | async unsafe const fn test() {}
+   | -------------^^^^^
+   | |            |
+   | |            expected one of `extern` or `fn`
+   | help: `const` must come before `async unsafe`: `const async unsafe`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/parser/issue-87217-keyword-order/wrong-async.rs b/src/test/ui/parser/issue-87217-keyword-order/wrong-async.rs
@@ -0,0 +1,14 @@
+// edition:2018
+
+// There is an order to respect for keywords before a function:
+// `<visibility>, const, async, unsafe, extern, "<ABI>"`
+//
+// This test ensures the compiler is helpful about them being misplaced.
+// Visibilities are tested elsewhere.
+
+unsafe async fn test() {}
+//~^ ERROR expected one of `extern` or `fn`, found keyword `async`
+//~| NOTE expected one of `extern` or `fn`
+//~| HELP `async` must come before `unsafe`
+//~| SUGGESTION async unsafe
+//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
diff --git a/src/test/ui/parser/issue-87217-keyword-order/wrong-async.stderr b/src/test/ui/parser/issue-87217-keyword-order/wrong-async.stderr
@@ -0,0 +1,13 @@
+error: expected one of `extern` or `fn`, found keyword `async`
+  --> $DIR/wrong-async.rs:9:8
+   |
+LL | unsafe async fn test() {}
+   | -------^^^^^
+   | |      |
+   | |      expected one of `extern` or `fn`
+   | help: `async` must come before `unsafe`: `async unsafe`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/parser/issue-87217-keyword-order/wrong-const.rs b/src/test/ui/parser/issue-87217-keyword-order/wrong-const.rs
@@ -0,0 +1,14 @@
+// edition:2018
+
+// There is an order to respect for keywords before a function:
+// `<visibility>, const, async, unsafe, extern, "<ABI>"`
+//
+// This test ensures the compiler is helpful about them being misplaced.
+// Visibilities are tested elsewhere.
+
+unsafe const fn test() {}
+//~^ ERROR expected one of `extern` or `fn`, found keyword `const`
+//~| NOTE expected one of `extern` or `fn`
+//~| HELP `const` must come before `unsafe`
+//~| SUGGESTION const unsafe
+//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
diff --git a/src/test/ui/parser/issue-87217-keyword-order/wrong-const.stderr b/src/test/ui/parser/issue-87217-keyword-order/wrong-const.stderr
@@ -0,0 +1,13 @@
+error: expected one of `extern` or `fn`, found keyword `const`
+  --> $DIR/wrong-const.rs:9:8
+   |
+LL | unsafe const fn test() {}
+   | -------^^^^^
+   | |      |
+   | |      expected one of `extern` or `fn`
+   | help: `const` must come before `unsafe`: `const unsafe`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.rs b/src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.rs
@@ -0,0 +1,14 @@
+// edition:2018
+
+// There is an order to respect for keywords before a function:
+// `<visibility>, const, async, unsafe, extern, "<ABI>"`
+//
+// This test ensures the compiler is helpful about them being misplaced.
+// Visibilities are tested elsewhere.
+
+extern unsafe fn test() {}
+//~^ ERROR expected `fn`, found keyword `unsafe`
+//~| NOTE expected `fn`
+//~| HELP `unsafe` must come before `extern`
+//~| SUGGESTION unsafe extern
+//~| NOTE keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
diff --git a/src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.stderr b/src/test/ui/parser/issue-87217-keyword-order/wrong-unsafe.stderr
@@ -0,0 +1,13 @@
+error: expected `fn`, found keyword `unsafe`
+  --> $DIR/wrong-unsafe.rs:9:8
+   |
+LL | extern unsafe fn test() {}
+   | -------^^^^^^
+   | |      |
+   | |      expected `fn`
+   | help: `unsafe` must come before `extern`: `unsafe extern`
+   |
+   = note: keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`
+
+error: aborting due to previous error
+
