Rollup merge of #110035 - Ezrashaw:improve-test-attr-expansion-code, r=davidtwco

matthiaskrgr · web-flow · commit d40c827e9a48 · 2023-04-12T22:04:32.000+02:00
fix: ensure bad `#[test]` invocs retain correct AST Fixes #109816 Ensures that a `StmtKind::Item` doesn't get converted into a plain `Item` (causing the ICE from the linked issue) Also unifies the error path a bit.
diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
@@ -118,34 +118,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(cx, attr_sp, 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(cx, attr_sp, 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
@@ -398,6 +386,36 @@ pub fn expand_test_or_bench(
     }
 }
 
+fn not_testable_error(cx: &ExtCtxt<'_>, attr_sp: Span, 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";
+    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();
+}
+
 fn get_location_info(cx: &ExtCtxt<'_>, item: &ast::Item) -> (Symbol, usize, usize, usize, usize) {
     let span = item.ident.span;
     let (source_file, lo_line, lo_col, hi_line, hi_col) =
diff --git a/tests/ui/test-attrs/issue-109816.rs b/tests/ui/test-attrs/issue-109816.rs
@@ -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);
+}
diff --git a/tests/ui/test-attrs/issue-109816.stderr b/tests/ui/test-attrs/issue-109816.stderr
@@ -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
+
diff --git a/tests/ui/test-attrs/test-attr-non-associated-functions.rs b/tests/ui/test-attrs/test-attr-non-associated-functions.rs
@@ -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 {}
     }
 }
diff --git a/tests/ui/test-attrs/test-attr-non-associated-functions.stderr b/tests/ui/test-attrs/test-attr-non-associated-functions.stderr
@@ -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
 
diff --git a/tests/ui/test-attrs/test-on-not-fn.stderr b/tests/ui/test-attrs/test-on-not-fn.stderr
@@ -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
    |
@@ -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
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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 | | }
@@ -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
    |
@@ -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,
@@ -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,
@@ -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
    |

Original file line number	Diff line number	Diff line change
`@@ -1,18 +1,16 @@`
`1`		`-// #[test] attribute is not allowed on associated functions or methods`
`2`		`-// reworded error message`
`3`	`1`	`// compile-flags:--test`
`4`	`2`
`5`	`3`	`struct A {}`
`6`	`4`
`7`	`5`	`impl A {`
`8`	`6`	`#[test]`
	`7`	+ //~^ ERROR the `#[test]` attribute may only be used on a non-associated function
`9`	`8`	`fn new() -> A {`
`10`		- //~^ ERROR `#[test]` attribute is only allowed on non associated functions
`11`	`9`	`A {}`
`12`	`10`	`}`
`13`	`11`	`#[test]`
	`12`	+ //~^ ERROR the `#[test]` attribute may only be used on a non-associated function
