about summary refs log tree commit diff
path: root/compiler/rustc_parse/src/parser/item.rs
diff options
context:
space:
mode:
Diffstat (limited to 'compiler/rustc_parse/src/parser/item.rs')
-rw-r--r--compiler/rustc_parse/src/parser/item.rs343
1 files changed, 274 insertions, 69 deletions
diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs
index d2167c7a5db..06849b31256 100644
--- a/compiler/rustc_parse/src/parser/item.rs
+++ b/compiler/rustc_parse/src/parser/item.rs
@@ -15,6 +15,7 @@ use rustc_ast::{MacArgs, MacCall, MacDelimiter};
 use rustc_ast_pretty::pprust;
 use rustc_errors::{struct_span_err, Applicability, PResult, StashKey};
 use rustc_span::edition::{Edition, LATEST_STABLE_EDITION};
+use rustc_span::lev_distance::lev_distance;
 use rustc_span::source_map::{self, Span};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 
@@ -26,7 +27,7 @@ impl<'a> Parser<'a> {
     /// Parses a source module as a crate. This is the main entry point for the parser.
     pub fn parse_crate_mod(&mut self) -> PResult<'a, ast::Crate> {
         let (attrs, items, span) = self.parse_mod(&token::Eof)?;
-        Ok(ast::Crate { attrs, items, span })
+        Ok(ast::Crate { attrs, items, span, id: DUMMY_NODE_ID, is_placeholder: false })
     }
 
     /// Parses a `mod <foo> { ... }` or `mod <foo>;` item.
