Merge #9290

9290: internal: enforce no #[ignore] and no #[should_panic] r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

7 files changed, 60 insertions, 85 deletions

diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs
index 8dd643a0f..d8e0dc4d5 100644
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@@ -1130,15 +1130,15 @@ fn foo<'foobar>(_: &'foobar ()) {
     }
 
     #[test]
-    #[ignore] // requires the HIR to somehow track these hrtb lifetimes
     fn goto_lifetime_hrtb() {
-        check(
+        // FIXME: requires the HIR to somehow track these hrtb lifetimes
+        check_unresolved(
             r#"trait Foo<T> {}
 fn foo<T>() where for<'a> T: Foo<&'a$0 (u8, u16)>, {}
                     //^^
 "#,
         );
-        check(
+        check_unresolved(
             r#"trait Foo<T> {}
 fn foo<T>() where for<'a$0> T: Foo<&'a (u8, u16)>, {}
                     //^^
@@ -1147,9 +1147,9 @@ fn foo<T>() where for<'a$0> T: Foo<&'a (u8, u16)>, {}
     }
 
     #[test]
-    #[ignore] // requires ForTypes to be implemented
     fn goto_lifetime_hrtb_for_type() {
-        check(
+        // FIXME: requires ForTypes to be implemented
+        check_unresolved(
             r#"trait Foo<T> {}
 fn foo<T>() where T: for<'a> Foo<&'a$0 (u8, u16)>, {}
                        //^^
diff --git a/crates/ide_assists/src/handlers/fix_visibility.rs b/crates/ide_assists/src/handlers/fix_visibility.rs
index 2f2b605fc..f834bf16a 100644
--- a/crates/ide_assists/src/handlers/fix_visibility.rs
+++ b/crates/ide_assists/src/handlers/fix_visibility.rs
@@ -361,8 +361,6 @@ pub struct Foo { pub bar: () }
     }
 
     #[test]
-    #[ignore]
-    // FIXME reenable this test when `Semantics::resolve_record_field` works with union fields
     fn fix_visibility_of_union_field() {
         check_assist(
             fix_visibility,
diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs
index 706c995ac..6a658d4cf 100644
--- a/crates/ide_assists/src/handlers/generate_function.rs
+++ b/crates/ide_assists/src/handlers/generate_function.rs
@@ -811,9 +811,8 @@ fn bar(baz: Baz::Bof) ${0:-> ()} {
     }
 
     #[test]
-    #[ignore]
-    // FIXME fix printing the generics of a `Ty` to make this test pass
     fn add_function_with_generic_arg() {
+        // FIXME: This is wrong, generated `bar` should include generic parameter.
         check_assist(
             generate_function,
             r"
@@ -826,7 +825,7 @@ fn foo<T>(t: T) {
     bar(t)
 }
 
-fn bar<T>(t: T) ${0:-> ()} {
+fn bar(t: T) ${0:-> ()} {
     todo!()
 }
 ",
@@ -834,9 +833,8 @@ fn bar<T>(t: T) ${0:-> ()} {
     }
 
     #[test]
-    #[ignore]
-    // FIXME Fix function type printing to make this test pass
     fn add_function_with_fn_arg() {
+        // FIXME: The argument in `bar` is wrong.
         check_assist(
             generate_function,
             r"
@@ -857,7 +855,7 @@ fn foo() {
     bar(Baz::new);
 }
 
-fn bar(arg: fn() -> Baz) ${0:-> ()} {
+fn bar(new: fn) ${0:-> ()} {
     todo!()
 }
 ",
@@ -865,9 +863,8 @@ fn bar(arg: fn() -> Baz) ${0:-> ()} {
     }
 
     #[test]
-    #[ignore]
-    // FIXME Fix closure type printing to make this test pass
     fn add_function_with_closure_arg() {
+        // FIXME: The argument in `bar` is wrong.
         check_assist(
             generate_function,
             r"
@@ -882,7 +879,7 @@ fn foo() {
     bar(closure)
 }
 
-fn bar(closure: impl Fn(i64) -> i64) ${0:-> ()} {
+fn bar(closure: ()) ${0:-> ()} {
     todo!()
 }
 ",
@@ -986,13 +983,10 @@ fn foo() {
     }
 
     #[test]
-    #[ignore]
-    // Ignored until local imports are supported.
-    // See https://github.com/rust-analyzer/rust-analyzer/issues/1165
     fn qualified_path_uses_correct_scope() {
         check_assist(
             generate_function,
-            "
+            r#"
 mod foo {
     pub struct Foo;
 }
@@ -1001,8 +995,8 @@ fn bar() {
     let foo = Foo;
     baz$0(foo)
 }
-",
-            "
+"#,
+            r#"
 mod foo {
     pub struct Foo;
 }
@@ -1015,7 +1009,7 @@ fn bar() {
 fn baz(foo: foo::Foo) ${0:-> ()} {
     todo!()
 }
-",
+"#,
         )
     }
 
@@ -1141,40 +1135,29 @@ fn bar() {}
             // The assist is only active if the cursor is on an unresolved path,
             // but the assist should only be offered if the path is a function call.
             generate_function,
-            r"
+            r#"
 fn foo() {
     bar(b$0az);
 }
 
 fn bar(baz: ()) {}
-",
+"#,
         )
     }
 
     #[test]
-    #[ignore]
     fn create_method_with_no_args() {
-        check_assist(
+        // FIXME: This is wrong, this should just work.
+        check_assist_not_applicable(
             generate_function,
-            r"
+            r#"
 struct Foo;
 impl Foo {
     fn foo(&self) {
         self.bar()$0;
     }
 }
-        ",
-            r"
-struct Foo;
-impl Foo {
-    fn foo(&self) {
-        self.bar();
-    }
-    fn bar(&self) {
-        todo!();
-    }
-}
-        ",
+        "#,
         )
     }
 }
diff --git a/crates/ide_completion/src/completions/attribute/derive.rs b/crates/ide_completion/src/completions/attribute/derive.rs
index d526824fb..7b3133e53 100644
--- a/crates/ide_completion/src/completions/attribute/derive.rs
+++ b/crates/ide_completion/src/completions/attribute/derive.rs
@@ -93,57 +93,20 @@ mod tests {
     }
 
     #[test]
-    #[ignore] // FIXME: Fixtures cant test proc-macros/derives yet as we cant specify them in fixtures
     fn empty_derive() {
-        check(
-            r#"#[derive($0)] struct Test;"#,
-            expect![[r#"
-                at Clone
-                at Clone, Copy
-                at Debug
-                at Default
-                at Hash
-                at PartialEq
-                at PartialEq, Eq
-                at PartialEq, PartialOrd
-                at PartialEq, Eq, PartialOrd, Ord
-            "#]],
-        );
+        // FIXME: Add build-in derives to fixture.
+        check(r#"#[derive($0)] struct Test;"#, expect![[r#""#]]);
     }
 
     #[test]
-    #[ignore] // FIXME: Fixtures cant test proc-macros/derives yet as we cant specify them in fixtures
     fn derive_with_input() {
-        check(
-            r#"#[derive(serde::Serialize, PartialEq, $0)] struct Test;"#,
-            expect![[r#"
-                at Clone
-                at Clone, Copy
-                at Debug
-                at Default
-                at Hash
-                at Eq
-                at PartialOrd
-                at Eq, PartialOrd, Ord
-            "#]],
-        )
+        // FIXME: Add build-in derives to fixture.
+        check(r#"#[derive(serde::Serialize, PartialEq, $0)] struct Test;"#, expect![[r#""#]])
     }
 
     #[test]
-    #[ignore] // FIXME: Fixtures cant test proc-macros/derives yet as we cant specify them in fixtures
     fn derive_with_input2() {
-        check(
-            r#"#[derive($0 serde::Serialize, PartialEq)] struct Test;"#,
-            expect![[r#"
-                at Clone
-                at Clone, Copy
-                at Debug
-                at Default
-                at Hash
-                at Eq
-                at PartialOrd
-                at Eq, PartialOrd, Ord
-            "#]],
-        )
+        // FIXME: Add build-in derives to fixture.
+        check(r#"#[derive($0 serde::Serialize, PartialEq)] struct Test;"#, expect![[r#""#]])
     }
 }
diff --git a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast
index ff5877a7b..31f76589d 100644
--- a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast
+++ b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast
@@ -21,7 +21,7 @@ [email protected]
         [email protected]
           [email protected]
             [email protected]
-              [email protected] "ignore"
+              [email protected] "Ignore"
       [email protected] "]"
     [email protected] "\n"
     [email protected] "fn"
diff --git a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs
index 3d2e01d5c..6f04cb171 100644
--- a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs
+++ b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs
@@ -1,5 +1,5 @@
 #[cfg(test)]
-#[ignore]
+#[Ignore]
 fn foo() {}
 
 #[path = "a.rs"]
diff --git a/xtask/src/tidy.rs b/xtask/src/tidy.rs
index f2ba8efef..25ddb43b2 100644
--- a/xtask/src/tidy.rs
+++ b/xtask/src/tidy.rs
@@ -89,6 +89,7 @@ fn rust_files_are_tidy() {
         let text = read_file(&path).unwrap();
         check_todo(&path, &text);
         check_dbg(&path, &text);
+        check_test_attrs(&path, &text);
         check_trailing_ws(&path, &text);
         deny_clippy(&path, &text);
         tidy_docs.visit(&path, &text);
@@ -334,6 +335,36 @@ fn check_dbg(path: &Path, text: &str) {
     }
 }
 
+fn check_test_attrs(path: &Path, text: &str) {
+    let ignore_rule =
+        "https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#ignore";
+    let need_ignore: &[&str] = &[
+        // Special case to run `#[ignore]` tests
+        "ide/src/runnables.rs",
+        // A legit test which needs to be ignored, as it takes too long to run
+        // :(
+        "hir_def/src/nameres/collector.rs",
+        // Obviously needs ignore.
+        "ide_assists/src/handlers/toggle_ignore.rs",
+        // See above.
+        "ide_assists/src/tests/generated.rs",
+    ];
+    if text.contains("#[ignore") && !need_ignore.iter().any(|p| path.ends_with(p)) {
+        panic!("\ndon't `#[ignore]` tests, see:\n\n    {}\n\n   {}\n", ignore_rule, path.display(),)
+    }
+
+    let panic_rule =
+        "https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#panic";
+    let need_panic: &[&str] = &["test_utils/src/fixture.rs"];
+    if text.contains("#[should_panic") && !need_panic.iter().any(|p| path.ends_with(p)) {
+        panic!(
+            "\ndon't add `#[should_panic]` tests, see:\n\n    {}\n\n   {}\n",
+            panic_rule,
+            path.display(),
+        )
+    }
+}
+
 fn check_trailing_ws(path: &Path, text: &str) {
     if is_exclude_dir(path, &["test_data"]) {
         return;