`14`	`13`	`fn recovery_witness() -> A {`
`15`		- //~^ ERROR `#[test]` attribute is only allowed on non associated functions
`16`	`14`	`A {}`
`17`	`15`	`}`
`18`	`16`	`}`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`2`	`2`	`--> $DIR/test-on-not-fn.rs:3:1`
`3`	`3`	`\|`
`4`	`4`	`LL \| #[test]`
`5`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`5`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`6`	`6`	`LL \| mod test {}`
`7`	`7`	`\| ----------- expected a non-associated function, found a module`
`8`	`8`	`\|`
@@ -15,7 +15,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`15`	`15`	`--> $DIR/test-on-not-fn.rs:6:1`
`16`	`16`	`\|`
`17`	`17`	`LL \| #[test]`
`18`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`18`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`19`	`19`	`LL \| / mod loooooooooooooong_teeeeeeeeeest {`
`20`	`20`	`LL \| \| /*`
`21`	`21`	`LL \| \| this is a comment`
@@ -34,7 +34,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`34`	`34`	`--> $DIR/test-on-not-fn.rs:20:1`
`35`	`35`	`\|`
`36`	`36`	`LL \| #[test]`
`37`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`37`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`38`	`38`	`LL \| extern "C" {}`
`39`	`39`	`\| ------------- expected a non-associated function, found an extern block`
`40`	`40`	`\|`
@@ -47,7 +47,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`47`	`47`	`--> $DIR/test-on-not-fn.rs:23:1`
`48`	`48`	`\|`
`49`	`49`	`LL \| #[test]`
`50`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`50`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`51`	`51`	`LL \| trait Foo {}`
`52`	`52`	`\| ------------ expected a non-associated function, found a trait`
`53`	`53`	`\|`
@@ -60,7 +60,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`60`	`60`	`--> $DIR/test-on-not-fn.rs:26:1`
`61`	`61`	`\|`
`62`	`62`	`LL \| #[test]`
`63`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`63`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`64`	`64`	`LL \| impl Foo for i32 {}`
`65`	`65`	`\| ------------------- expected a non-associated function, found an implementation`
`66`	`66`	`\|`
@@ -73,7 +73,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`73`	`73`	`--> $DIR/test-on-not-fn.rs:29:1`
`74`	`74`	`\|`
`75`	`75`	`LL \| #[test]`
`76`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`76`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`77`	`77`	`LL \| const FOO: i32 = -1_i32;`
`78`	`78`	`\| ------------------------ expected a non-associated function, found a constant item`
`79`	`79`	`\|`
@@ -86,7 +86,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`86`	`86`	`--> $DIR/test-on-not-fn.rs:32:1`
`87`	`87`	`\|`
`88`	`88`	`LL \| #[test]`
`89`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`89`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`90`	`90`	`LL \| static BAR: u64 = 10_000_u64;`
`91`	`91`	`\| ----------------------------- expected a non-associated function, found a static item`
`92`	`92`	`\|`
@@ -99,7 +99,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`99`	`99`	`--> $DIR/test-on-not-fn.rs:35:1`
`100`	`100`	`\|`
`101`	`101`	`LL \| #[test]`
`102`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`102`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`103`	`103`	`LL \| / enum MyUnit {`
`104`	`104`	`LL \| \| Unit,`
`105`	`105`	`LL \| \| }`
@@ -114,7 +114,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`114`	`114`	`--> $DIR/test-on-not-fn.rs:40:1`
`115`	`115`	`\|`
`116`	`116`	`LL \| #[test]`
`117`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`117`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`118`	`118`	`LL \| struct NewI32(i32);`
`119`	`119`	`\| ------------------- expected a non-associated function, found a struct`
`120`	`120`	`\|`
@@ -127,7 +127,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`127`	`127`	`--> $DIR/test-on-not-fn.rs:43:1`
`128`	`128`	`\|`
`129`	`129`	`LL \| #[test]`
`130`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`130`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`131`	`131`	`LL \| / union Spooky {`
`132`	`132`	`LL \| \| x: i32,`
`133`	`133`	`LL \| \| y: u32,`
@@ -143,7 +143,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
`143`	`143`	`--> $DIR/test-on-not-fn.rs:50:1`
`144`	`144`	`\|`
`145`	`145`	`LL \| #[test]`
`146`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`146`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`147`	`147`	`LL \| #[derive(Copy, Clone, Debug)]`
`148`	`148`	`LL \| / struct MoreAttrs {`
`149`	`149`	`LL \| \| a: i32,`
@@ -160,7 +160,7 @@ warning: the `#[test]` attribute may only be used on a non-associated function
`160`	`160`	`--> $DIR/test-on-not-fn.rs:61:1`
`161`	`161`	`\|`
`162`	`162`	`LL \| #[test]`
`163`		- \| ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
	`163`	+ \| ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
`164`	`164`	`LL \| foo!();`
`165`	`165`	`\| ------- expected a non-associated function, found an item macro invocation`
`166`	`166`	`\|`