Address review comments.

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -817,10 +817,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             self.has_proc_macro_decls = true;
         }
 
-        if attr::contains_name(&item.attrs, sym::no_mangle) {
+        if let Some(ident) = item.kind.ident()
-            if let Some(ident) = item.kind.ident() {
+            && attr::contains_name(&item.attrs, sym::no_mangle)
-                self.check_nomangle_item_asciionly(ident, item.span);
+        {
-            }
+            self.check_nomangle_item_asciionly(ident, item.span);
         }
 
         match &item.kind {
@@ -1412,10 +1412,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     }
 
     fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) {
-        if attr::contains_name(&item.attrs, sym::no_mangle) {
+        if let Some(ident) = item.kind.ident()
-            if let Some(ident) = item.kind.ident() {
+            && attr::contains_name(&item.attrs, sym::no_mangle)
-                self.check_nomangle_item_asciionly(ident, item.span);
+        {
-            }
+            self.check_nomangle_item_asciionly(ident, item.span);
         }
 
         if ctxt == AssocCtxt::Trait || self.outer_trait_or_trait_impl.is_none() {

compiler/rustc_builtin_macros/src/proc_macro_harness.rs

@@ -213,10 +213,8 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
             return;
         };
 
-        // First up, make sure we're checking a bare function. If we're not then
+        // Make sure we're checking a bare function. If we're not then we're
-        // we're just not interested in this item.
+        // just not interested any further in this item.
-        //
-        // If we find one, try to locate a `#[proc_macro_derive]` attribute on it.
         let fn_ident = if let ast::ItemKind::Fn(fn_) = &item.kind {
             fn_.ident
         } else {
@@ -243,6 +241,7 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
             return;
         }
 
+        // Try to locate a `#[proc_macro_derive]` attribute.
         if attr.has_name(sym::proc_macro_derive) {
             self.collect_custom_derive(item, fn_ident, attr);
         } else if attr.has_name(sym::proc_macro_attribute) {

compiler/rustc_expand/src/expand.rs

@@ -1223,9 +1223,7 @@ impl InvocationCollectorNode for P<ast::Item> {
 
         // Work around borrow checker not seeing through `P`'s deref.
         let (span, mut attrs) = (node.span, mem::take(&mut node.attrs));
-        let ItemKind::Mod(_, ident, mod_kind) = &mut node.kind else { unreachable!() };
+        let ItemKind::Mod(_, ident, ref mut mod_kind) = node.kind else { unreachable!() };
-        let ident = *ident;
-
         let ecx = &mut collector.cx;
         let (file_path, dir_path, dir_ownership) = match mod_kind {
             ModKind::Loaded(_, inline, _, _) => {

compiler/rustc_parse/src/parser/item.rs

@@ -243,8 +243,7 @@ impl<'a> Parser<'a> {
             // STATIC ITEM
             self.bump(); // `static`
             let mutability = self.parse_mutability();
-            let item = self.parse_static_item(safety, mutability)?;
+            self.parse_static_item(safety, mutability)?
-            ItemKind::Static(Box::new(item))
         } else if let Const::Yes(const_span) = self.parse_constness(Case::Sensitive) {
             // CONST ITEM
             if self.token.is_keyword(kw::Impl) {
@@ -681,7 +680,7 @@ impl<'a> Parser<'a> {
             }
             None => (None, ty_first), // impl Type
         };
-        let item_kind = ItemKind::Impl(Box::new(Impl {
+        Ok(ItemKind::Impl(Box::new(Impl {
             safety,
             polarity,
             defaultness,
@@ -690,9 +689,7 @@ impl<'a> Parser<'a> {
             of_trait,
             self_ty,
             items: impl_items,
-        }));
+        })))
-
-        Ok(item_kind)
     }
 
     fn parse_item_delegation(&mut self) -> PResult<'a, ItemKind> {
@@ -1222,13 +1219,12 @@ impl<'a> Parser<'a> {
             safety = Safety::Unsafe(self.token.span);
             let _ = self.eat_keyword(exp!(Unsafe));
         }
-        let module = ast::ForeignMod {
+        Ok(ItemKind::ForeignMod(ast::ForeignMod {
             extern_span,
             safety,
             abi,
             items: self.parse_item_list(attrs, |p| p.parse_foreign_item(ForceCollect::No))?,
-        };
+        }))
-        Ok(ItemKind::ForeignMod(module))
     }
 
     /// Parses a foreign item (one in an `extern { ... }` block).
@@ -1370,7 +1366,8 @@ impl<'a> Parser<'a> {
         Ok(item_kind)
     }
 
-    /// Parse a static item with the prefix `"static" "mut"?` already parsed and stored in `mutability`.
+    /// Parse a static item with the prefix `"static" "mut"?` already parsed and stored in
+    /// `mutability`.
     ///
     /// ```ebnf
     /// Static = "static" "mut"? $ident ":" $ty (= $expr)? ";" ;
@@ -1379,7 +1376,7 @@ impl<'a> Parser<'a> {
         &mut self,
         safety: Safety,
         mutability: Mutability,
-    ) -> PResult<'a, StaticItem> {
+    ) -> PResult<'a, ItemKind> {
         let ident = self.parse_ident()?;
 
         if self.token == TokenKind::Lt && self.may_recover() {
@@ -1391,7 +1388,8 @@ impl<'a> Parser<'a> {
         // FIXME: This could maybe benefit from `.may_recover()`?
         let ty = match (self.eat(exp!(Colon)), self.check(exp!(Eq)) | self.check(exp!(Semi))) {
             (true, false) => self.parse_ty()?,
-            // If there wasn't a `:` or the colon was followed by a `=` or `;`, recover a missing type.
+            // If there wasn't a `:` or the colon was followed by a `=` or `;`, recover a missing
+            // type.
             (colon, _) => self.recover_missing_global_item_type(colon, Some(mutability)),
         };
 
@@ -1399,7 +1397,8 @@ impl<'a> Parser<'a> {
 
         self.expect_semi()?;
 
-        Ok(StaticItem { ident, ty, safety, mutability, expr, define_opaque: None })
+        let item = StaticItem { ident, ty, safety, mutability, expr, define_opaque: None };
+        Ok(ItemKind::Static(Box::new(item)))
     }
 
     /// Parse a constant item with the prefix `"const"` already parsed.
@@ -1537,7 +1536,7 @@ impl<'a> Parser<'a> {
         }
 
         let prev_span = self.prev_token.span;
-        let id = self.parse_ident()?;
+        let ident = self.parse_ident()?;
         let mut generics = self.parse_generics()?;
         generics.where_clause = self.parse_where_clause()?;
 
@@ -1548,10 +1547,10 @@ impl<'a> Parser<'a> {
             (thin_vec![], Trailing::No)
         } else {
             self.parse_delim_comma_seq(exp!(OpenBrace), exp!(CloseBrace), |p| {
-                p.parse_enum_variant(id.span)
+                p.parse_enum_variant(ident.span)
             })
             .map_err(|mut err| {
-                err.span_label(id.span, "while parsing this enum");
+                err.span_label(ident.span, "while parsing this enum");
                 if self.token == token::Colon {
                     let snapshot = self.create_snapshot_for_diagnostic();
                     self.bump();
@@ -1577,7 +1576,7 @@ impl<'a> Parser<'a> {
         };
 
         let enum_definition = EnumDef { variants: variants.into_iter().flatten().collect() };
-        Ok(ItemKind::Enum(id, enum_definition, generics))
+        Ok(ItemKind::Enum(ident, enum_definition, generics))
     }
 
     fn parse_enum_variant(&mut self, span: Span) -> PResult<'a, Option<Variant>> {

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -10,8 +10,8 @@ use std::sync::Arc;
 
 use rustc_ast::visit::{self, AssocCtxt, Visitor, WalkItemKind};
 use rustc_ast::{
-    self as ast, AssocItem, AssocItemKind, Block, ConstItem, Delegation, ForeignItem,
+    self as ast, AssocItem, AssocItemKind, Block, ConstItem, Delegation, Fn, ForeignItem,
-    ForeignItemKind, Impl, Item, ItemKind, MetaItemKind, NodeId, StaticItem, StmtKind,
+    ForeignItemKind, Impl, Item, ItemKind, MetaItemKind, NodeId, StaticItem, StmtKind, TyAlias,
 };
 use rustc_attr_parsing as attr;
 use rustc_expand::base::ResolverExpand;
@@ -764,8 +764,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             ItemKind::ExternCrate(orig_name, ident) => {
                 self.build_reduced_graph_for_extern_crate(
                     orig_name,
-                    ident,
                     item,
+                    ident,
                     local_def_id,
                     vis,
                     parent,
@@ -797,7 +797,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             | ItemKind::Static(box StaticItem { ident, .. }) => {
                 self.r.define(parent, ident, ValueNS, (res, vis, sp, expansion));
             }
-            ItemKind::Fn(box ast::Fn { ident, .. }) => {
+            ItemKind::Fn(box Fn { ident, .. }) => {
                 self.r.define(parent, ident, ValueNS, (res, vis, sp, expansion));
 
                 // Functions introducing procedural macros reserve a slot
@@ -806,7 +806,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             }
 
             // These items live in the type namespace.
-            ItemKind::TyAlias(box ast::TyAlias { ident, .. }) | ItemKind::TraitAlias(ident, ..) => {
+            ItemKind::TyAlias(box TyAlias { ident, .. }) | ItemKind::TraitAlias(ident, ..) => {
                 self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion));
             }
 
@@ -900,8 +900,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
     fn build_reduced_graph_for_extern_crate(
         &mut self,
         orig_name: Option<Symbol>,
-        ident: Ident,
         item: &Item,
+        ident: Ident,
         local_def_id: LocalDefId,
         vis: ty::Visibility,
         parent: Module<'ra>,
@@ -1362,14 +1362,16 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
     }
 
     fn visit_foreign_item(&mut self, foreign_item: &'a ForeignItem) {
-        if let ForeignItemKind::MacCall(_) = foreign_item.kind {
+        let ident = match foreign_item.kind {
-            self.visit_invoc_in_module(foreign_item.id);
+            ForeignItemKind::Static(box StaticItem { ident, .. })
-            return;
+            | ForeignItemKind::Fn(box Fn { ident, .. })
-        }
+            | ForeignItemKind::TyAlias(box TyAlias { ident, .. }) => ident,
+            ForeignItemKind::MacCall(_) => {
+                self.visit_invoc_in_module(foreign_item.id);
+                return;
+            }
+        };
 
-        // `unwrap` is safe because `MacCall` has been excluded, and other foreign item kinds have
-        // an ident.
-        let ident = foreign_item.kind.ident().unwrap();
         self.build_reduced_graph_for_foreign_item(foreign_item, ident);
         visit::walk_item(self, foreign_item);
     }
@@ -1384,26 +1386,35 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
     }
 
     fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) {
-        if let AssocItemKind::MacCall(_) = item.kind {
+        let (ident, ns) = match item.kind {
-            match ctxt {
+            AssocItemKind::Const(box ConstItem { ident, .. })
-                AssocCtxt::Trait => {
+            | AssocItemKind::Fn(box Fn { ident, .. })
-                    self.visit_invoc_in_module(item.id);
+            | AssocItemKind::Delegation(box Delegation { ident, .. }) => (ident, ValueNS),
-                }
-                AssocCtxt::Impl { .. } => {
-                    let invoc_id = item.id.placeholder_to_expn_id();
-                    if !self.r.glob_delegation_invoc_ids.contains(&invoc_id) {
-                        self.r
-                            .impl_unexpanded_invocations
-                            .entry(self.r.invocation_parent(invoc_id))
-                            .or_default()
-                            .insert(invoc_id);
-                    }
-                    self.visit_invoc(item.id);
-                }
-            }
-            return;
-        }
 
+            AssocItemKind::Type(box TyAlias { ident, .. }) => (ident, TypeNS),
+
+            AssocItemKind::MacCall(_) => {
+                match ctxt {
+                    AssocCtxt::Trait => {
+                        self.visit_invoc_in_module(item.id);
+                    }
+                    AssocCtxt::Impl { .. } => {
+                        let invoc_id = item.id.placeholder_to_expn_id();
+                        if !self.r.glob_delegation_invoc_ids.contains(&invoc_id) {
+                            self.r
+                                .impl_unexpanded_invocations
+                                .entry(self.r.invocation_parent(invoc_id))
+                                .or_default()
+                                .insert(invoc_id);
+                        }
+                        self.visit_invoc(item.id);
+                    }
+                }
+                return;
+            }
+
+            AssocItemKind::DelegationMac(..) => bug!(),
+        };
         let vis = self.resolve_visibility(&item.vis);
         let feed = self.r.feed(item.id);
         let local_def_id = feed.key();
@@ -1418,16 +1429,6 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             self.r.feed_visibility(feed, vis);
         }
 
-        let ns = match item.kind {
-            AssocItemKind::Const(..) | AssocItemKind::Delegation(..) | AssocItemKind::Fn(..) => {
-                ValueNS
-            }
-            AssocItemKind::Type(..) => TypeNS,
-            AssocItemKind::MacCall(_) | AssocItemKind::DelegationMac(..) => bug!(), // handled above
-        };
-        // `unwrap` is safe because `MacCall`/`DelegationMac` have been excluded, and other foreign
-        // item kinds have an ident.
-        let ident = item.kind.ident().unwrap();
         if ctxt == AssocCtxt::Trait {
             let parent = self.parent_scope.module;
             let expansion = self.parent_scope.expansion;

compiler/rustc_resolve/src/def_collector.rs

@@ -186,8 +186,11 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
             FnKind::Fn(
                 _ctxt,
                 _vis,
-                Fn { sig: FnSig { header, decl, span: _ }, generics, contract, body, .. },
+                Fn {
+                    sig: FnSig { header, decl, span: _ }, ident, generics, contract, body, ..
+                },
             ) if let Some(coroutine_kind) = header.coroutine_kind => {
+                self.visit_ident(ident);
                 self.visit_fn_header(header);
                 self.visit_generics(generics);
                 if let Some(contract) = contract {

compiler/rustc_resolve/src/late.rs

@@ -1025,9 +1025,10 @@ impl<'ra: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'r
         match fn_kind {
             // Bail if the function is foreign, and thus cannot validly have
             // a body, or if there's no body for some other reason.
-            FnKind::Fn(FnCtxt::Foreign, _, Fn { sig, generics, .. })
+            FnKind::Fn(FnCtxt::Foreign, _, Fn { sig, ident, generics, .. })
-            | FnKind::Fn(_, _, Fn { sig, generics, body: None, .. }) => {
+            | FnKind::Fn(_, _, Fn { sig, ident, generics, body: None, .. }) => {
                 self.visit_fn_header(&sig.header);
+                self.visit_ident(ident);
                 self.visit_generics(generics);
                 self.with_lifetime_rib(
                     LifetimeRibKind::AnonymousCreateParameter {

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -227,14 +227,10 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                     && let FnKind::Fn(_, _, ast::Fn { sig, .. }) = fn_kind
                     && let Some(items) = self.diag_metadata.current_impl_items
                     && let Some(item) = items.iter().find(|i| {
-                        if let Some(ident) = i.kind.ident()
+                        i.kind.ident().is_some_and(|ident| {
-                            && ident.name == item_str.name
-                        {
                             // Don't suggest if the item is in Fn signature arguments (#112590).
-                            !sig.span.contains(item_span)
+                            ident.name == item_str.name && !sig.span.contains(item_span)
-                        } else {
+                        })
-                            false
-                        }
                     }) {
                     let sp = item_span.shrink_to_lo();
 

src/tools/clippy/clippy_lints/src/empty_line_after.rs

@@ -8,8 +8,7 @@ use rustc_errors::{Applicability, Diag, SuggestionStyle};
 use rustc_lexer::TokenKind;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
 use rustc_session::impl_lint_pass;
-use rustc_span::symbol::kw;
+use rustc_span::{BytePos, ExpnKind, Ident, InnerSpan, Span, SpanData, Symbol, kw};
-use rustc_span::{BytePos, ExpnKind, Ident, InnerSpan, Span, SpanData, Symbol, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -375,14 +374,16 @@ impl EmptyLineAfter {
         &mut self,
         cx: &EarlyContext<'_>,
         kind: &ItemKind,
-        ident: &Option<Ident>,
+        ident: Option<Ident>,
         span: Span,
         attrs: &[Attribute],
         id: NodeId,
     ) {
         self.items.push(ItemInfo {
             kind: kind.descr(),
-            name: if let Some(ident) = ident { ident.name } else { sym::dummy },
+            // FIXME: this `sym::empty` can be leaked, see
+            // https://github.com/rust-lang/rust/pull/138740#discussion_r2021979899
+            name: if let Some(ident) = ident { ident.name } else { kw::Empty },
             span: if let Some(ident) = ident {
                 span.with_hi(ident.span.hi())
             } else {
@@ -471,7 +472,7 @@ impl EarlyLintPass for EmptyLineAfter {
         self.check_item_kind(
             cx,
             &item.kind.clone().into(),
-            &item.kind.ident(),
+            item.kind.ident(),
             item.span,
             &item.attrs,
             item.id,
@@ -482,7 +483,7 @@ impl EarlyLintPass for EmptyLineAfter {
         self.check_item_kind(
             cx,
             &item.kind.clone().into(),
-            &item.kind.ident(),
+            item.kind.ident(),
             item.span,
             &item.attrs,
             item.id,
@@ -490,6 +491,6 @@ impl EarlyLintPass for EmptyLineAfter {
     }
 
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
-        self.check_item_kind(cx, &item.kind, &item.kind.ident(), item.span, &item.attrs, item.id);
+        self.check_item_kind(cx, &item.kind, item.kind.ident(), item.span, &item.attrs, item.id);
     }
 }

src/tools/clippy/clippy_utils/src/ast_utils/mod.rs

@@ -567,20 +567,23 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool {
         (
             TyAlias(box ast::TyAlias {
                 defaultness: ld,
+                ident: li,
                 generics: lg,
+                where_clauses: _,
                 bounds: lb,
                 ty: lt,
-                ..
             }),
             TyAlias(box ast::TyAlias {
                 defaultness: rd,
+                ident: ri,
                 generics: rg,
+                where_clauses: _,
                 bounds: rb,
                 ty: rt,
-                ..
             }),
         ) => {
             eq_defaultness(*ld, *rd)
+                && eq_id(*li, *ri)
                 && eq_generics(lg, rg)
                 && over(lb, rb, eq_generic_bound)
                 && both(lt.as_ref(), rt.as_ref(), |l, r| eq_ty(l, r))
@@ -647,20 +650,23 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool {
         (
             Type(box TyAlias {
                 defaultness: ld,
+                ident: li,
                 generics: lg,
+                where_clauses: _,
                 bounds: lb,
                 ty: lt,
-                ..
             }),
             Type(box TyAlias {
                 defaultness: rd,
+                ident: ri,
                 generics: rg,
+                where_clauses: _,
                 bounds: rb,
                 ty: rt,
-                ..
             }),
         ) => {
             eq_defaultness(*ld, *rd)
+                && eq_id(*li, *ri)
                 && eq_generics(lg, rg)
                 && over(lb, rb, eq_generic_bound)
                 && both(lt.as_ref(), rt.as_ref(), |l, r| eq_ty(l, r))

src/tools/rustfmt/src/visitor.rs

@@ -623,13 +623,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
 
     fn visit_assoc_item(&mut self, ai: &ast::AssocItem, visitor_kind: ItemVisitorKind) {
         use ItemVisitorKind::*;
-        // TODO(calebcartwright): Not sure the skip spans are correct
         let assoc_ctxt = match visitor_kind {
             AssocTraitItem => visit::AssocCtxt::Trait,
             // There is no difference between trait and inherent assoc item formatting
             AssocImplItem => visit::AssocCtxt::Impl { of_trait: false },
             _ => unreachable!(),
         };
+        // TODO(calebcartwright): Not sure the skip spans are correct
         let skip_span = ai.span;
         skip_out_of_file_lines_range_visitor!(self, ai.span);
 

tests/ui/custom_test_frameworks/full.rs

@@ -28,12 +28,13 @@ const TEST_1: IsFoo = IsFoo("hello");
 static TEST_2: IsFoo = IsFoo("foo");
 
 // FIXME: `test_case` is currently ignored on anything other than
-// fn/const/static. Should this be a warning/error?
+// fn/const/static. This should be an error. Compare this with `#[test]` and
+// #[bench] whose expanders emit "error: expected a non-associated function,
+// found […]" if applied to invalid items.
 #[test_case]
 struct _S;
 
-// FIXME: `test_case` is currently ignored on anything other than
+// FIXME: as above.
-// fn/const/static. Should this be a warning/error?
 #[test_case]
 impl _S {
     fn _f() {}