Factor some code out of `AstValidator::visit_items`.

Currently it uses `walk_item` on some item kinds. For other item kinds
it visits the fields individually. For the latter group, this commit
adds `visit_attrs_vis` and `visit_attrs_vis_ident` which bundle up
visits to the fields that don't need special handling. This makes it
clearer that they haven't been forgotten about.

Also, it's better to do the attribute visits at the start because
attributes precede the items in the source code. Because of this, a
couple of tests have their output improved: errors appear in an order
that matches the source code order.

3 files changed, 52 insertions, 51 deletions

diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
index 1390895daac..dc617965f9f 100644
--- a/compiler/rustc_ast_passes/src/ast_validation.rs
+++ b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -727,6 +727,19 @@ impl<'a> AstValidator<'a> {
             )
         }
     }
+
+    // Used within `visit_item` for item kinds where we don't call `visit::walk_item`.
+    fn visit_attrs_vis(&mut self, attrs: &'a AttrVec, vis: &'a Visibility) {
+        walk_list!(self, visit_attribute, attrs);
+        self.visit_vis(vis);
+    }
+
+    // Used within `visit_item` for item kinds where we don't call `visit::walk_item`.
+    fn visit_attrs_vis_ident(&mut self, attrs: &'a AttrVec, vis: &'a Visibility, ident: &'a Ident) {
+        walk_list!(self, visit_attribute, attrs);
+        self.visit_vis(vis);
+        self.visit_ident(ident);
+    }
 }
 
 /// Checks that generic parameters are in the correct order,
@@ -834,6 +847,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 self_ty,
                 items,
             }) => {
+                self.visit_attrs_vis(&item.attrs, &item.vis);
                 self.with_in_trait_impl(Some((*constness, *polarity, t)), |this| {
                     this.visibility_not_permitted(
                         &item.vis,
@@ -853,7 +867,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                         });
                     }
 
-                    this.visit_vis(&item.vis);
                     let disallowed = matches!(constness, Const::No)
                         .then(|| TildeConstReason::TraitImpl { span: item.span });
                     this.with_tilde_const(disallowed, |this| this.visit_generics(generics));
@@ -862,7 +875,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
 
                     walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: true });
                 });
-                walk_list!(self, visit_attribute, &item.attrs);
             }
             ItemKind::Impl(box Impl {
                 safety,
@@ -882,6 +894,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     only_trait,
                 };
 
+                self.visit_attrs_vis(&item.attrs, &item.vis);
                 self.with_in_trait_impl(None, |this| {
                     this.visibility_not_permitted(
                         &item.vis,
@@ -905,7 +918,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                         this.dcx().emit_err(error(span, "`const`", true));
                     }
 
-                    this.visit_vis(&item.vis);
                     this.with_tilde_const(
                         Some(TildeConstReason::Impl { span: item.span }),
                         |this| this.visit_generics(generics),
@@ -913,7 +925,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     this.visit_ty(self_ty);
                     walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: false });
                 });
-                walk_list!(self, visit_attribute, &item.attrs);
             }
             ItemKind::Fn(
                 func @ box Fn {
@@ -926,6 +937,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     define_opaque: _,
                 },
             ) => {
+                self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
                 self.check_defaultness(item.span, *defaultness);
 
                 let is_intrinsic =
@@ -953,11 +965,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     });
                 }
 
-                self.visit_vis(&item.vis);
-                self.visit_ident(ident);
                 let kind = FnKind::Fn(FnCtxt::Free, &item.vis, &*func);
                 self.visit_fn(kind, item.span, item.id);
-                walk_list!(self, visit_attribute, &item.attrs);
             }
             ItemKind::ForeignMod(ForeignMod { extern_span, abi, safety, .. }) => {
                 self.with_in_extern_mod(*safety, |this| {
@@ -1005,6 +1014,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 visit::walk_item(self, item)
             }
             ItemKind::Trait(box Trait { is_auto, generics, ident, bounds, items, .. }) => {
+                self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
                 let is_const_trait =
                     attr::find_by_name(&item.attrs, sym::const_trait).map(|attr| attr.span);
                 self.with_in_trait(item.span, is_const_trait, |this| {
@@ -1018,8 +1028,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
 
                     // Equivalent of `visit::walk_item` for `ItemKind::Trait` that inserts a bound
                     // context for the supertraits.
-                    this.visit_vis(&item.vis);
-                    this.visit_ident(ident);
                     let disallowed = is_const_trait
                         .is_none()
                         .then(|| TildeConstReason::Trait { span: item.span });
@@ -1029,7 +1037,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     });
                     walk_list!(this, visit_assoc_item, items, AssocCtxt::Trait);
                 });
-                walk_list!(self, visit_attribute, &item.attrs);
             }
             ItemKind::Mod(safety, ident, mod_kind) => {
                 if let &Safety::Unsafe(span) = safety {
@@ -1045,12 +1052,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             }
             ItemKind::Struct(ident, vdata, generics) => match vdata {
                 VariantData::Struct { fields, .. } => {
-                    self.visit_vis(&item.vis);
-                    self.visit_ident(ident);
+                    self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
                     self.visit_generics(generics);
                     // Permit `Anon{Struct,Union}` as field type.
                     walk_list!(self, visit_struct_field_def, fields);
-                    walk_list!(self, visit_attribute, &item.attrs);
                 }
                 _ => visit::walk_item(self, item),
             },
@@ -1060,12 +1065,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 }
                 match vdata {
                     VariantData::Struct { fields, .. } => {
-                        self.visit_vis(&item.vis);
-                        self.visit_ident(ident);
+                        self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
                         self.visit_generics(generics);
                         // Permit `Anon{Struct,Union}` as field type.
                         walk_list!(self, visit_struct_field_def, fields);
-                        walk_list!(self, visit_attribute, &item.attrs);
                     }
                     _ => visit::walk_item(self, item),
                 }