@@ -78,16 +79,17 @@ pub(super) type ItemInfo = (Ident, ItemKind);
 
 impl<'a> Parser<'a> {
     pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<P<Item>>> {
-        self.parse_item_(|_| true, force_collect).map(|i| i.map(P))
+        let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
+        self.parse_item_(fn_parse_mode, force_collect).map(|i| i.map(P))
     }
 
     fn parse_item_(
         &mut self,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Item>> {
         let attrs = self.parse_outer_attributes()?;
-        self.parse_item_common(attrs, true, false, req_name, force_collect)
+        self.parse_item_common(attrs, true, false, fn_parse_mode, force_collect)
     }
 
     pub(super) fn parse_item_common(
@@ -95,7 +97,7 @@ impl<'a> Parser<'a> {
         attrs: AttrWrapper,
         mac_allowed: bool,
         attrs_allowed: bool,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Item>> {
         // Don't use `maybe_whole` so that we have precise control
@@ -113,7 +115,8 @@ impl<'a> Parser<'a> {
         let mut unclosed_delims = vec![];
         let item =
             self.collect_tokens_trailing_token(attrs, force_collect, |this: &mut Self, attrs| {
-                let item = this.parse_item_common_(attrs, mac_allowed, attrs_allowed, req_name);
+                let item =
+                    this.parse_item_common_(attrs, mac_allowed, attrs_allowed, fn_parse_mode);
                 unclosed_delims.append(&mut this.unclosed_delims);
                 Ok((item?, TrailingToken::None))
             })?;
@@ -127,12 +130,13 @@ impl<'a> Parser<'a> {
         mut attrs: Vec<Attribute>,
         mac_allowed: bool,
         attrs_allowed: bool,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
     ) -> PResult<'a, Option<Item>> {
         let lo = self.token.span;
         let vis = self.parse_visibility(FollowedByType::No)?;
         let mut def = self.parse_defaultness();
-        let kind = self.parse_item_kind(&mut attrs, mac_allowed, lo, &vis, &mut def, req_name)?;
+        let kind =
+            self.parse_item_kind(&mut attrs, mac_allowed, lo, &vis, &mut def, fn_parse_mode)?;
         if let Some((ident, kind)) = kind {
             self.error_on_unconsumed_default(def, &kind);
             let span = lo.to(self.prev_token.span);
@@ -192,7 +196,7 @@ impl<'a> Parser<'a> {
         lo: Span,
         vis: &Visibility,
         def: &mut Defaultness,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
     ) -> PResult<'a, Option<ItemInfo>> {
         let def_final = def == &Defaultness::Final;
         let mut def = || mem::replace(def, Defaultness::Final);
@@ -219,8 +223,8 @@ impl<'a> Parser<'a> {
             (Ident::empty(), ItemKind::Use(tree))
         } 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::new(FnKind(def(), sig, generics, body))))
+            let (ident, sig, generics, body) = self.parse_fn(attrs, fn_parse_mode, lo, vis)?;
+            (ident, ItemKind::Fn(Box::new(Fn { defaultness: def(), sig, generics, body })))
         } else if self.eat_keyword(kw::Extern) {
             if self.eat_keyword(kw::Crate) {
                 // EXTERN CRATE
@@ -407,10 +411,30 @@ impl<'a> Parser<'a> {
     fn parse_item_macro(&mut self, vis: &Visibility) -> PResult<'a, MacCall> {
         let path = self.parse_path(PathStyle::Mod)?; // `foo::bar`
         self.expect(&token::Not)?; // `!`
-        let args = self.parse_mac_args()?; // `( .. )` or `[ .. ]` (followed by `;`), or `{ .. }`.
-        self.eat_semi_for_macro_if_needed(&args);
-        self.complain_if_pub_macro(vis, false);
-        Ok(MacCall { path, args, prior_type_ascription: self.last_type_ascription })
+        match self.parse_mac_args() {
+            // `( .. )` or `[ .. ]` (followed by `;`), or `{ .. }`.
+            Ok(args) => {
+                self.eat_semi_for_macro_if_needed(&args);
+                self.complain_if_pub_macro(vis, false);
+                Ok(MacCall { path, args, prior_type_ascription: self.last_type_ascription })
+            }
+
+            Err(mut err) => {
+                // Maybe the user misspelled `macro_rules` (issue #91227)
+                if self.token.is_ident()
+                    && path.segments.len() == 1
+                    && lev_distance("macro_rules", &path.segments[0].ident.to_string(), 3).is_some()
+                {
+                    err.span_suggestion(
+                        path.span,
+                        "perhaps you meant to define a macro",
+                        "macro_rules".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                }
+                Err(err)
+            }
+        }
     }
 
     /// Recover if we parsed attributes and expected an item but there was none.
@@ -514,7 +538,7 @@ impl<'a> Parser<'a> {
                 tokens: None,
             })
         } else {
-            self.parse_ty()?
+            self.parse_ty_with_generics_recovery(&generics)?
         };
 
         // If `for` is missing we try to recover.
@@ -560,7 +584,7 @@ impl<'a> Parser<'a> {
                 };
                 let trait_ref = TraitRef { path, ref_id: ty_first.id };
 
-                ItemKind::Impl(Box::new(ImplKind {
+                ItemKind::Impl(Box::new(Impl {
                     unsafety,
                     polarity,
                     defaultness,
@@ -573,7 +597,7 @@ impl<'a> Parser<'a> {
             }
             None => {
                 // impl Type
-                ItemKind::Impl(Box::new(ImplKind {
+                ItemKind::Impl(Box::new(Impl {
                     unsafety,
                     polarity,
                     defaultness,
@@ -682,7 +706,7 @@ impl<'a> Parser<'a> {
 
         self.expect_keyword(kw::Trait)?;
         let ident = self.parse_ident()?;
-        let mut tps = self.parse_generics()?;
+        let mut generics = self.parse_generics()?;
 
         // Parse optional colon and supertrait bounds.
         let had_colon = self.eat(&token::Colon);
@@ -702,7 +726,7 @@ impl<'a> Parser<'a> {
             }
 
             let bounds = self.parse_generic_bounds(None)?;
-            tps.where_clause = self.parse_where_clause()?;
+            generics.where_clause = self.parse_where_clause()?;
             self.expect_semi()?;
 
             let whole_span = lo.to(self.prev_token.span);
@@ -717,12 +741,15 @@ impl<'a> Parser<'a> {
 
             self.sess.gated_spans.gate(sym::trait_alias, whole_span);
 
-            Ok((ident, ItemKind::TraitAlias(tps, bounds)))
+            Ok((ident, ItemKind::TraitAlias(generics, bounds)))
         } else {
             // It's a normal trait.
-            tps.where_clause = self.parse_where_clause()?;
+            generics.where_clause = self.parse_where_clause()?;
             let items = self.parse_item_list(attrs, |p| p.parse_trait_item(ForceCollect::No))?;
-            Ok((ident, ItemKind::Trait(Box::new(TraitKind(is_auto, unsafety, tps, bounds, items)))))
+            Ok((
+                ident,
+                ItemKind::Trait(Box::new(Trait { is_auto, unsafety, generics, bounds, items })),
+            ))
         }
     }
 
@@ -730,23 +757,26 @@ impl<'a> Parser<'a> {
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<AssocItem>>>> {
-        self.parse_assoc_item(|_| true, force_collect)
+        let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
+        self.parse_assoc_item(fn_parse_mode, force_collect)
     }
 
     pub fn parse_trait_item(
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<AssocItem>>>> {
-        self.parse_assoc_item(|edition| edition >= Edition::Edition2018, force_collect)
+        let fn_parse_mode =
+            FnParseMode { req_name: |edition| edition >= Edition::Edition2018, req_body: false };
+        self.parse_assoc_item(fn_parse_mode, force_collect)
     }
 
     /// Parses associated items.
     fn parse_assoc_item(
         &mut self,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<AssocItem>>>> {
-        Ok(self.parse_item_(req_name, force_collect)?.map(
+        Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
             |Item { attrs, id, span, vis, ident, kind, tokens }| {
                 let kind = match AssocItemKind::try_from(kind) {
                     Ok(kind) => kind,
@@ -764,24 +794,77 @@ impl<'a> Parser<'a> {
         ))
     }
 
+    /// Emits an error that the where clause at the end of a type alias is not
+    /// allowed and suggests moving it.
+    fn error_ty_alias_where(
+        &self,
+        before_where_clause_present: bool,
+        before_where_clause_span: Span,
+        after_predicates: &[WherePredicate],
+        after_where_clause_span: Span,
+    ) {
+        let mut err =
+            self.struct_span_err(after_where_clause_span, "where clause not allowed here");
+        if !after_predicates.is_empty() {
+            let mut state = crate::pprust::State::new();
+            if !before_where_clause_present {
+                state.space();
+                state.word_space("where");
+            } else {
+                state.word_space(",");
+            }
+            let mut first = true;
+            for p in after_predicates.iter() {
+                if !first {
+                    state.word_space(",");
+                }
+                first = false;
+                state.print_where_predicate(p);
+            }
+            let suggestion = state.s.eof();
+            err.span_suggestion(
+                before_where_clause_span.shrink_to_hi(),
+                "move it here",
+                suggestion,
+                Applicability::MachineApplicable,
+            );
+        }
+        err.emit()
+    }
+
     /// Parses a `type` alias with the following grammar:
     /// ```
     /// TypeAlias = "type" Ident Generics {":" GenericBounds}? {"=" Ty}? ";" ;
     /// ```
     /// The `"type"` has already been eaten.
-    fn parse_type_alias(&mut self, def: Defaultness) -> PResult<'a, ItemInfo> {
+    fn parse_type_alias(&mut self, defaultness: Defaultness) -> PResult<'a, ItemInfo> {
         let ident = self.parse_ident()?;
         let mut generics = self.parse_generics()?;
 
         // Parse optional colon and param bounds.
         let bounds =
             if self.eat(&token::Colon) { self.parse_generic_bounds(None)? } else { Vec::new() };
+
         generics.where_clause = self.parse_where_clause()?;
 
-        let default = if self.eat(&token::Eq) { Some(self.parse_ty()?) } else { None };
+        let ty = if self.eat(&token::Eq) { Some(self.parse_ty()?) } else { None };
+
+        if self.token.is_keyword(kw::Where) {
+            let after_where_clause = self.parse_where_clause()?;
+
+            self.error_ty_alias_where(
+                generics.where_clause.has_where_token,
+                generics.where_clause.span,
+                &after_where_clause.predicates,
+                after_where_clause.span,
+            );
+
+            generics.where_clause.predicates.extend(after_where_clause.predicates.into_iter());
+        }
+
         self.expect_semi()?;
 
-        Ok((ident, ItemKind::TyAlias(Box::new(TyAliasKind(def, generics, bounds, default)))))
+        Ok((ident, ItemKind::TyAlias(Box::new(TyAlias { defaultness, generics, bounds, ty }))))
     }
 
     /// Parses a `UseTree`.
@@ -941,7 +1024,8 @@ impl<'a> Parser<'a> {
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<ForeignItem>>>> {
-        Ok(self.parse_item_(|_| true, force_collect)?.map(
+        let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: false };
+        Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
             |Item { attrs, id, span, vis, ident, kind, tokens }| {
                 let kind = match ForeignItemKind::try_from(kind) {
                     Ok(kind) => kind,
@@ -1039,9 +1123,7 @@ impl<'a> Parser<'a> {
         };
 
         match impl_info.1 {
-            ItemKind::Impl(box ImplKind {
-                of_trait: Some(ref trai), ref mut constness, ..
-            }) => {
+            ItemKind::Impl(box Impl { of_trait: Some(ref trai), ref mut constness, .. }) => {
                 *constness = Const::Yes(const_span);
 
                 let before_trait = trai.path.span.shrink_to_lo();
@@ -1482,8 +1564,16 @@ impl<'a> Parser<'a> {
         let (ident, is_raw) = self.ident_or_err()?;
         if !is_raw && ident.is_reserved() {
             let err = if self.check_fn_front_matter(false) {
+                let inherited_vis = Visibility {
+                    span: rustc_span::DUMMY_SP,
+                    kind: VisibilityKind::Inherited,
+                    tokens: None,
+                };
                 // We use `parse_fn` to get a span for the function
-                if let Err(mut db) = self.parse_fn(&mut Vec::new(), |_| true, lo) {
+                let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
+                if let Err(mut db) =
+                    self.parse_fn(&mut Vec::new(), fn_parse_mode, lo, &inherited_vis)
+                {
                     db.delay_as_bug();
                 }
                 let mut err = self.struct_span_err(
@@ -1697,25 +1787,83 @@ impl<'a> Parser<'a> {
 /// The parsing configuration used to parse a parameter list (see `parse_fn_params`).
 ///
 /// The function decides if, per-parameter `p`, `p` must have a pattern or just a type.
+///
+/// This function pointer accepts an edition, because in edition 2015, trait declarations
+/// were allowed to omit parameter names. In 2018, they became required.
 type ReqName = fn(Edition) -> bool;
 
+/// Parsing configuration for functions.
+///
+/// The syntax of function items is slightly different within trait definitions,
+/// impl blocks, and modules. It is still parsed using the same code, just with
+/// different flags set, so that even when the input is wrong and produces a parse
+/// error, it still gets into the AST and the rest of the parser and
+/// type checker can run.
+#[derive(Clone, Copy)]
+pub(crate) struct FnParseMode {
+    /// A function pointer that decides if, per-parameter `p`, `p` must have a
+    /// pattern or just a type. This field affects parsing of the parameters list.
+    ///
+    /// ```text
+    /// fn foo(alef: A) -> X { X::new() }
+    ///        -----^^ affects parsing this part of the function signature
+    ///        |
+    ///        if req_name returns false, then this name is optional
+    ///
+    /// fn bar(A) -> X;
+    ///        ^
+    ///        |
+    ///        if req_name returns true, this is an error
+    /// ```
+    ///
+    /// Calling this function pointer should only return false if:
+    ///
+    ///   * The item is being parsed inside of a trait definition.
+    ///     Within an impl block or a module, it should always evaluate
+    ///     to true.
+    ///   * The span is from Edition 2015. In particular, you can get a
+    ///     2015 span inside a 2021 crate using macros.
+    pub req_name: ReqName,
+    /// If this flag is set to `true`, then plain, semicolon-terminated function
+    /// prototypes are not allowed here.
+    ///
+    /// ```text
+    /// fn foo(alef: A) -> X { X::new() }
+    ///                      ^^^^^^^^^^^^
+    ///                      |
+    ///                      this is always allowed
+    ///
+    /// fn bar(alef: A, bet: B) -> X;
+    ///                             ^
+    ///                             |
+    ///                             if req_body is set to true, this is an error
+    /// ```
+    ///
+    /// This field should only be set to false if the item is inside of a trait
+    /// definition or extern block. Within an impl block or a module, it should
+    /// always be set to true.
+    pub req_body: bool,
+}
+
 /// Parsing of functions and methods.
 impl<'a> Parser<'a> {
     /// Parse a function starting from the front matter (`const ...`) to the body `{ ... }` or `;`.
     fn parse_fn(
         &mut self,
         attrs: &mut Vec<Attribute>,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         sig_lo: Span,
+        vis: &Visibility,
     ) -> PResult<'a, (Ident, FnSig, Generics, Option<P<Block>>)> {
-        let header = self.parse_fn_front_matter()?; // `const ... fn`
+        let header = self.parse_fn_front_matter(vis)?; // `const ... fn`
         let ident = self.parse_ident()?; // `foo`
         let mut generics = self.parse_generics()?; // `<'a, T, ...>`
-        let decl = self.parse_fn_decl(req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
+        let decl =
+            self.parse_fn_decl(fn_parse_mode.req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
         generics.where_clause = self.parse_where_clause()?; // `where T: Ord`
 
         let mut sig_hi = self.prev_token.span;
-        let body = self.parse_fn_body(attrs, &ident, &mut sig_hi)?; // `;` or `{ ... }`.
+        let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`.
         let fn_sig_span = sig_lo.to(sig_hi);
         Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
     }
@@ -1728,9 +1876,17 @@ impl<'a> Parser<'a> {
         attrs: &mut Vec<Attribute>,
         ident: &Ident,
         sig_hi: &mut Span,
+        req_body: bool,
     ) -> PResult<'a, Option<P<Block>>> {
-        let (inner_attrs, body) = if self.eat(&token::Semi) {
+        let has_semi = if req_body {
+            self.token.kind == TokenKind::Semi
+        } else {
+            // Only include `;` in list of expected tokens if body is not required
+            self.check(&TokenKind::Semi)
+        };
+        let (inner_attrs, body) = if has_semi {
             // Include the trailing semicolon in the span of the signature
+            self.expect_semi()?;
             *sig_hi = self.prev_token.span;
             (Vec::new(), None)
         } else if self.check(&token::OpenDelim(token::Brace)) || self.token.is_whole_block() {
@@ -1751,9 +1907,12 @@ impl<'a> Parser<'a> {
                 .emit();
             (Vec::new(), Some(self.mk_block_err(span)))
         } else {
-            if let Err(mut err) =
-                self.expected_one_of_not_found(&[], &[token::Semi, token::OpenDelim(token::Brace)])
-            {
+            let expected = if req_body {
+                &[token::OpenDelim(token::Brace)][..]
+            } else {
+                &[token::Semi, token::OpenDelim(token::Brace)]
+            };
+            if let Err(mut err) = self.expected_one_of_not_found(&[], &expected) {
                 if self.token.kind == token::CloseDelim(token::Brace) {
                     // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
                     // the AST for typechecking.
@@ -1805,12 +1964,15 @@ impl<'a> Parser<'a> {
     /// Parses all the "front matter" (or "qualifiers") for a `fn` declaration,
     /// up to and including the `fn` keyword. The formal grammar is:
     ///
-    /// ```
+    /// ```text
     /// Extern = "extern" StringLit? ;
     /// FnQual = "const"? "async"? "unsafe"? Extern? ;
     /// FnFrontMatter = FnQual "fn" ;
     /// ```
-    pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
+    ///
+    /// `vis` represents the visibility that was already parsed, if any. Use
+    /// `Visibility::Inherited` when no visibility is known.
+    pub(super) fn parse_fn_front_matter(&mut self, orig_vis: &Visibility) -> PResult<'a, FnHeader> {
         let sp_start = self.token.span;
         let constness = self.parse_constness();
 
@@ -1836,51 +1998,94 @@ impl<'a> Parser<'a> {
                 Ok(false) => unreachable!(),
                 Err(mut err) => {
                     // Qualifier keywords ordering check
+                    enum WrongKw {
+                        Duplicated(Span),
+                        Misplaced(Span),
+                    }
 
-                    // 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)
+                    // This will allow the machine fix to directly place the keyword in the correct place or to indicate
+                    // that the keyword is already present and the second instance should be removed.
+                    let wrong_kw = if self.check_keyword(kw::Const) {
+                        match constness {
+                            Const::Yes(sp) => Some(WrongKw::Duplicated(sp)),
+                            Const::No => Some(WrongKw::Misplaced(async_start_sp)),
+                        }
                     } else if self.check_keyword(kw::Async) {
-                        Some(unsafe_start_sp)
+                        match asyncness {
+                            Async::Yes { span, .. } => Some(WrongKw::Duplicated(span)),
+                            Async::No => Some(WrongKw::Misplaced(unsafe_start_sp)),
+                        }
                     } else if self.check_keyword(kw::Unsafe) {
-                        Some(ext_start_sp)
+                        match unsafety {
+                            Unsafe::Yes(sp) => Some(WrongKw::Duplicated(sp)),
+                            Unsafe::No => Some(WrongKw::Misplaced(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();
+                    // The keyword is already present, suggest removal of the second instance
+                    if let Some(WrongKw::Duplicated(original_sp)) = wrong_kw {
+                        let original_kw = self
+                            .span_to_snippet(original_sp)
+                            .expect("Span extracted directly from keyword should always work");
+
+                        err.span_suggestion(
+                            self.token.uninterpolated_span(),
+                            &format!("`{}` already used earlier, remove this one", original_kw),
+                            "".to_string(),
+                            Applicability::MachineApplicable,
+                        )
+                        .span_note(original_sp, &format!("`{}` first seen here", original_kw));
+                    }
+                    // The keyword has not been seen yet, suggest correct placement in the function front matter
+                    else if let Some(WrongKw::Misplaced(correct_pos_sp)) = wrong_kw {
+                        let correct_pos_sp = correct_pos_sp.to(self.prev_token.span);
+                        if let Ok(current_qual) = self.span_to_snippet(correct_pos_sp) {
+                            let misplaced_qual_sp = self.token.uninterpolated_span();
+                            let misplaced_qual = self.span_to_snippet(misplaced_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`");
+                                    correct_pos_sp.to(misplaced_qual_sp),
+                                    &format!("`{}` must come before `{}`", misplaced_qual, current_qual),
+                                    format!("{} {}", misplaced_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`.
+                    // Recover incorrect visibility order such as `async 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) {
+                            let current_vis = match self.parse_visibility(FollowedByType::No) {
                                 Ok(v) => v,
                                 Err(mut d) => {
                                     d.cancel();
                                     return Err(err);
                                 }
                             };
-                            let vs = pprust::vis_to_string(&vis);
+                            let vs = pprust::vis_to_string(&current_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,
-                            );
+
+                            // There was no explicit visibility
+                            if matches!(orig_vis.kind, VisibilityKind::Inherited) {
+                                err.span_suggestion(
+                                    sp_start.to(self.prev_token.span),
+                                    &format!("visibility `{}` must come before `{}`", vs, snippet),
+                                    format!("{} {}", vs, snippet),
+                                    Applicability::MachineApplicable,
+                                );
+                            }
+                            // There was an explicit visibility
+                            else {
+                                err.span_suggestion(
+                                    current_vis.span,
+                                    "there is already a visibility modifier, remove one",
+                                    "".to_string(),
+                                    Applicability::MachineApplicable,
+                                )
+                                .span_note(orig_vis.span, "explicit visibility first seen here");
+                            }
                         }
                     }
                     return Err(err);