Merge #5373

5373: Clenup diagnostics tests r=matklad a=matklad

bors r+
🤖

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

6 files changed, 278 insertions, 474 deletions

diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index 3870c6d9c..d3ee9cf55 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -246,25 +246,233 @@ impl AstDiagnostic for MismatchedArgCount {
 }
 
 #[cfg(test)]
-fn check_diagnostics(ra_fixture: &str) {
-    use ra_db::{fixture::WithFixture, FileId};
-    use ra_syntax::TextRange;
+mod tests {
+    use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId};
+    use hir_expand::diagnostics::{Diagnostic, DiagnosticSink};
+    use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
+    use ra_syntax::{TextRange, TextSize};
     use rustc_hash::FxHashMap;
 
-    use crate::test_db::TestDB;
+    use crate::{diagnostics::validate_body, test_db::TestDB};
+
+    impl TestDB {
+        fn diagnostics<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) {
+            let crate_graph = self.crate_graph();
+            for krate in crate_graph.iter() {
+                let crate_def_map = self.crate_def_map(krate);
+
+                let mut fns = Vec::new();
+                for (module_id, _) in crate_def_map.modules.iter() {
+                    for decl in crate_def_map[module_id].scope.declarations() {
+                        if let ModuleDefId::FunctionId(f) = decl {
+                            fns.push(f)
+                        }
+                    }
+
+                    for impl_id in crate_def_map[module_id].scope.impls() {
+                        let impl_data = self.impl_data(impl_id);
+                        for item in impl_data.items.iter() {
+                            if let AssocItemId::FunctionId(f) = item {
+                                fns.push(*f)
+                            }
+                        }
+                    }
+                }
+
+                for f in fns {
+                    let mut sink = DiagnosticSink::new(&mut cb);
+                    validate_body(self, f.into(), &mut sink);
+                }
+            }
+        }
+    }
+
+    pub(crate) fn check_diagnostics(ra_fixture: &str) {
+        let db = TestDB::with_files(ra_fixture);
+        let annotations = db.extract_annotations();
+
+        let mut actual: FxHashMap<FileId, Vec<(TextRange, String)>> = FxHashMap::default();
+        db.diagnostics(|d| {
+            // FXIME: macros...
+            let file_id = d.source().file_id.original_file(&db);
+            let range = d.syntax_node(&db).text_range();
+            let message = d.message().to_owned();
+            actual.entry(file_id).or_default().push((range, message));
+        });
+
+        for (file_id, diags) in actual.iter_mut() {
+            diags.sort_by_key(|it| it.0.start());
+            let text = db.file_text(*file_id);
+            // For multiline spans, place them on line start
+            for (range, content) in diags {
+                if text[*range].contains('\n') {
+                    *range = TextRange::new(range.start(), range.start() + TextSize::from(1));
+                    *content = format!("... {}", content);
+                }
+            }
+        }
+
+        assert_eq!(annotations, actual);
+    }
+
+    #[test]
+    fn no_such_field_diagnostics() {
+        check_diagnostics(
+            r#"
+struct S { foo: i32, bar: () }
+impl S {
+    fn new() -> S {
+        S {
+        //^... Missing structure fields:
+        //|    - bar
+            foo: 92,
+            baz: 62,
+          //^^^^^^^ no such field
+        }
+    }
+}
+"#,
+        );
+    }
+    #[test]
+    fn no_such_field_with_feature_flag_diagnostics() {
+        check_diagnostics(
+            r#"
+//- /lib.rs crate:foo cfg:feature=foo
+struct MyStruct {
+    my_val: usize,
+    #[cfg(feature = "foo")]
+    bar: bool,
+}
+
+impl MyStruct {
+    #[cfg(feature = "foo")]
+    pub(crate) fn new(my_val: usize, bar: bool) -> Self {
+        Self { my_val, bar }
+    }
+    #[cfg(not(feature = "foo"))]
+    pub(crate) fn new(my_val: usize, _bar: bool) -> Self {
+        Self { my_val }
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn no_such_field_enum_with_feature_flag_diagnostics() {
+        check_diagnostics(
+            r#"
+//- /lib.rs crate:foo cfg:feature=foo
+enum Foo {
+    #[cfg(not(feature = "foo"))]
+    Buz,
+    #[cfg(feature = "foo")]
+    Bar,
+    Baz
+}
+
+fn test_fn(f: Foo) {
+    match f {
+        Foo::Bar => {},
+        Foo::Baz => {},
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn no_such_field_with_feature_flag_diagnostics_on_struct_lit() {
+        check_diagnostics(
+            r#"
+//- /lib.rs crate:foo cfg:feature=foo
+struct S {
+    #[cfg(feature = "foo")]
+    foo: u32,
+    #[cfg(not(feature = "foo"))]
+    bar: u32,
+}
+
+impl S {
+    #[cfg(feature = "foo")]
+    fn new(foo: u32) -> Self {
+        Self { foo }
+    }
+    #[cfg(not(feature = "foo"))]
+    fn new(bar: u32) -> Self {
+        Self { bar }
+    }
+    fn new2(bar: u32) -> Self {
+        #[cfg(feature = "foo")]
+        { Self { foo: bar } }
+        #[cfg(not(feature = "foo"))]
+        { Self { bar } }
+    }
+    fn new2(val: u32) -> Self {
+        Self {
+            #[cfg(feature = "foo")]
+            foo: val,
+            #[cfg(not(feature = "foo"))]
+            bar: val,
+        }
+    }
+}
+"#,
+        );
+    }
 
-    let db = TestDB::with_files(ra_fixture);
-    let annotations = db.extract_annotations();
+    #[test]
+    fn no_such_field_with_type_macro() {
+        check_diagnostics(
+            r#"
+macro_rules! Type { () => { u32 }; }
+struct Foo { bar: Type![] }
 
-    let mut actual: FxHashMap<FileId, Vec<(TextRange, String)>> = FxHashMap::default();
-    db.diag(|d| {
-        // FXIME: macros...
-        let file_id = d.source().file_id.original_file(&db);
-        let range = d.syntax_node(&db).text_range();
-        let message = d.message().to_owned();
-        actual.entry(file_id).or_default().push((range, message));
-    });
-    actual.values_mut().for_each(|diags| diags.sort_by_key(|it| it.0.start()));
+impl Foo {
+    fn new() -> Self {
+        Foo { bar: 0 }
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn missing_record_pat_field_diagnostic() {
+        check_diagnostics(
+            r#"
+struct S { foo: i32, bar: () }
+fn baz(s: S) {
+    let S { foo: _ } = s;
+        //^^^^^^^^^^ Missing structure fields:
+        //         | - bar
+}
+"#,
+        );
+    }
 
-    assert_eq!(annotations, actual);
+    #[test]
+    fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() {
+        check_diagnostics(
+            r"
+struct S { foo: i32, bar: () }
+fn baz(s: S) -> i32 {
+    match s {
+        S { foo, .. } => foo,
+    }
+}
+",
+        )
+    }
+
+    #[test]
+    fn break_outside_of_loop() {
+        check_diagnostics(
+            r#"
+fn foo() { break; }
+         //^^^^^ break outside of loop
+"#,
+        );
+    }
 }
diff --git a/crates/ra_hir_ty/src/diagnostics/expr.rs b/crates/ra_hir_ty/src/diagnostics/expr.rs
index 277ace180..91caedc3d 100644
--- a/crates/ra_hir_ty/src/diagnostics/expr.rs
+++ b/crates/ra_hir_ty/src/diagnostics/expr.rs
@@ -376,7 +376,7 @@ pub fn record_pattern_missing_fields(
 
 #[cfg(test)]
 mod tests {
-    use crate::diagnostics::check_diagnostics;
+    use crate::diagnostics::tests::check_diagnostics;
 
     #[test]
     fn simple_free_fn_zero() {
diff --git a/crates/ra_hir_ty/src/diagnostics/match_check.rs b/crates/ra_hir_ty/src/diagnostics/match_check.rs
index 95f272811..507edcb7d 100644
--- a/crates/ra_hir_ty/src/diagnostics/match_check.rs
+++ b/crates/ra_hir_ty/src/diagnostics/match_check.rs
@@ -837,7 +837,7 @@ fn enum_variant_matches(cx: &MatchCheckCtx, pat_id: PatId, enum_variant_id: Enum
 
 #[cfg(test)]
 mod tests {
-    use crate::diagnostics::check_diagnostics;
+    use crate::diagnostics::tests::check_diagnostics;
 
     #[test]
     fn empty_tuple() {
diff --git a/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs b/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
index b8ff95ee1..9e4ed9a8b 100644
--- a/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
+++ b/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
@@ -121,3 +121,53 @@ fn walk_unsafe(
         walk_unsafe(unsafe_exprs, db, infer, body, child, inside_unsafe_block);
     });
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostics::tests::check_diagnostics;
+
+    #[test]
+    fn missing_unsafe_diagnostic_with_raw_ptr() {
+        check_diagnostics(
+            r#"
+fn main() {
+    let x = &5 as *const usize;
+    unsafe { let y = *x; }
+    let z = *x;
+}         //^^ This operation is unsafe and requires an unsafe function or block
+"#,
+        )
+    }
+
+    #[test]
+    fn missing_unsafe_diagnostic_with_unsafe_call() {
+        check_diagnostics(
+            r#"
+struct HasUnsafe;
+
+impl HasUnsafe {
+    unsafe fn unsafe_fn(&self) {
+        let x = &5 as *const usize;
+        let y = *x;
+    }
+}
+
+unsafe fn unsafe_fn() {
+    let x = &5 as *const usize;
+    let y = *x;
+}
+
+fn main() {
+    unsafe_fn();
+  //^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+    HasUnsafe.unsafe_fn();
+  //^^^^^^^^^^^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+    unsafe {
+        unsafe_fn();
+        HasUnsafe.unsafe_fn();
+    }
+}
+"#,
+        );
+    }
+}
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index fb8723fb7..a1714ff0f 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -5,19 +5,13 @@ use std::{
     sync::{Arc, Mutex},
 };
 
-use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId, ModuleId};
-use hir_expand::{
-    db::AstDatabase,
-    diagnostics::{Diagnostic, DiagnosticSink},
-};
+use hir_def::{db::DefDatabase, ModuleId};
+use hir_expand::db::AstDatabase;
 use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, SourceDatabase, Upcast};
 use ra_syntax::TextRange;
 use rustc_hash::{FxHashMap, FxHashSet};
-use stdx::format_to;
 use test_utils::extract_annotations;
 
-use crate::diagnostics::validate_body;
-
 #[salsa::database(
     ra_db::SourceDatabaseExtStorage,
     ra_db::SourceDatabaseStorage,
@@ -94,46 +88,6 @@ impl TestDB {
         panic!("Can't find module for file")
     }
 
-    pub(crate) fn diag<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) {
-        let crate_graph = self.crate_graph();
-        for krate in crate_graph.iter() {
-            let crate_def_map = self.crate_def_map(krate);
-
-            let mut fns = Vec::new();
-            for (module_id, _) in crate_def_map.modules.iter() {
-                for decl in crate_def_map[module_id].scope.declarations() {
-                    if let ModuleDefId::FunctionId(f) = decl {
-                        fns.push(f)
-                    }
-                }
-
-                for impl_id in crate_def_map[module_id].scope.impls() {
-                    let impl_data = self.impl_data(impl_id);
-                    for item in impl_data.items.iter() {
-                        if let AssocItemId::FunctionId(f) = item {
-                            fns.push(*f)
-                        }
-                    }
-                }
-            }
-
-            for f in fns {
-                let mut sink = DiagnosticSink::new(&mut cb);
-                validate_body(self, f.into(), &mut sink);
-            }
-        }
-    }
-
-    pub(crate) fn diagnostics(&self) -> (String, u32) {
-        let mut buf = String::new();
-        let mut count = 0;
-        self.diag(|d| {
-            format_to!(buf, "{:?}: {}\n", d.syntax_node(self).text(), d.message());
-            count += 1;
-        });
-        (buf, count)
-    }
-
     pub(crate) fn extract_annotations(&self) -> FxHashMap<FileId, Vec<(TextRange, String)>> {
         let mut files = Vec::new();
         let crate_graph = self.crate_graph();
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 27f5a60bf..d57b3f288 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -20,7 +20,6 @@ use hir_def::{
     AssocItemId, DefWithBodyId, LocalModuleId, Lookup, ModuleDefId,
 };
 use hir_expand::{db::AstDatabase, InFile};
-use insta::assert_snapshot;
 use ra_db::{fixture::WithFixture, FileRange, SourceDatabase, SourceDatabaseExt};
 use ra_syntax::{
     algo,
@@ -341,410 +340,3 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
         assert!(!format!("{:?}", events).contains("infer"), "{:#?}", events)
     }
 }
-
-#[test]
-fn no_such_field_diagnostics() {
-    let diagnostics = TestDB::with_files(
-        r"
-        //- /lib.rs
-        struct S { foo: i32, bar: () }
-        impl S {
-            fn new() -> S {
-                S {
-                    foo: 92,
-                    baz: 62,
-                }
-            }
-        }
-        ",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###"
-    "baz: 62": no such field
-    "{\n            foo: 92,\n            baz: 62,\n        }": Missing structure fields:
-    - bar
-    "###
-    );
-}
-
-#[test]
-fn no_such_field_with_feature_flag_diagnostics() {
-    let diagnostics = TestDB::with_files(
-        r#"
-        //- /lib.rs crate:foo cfg:feature=foo
-        struct MyStruct {
-            my_val: usize,
-            #[cfg(feature = "foo")]
-            bar: bool,
-        }
-
-        impl MyStruct {
-            #[cfg(feature = "foo")]
-            pub(crate) fn new(my_val: usize, bar: bool) -> Self {
-                Self { my_val, bar }
-            }
-
-            #[cfg(not(feature = "foo"))]
-            pub(crate) fn new(my_val: usize, _bar: bool) -> Self {
-                Self { my_val }
-            }
-        }
-        "#,
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""###);
-}
-
-#[test]
-fn no_such_field_enum_with_feature_flag_diagnostics() {
-    let diagnostics = TestDB::with_files(
-        r#"
-        //- /lib.rs crate:foo cfg:feature=foo
-        enum Foo {
-            #[cfg(not(feature = "foo"))]
-            Buz,
-            #[cfg(feature = "foo")]
-            Bar,
-            Baz
-        }
-
-        fn test_fn(f: Foo) {
-            match f {
-                Foo::Bar => {},
-                Foo::Baz => {},
-            }
-        }
-        "#,
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""###);
-}
-
-#[test]
-fn no_such_field_with_feature_flag_diagnostics_on_struct_lit() {
-    let diagnostics = TestDB::with_files(
-        r#"
-        //- /lib.rs crate:foo cfg:feature=foo
-        struct S {
-            #[cfg(feature = "foo")]
-            foo: u32,
-            #[cfg(not(feature = "foo"))]
-            bar: u32,
-        }
-
-        impl S {
-            #[cfg(feature = "foo")]
-            fn new(foo: u32) -> Self {
-                Self { foo }
-            }
-            #[cfg(not(feature = "foo"))]
-            fn new(bar: u32) -> Self {
-                Self { bar }
-            }
-        }
-        "#,
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""###);
-}
-
-#[test]
-fn no_such_field_with_feature_flag_diagnostics_on_block_expr() {
-    let diagnostics = TestDB::with_files(
-        r#"
-        //- /lib.rs crate:foo cfg:feature=foo
-        struct S {
-            #[cfg(feature = "foo")]
-            foo: u32,
-            #[cfg(not(feature = "foo"))]
-            bar: u32,
-        }
-
-        impl S {
-            fn new(bar: u32) -> Self {
-                #[cfg(feature = "foo")]
-                {
-                Self { foo: bar }
-                }
-                #[cfg(not(feature = "foo"))]
-                {
-                Self { bar }
-                }
-            }
-        }
-        "#,
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""###);
-}
-
-#[test]
-fn no_such_field_with_feature_flag_diagnostics_on_struct_fields() {
-    let diagnostics = TestDB::with_files(
-        r#"
-        //- /lib.rs crate:foo cfg:feature=foo
-        struct S {
-            #[cfg(feature = "foo")]
-            foo: u32,
-            #[cfg(not(feature = "foo"))]
-            bar: u32,
-        }
-
-        impl S {
-            fn new(val: u32) -> Self {
-                Self {
-                    #[cfg(feature = "foo")]
-                    foo: val,
-                    #[cfg(not(feature = "foo"))]
-                    bar: val,
-                }
-            }
-        }
-        "#,
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""###);
-}
-
-#[test]
-fn no_such_field_with_type_macro() {
-    let diagnostics = TestDB::with_files(
-        r"
-        macro_rules! Type {
-            () => { u32 };
-        }
-
-        struct Foo {
-            bar: Type![],
-        }
-        impl Foo {
-            fn new() -> Self {
-                Foo { bar: 0 }
-            }
-        }
-        ",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""###);
-}
-
-#[test]
-fn missing_record_pat_field_diagnostic() {
-    let diagnostics = TestDB::with_files(
-        r"
-        //- /lib.rs
-        struct S { foo: i32, bar: () }
-        fn baz(s: S) {
-            let S { foo: _ } = s;
-        }
-        ",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###"
-    "{ foo: _ }": Missing structure fields:
-    - bar
-    "###
-    );
-}
-
-#[test]
-fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() {
-    let diagnostics = TestDB::with_files(
-        r"
-        //- /lib.rs
-        struct S { foo: i32, bar: () }
-        fn baz(s: S) -> i32 {
-            match s {
-                S { foo, .. } => foo,
-            }
-        }
-        ",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @"");
-}
-
-#[test]
-fn missing_unsafe_diagnostic_with_raw_ptr() {
-    let diagnostics = TestDB::with_files(
-        r"
-//- /lib.rs
-fn missing_unsafe() {
-    let x = &5 as *const usize;
-    let y = *x;
-}
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#);
-}
-
-#[test]
-fn missing_unsafe_diagnostic_with_unsafe_call() {
-    let diagnostics = TestDB::with_files(
-        r"
-//- /lib.rs
-unsafe fn unsafe_fn() {
-    let x = &5 as *const usize;
-    let y = *x;
-}
-
-fn missing_unsafe() {
-    unsafe_fn();
-}
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r#""unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#);
-}
-
-#[test]
-fn missing_unsafe_diagnostic_with_unsafe_method_call() {
-    let diagnostics = TestDB::with_files(
-        r"
-struct HasUnsafe;
-
-impl HasUnsafe {
-    unsafe fn unsafe_fn(&self) {
-        let x = &5 as *const usize;
-        let y = *x;
-    }
-}
-
-fn missing_unsafe() {
-    HasUnsafe.unsafe_fn();
-}
-
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r#""HasUnsafe.unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#);
-}
-
-#[test]
-fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() {
-    let diagnostics = TestDB::with_files(
-        r"
-fn nothing_to_see_move_along() {
-    let x = &5 as *const usize;
-    unsafe {
-        let y = *x;
-    }
-}
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @"");
-}
-
-#[test]
-fn missing_unsafe_diagnostic_with_raw_ptr_outside_unsafe_block() {
-    let diagnostics = TestDB::with_files(
-        r"
-fn nothing_to_see_move_along() {
-    let x = &5 as *const usize;
-    unsafe {
-        let y = *x;
-    }
-    let z = *x;
-}
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#);
-}
-
-#[test]
-fn no_missing_unsafe_diagnostic_with_unsafe_call_in_unsafe_block() {
-    let diagnostics = TestDB::with_files(
-        r"
-unsafe fn unsafe_fn() {
-    let x = &5 as *const usize;
-    let y = *x;
-}
-
-fn nothing_to_see_move_along() {
-    unsafe {
-        unsafe_fn();
-    }
-}
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @"");
-}
-
-#[test]
-fn no_missing_unsafe_diagnostic_with_unsafe_method_call_in_unsafe_block() {
-    let diagnostics = TestDB::with_files(
-        r"
-struct HasUnsafe;
-
-impl HasUnsafe {
-    unsafe fn unsafe_fn() {
-        let x = &5 as *const usize;
-        let y = *x;
-    }
-}
-
-fn nothing_to_see_move_along() {
-    unsafe {
-        HasUnsafe.unsafe_fn();
-    }
-}
-
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @"");
-}
-
-#[test]
-fn break_outside_of_loop() {
-    let diagnostics = TestDB::with_files(
-        r"
-        //- /lib.rs
-        fn foo() {
-            break;
-        }
-        ",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r###""break": break outside of loop
-    "###
-    );
-}