Merge #784

784: WIP: improve multi-crate fixtures r=matklad a=matklad



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

3 files changed, 146 insertions, 116 deletions

diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index a9cd955cf..8ed8c4bdc 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -18,6 +18,7 @@ macro_rules! impl_froms {
 }
 
 pub mod db;
+#[macro_use]
 pub mod mock;
 mod query_definitions;
 mod path;
diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs
index 950f89948..5ca870867 100644
--- a/crates/ra_hir/src/mock.rs
+++ b/crates/ra_hir/src/mock.rs
@@ -6,6 +6,7 @@ use ra_db::{
 };
 use relative_path::RelativePathBuf;
 use test_utils::{parse_fixture, CURSOR_MARKER, extract_offset};
+use rustc_hash::FxHashMap;
 
 use crate::{db, HirInterner};
 
@@ -21,82 +22,125 @@ pub struct MockDatabase {
     events: Mutex<Option<Vec<salsa::Event<MockDatabase>>>>,
     runtime: salsa::Runtime<MockDatabase>,
     interner: Arc<HirInterner>,
-    file_counter: u32,
+    files: FxHashMap<String, FileId>,
 }
 
 impl panic::RefUnwindSafe for MockDatabase {}
 
 impl MockDatabase {
-    pub fn with_files(fixture: &str) -> (MockDatabase, SourceRoot) {
-        let (db, source_root, position) = MockDatabase::from_fixture(fixture);
+    pub fn with_files(fixture: &str) -> MockDatabase {
+        let (db, position) = MockDatabase::from_fixture(fixture);
         assert!(position.is_none());
-        (db, source_root)
+        db
     }
 
     pub fn with_single_file(text: &str) -> (MockDatabase, SourceRoot, FileId) {
         let mut db = MockDatabase::default();
         let mut source_root = SourceRoot::default();
-        let file_id = db.add_file(WORKSPACE, &mut source_root, "/main.rs", text);
+        let file_id = db.add_file(WORKSPACE, "/", &mut source_root, "/main.rs", text);
         db.set_source_root(WORKSPACE, Arc::new(source_root.clone()));
         (db, source_root, file_id)
     }
 
     pub fn with_position(fixture: &str) -> (MockDatabase, FilePosition) {
-        let (db, _, position) = MockDatabase::from_fixture(fixture);
+        let (db, position) = MockDatabase::from_fixture(fixture);
         let position = position.expect("expected a marker ( <|> )");
         (db, position)
     }
 
-    fn from_fixture(fixture: &str) -> (MockDatabase, SourceRoot, Option<FilePosition>) {
+    pub fn file_id_of(&self, path: &str) -> FileId {
+        match self.files.get(path) {
+            Some(it) => *it,
+            None => panic!("unknown file: {:?}\nexisting files:\n{:#?}", path, self.files),
+        }
+    }
+
+    pub fn set_crate_graph_from_fixture(&mut self, graph: CrateGraphFixture) {
+        let mut ids = FxHashMap::default();
+        let mut crate_graph = CrateGraph::default();
+        for (crate_name, (crate_root, _)) in graph.0.iter() {
+            let crate_root = self.file_id_of(&crate_root);
+            let crate_id = crate_graph.add_crate_root(crate_root);
+            ids.insert(crate_name, crate_id);
+        }
+        for (crate_name, (_, deps)) in graph.0.iter() {
+            let from = ids[crate_name];
+            for dep in deps {
+                let to = ids[dep];
+                crate_graph.add_dep(from, dep.as_str().into(), to).unwrap();
+            }
+        }
+        self.set_crate_graph(Arc::new(crate_graph))
+    }
+
+    fn from_fixture(fixture: &str) -> (MockDatabase, Option<FilePosition>) {
         let mut db = MockDatabase::default();
 
-        let (source_root, pos) = db.add_fixture(WORKSPACE, fixture);
+        let pos = db.add_fixture(fixture);
 
-        (db, source_root, pos)
+        (db, pos)
     }
 
-    pub fn add_fixture(
-        &mut self,
-        source_root_id: SourceRootId,
-        fixture: &str,
-    ) -> (SourceRoot, Option<FilePosition>) {
+    fn add_fixture(&mut self, fixture: &str) -> Option<FilePosition> {
         let mut position = None;
         let mut source_root = SourceRoot::default();
+        let mut source_root_id = WORKSPACE;
+        let mut source_root_prefix = "/".to_string();
         for entry in parse_fixture(fixture) {
+            if entry.meta.starts_with("root") {
+                self.set_source_root(source_root_id, Arc::new(source_root));
+                source_root = SourceRoot::default();
+
+                source_root_id = SourceRootId(source_root_id.0 + 1);
+                source_root_prefix = entry.meta["root".len()..].trim().to_string();
+                continue;
+            }
             if entry.text.contains(CURSOR_MARKER) {
                 assert!(position.is_none(), "only one marker (<|>) per fixture is allowed");
                 position = Some(self.add_file_with_position(
                     source_root_id,
+                    &source_root_prefix,
                     &mut source_root,
                     &entry.meta,
                     &entry.text,
                 ));
             } else {
-                self.add_file(source_root_id, &mut source_root, &entry.meta, &entry.text);
+                self.add_file(
+                    source_root_id,
+                    &source_root_prefix,
+                    &mut source_root,
+                    &entry.meta,
+                    &entry.text,
+                );
             }
         }
-        self.set_source_root(source_root_id, Arc::new(source_root.clone()));
-        (source_root, position)
+        self.set_source_root(source_root_id, Arc::new(source_root));
+        position
     }
 
     fn add_file(
         &mut self,
         source_root_id: SourceRootId,
+        source_root_prefix: &str,
         source_root: &mut SourceRoot,
         path: &str,
         text: &str,
     ) -> FileId {
-        assert!(path.starts_with('/'));
-        let is_crate_root = path == "/lib.rs" || path == "/main.rs";
+        assert!(source_root_prefix.starts_with('/'));
+        assert!(source_root_prefix.ends_with('/'));
+        assert!(path.starts_with(source_root_prefix));
+        let rel_path = RelativePathBuf::from_path(&path[source_root_prefix.len()..]).unwrap();
+
+        let is_crate_root = rel_path == "lib.rs" || rel_path == "/main.rs";
 
-        let path = RelativePathBuf::from_path(&path[1..]).unwrap();
-        let file_id = FileId(self.file_counter);
-        self.file_counter += 1;
+        let file_id = FileId(self.files.len() as u32);
+        let prev = self.files.insert(path.to_string(), file_id);
+        assert!(prev.is_none(), "duplicate files in the text fixture");
         let text = Arc::new(text.to_string());
         self.set_file_text(file_id, text);
-        self.set_file_relative_path(file_id, path.clone());
+        self.set_file_relative_path(file_id, rel_path.clone());
         self.set_file_source_root(file_id, source_root_id);
-        source_root.files.insert(path, file_id);
+        source_root.files.insert(rel_path, file_id);
 
         if is_crate_root {
             let mut crate_graph = CrateGraph::default();
@@ -109,12 +153,13 @@ impl MockDatabase {
     fn add_file_with_position(
         &mut self,
         source_root_id: SourceRootId,
+        source_root_prefix: &str,
         source_root: &mut SourceRoot,
         path: &str,
         text: &str,
     ) -> FilePosition {
         let (offset, text) = extract_offset(text);
-        let file_id = self.add_file(source_root_id, source_root, path, &text);
+        let file_id = self.add_file(source_root_id, source_root_prefix, source_root, path, &text);
         FilePosition { file_id, offset }
     }
 }
@@ -138,7 +183,7 @@ impl Default for MockDatabase {
             events: Default::default(),
             runtime: salsa::Runtime::default(),
             interner: Default::default(),
-            file_counter: 0,
+            files: FxHashMap::default(),
         };
         db.set_crate_graph(Default::default());
         db
@@ -151,7 +196,8 @@ impl salsa::ParallelDatabase for MockDatabase {
             events: Default::default(),
             runtime: self.runtime.snapshot(self),
             interner: Arc::clone(&self.interner),
-            file_counter: self.file_counter,
+            // only the root database can be used to get file_id by path.
+            files: FxHashMap::default(),
         })
     }
 }
@@ -184,3 +230,20 @@ impl MockDatabase {
             .collect()
     }
 }
+
+#[derive(Default)]
+pub struct CrateGraphFixture(pub FxHashMap<String, (String, Vec<String>)>);
+
+#[macro_export]
+macro_rules! crate_graph {
+    ($($crate_name:literal: ($crate_path:literal, [$($dep:literal),*]),)*) => {{
+        let mut res = $crate::mock::CrateGraphFixture::default();
+        $(
+            res.0.insert(
+                $crate_name.to_string(),
+                ($crate_path.to_string(), vec![$($dep.to_string()),*])
+            );
+        )*
+        res
+    }}
+}
diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs
index 9c0e4ef29..9b621fbc2 100644
--- a/crates/ra_hir/src/nameres/tests.rs
+++ b/crates/ra_hir/src/nameres/tests.rs
@@ -1,7 +1,6 @@
 use std::sync::Arc;
 
-use ra_db::{CrateGraph, SourceRootId, SourceDatabase};
-use relative_path::RelativePath;
+use ra_db::SourceDatabase;
 use test_utils::{assert_eq_text, covers};
 
 use crate::{
@@ -20,20 +19,6 @@ fn item_map(fixture: &str) -> (Arc<ItemMap>, ModuleId) {
     (db.item_map(krate), module_id)
 }
 
-/// Sets the crate root to the file of the cursor marker
-fn item_map_custom_crate_root(fixture: &str) -> (Arc<ItemMap>, ModuleId) {
-    let (mut db, pos) = MockDatabase::with_position(fixture);
-
-    let mut crate_graph = CrateGraph::default();
-    crate_graph.add_crate_root(pos.file_id);
-    db.set_crate_graph(Arc::new(crate_graph));
-
-    let module = crate::source_binder::module_from_position(&db, pos).unwrap();
-    let krate = module.krate(&db).unwrap();
-    let module_id = module.module_id;
-    (db.item_map(krate), module_id)
-}
-
 fn check_module_item_map(map: &ItemMap, module_id: ModuleId, expected: &str) {
     let mut lines = map[module_id]
         .items
@@ -252,24 +237,20 @@ fn glob_enum() {
 #[test]
 fn glob_across_crates() {
     covers!(glob_across_crates);
-    let (mut db, sr) = MockDatabase::with_files(
+    let mut db = MockDatabase::with_files(
         "
         //- /main.rs
         use test_crate::*;
 
         //- /lib.rs
         pub struct Baz;
-    ",
+        ",
     );
-    let main_id = sr.files[RelativePath::new("/main.rs")];
-    let lib_id = sr.files[RelativePath::new("/lib.rs")];
-
-    let mut crate_graph = CrateGraph::default();
-    let main_crate = crate_graph.add_crate_root(main_id);
-    let lib_crate = crate_graph.add_crate_root(lib_id);
-    crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate).unwrap();
-
-    db.set_crate_graph(Arc::new(crate_graph));
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main.rs", ["test_crate"]),
+        "test_crate": ("/lib.rs", []),
+    });
+    let main_id = db.file_id_of("/main.rs");
 
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
     let krate = module.krate(&db).unwrap();
@@ -286,22 +267,31 @@ fn glob_across_crates() {
 
 #[test]
 fn module_resolution_works_for_non_standard_filenames() {
-    let (item_map, module_id) = item_map_custom_crate_root(
+    let mut db = MockDatabase::with_files(
         "
         //- /my_library.rs
         mod foo;
         use self::foo::Bar;
-        <|>
+
         //- /foo/mod.rs
         pub struct Bar;
-    ",
+        ",
     );
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "my_library": ("/my_library.rs", []),
+    });
+    let file_id = db.file_id_of("/my_library.rs");
+
+    let module = crate::source_binder::module_from_file_id(&db, file_id).unwrap();
+    let krate = module.krate(&db).unwrap();
+    let module_id = module.module_id;
+    let item_map = db.item_map(krate);
     check_module_item_map(
         &item_map,
         module_id,
         "
-            Bar: t v
-            foo: t
+        Bar: t v
+        foo: t
         ",
     );
 }
@@ -411,24 +401,20 @@ fn item_map_enum_importing() {
 
 #[test]
 fn item_map_across_crates() {
-    let (mut db, sr) = MockDatabase::with_files(
+    let mut db = MockDatabase::with_files(
         "
         //- /main.rs
         use test_crate::Baz;
 
         //- /lib.rs
         pub struct Baz;
-    ",
+        ",
     );
-    let main_id = sr.files[RelativePath::new("/main.rs")];
-    let lib_id = sr.files[RelativePath::new("/lib.rs")];
-
-    let mut crate_graph = CrateGraph::default();
-    let main_crate = crate_graph.add_crate_root(main_id);
-    let lib_crate = crate_graph.add_crate_root(lib_id);
-    crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate).unwrap();
-
-    db.set_crate_graph(Arc::new(crate_graph));
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main.rs", ["test_crate"]),
+        "test_crate": ("/lib.rs", []),
+    });
+    let main_id = db.file_id_of("/main.rs");
 
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
     let krate = module.krate(&db).unwrap();
@@ -438,14 +424,14 @@ fn item_map_across_crates() {
         &item_map,
         module.module_id,
         "
-            Baz: t v
+        Baz: t v
         ",
     );
 }
 
 #[test]
 fn extern_crate_rename() {
-    let (mut db, sr) = MockDatabase::with_files(
+    let mut db = MockDatabase::with_files(
         "
         //- /main.rs
         extern crate alloc as alloc_crate;
@@ -458,18 +444,13 @@ fn extern_crate_rename() {
 
         //- /lib.rs
         struct Arc;
-    ",
+        ",
     );
-    let main_id = sr.files[RelativePath::new("/main.rs")];
-    let sync_id = sr.files[RelativePath::new("/sync.rs")];
-    let lib_id = sr.files[RelativePath::new("/lib.rs")];
-
-    let mut crate_graph = CrateGraph::default();
-    let main_crate = crate_graph.add_crate_root(main_id);
-    let lib_crate = crate_graph.add_crate_root(lib_id);
-    crate_graph.add_dep(main_crate, "alloc".into(), lib_crate).unwrap();
-
-    db.set_crate_graph(Arc::new(crate_graph));
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main.rs", ["alloc"]),
+        "alloc": ("/lib.rs", []),
+    });
+    let sync_id = db.file_id_of("/sync.rs");
 
     let module = crate::source_binder::module_from_file_id(&db, sync_id).unwrap();
     let krate = module.krate(&db).unwrap();
@@ -479,14 +460,14 @@ fn extern_crate_rename() {
         &item_map,
         module.module_id,
         "
-            Arc: t v
+        Arc: t v
         ",
     );
 }
 
 #[test]
 fn import_across_source_roots() {
-    let (mut db, sr) = MockDatabase::with_files(
+    let mut db = MockDatabase::with_files(
         "
         //- /lib.rs
         pub mod a {
@@ -494,29 +475,18 @@ fn import_across_source_roots() {
                 pub struct C;
             }
         }
-    ",
-    );
-    let lib_id = sr.files[RelativePath::new("/lib.rs")];
 
-    let source_root = SourceRootId(1);
+        //- root /main/
 
-    let (sr2, pos) = db.add_fixture(
-        source_root,
-        "
-        //- /main.rs
+        //- /main/main.rs
         use test_crate::a::b::C;
-    ",
+        ",
     );
-    assert!(pos.is_none());
-
-    let main_id = sr2.files[RelativePath::new("/main.rs")];
-
-    let mut crate_graph = CrateGraph::default();
-    let main_crate = crate_graph.add_crate_root(main_id);
-    let lib_crate = crate_graph.add_crate_root(lib_id);
-    crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate).unwrap();
-
-    db.set_crate_graph(Arc::new(crate_graph));
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main/main.rs", ["test_crate"]),
+        "test_crate": ("/lib.rs", []),
+    });
+    let main_id = db.file_id_of("/main/main.rs");
 
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
     let krate = module.krate(&db).unwrap();
@@ -533,7 +503,7 @@ fn import_across_source_roots() {
 
 #[test]
 fn reexport_across_crates() {
-    let (mut db, sr) = MockDatabase::with_files(
+    let mut db = MockDatabase::with_files(
         "
         //- /main.rs
         use test_crate::Baz;
@@ -545,17 +515,13 @@ fn reexport_across_crates() {
 
         //- /foo.rs
         pub struct Baz;
-    ",
+        ",
     );
-    let main_id = sr.files[RelativePath::new("/main.rs")];
-    let lib_id = sr.files[RelativePath::new("/lib.rs")];
-
-    let mut crate_graph = CrateGraph::default();
-    let main_crate = crate_graph.add_crate_root(main_id);
-    let lib_crate = crate_graph.add_crate_root(lib_id);
-    crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate).unwrap();
-
-    db.set_crate_graph(Arc::new(crate_graph));
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main.rs", ["test_crate"]),
+        "test_crate": ("/lib.rs", []),
+    });
+    let main_id = db.file_id_of("/main.rs");
 
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
     let krate = module.krate(&db).unwrap();
@@ -565,7 +531,7 @@ fn reexport_across_crates() {
         &item_map,
         module.module_id,
         "
-            Baz: t v
+        Baz: t v
         ",
     );
 }