Skip to content

Commit 43018ea

Browse files
committed
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.
1 parent deed0f2 commit 43018ea

File tree

2 files changed

+39
-21
lines changed
  • compiler/rustc_builtin_macros/src
  • tests/ui/custom_test_frameworks

2 files changed

+39
-21
lines changed

Diff for: compiler/rustc_builtin_macros/src/test.rs

+26-20
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,26 @@ pub(crate) fn expand_test_case(
5151
return vec![];
5252
}
5353
};
54-
item = item.map(|mut item| {
55-
let test_path_symbol = Symbol::intern(&item_path(
56-
// skip the name of the root module
57-
&ecx.current_expansion.module.mod_path[1..],
58-
&item.ident,
59-
));
60-
item.vis = ast::Visibility {
61-
span: item.vis.span,
62-
kind: ast::VisibilityKind::Public,
63-
tokens: None,
64-
};
65-
item.ident.span = item.ident.span.with_ctxt(sp.ctxt());
66-
item.attrs.push(ecx.attr_name_value_str(sym::rustc_test_marker, test_path_symbol, sp));
67-
item
68-
});
54+
55+
// `#[test_case]` is valid on functions, consts, and statics. Only modify
56+
// the item in those cases.
57+
match &mut item.kind {
58+
ast::ItemKind::Fn(_) | ast::ItemKind::Const(_) | ast::ItemKind::Static(_) => {
59+
item.ident.span = item.ident.span.with_ctxt(sp.ctxt());
60+
let test_path_symbol = Symbol::intern(&item_path(
61+
// skip the name of the root module
62+
&ecx.current_expansion.module.mod_path[1..],
63+
&item.ident,
64+
));
65+
item.vis = ast::Visibility {
66+
span: item.vis.span,
67+
kind: ast::VisibilityKind::Public,
68+
tokens: None,
69+
};
70+
item.attrs.push(ecx.attr_name_value_str(sym::rustc_test_marker, test_path_symbol, sp));
71+
}
72+
_ => {}
73+
}
6974

7075
let ret = if is_stmt {
7176
Annotatable::Stmt(P(ecx.stmt_item(item.span, item)))
@@ -162,17 +167,17 @@ pub(crate) fn expand_test_or_bench(
162167
let ret_ty_sp = cx.with_def_site_ctxt(fn_.sig.decl.output.span());
163168
let attr_sp = cx.with_def_site_ctxt(attr_sp);
164169

165-
let test_id = Ident::new(sym::test, attr_sp);
170+
let test_ident = Ident::new(sym::test, attr_sp);
166171

167172
// creates test::$name
168-
let test_path = |name| cx.path(ret_ty_sp, vec![test_id, Ident::from_str_and_span(name, sp)]);
173+
let test_path = |name| cx.path(ret_ty_sp, vec![test_ident, Ident::from_str_and_span(name, sp)]);
169174

170175
// creates test::ShouldPanic::$name
171176
let should_panic_path = |name| {
172177
cx.path(
173178
sp,
174179
vec![
175-
test_id,
180+
test_ident,
176181
Ident::from_str_and_span("ShouldPanic", sp),
177182
Ident::from_str_and_span(name, sp),
178183
],
@@ -184,7 +189,7 @@ pub(crate) fn expand_test_or_bench(
184189
cx.path(
185190
sp,
186191
vec![
187-
test_id,
192+
test_ident,
188193
Ident::from_str_and_span("TestType", sp),
189194
Ident::from_str_and_span(name, sp),
190195
],
@@ -380,7 +385,8 @@ pub(crate) fn expand_test_or_bench(
380385
});
381386

382387
// extern crate test
383-
let test_extern = cx.item(sp, test_id, ast::AttrVec::new(), ast::ItemKind::ExternCrate(None));
388+
let test_extern =
389+
cx.item(sp, test_ident, ast::AttrVec::new(), ast::ItemKind::ExternCrate(None));
384390

385391
debug!("synthetic test item:\n{}\n", pprust::item_to_string(&test_const));
386392

Diff for: tests/ui/custom_test_frameworks/full.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,16 @@ impl example_runner::Testable for IsFoo {
2525
const TEST_1: IsFoo = IsFoo("hello");
2626

2727
#[test_case]
28-
const TEST_2: IsFoo = IsFoo("foo");
28+
static TEST_2: IsFoo = IsFoo("foo");
29+
30+
// FIXME: `test_case` is currently ignored on anything other than
31+
// fn/const/static. Should this be a warning/error?
32+
#[test_case]
33+
struct _S;
34+
35+
// FIXME: `test_case` is currently ignored on anything other than
36+
// fn/const/static. Should this be a warning/error?
37+
#[test_case]
38+
impl _S {
39+
fn _f() {}
40+
}

0 commit comments

Comments
 (0)