@@ -1484,10 +1487,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     || ctxt == AssocCtxt::Trait
                     || matches!(func.sig.header.constness, Const::Yes(_)) =>
             {
-                self.visit_vis(&item.vis);
-                self.visit_ident(&func.ident);
+                self.visit_attrs_vis_ident(&item.attrs, &item.vis, &func.ident);
                 let kind = FnKind::Fn(FnCtxt::Assoc(ctxt), &item.vis, &*func);
-                walk_list!(self, visit_attribute, &item.attrs);
                 self.visit_fn(kind, item.span, item.id);
             }
             AssocItemKind::Type(_) => {
diff --git a/tests/ui/coverage-attr/name-value.stderr b/tests/ui/coverage-attr/name-value.stderr
index 31a635b57e5..f24db78415e 100644
--- a/tests/ui/coverage-attr/name-value.stderr
+++ b/tests/ui/coverage-attr/name-value.stderr
@@ -44,6 +44,21 @@ LL + #[coverage(on)]
    |
 
 error: malformed `coverage` attribute input
+  --> $DIR/name-value.rs:26:1
+   |
+LL | #[coverage = "off"]
+   | ^^^^^^^^^^^^^^^^^^^
+   |
+help: the following are the possible correct uses
+   |
+LL - #[coverage = "off"]
+LL + #[coverage(off)]
+   |
+LL - #[coverage = "off"]
+LL + #[coverage(on)]
+   |
+
+error: malformed `coverage` attribute input
   --> $DIR/name-value.rs:29:5
    |
 LL |     #[coverage = "off"]
@@ -59,7 +74,7 @@ LL +     #[coverage(on)]
    |
 
 error: malformed `coverage` attribute input
-  --> $DIR/name-value.rs:26:1
+  --> $DIR/name-value.rs:35:1
    |
 LL | #[coverage = "off"]
    | ^^^^^^^^^^^^^^^^^^^
@@ -104,7 +119,7 @@ LL +     #[coverage(on)]
    |
 
 error: malformed `coverage` attribute input
-  --> $DIR/name-value.rs:35:1
+  --> $DIR/name-value.rs:50:1
    |
 LL | #[coverage = "off"]
    | ^^^^^^^^^^^^^^^^^^^
@@ -149,21 +164,6 @@ LL +     #[coverage(on)]
    |
 
 error: malformed `coverage` attribute input
-  --> $DIR/name-value.rs:50:1
-   |
-LL | #[coverage = "off"]
-   | ^^^^^^^^^^^^^^^^^^^
-   |
-help: the following are the possible correct uses
-   |
-LL - #[coverage = "off"]
-LL + #[coverage(off)]
-   |
-LL - #[coverage = "off"]
-LL + #[coverage(on)]
-   |
-
-error: malformed `coverage` attribute input
   --> $DIR/name-value.rs:64:1
    |
 LL | #[coverage = "off"]
diff --git a/tests/ui/coverage-attr/word-only.stderr b/tests/ui/coverage-attr/word-only.stderr
index 612301885dc..2773db9c857 100644
--- a/tests/ui/coverage-attr/word-only.stderr
+++ b/tests/ui/coverage-attr/word-only.stderr
@@ -38,6 +38,19 @@ LL | #[coverage(on)]
    |           ++++
 
 error: malformed `coverage` attribute input
+  --> $DIR/word-only.rs:26:1
+   |
+LL | #[coverage]
+   | ^^^^^^^^^^^
+   |
+help: the following are the possible correct uses
+   |
+LL | #[coverage(off)]
+   |           +++++
+LL | #[coverage(on)]
+   |           ++++
+
+error: malformed `coverage` attribute input
   --> $DIR/word-only.rs:29:5
    |
 LL |     #[coverage]
@@ -51,7 +64,7 @@ LL |     #[coverage(on)]
    |               ++++
 
 error: malformed `coverage` attribute input
-  --> $DIR/word-only.rs:26:1
+  --> $DIR/word-only.rs:35:1
    |
 LL | #[coverage]
    | ^^^^^^^^^^^
@@ -90,7 +103,7 @@ LL |     #[coverage(on)]
    |               ++++
 
 error: malformed `coverage` attribute input
-  --> $DIR/word-only.rs:35:1
+  --> $DIR/word-only.rs:50:1
    |
 LL | #[coverage]
    | ^^^^^^^^^^^
@@ -129,19 +142,6 @@ LL |     #[coverage(on)]
    |               ++++
 
 error: malformed `coverage` attribute input
-  --> $DIR/word-only.rs:50:1
-   |
-LL | #[coverage]
-   | ^^^^^^^^^^^
-   |
-help: the following are the possible correct uses
-   |
-LL | #[coverage(off)]
-   |           +++++
-LL | #[coverage(on)]
-   |           ++++
-
-error: malformed `coverage` attribute input
   --> $DIR/word-only.rs:64:1
    |
 LL | #[coverage]