From 43018eacb61da96b718f70b7719bf5e51207df61 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Fri, 21 Mar 2025 15:25:30 +1100
Subject: [PATCH] Ignore `#[test_case]` on anything other than
 `fn`/`const`/`static`.

`expand_test_case` looks for any item with a `#[test_case]` attribute
and adds a `test_path_symbol` attribute to it while also fiddling with
the item's ident's span.

This is pretty weird, because `#[test_case]` is only valid on
`fn`/`const`/`static` items, as far as I can tell. But you don't
currently get an error or warning if you use it on other kinds of items.

This commit changes things so that a `#[test_case]` item is modified
only if it is `fn`/`const`/`static`. This is relevant for moving idents
from `Item` to `ItemKind`, because some item kinds don't have an ident,
e.g. `impl` blocks.

The commit also does the following.
- Renames a local variable `test_id` as `test_ident`.
- Changes a `const` to `static` in
  `tests/ui/custom_test_frameworks/full.rs` to give the `static` case
  some test coverage.
- Adds a `struct` and `impl` to the same test to give some test coverage
  to the non-affected item kinds. These have a `FIXME` comment
  identifying the weirdness here. Hopefully this will be useful
  breadcrumbs for somebody else in the future.
---
 compiler/rustc_builtin_macros/src/test.rs | 46 +++++++++++++----------
 tests/ui/custom_test_frameworks/full.rs   | 14 ++++++-
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
index 239f8657284..db3e431495b 100644
--- a/compiler/rustc_builtin_macros/src/test.rs
+++ b/compiler/rustc_builtin_macros/src/test.rs
@@ -51,21 +51,26 @@ pub(crate) fn expand_test_case(
             return vec![];
         }
     };
-    item = item.map(|mut item| {
-        let test_path_symbol = Symbol::intern(&item_path(
-            // skip the name of the root module
-            &ecx.current_expansion.module.mod_path[1..],
-            &item.ident,
-        ));
-        item.vis = ast::Visibility {
-            span: item.vis.span,
-            kind: ast::VisibilityKind::Public,
-            tokens: None,
-        };
-        item.ident.span = item.ident.span.with_ctxt(sp.ctxt());
-        item.attrs.push(ecx.attr_name_value_str(sym::rustc_test_marker, test_path_symbol, sp));
-        item
-    });
+
+    // `#[test_case]` is valid on functions, consts, and statics. Only modify
+    // the item in those cases.
+    match &mut item.kind {
+        ast::ItemKind::Fn(_) | ast::ItemKind::Const(_) | ast::ItemKind::Static(_) => {
+            item.ident.span = item.ident.span.with_ctxt(sp.ctxt());
+            let test_path_symbol = Symbol::intern(&item_path(
+                // skip the name of the root module
+                &ecx.current_expansion.module.mod_path[1..],
+                &item.ident,
+            ));
+            item.vis = ast::Visibility {
+                span: item.vis.span,
+                kind: ast::VisibilityKind::Public,
+                tokens: None,
+            };
+            item.attrs.push(ecx.attr_name_value_str(sym::rustc_test_marker, test_path_symbol, sp));
+        }
+        _ => {}
+    }
 
     let ret = if is_stmt {
         Annotatable::Stmt(P(ecx.stmt_item(item.span, item)))
@@ -162,17 +167,17 @@ pub(crate) fn expand_test_or_bench(
     let ret_ty_sp = cx.with_def_site_ctxt(fn_.sig.decl.output.span());
     let attr_sp = cx.with_def_site_ctxt(attr_sp);
 
-    let test_id = Ident::new(sym::test, attr_sp);
+    let test_ident = Ident::new(sym::test, attr_sp);
 
     // creates test::$name
-    let test_path = |name| cx.path(ret_ty_sp, vec![test_id, Ident::from_str_and_span(name, sp)]);
+    let test_path = |name| cx.path(ret_ty_sp, vec![test_ident, Ident::from_str_and_span(name, sp)]);
 
     // creates test::ShouldPanic::$name
     let should_panic_path = |name| {
         cx.path(
             sp,
             vec![
-                test_id,
+                test_ident,
                 Ident::from_str_and_span("ShouldPanic", sp),
                 Ident::from_str_and_span(name, sp),
             ],
@@ -184,7 +189,7 @@ pub(crate) fn expand_test_or_bench(
         cx.path(
             sp,
             vec![
-                test_id,
+                test_ident,
                 Ident::from_str_and_span("TestType", sp),
                 Ident::from_str_and_span(name, sp),
             ],
@@ -380,7 +385,8 @@ pub(crate) fn expand_test_or_bench(
     });
 
     // extern crate test
-    let test_extern = cx.item(sp, test_id, ast::AttrVec::new(), ast::ItemKind::ExternCrate(None));
+    let test_extern =
+        cx.item(sp, test_ident, ast::AttrVec::new(), ast::ItemKind::ExternCrate(None));
 
     debug!("synthetic test item:\n{}\n", pprust::item_to_string(&test_const));
 
diff --git a/tests/ui/custom_test_frameworks/full.rs b/tests/ui/custom_test_frameworks/full.rs
index 289767b1f69..6be29f0418d 100644
--- a/tests/ui/custom_test_frameworks/full.rs
+++ b/tests/ui/custom_test_frameworks/full.rs
@@ -25,4 +25,16 @@ impl example_runner::Testable for IsFoo {
 const TEST_1: IsFoo = IsFoo("hello");
 
 #[test_case]
-const TEST_2: IsFoo = IsFoo("foo");
+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?
+#[test_case]
+struct _S;
+
+// FIXME: `test_case` is currently ignored on anything other than
+// fn/const/static. Should this be a warning/error?
+#[test_case]
+impl _S {
+    fn _f() {}
+}