internal: document that we don't #[ignore] tests

8 files changed, 86 insertions, 71 deletions

diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 6fab58f15..4ae02e576 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -1992,8 +1992,8 @@ mod tests {
         collector.def_map
     }
 
-    fn do_resolve(code: &str) -> DefMap {
-        let (db, _file_id) = TestDB::with_single_file(code);
+    fn do_resolve(not_ra_fixture: &str) -> DefMap {
+        let (db, _file_id) = TestDB::with_single_file(not_ra_fixture);
         let krate = db.test_crate();
 
         let edition = db.crate_graph()[krate].edition;
@@ -2013,16 +2013,21 @@ mod tests {
         );
     }
 
-    #[ignore] // this test does succeed, but takes quite a while :/
+    #[ignore]
     #[test]
     fn test_macro_expand_will_stop_2() {
+        // FIXME: this test does succeed, but takes quite a while: 90 seconds in
+        // the release mode. That's why the argument is not an ra_fixture --
+        // otherwise injection highlighting gets stuck.
+        //
+        // We need to find a way to fail this faster.
         do_resolve(
             r#"
-        macro_rules! foo {
-            ($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
-        }
-        foo!(KABOOM);
-        "#,
+macro_rules! foo {
+    ($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
+}
+foo!(KABOOM);
+"#,
         );
     }
 }
diff --git a/crates/hir_ty/src/tests/coercion.rs b/crates/hir_ty/src/tests/coercion.rs
index 71047703d..4f859fc85 100644
--- a/crates/hir_ty/src/tests/coercion.rs
+++ b/crates/hir_ty/src/tests/coercion.rs
@@ -741,10 +741,24 @@ fn coerce_unsize_trait_object_simple() {
 }
 
 #[test]
-// The rust reference says this should be possible, but rustc doesn't implement
-// it. We used to support it, but Chalk doesn't.
-#[ignore]
 fn coerce_unsize_trait_object_to_trait_object() {
+    // FIXME: The rust reference says this should be possible, but rustc doesn't
+    // implement it. We used to support it, but Chalk doesn't. Here's the
+    // correct expect:
+    //
+    //     424..609 '{     ...bj2; }': ()
+    //     434..437 'obj': &dyn Baz<i8, i16>
+    //     459..461 '&S': &S<i8, i16>
+    //     460..461 'S': S<i8, i16>
+    //     471..474 'obj': &dyn Bar<usize, i8, i16>
+    //     496..499 'obj': &dyn Baz<i8, i16>
+    //     509..512 'obj': &dyn Foo<i8, usize>
+    //     531..534 'obj': &dyn Bar<usize, i8, i16>
+    //     544..548 'obj2': &dyn Baz<i8, i16>
+    //     570..572 '&S': &S<i8, i16>
+    //     571..572 'S': S<i8, i16>
+    //     582..583 '_': &dyn Foo<i8, usize>
+    //     602..606 'obj2': &dyn Baz<i8, i16>
     check_infer_with_mismatches(
         r#"
         #[lang = "sized"]
@@ -773,21 +787,24 @@ fn coerce_unsize_trait_object_to_trait_object() {
             let _: &dyn Foo<_, _> = obj2;
         }
         "#,
-        expect![[r"
+        expect![[r#"
             424..609 '{     ...bj2; }': ()
             434..437 'obj': &dyn Baz<i8, i16>
             459..461 '&S': &S<i8, i16>
             460..461 'S': S<i8, i16>
-            471..474 'obj': &dyn Bar<usize, i8, i16>
+            471..474 'obj': &dyn Bar<{unknown}, {unknown}, {unknown}>
             496..499 'obj': &dyn Baz<i8, i16>
-            509..512 'obj': &dyn Foo<i8, usize>
-            531..534 'obj': &dyn Bar<usize, i8, i16>
+            509..512 'obj': &dyn Foo<{unknown}, {unknown}>
+            531..534 'obj': &dyn Bar<{unknown}, {unknown}, {unknown}>
             544..548 'obj2': &dyn Baz<i8, i16>
             570..572 '&S': &S<i8, i16>
             571..572 'S': S<i8, i16>
-            582..583 '_': &dyn Foo<i8, usize>
+            582..583 '_': &dyn Foo<{unknown}, {unknown}>
             602..606 'obj2': &dyn Baz<i8, i16>
-        "]],
+            496..499: expected &dyn Bar<{unknown}, {unknown}, {unknown}>, got &dyn Baz<i8, i16>
+            531..534: expected &dyn Foo<{unknown}, {unknown}>, got &dyn Bar<{unknown}, {unknown}, {unknown}>
+            602..606: expected &dyn Foo<{unknown}, {unknown}>, got &dyn Baz<i8, i16>
+        "#]],
     );
 }
 
diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs
index 6bcede4c4..c830e576e 100644
--- a/crates/hir_ty/src/tests/traits.rs
+++ b/crates/hir_ty/src/tests/traits.rs
@@ -1475,7 +1475,6 @@ fn test(
 }
 
 #[test]
-#[ignore]
 fn error_bound_chalk() {
     check_types(
         r#"
diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs
index 57ae9455b..7ac0118fe 100644
--- a/crates/ide/src/doc_links.rs
+++ b/crates/ide/src/doc_links.rs
@@ -241,6 +241,10 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option<String> {
         Definition::ModuleDef(ModuleDef::Module(module)) => module.krate(),
         _ => definition.module(db)?.krate(),
     };
+    // FIXME: using import map doesn't make sense here. What we want here is
+    // canonical path. What import map returns is the shortest path suitable for
+    // import. See this test:
+    cov_mark::hit!(test_reexport_order);
     let import_map = db.import_map(krate.into());
 
     let mut base = krate.display_name(db)?.to_string();
@@ -642,13 +646,15 @@ pub mod foo {
         )
     }
 
-    // FIXME: ImportMap will return re-export paths instead of public module
-    // paths. The correct path to documentation will never be a re-export.
-    // This problem stops us from resolving stdlib items included in the prelude
-    // such as `Option::Some` correctly.
-    #[ignore = "ImportMap may return re-exports"]
     #[test]
     fn test_reexport_order() {
+        cov_mark::check!(test_reexport_order);
+        // FIXME: This should return
+        //
+        //    https://docs.rs/test/*/test/wrapper/modulestruct.Item.html
+        //
+        // That is, we should point inside the module, rather than at the
+        // re-export.
         check(
             r#"
 pub mod wrapper {
@@ -663,7 +669,7 @@ fn foo() {
     let bar: wrapper::It$0em;
 }
         "#,
-            expect![[r#"https://docs.rs/test/*/test/wrapper/module/struct.Item.html"#]],
+            expect![[r#"https://docs.rs/test/*/test/wrapper/struct.Item.html"#]],
         )
     }
 }
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index c08516805..afeded315 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -1821,9 +1821,10 @@ pub struct B$0ar
         );
     }
 
-    #[ignore = "path based links currently only support documentation on ModuleDef items"]
     #[test]
     fn test_hover_path_link_field() {
+        // FIXME: Should be
+        //  [Foo](https://docs.rs/test/*/test/struct.Foo.html)
         check(
             r#"
 pub struct Foo;
@@ -1845,7 +1846,7 @@ pub struct Bar {
 
                 ---
 
-                [Foo](https://docs.rs/test/*/test/struct.Foo.html)
+                [Foo](struct.Foo.html)
             "#]],
         );
     }
diff --git a/crates/ide_assists/src/handlers/fix_visibility.rs b/crates/ide_assists/src/handlers/fix_visibility.rs
index 9b432e92f..2f2b605fc 100644
--- a/crates/ide_assists/src/handlers/fix_visibility.rs
+++ b/crates/ide_assists/src/handlers/fix_visibility.rs
@@ -583,25 +583,25 @@ pub struct Foo { pub(crate) bar: () }
     }
 
     #[test]
-    #[ignore]
-    // FIXME handle reexports properly
     fn fix_visibility_of_reexport() {
+        // FIXME: broken test, this should fix visibility of the re-export
+        // rather than the struct.
         check_assist(
             fix_visibility,
-            r"
-            mod foo {
-                use bar::Baz;
-                mod bar { pub(super) struct Baz; }
-            }
-            foo::Baz$0
-            ",
-            r"
-            mod foo {
-                $0pub(crate) use bar::Baz;
-                mod bar { pub(super) struct Baz; }
-            }
-            foo::Baz
-            ",
+            r#"
+mod foo {
+    use bar::Baz;
+    mod bar { pub(super) struct Baz; }
+}
+foo::Baz$0
+"#,
+            r#"
+mod foo {
+    use bar::Baz;
+    mod bar { $0pub(crate) struct Baz; }
+}
+foo::Baz
+"#,
         )
     }
 }
diff --git a/crates/ide_diagnostics/src/handlers/incorrect_case.rs b/crates/ide_diagnostics/src/handlers/incorrect_case.rs
index 72f251961..68f25f284 100644
--- a/crates/ide_diagnostics/src/handlers/incorrect_case.rs
+++ b/crates/ide_diagnostics/src/handlers/incorrect_case.rs
@@ -341,43 +341,27 @@ mod F {
     }
 
     #[test]
-    #[ignore]
-    fn bug_trait_inside_fn() {
-        // FIXME:
-        // This is broken, and in fact, should not even be looked at by this
-        // lint in the first place. There's weird stuff going on in the
-        // collection phase.
-        // It's currently being brought in by:
-        // * validate_func on `a` recursing into modules
-        // * then it finds the trait and then the function while iterating
-        //   through modules
-        // * then validate_func is called on Dirty
-        // * ... which then proceeds to look at some unknown module taking no
-        //   attrs from either the impl or the fn a, and then finally to the root
-        //   module
-        //
-        // It should find the attribute on the trait, but it *doesn't even see
-        // the trait* as far as I can tell.
-
+    fn complex_ignore() {
+        // FIXME: this should trigger errors for the second case.
         check_diagnostics(
             r#"
 trait T { fn a(); }
 struct U {}
 impl T for U {
     fn a() {
-        // this comes out of bitflags, mostly
         #[allow(non_snake_case)]
-        trait __BitFlags {
+        trait __BitFlagsOk {
             const HiImAlsoBad: u8 = 2;
-            #[inline]
-            fn Dirty(&self) -> bool {
-                false
-            }
+            fn Dirty(&self) -> bool { false }
         }
 
+        trait __BitFlagsBad {
+            const HiImAlsoBad: u8 = 2;
+            fn Dirty(&self) -> bool { false }
+        }
     }
 }
-    "#,
+"#,
         );
     }
 
@@ -414,18 +398,14 @@ extern {
     }
 
     #[test]
-    #[ignore]
     fn bug_traits_arent_checked() {
         // FIXME: Traits and functions in traits aren't currently checked by
         // r-a, even though rustc will complain about them.
         check_diagnostics(
             r#"
 trait BAD_TRAIT {
-    // ^^^^^^^^^ 💡 weak: Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait`
     fn BAD_FUNCTION();
-    // ^^^^^^^^^^^^ 💡 weak: Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function`
     fn BadFunction();
-    // ^^^^^^^^^^^^ 💡 weak: Function `BadFunction` should have snake_case name, e.g. `bad_function`
 }
     "#,
         );
diff --git a/docs/dev/style.md b/docs/dev/style.md
index 96dd684b3..84485ea28 100644
--- a/docs/dev/style.md
+++ b/docs/dev/style.md
@@ -174,6 +174,13 @@ Instead, explicitly check for `None`, `Err`, etc.
 `rust-analyzer` is not a library, we don't need to test for API misuse, and we have to handle any user input without panics.
 Panic messages in the logs from the `#[should_panic]` tests are confusing.
 
+## `#[ignore]`
+
+Do not `#[ignore]` tests.
+If the test currently does not work, assert the wrong behavior and add a fixme explaining why it is wrong.
+
+**Rationale:** noticing when the behavior is fixed, making sure that even the wrong behavior is acceptable (ie, not a panic).
+
 ## Function Preconditions
 
 Express function preconditions in types and force the caller to provide them (rather than checking in callee):