Detect pub fn attr wrong order like async pub

Redirects `const? async? unsafe? pub` to `pub const? async? unsafe?`.

Fix #76437

Author: pickfire

compiler/rustc_parse/src/parser/item.rs

@@ -1639,7 +1639,8 @@ impl<'a> Parser<'a> {
     pub(super) fn check_fn_front_matter(&mut self) -> bool {
         // We use an over-approximation here.
         // `const const`, `fn const` won't parse, but we're not stepping over other syntax either.
-        const QUALS: [Symbol; 4] = [kw::Const, kw::Async, kw::Unsafe, kw::Extern];
+        // `pub` is added in case users got confused with the ordering like `async pub fn`.
+        const QUALS: [Symbol; 5] = [kw::Pub, kw::Const, kw::Async, kw::Unsafe, kw::Extern];
         self.check_keyword(kw::Fn) // Definitely an `fn`.
             // `$qual fn` or `$qual $qual`:
             || QUALS.iter().any(|&kw| self.check_keyword(kw))
@@ -1668,6 +1669,7 @@ impl<'a> Parser<'a> {
     /// FnFrontMatter = FnQual "fn" ;
     /// ```
     pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
+        let sp_start = self.token.span;
         let constness = self.parse_constness();
         let asyncness = self.parse_asyncness();
         let unsafety = self.parse_unsafety();
@@ -1681,8 +1683,27 @@ impl<'a> Parser<'a> {
             // It is possible for `expect_one_of` to recover given the contents of
             // `self.expected_tokens`, therefore, do not use `self.unexpected()` which doesn't
             // account for this.
-            if !self.expect_one_of(&[], &[])? {
-                unreachable!()
+            match self.expect_one_of(&[], &[]) {
+                Ok(true) => {}
+                Ok(false) => unreachable!(),
+                Err(mut err) => {
+                    // Recover incorrect visibility order such as `async pub`.
+                    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 = self.parse_visibility(FollowedByType::No)?;
+                            let vs = pprust::vis_to_string(&vis);
+                            let vs = vs.trim_end();
+                            err.span_suggestion(
+                                sp_start.to(self.prev_token.span),
+                                &format!("visibility `{}` must come before `{}`", vs, snippet),
+                                format!("{} {}", vs, snippet),
+                                Applicability::MachineApplicable,
+                            );
+                        }
+                    }
+                    return Err(err);
+                }
             }
         }
 

src/test/ui/parser/default.rs

@@ -1,3 +1,4 @@
+// ignore-tidy-linelength
 // Test successful and unsuccessful parsing of the `default` contextual keyword
 
 #![feature(specialization)]
@@ -21,8 +22,7 @@ impl Foo for u16 {
 
 impl Foo for u32 { //~ ERROR not all trait items implemented, missing: `foo`
     default pub fn foo<T: Default>() -> T { T::default() }
-    //~^ ERROR `default` is not followed by an item
-    //~| ERROR non-item in item list
+    //~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub`
 }
 
 fn main() {}

src/test/ui/parser/default.stderr

@@ -1,30 +1,25 @@
-error: `default` is not followed by an item
-  --> $DIR/default.rs:23:5
-   |
-LL |     default pub fn foo<T: Default>() -> T { T::default() }
-   |     ^^^^^^^ the `default` qualifier
-   |
-   = note: only `fn`, `const`, `type`, or `impl` items may be prefixed by `default`
-
-error: non-item in item list
-  --> $DIR/default.rs:23:13
+error: expected one of `async`, `const`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub`
+  --> $DIR/default.rs:24:13
    |
 LL | impl Foo for u32 {
-   |                  - item list starts here
+   |                  - while parsing this item list starting here
 LL |     default pub fn foo<T: Default>() -> T { T::default() }
-   |             ^^^ non-item starts here
-...
+   |             ^^^
+   |             |
+   |             expected one of 7 possible tokens
+   |             help: visibility `pub` must come before `default pub`: `pub default pub`
+LL |
 LL | }
-   | - item list ends here
+   | - the item list ends here
 
 error[E0449]: unnecessary visibility qualifier
-  --> $DIR/default.rs:17:5
+  --> $DIR/default.rs:18:5
    |
 LL |     pub default fn foo<T: Default>() -> T {
    |     ^^^ `pub` not permitted here because it's implied
 
 warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes
-  --> $DIR/default.rs:3:12
+  --> $DIR/default.rs:4:12
    |
 LL | #![feature(specialization)]
    |            ^^^^^^^^^^^^^^
@@ -34,15 +29,15 @@ LL | #![feature(specialization)]
    = help: consider using `min_specialization` instead, which is more stable and complete
 
 error[E0046]: not all trait items implemented, missing: `foo`
-  --> $DIR/default.rs:22:1
+  --> $DIR/default.rs:23:1
    |
 LL |     fn foo<T: Default>() -> T;
    |     -------------------------- `foo` from trait
 ...
 LL | impl Foo for u32 {
    | ^^^^^^^^^^^^^^^^ missing `foo` in implementation
 
-error: aborting due to 4 previous errors; 1 warning emitted
+error: aborting due to 3 previous errors; 1 warning emitted
 
 Some errors have detailed explanations: E0046, E0449.
 For more information about an error, try `rustc --explain E0046`.

src/test/ui/parser/duplicate-visibility.rs

@@ -1,7 +1,8 @@
+// ignore-tidy-linelength
+
 fn main() {}
 
 extern "C" {
     pub pub fn foo();
-    //~^ ERROR visibility `pub` is not followed by an item
-    //~| ERROR non-item in item list
+    //~^ ERROR expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub`
 }

src/test/ui/parser/duplicate-visibility.stderr

@@ -1,21 +1,16 @@
-error: visibility `pub` is not followed by an item
-  --> $DIR/duplicate-visibility.rs:4:5
-   |
-LL |     pub pub fn foo();
-   |     ^^^ the visibility
-   |
-   = help: you likely meant to define an item, e.g., `pub fn foo() {}`
-
-error: non-item in item list
-  --> $DIR/duplicate-visibility.rs:4:9
+error: expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub`
+  --> $DIR/duplicate-visibility.rs:6:9
    |
 LL | extern "C" {
-   |            - item list starts here
+   |            - while parsing this item list starting here
 LL |     pub pub fn foo();
-   |         ^^^ non-item starts here
-...
+   |         ^^^
+   |         |
+   |         expected one of 9 possible tokens
+   |         help: visibility `pub` must come before `pub pub`: `pub pub pub`
+LL |
 LL | }
-   | - item list ends here
+   | - the item list ends here
 
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 

src/test/ui/parser/issue-63116.stderr

@@ -1,4 +1,4 @@
-error: this file contains an unclosed delimiter
+=rror: this file contains an unclosed delimiter
   --> $DIR/issue-63116.rs:3:18
    |
 LL | impl W <s(f;Y(;]
@@ -12,7 +12,7 @@ error: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `;`
 LL | impl W <s(f;Y(;]
    |            ^ expected one of 7 possible tokens
 
-error: expected one of `!`, `&&`, `&`, `(`, `)`, `*`, `+`, `,`, `->`, `...`, `::`, `:`, `<`, `=`, `>`, `?`, `[`, `_`, `async`, `const`, `dyn`, `extern`, `fn`, `for`, `impl`, `unsafe`, lifetime, or path, found `;`
+error: expected one of `!`, `&&`, `&`, `(`, `)`, `*`, `+`, `,`, `->`, `...`, `::`, `<`, `>`, `?`, `[`, `_`, `async`, `const`, `dyn`, `extern`, `fn`, `for`, `impl`, `pub`, `unsafe`, lifetime, or path, found `;`
   --> $DIR/issue-63116.rs:3:15
    |
 LL | impl W <s(f;Y(;]

src/test/ui/parser/issue-76437-async.rs

@@ -0,0 +1,6 @@
+// edition:2018
+
+mod t {
+    async pub fn t() {}
+    //~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `pub`
+}

src/test/ui/parser/issue-76437-async.stderr

@@ -0,0 +1,11 @@
+error: expected one of `extern`, `fn`, or `unsafe`, found keyword `pub`
+  --> $DIR/issue-76437-async.rs:4:11
+   |
+LL |     async pub fn t() {}
+   |     ------^^^
+   |     |     |
+   |     |     expected one of `extern`, `fn`, or `unsafe`
+   |     help: visibility `pub` must come before `async`: `pub async`
+
+error: aborting due to previous error
+

src/test/ui/parser/issue-76437-const-async-unsafe.rs

@@ -0,0 +1,6 @@
+// edition:2018
+
+mod t {
+    const async unsafe pub fn t() {}
+    //~^ ERROR expected one of `extern` or `fn`, found keyword `pub`
+}

src/test/ui/parser/issue-76437-const-async-unsafe.stderr

@@ -0,0 +1,11 @@
+error: expected one of `extern` or `fn`, found keyword `pub`
+  --> $DIR/issue-76437-const-async-unsafe.rs:4:24
+   |
+LL |     const async unsafe pub fn t() {}
+   |     -------------------^^^
+   |     |                  |
+   |     |                  expected one of `extern` or `fn`
+   |     help: visibility `pub` must come before `const async unsafe`: `pub const async unsafe`
+
+error: aborting due to previous error
+

src/test/ui/parser/issue-76437-const-async.rs

@@ -0,0 +1,6 @@
+// edition:2018
+
+mod t {
+    const async pub fn t() {}
+    //~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `pub`
+}

src/test/ui/parser/issue-76437-const-async.stderr

@@ -0,0 +1,11 @@
+error: expected one of `extern`, `fn`, or `unsafe`, found keyword `pub`
+  --> $DIR/issue-76437-const-async.rs:4:17
+   |
+LL |     const async pub fn t() {}
+   |     ------------^^^
+   |     |           |
+   |     |           expected one of `extern`, `fn`, or `unsafe`
+   |     help: visibility `pub` must come before `const async`: `pub const async`
+
+error: aborting due to previous error
+

src/test/ui/parser/issue-76437-const.rs

@@ -0,0 +1,6 @@
+// edition:2018
+
+mod t {
+    const pub fn t() {}
+    //~^ ERROR expected one of `async`, `extern`, `fn`, or `unsafe`, found keyword `pub`
+}

src/test/ui/parser/issue-76437-const.stderr

@@ -0,0 +1,11 @@
+error: expected one of `async`, `extern`, `fn`, or `unsafe`, found keyword `pub`
+  --> $DIR/issue-76437-const.rs:4:11
+   |
+LL |     const pub fn t() {}
+   |     ------^^^
+   |     |     |
+   |     |     expected one of `async`, `extern`, `fn`, or `unsafe`
+   |     help: visibility `pub` must come before `const`: `pub const`
+
+error: aborting due to previous error
+

src/test/ui/parser/issue-76437-pub-crate-unsafe.rs

@@ -0,0 +1,6 @@
+// edition:2018
+
+mod t {
+    unsafe pub(crate) fn t() {}
+    //~^ ERROR expected one of `extern` or `fn`, found keyword `pub`
+}

src/test/ui/parser/issue-76437-pub-crate-unsafe.stderr

@@ -0,0 +1,11 @@
+error: expected one of `extern` or `fn`, found keyword `pub`
+  --> $DIR/issue-76437-pub-crate-unsafe.rs:4:12
+   |
+LL |     unsafe pub(crate) fn t() {}
+   |     -------^^^-------
+   |     |      |
+   |     |      expected one of `extern` or `fn`
+   |     help: visibility `pub(crate)` must come before `unsafe`: `pub(crate) unsafe`
+
+error: aborting due to previous error
+

src/test/ui/parser/issue-76437-unsafe.rs

@@ -0,0 +1,6 @@
+// edition:2018
+
+mod t {
+    unsafe pub fn t() {}
+    //~^ ERROR expected one of `extern` or `fn`, found keyword `pub`
+}

src/test/ui/parser/issue-76437-unsafe.stderr

@@ -0,0 +1,11 @@
+error: expected one of `extern` or `fn`, found keyword `pub`
+  --> $DIR/issue-76437-unsafe.rs:4:12
+   |
+LL |     unsafe pub fn t() {}
+   |     -------^^^
+   |     |      |
+   |     |      expected one of `extern` or `fn`
+   |     help: visibility `pub` must come before `unsafe`: `pub unsafe`
+
+error: aborting due to previous error
+