Skip to content

fix: ensure bad #[test] invocs retain correct AST #110035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 40 additions & 22 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,36 @@ pub fn expand_test_or_bench(
return vec![];
}

let not_testable_error = |item: Option<&ast::Item>| {
let diag = &cx.sess.parse_sess.span_diagnostic;
let msg = "the `#[test]` attribute may only be used on a non-associated function";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this translatable? See #100717.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'd prefer not to do this right now. The whole test.rs file hasn't been converted yet so I feel like that should happen all at once. Also there is some complexity with the error being downgraded to a lint in some cases.

I'm happy to do this if you really want, just not sure it's currently worth the effort.

let mut err = match item.map(|i| &i.kind) {
// These were a warning before #92959 and need to continue being that to avoid breaking
// stable user code (#94508).
Some(ast::ItemKind::MacCall(_)) => diag.struct_span_warn(attr_sp, msg),
// `.forget_guarantee()` needed to get these two arms to match types. Because of how
// locally close the `.emit()` call is I'm comfortable with it, but if it can be
// reworked in the future to not need it, it'd be nice.
_ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
};
if let Some(item) = item {
err.span_label(
item.span,
format!(
"expected a non-associated function, found {} {}",
item.kind.article(),
item.kind.descr()
),
);
}
err.span_label(attr_sp, "the `#[test]` macro causes a function to be run as a test and has no effect on non-functions")
.span_suggestion(attr_sp,
"replace with conditional compilation to make the item only exist when tests are being run",
"#[cfg(test)]",
Applicability::MaybeIncorrect)
.emit();
};

let (item, is_stmt) = match item {
Annotatable::Item(i) => (i, false),
Annotatable::Stmt(stmt) if matches!(stmt.kind, ast::StmtKind::Item(_)) => {
Expand All @@ -118,34 +148,22 @@ pub fn expand_test_or_bench(
}
}
other => {
cx.struct_span_err(
other.span(),
"`#[test]` attribute is only allowed on non associated functions",
)
.emit();
not_testable_error(None);
return vec![other];
}
};

// Note: non-associated fn items are already handled by `expand_test_or_bench`
let ast::ItemKind::Fn(fn_) = &item.kind else {
let diag = &cx.sess.parse_sess.span_diagnostic;
let msg = "the `#[test]` attribute may only be used on a non-associated function";
let mut err = match item.kind {
// These were a warning before #92959 and need to continue being that to avoid breaking
// stable user code (#94508).
ast::ItemKind::MacCall(_) => diag.struct_span_warn(attr_sp, msg),
// `.forget_guarantee()` needed to get these two arms to match types. Because of how
// locally close the `.emit()` call is I'm comfortable with it, but if it can be
// reworked in the future to not need it, it'd be nice.
_ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
not_testable_error(Some(&item));
return if is_stmt {
vec![Annotatable::Stmt(P(ast::Stmt {
id: ast::DUMMY_NODE_ID,
span: item.span,
kind: ast::StmtKind::Item(item),
}))]
} else {
vec![Annotatable::Item(item)]
};
err.span_label(attr_sp, "the `#[test]` macro causes a function to be run on a test and has no effect on non-functions")
.span_label(item.span, format!("expected a non-associated function, found {} {}", item.kind.article(), item.kind.descr()))
.span_suggestion(attr_sp, "replace with conditional compilation to make the item only exist when tests are being run", "#[cfg(test)]", Applicability::MaybeIncorrect)
.emit();

return vec![Annotatable::Item(item)];
};

// has_*_signature will report any errors in the type so compilation
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/test-attrs/issue-109816.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// compile-flags: --test

fn align_offset_weird_strides() {
#[test]
//~^ ERROR the `#[test]` attribute may only be used on a non-associated function
struct A5(u32, u8);
}
16 changes: 16 additions & 0 deletions tests/ui/test-attrs/issue-109816.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/issue-109816.rs:4:5
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL |
LL | struct A5(u32, u8);
| ------------------- expected a non-associated function, found a struct
|
help: replace with conditional compilation to make the item only exist when tests are being run
|
LL | #[cfg(test)]
|

error: aborting due to previous error

6 changes: 2 additions & 4 deletions tests/ui/test-attrs/test-attr-non-associated-functions.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
// #[test] attribute is not allowed on associated functions or methods
// reworded error message
// compile-flags:--test

struct A {}

impl A {
#[test]
//~^ ERROR the `#[test]` attribute may only be used on a non-associated function
fn new() -> A {
//~^ ERROR `#[test]` attribute is only allowed on non associated functions
A {}
}
#[test]
//~^ ERROR the `#[test]` attribute may only be used on a non-associated function
fn recovery_witness() -> A {
//~^ ERROR `#[test]` attribute is only allowed on non associated functions
A {}
}
}
Expand Down
36 changes: 20 additions & 16 deletions tests/ui/test-attrs/test-attr-non-associated-functions.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
error: `#[test]` attribute is only allowed on non associated functions
--> $DIR/test-attr-non-associated-functions.rs:9:5
|
LL | / fn new() -> A {
LL | |
LL | | A {}
LL | | }
| |_____^
error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-attr-non-associated-functions.rs:6:5
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
|
help: replace with conditional compilation to make the item only exist when tests are being run
|
LL | #[cfg(test)]
|

error: `#[test]` attribute is only allowed on non associated functions
--> $DIR/test-attr-non-associated-functions.rs:14:5
|
LL | / fn recovery_witness() -> A {
LL | |
LL | | A {}
LL | | }
| |_____^
error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-attr-non-associated-functions.rs:11:5
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
|
help: replace with conditional compilation to make the item only exist when tests are being run
|
LL | #[cfg(test)]
|

error: aborting due to 2 previous errors

24 changes: 12 additions & 12 deletions tests/ui/test-attrs/test-on-not-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:3:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | mod test {}
| ----------- expected a non-associated function, found a module
|
Expand All @@ -15,7 +15,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:6:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | / mod loooooooooooooong_teeeeeeeeeest {
LL | | /*
LL | | this is a comment
Expand All @@ -34,7 +34,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:20:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | extern "C" {}
| ------------- expected a non-associated function, found an extern block
|
Expand All @@ -47,7 +47,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:23:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | trait Foo {}
| ------------ expected a non-associated function, found a trait
|
Expand All @@ -60,7 +60,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:26:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | impl Foo for i32 {}
| ------------------- expected a non-associated function, found an implementation
|
Expand All @@ -73,7 +73,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:29:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | const FOO: i32 = -1_i32;
| ------------------------ expected a non-associated function, found a constant item
|
Expand All @@ -86,7 +86,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:32:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | static BAR: u64 = 10_000_u64;
| ----------------------------- expected a non-associated function, found a static item
|
Expand All @@ -99,7 +99,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:35:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | / enum MyUnit {
LL | | Unit,
LL | | }
Expand All @@ -114,7 +114,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:40:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | struct NewI32(i32);
| ------------------- expected a non-associated function, found a struct
|
Expand All @@ -127,7 +127,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:43:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | / union Spooky {
LL | | x: i32,
LL | | y: u32,
Expand All @@ -143,7 +143,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:50:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | #[derive(Copy, Clone, Debug)]
LL | / struct MoreAttrs {
LL | | a: i32,
Expand All @@ -160,7 +160,7 @@ warning: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:61:1
|
LL | #[test]
| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | foo!();
| ------- expected a non-associated function, found an item macro invocation
|
Expand Down