Auto merge of #76447 - pickfire:async-pub, r=estebank

Detect async visibility wrong order, `async pub`

Partially address #76437.

Author: bors

compiler/rustc_parse/src/parser/item.rs

@@ -204,6 +204,7 @@ impl<'a> Parser<'a> {
         def: &mut Defaultness,
         req_name: ReqName,
     ) -> PResult<'a, Option<ItemInfo>> {
+        let def_final = def == &Defaultness::Final;
         let mut def = || mem::replace(def, Defaultness::Final);
 
         let info = if self.eat_keyword(kw::Use) {
@@ -226,7 +227,7 @@ impl<'a> Parser<'a> {
             }
 
             (Ident::invalid(), ItemKind::Use(tree))
-        } else if self.check_fn_front_matter() {
+        } else if self.check_fn_front_matter(def_final) {
             // FUNCTION ITEM
             let (ident, sig, generics, body) = self.parse_fn(attrs, req_name, lo)?;
             (ident, ItemKind::Fn(box FnKind(def(), sig, generics, body)))
@@ -1634,18 +1635,27 @@ impl<'a> Parser<'a> {
     }
 
     /// Is the current token the start of an `FnHeader` / not a valid parse?
-    pub(super) fn check_fn_front_matter(&mut self) -> bool {
+    ///
+    /// `check_pub` adds additional `pub` to the checks in case users place it
+    /// wrongly, can be used to ensure `pub` never comes after `default`.
+    pub(super) fn check_fn_front_matter(&mut self, check_pub: bool) -> 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`,
+        // only if it wasn't preceeded by `default` as `default pub` is invalid.
+        let quals: &[Symbol] = if check_pub {
+            &[kw::Pub, kw::Const, kw::Async, kw::Unsafe, kw::Extern]
+        } else {
+            &[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))
+            || quals.iter().any(|&kw| self.check_keyword(kw))
                 && self.look_ahead(1, |t| {
                     // `$qual fn`, e.g. `const fn` or `async fn`.
                     t.is_keyword(kw::Fn)
                     // Two qualifiers `$qual $qual` is enough, e.g. `async unsafe`.
-                    || t.is_non_raw_ident_where(|i| QUALS.contains(&i.name)
+                    || t.is_non_raw_ident_where(|i| quals.contains(&i.name)
                         // Rule out 2015 `const async: T = val`.
                         && i.is_reserved()
                         // Rule out unsafe extern block.
@@ -1666,6 +1676,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();
@@ -1679,8 +1690,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);
+                }
             }
         }
 

compiler/rustc_parse/src/parser/ty.rs

@@ -209,15 +209,15 @@ impl<'a> Parser<'a> {
         } else if self.eat_keyword(kw::Underscore) {
             // A type to be inferred `_`
             TyKind::Infer
-        } else if self.check_fn_front_matter() {
+        } else if self.check_fn_front_matter(false) {
             // Function pointer type
             self.parse_ty_bare_fn(lo, Vec::new(), recover_return_sign)?
         } else if self.check_keyword(kw::For) {
             // Function pointer type or bound list (trait object type) starting with a poly-trait.
             //   `for<'lt> [unsafe] [extern "ABI"] fn (&'lt S) -> T`
             //   `for<'lt> Trait1<'lt> + Trait2 + 'a`
             let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
-            if self.check_fn_front_matter() {
+            if self.check_fn_front_matter(false) {
                 self.parse_ty_bare_fn(lo, lifetime_defs, recover_return_sign)?
             } else {
                 let path = self.parse_path(PathStyle::Type)?;

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-76437-async.rs

@@ -0,0 +1,7 @@
+// edition:2018
+
+mod t {
+    async pub fn t() {}
+    //~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `pub`
+    //~| HELP visibility `pub` must come before `async`
+}

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,7 @@
+// edition:2018
+
+mod t {
+    const async unsafe pub fn t() {}
+    //~^ ERROR expected one of `extern` or `fn`, found keyword `pub`
+    //~| HELP visibility `pub` must come before `const async unsafe`
+}

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,7 @@
+// edition:2018
+
+mod t {
+    const async pub fn t() {}
+    //~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `pub`
+    //~| HELP visibility `pub` must come before `const async`
+}

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
+