Merge #248

248: Hir tests r=matklad a=matklad

bors r+

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

9 files changed, 338 insertions, 196 deletions

diff --git a/crates/ra_analysis/src/db.rs b/crates/ra_analysis/src/db.rs
index 7fc3fe31b..df2ef293d 100644
--- a/crates/ra_analysis/src/db.rs
+++ b/crates/ra_analysis/src/db.rs
@@ -1,6 +1,4 @@
 use std::sync::Arc;
-#[cfg(test)]
-use parking_lot::Mutex;
 use salsa::{self, Database};
 use ra_db::{LocationIntener, BaseDatabase};
 use hir::{self, DefId, DefLoc, FnId, SourceItemId};
@@ -11,11 +9,6 @@ use crate::{
 
 #[derive(Debug)]
 pub(crate) struct RootDatabase {
-    #[cfg(test)]
-    events: Mutex<Option<Vec<salsa::Event<RootDatabase>>>>,
-    #[cfg(not(test))]
-    events: (),
-
     runtime: salsa::Runtime<RootDatabase>,
     id_maps: Arc<IdMaps>,
 }
@@ -30,23 +23,11 @@ impl salsa::Database for RootDatabase {
     fn salsa_runtime(&self) -> &salsa::Runtime<RootDatabase> {
         &self.runtime
     }
-
-    #[allow(unused)]
-    fn salsa_event(&self, event: impl Fn() -> salsa::Event<RootDatabase>) {
-        #[cfg(test)]
-        {
-            let mut events = self.events.lock();
-            if let Some(events) = &mut *events {
-                events.push(event());
-            }
-        }
-    }
 }
 
 impl Default for RootDatabase {
     fn default() -> RootDatabase {
         let mut db = RootDatabase {
-            events: Default::default(),
             runtime: salsa::Runtime::default(),
             id_maps: Default::default(),
         };
@@ -63,7 +44,6 @@ impl Default for RootDatabase {
 impl salsa::ParallelDatabase for RootDatabase {
     fn snapshot(&self) -> salsa::Snapshot<RootDatabase> {
         salsa::Snapshot::new(RootDatabase {
-            events: Default::default(),
             runtime: self.runtime.snapshot(self),
             id_maps: self.id_maps.clone(),
         })
@@ -84,29 +64,6 @@ impl AsRef<LocationIntener<hir::SourceItemId, FnId>> for RootDatabase {
     }
 }
 
-#[cfg(test)]
-impl RootDatabase {
-    pub(crate) fn log(&self, f: impl FnOnce()) -> Vec<salsa::Event<RootDatabase>> {
-        *self.events.lock() = Some(Vec::new());
-        f();
-        let events = self.events.lock().take().unwrap();
-        events
-    }
-
-    pub(crate) fn log_executed(&self, f: impl FnOnce()) -> Vec<String> {
-        let events = self.log(f);
-        events
-            .into_iter()
-            .filter_map(|e| match e.kind {
-                // This pretty horrible, but `Debug` is the only way to inspect
-                // QueryDescriptor at the moment.
-                salsa::EventKind::WillExecute { descriptor } => Some(format!("{:?}", descriptor)),
-                _ => None,
-            })
-            .collect()
-    }
-}
-
 salsa::database_storage! {
     pub(crate) struct RootDatabaseStorage for RootDatabase {
         impl ra_db::FilesDatabase {
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index 350a6d627..12df580ba 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -317,112 +317,3 @@ fn analysis_is_send() {
     fn is_send<T: Send>() {}
     is_send::<Analysis>();
 }
-
-//TODO: move to hir
-#[cfg(test)]
-mod hir_namres_tests {
-    use std::sync::Arc;
-    use ra_db::FilesDatabase;
-    use ra_syntax::SmolStr;
-    use hir::{self, db::HirDatabase};
-
-    use crate::{
-        AnalysisChange,
-        mock_analysis::{MockAnalysis, analysis_and_position},
-};
-
-    fn item_map(fixture: &str) -> (Arc<hir::ItemMap>, hir::ModuleId) {
-        let (analysis, pos) = analysis_and_position(fixture);
-        let db = analysis.imp.db;
-        let source_root = db.file_source_root(pos.file_id);
-        let descr = hir::Module::guess_from_position(&*db, pos)
-            .unwrap()
-            .unwrap();
-        let module_id = descr.module_id;
-        (db.item_map(source_root).unwrap(), module_id)
-    }
-
-    #[test]
-    fn test_item_map() {
-        let (item_map, module_id) = item_map(
-            "
-            //- /lib.rs
-            mod foo;
-
-            use crate::foo::bar::Baz;
-            <|>
-
-            //- /foo/mod.rs
-            pub mod bar;
-
-            //- /foo/bar.rs
-            pub struct Baz;
-        ",
-        );
-        let name = SmolStr::from("Baz");
-        let resolution = &item_map.per_module[&module_id].items[&name];
-        assert!(resolution.def_id.is_some());
-    }
-
-    #[test]
-    fn typing_inside_a_function_should_not_invalidate_item_map() {
-        let mock_analysis = MockAnalysis::with_files(
-            "
-            //- /lib.rs
-            mod foo;
-
-            use crate::foo::bar::Baz;
-
-            fn foo() -> i32 {
-                1 + 1
-            }
-            //- /foo/mod.rs
-            pub mod bar;
-
-            //- /foo/bar.rs
-            pub struct Baz;
-        ",
-        );
-
-        let file_id = mock_analysis.id_of("/lib.rs");
-        let mut host = mock_analysis.analysis_host();
-
-        let source_root = host.analysis().imp.db.file_source_root(file_id);
-
-        {
-            let db = host.analysis().imp.db;
-            let events = db.log_executed(|| {
-                db.item_map(source_root).unwrap();
-            });
-            assert!(format!("{:?}", events).contains("item_map"))
-        }
-
-        let mut change = AnalysisChange::new();
-
-        change.change_file(
-            file_id,
-            "
-            mod foo;
-
-            use crate::foo::bar::Baz;
-
-            fn foo() -> i32 { 92 }
-        "
-            .to_string(),
-        );
-
-        host.apply_change(change);
-
-        {
-            let db = host.analysis().imp.db;
-            let events = db.log_executed(|| {
-                db.item_map(source_root).unwrap();
-            });
-            assert!(
-                !format!("{:?}", events).contains("_item_map"),
-                "{:#?}",
-                events
-            )
-        }
-    }
-}
diff --git a/crates/ra_analysis/src/mock_analysis.rs b/crates/ra_analysis/src/mock_analysis.rs
index 8e8f969f4..0d9a7a147 100644
--- a/crates/ra_analysis/src/mock_analysis.rs
+++ b/crates/ra_analysis/src/mock_analysis.rs
@@ -1,9 +1,10 @@
 use std::sync::Arc;
 
-use relative_path::{RelativePath, RelativePathBuf};
+use relative_path::{RelativePathBuf};
 use test_utils::{extract_offset, parse_fixture, CURSOR_MARKER};
+use ra_db::mock::FileMap;
 
-use crate::{Analysis, AnalysisChange, AnalysisHost, FileId, FileResolver, FilePosition};
+use crate::{Analysis, AnalysisChange, AnalysisHost, FileId, FilePosition};
 
 /// Mock analysis is used in test to bootstrap an AnalysisHost/Analysis
 /// from a set of in-memory files.
@@ -76,16 +77,15 @@ impl MockAnalysis {
     }
     pub fn analysis_host(self) -> AnalysisHost {
         let mut host = AnalysisHost::default();
-        let mut file_map = Vec::new();
+        let mut file_map = FileMap::default();
         let mut change = AnalysisChange::new();
-        for (id, (path, contents)) in self.files.into_iter().enumerate() {
-            let file_id = FileId((id + 1) as u32);
+        for (path, contents) in self.files.into_iter() {
             assert!(path.starts_with('/'));
             let path = RelativePathBuf::from_path(&path[1..]).unwrap();
+            let file_id = file_map.add(path);
             change.add_file(file_id, contents);
-            file_map.push((file_id, path));
         }
-        change.set_file_resolver(Arc::new(FileMap(file_map)));
+        change.set_file_resolver(Arc::new(file_map));
         host.apply_change(change);
         host
     }
@@ -113,29 +113,3 @@ pub fn single_file_with_position(code: &str) -> (Analysis, FilePosition) {
     let pos = mock.add_file_with_position("/main.rs", code);
     (mock.analysis(), pos)
 }
-
-#[derive(Debug)]
-struct FileMap(Vec<(FileId, RelativePathBuf)>);
-
-impl FileMap {
-    fn iter<'a>(&'a self) -> impl Iterator<Item = (FileId, &'a RelativePath)> + 'a {
-        self.0
-            .iter()
-            .map(|(id, path)| (*id, path.as_relative_path()))
-    }
-
-    fn path(&self, id: FileId) -> &RelativePath {
-        self.iter().find(|&(it, _)| it == id).unwrap().1
-    }
-}
-
-impl FileResolver for FileMap {
-    fn file_stem(&self, id: FileId) -> String {
-        self.path(id).file_stem().unwrap().to_string()
-    }
-    fn resolve(&self, id: FileId, rel: &RelativePath) -> Option<FileId> {
-        let path = self.path(id).join(rel).normalize();
-        let id = self.iter().find(|&(_, p)| path == p)?.0;
-        Some(id)
-    }
-}
diff --git a/crates/ra_db/src/lib.rs b/crates/ra_db/src/lib.rs
index 33cb0e2ec..6a996c7f4 100644
--- a/crates/ra_db/src/lib.rs
+++ b/crates/ra_db/src/lib.rs
@@ -10,6 +10,7 @@ mod syntax_ptr;
 mod file_resolver;
 mod input;
 mod loc2id;
+pub mod mock;
 
 use std::sync::Arc;
 use ra_editor::LineIndex;
diff --git a/crates/ra_db/src/mock.rs b/crates/ra_db/src/mock.rs
new file mode 100644
index 000000000..2840f9655
--- /dev/null
+++ b/crates/ra_db/src/mock.rs
@@ -0,0 +1,51 @@
+use std::sync::Arc;
+
+use rustc_hash::FxHashSet;
+use relative_path::{RelativePath, RelativePathBuf};
+
+use crate::{FileId, FileResolver, SourceRoot, FileResolverImp};
+
+#[derive(Default, Debug)]
+pub struct FileMap(Vec<(FileId, RelativePathBuf)>);
+
+impl FileMap {
+    pub fn add(&mut self, path: RelativePathBuf) -> FileId {
+        let file_id = FileId((self.0.len() + 1) as u32);
+        self.0.push((file_id, path));
+        file_id
+    }
+
+    pub fn into_source_root(self) -> SourceRoot {
+        let files = self.files();
+        let file_resolver = FileResolverImp::new(Arc::new(self));
+        SourceRoot {
+            file_resolver,
+            files,
+        }
+    }
+
+    pub fn files(&self) -> FxHashSet<FileId> {
+        self.iter().map(|(id, _)| id).collect()
+    }
+
+    fn iter<'a>(&'a self) -> impl Iterator<Item = (FileId, &'a RelativePath)> + 'a {
+        self.0
+            .iter()
+            .map(|(id, path)| (*id, path.as_relative_path()))
+    }
+
+    fn path(&self, id: FileId) -> &RelativePath {
+        self.iter().find(|&(it, _)| it == id).unwrap().1
+    }
+}
+
+impl FileResolver for FileMap {
+    fn file_stem(&self, id: FileId) -> String {
+        self.path(id).file_stem().unwrap().to_string()
+    }
+    fn resolve(&self, id: FileId, rel: &RelativePath) -> Option<FileId> {
+        let path = self.path(id).join(rel).normalize();
+        let id = self.iter().find(|&(_, p)| path == p)?.0;
+        Some(id)
+    }
+}
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index f13f0107e..e7b6a81f4 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -15,6 +15,8 @@ macro_rules! ctry {
 }
 
 pub mod db;
+#[cfg(test)]
+mod mock;
 mod query_definitions;
 mod function;
 mod module;
diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs
new file mode 100644
index 000000000..8e256b89f
--- /dev/null
+++ b/crates/ra_hir/src/mock.rs
@@ -0,0 +1,172 @@
+use std::sync::Arc;
+
+use parking_lot::Mutex;
+use salsa::{self, Database};
+use ra_db::{LocationIntener, BaseDatabase, FilePosition, mock::FileMap, FileId, WORKSPACE};
+use relative_path::RelativePathBuf;
+use test_utils::{parse_fixture, CURSOR_MARKER, extract_offset};
+
+use crate::{db, DefId, DefLoc, FnId, SourceItemId};
+
+#[derive(Debug)]
+pub(crate) struct MockDatabase {
+    events: Mutex<Option<Vec<salsa::Event<MockDatabase>>>>,
+    runtime: salsa::Runtime<MockDatabase>,
+    id_maps: Arc<IdMaps>,
+}
+
+impl MockDatabase {
+    pub(crate) fn with_position(fixture: &str) -> (MockDatabase, FilePosition) {
+        let mut db = MockDatabase::default();
+
+        let mut position = None;
+        let mut file_map = FileMap::default();
+        for entry in parse_fixture(fixture) {
+            if entry.text.contains(CURSOR_MARKER) {
+                assert!(
+                    position.is_none(),
+                    "only one marker (<|>) per fixture is allowed"
+                );
+                position = Some(db.add_file_with_position(&mut file_map, &entry.meta, &entry.text));
+            } else {
+                db.add_file(&mut file_map, &entry.meta, &entry.text);
+            }
+        }
+        let position = position.expect("expected a marker (<|>)");
+        let source_root = file_map.into_source_root();
+        db.query_mut(ra_db::SourceRootQuery)
+            .set(WORKSPACE, Arc::new(source_root));
+        (db, position)
+    }
+
+    fn add_file(&mut self, file_map: &mut FileMap, path: &str, text: &str) -> FileId {
+        assert!(path.starts_with('/'));
+        let path = RelativePathBuf::from_path(&path[1..]).unwrap();
+
+        let file_id = file_map.add(path);
+        let text = Arc::new(text.to_string());
+        self.query_mut(ra_db::FileTextQuery).set(file_id, text);
+        self.query_mut(ra_db::FileSourceRootQuery)
+            .set(file_id, WORKSPACE);
+        file_id
+    }
+
+    fn add_file_with_position(
+        &mut self,
+        file_map: &mut FileMap,
+        path: &str,
+        text: &str,
+    ) -> FilePosition {
+        let (offset, text) = extract_offset(text);
+        let file_id = self.add_file(file_map, path, &text);
+        FilePosition { file_id, offset }
+    }
+}
+
+#[derive(Debug, Default)]
+struct IdMaps {
+    fns: LocationIntener<SourceItemId, FnId>,
+    defs: LocationIntener<DefLoc, DefId>,
+}
+
+impl salsa::Database for MockDatabase {
+    fn salsa_runtime(&self) -> &salsa::Runtime<MockDatabase> {
+        &self.runtime
+    }
+
+    fn salsa_event(&self, event: impl Fn() -> salsa::Event<MockDatabase>) {
+        let mut events = self.events.lock();
+        if let Some(events) = &mut *events {
+            events.push(event());
+        }
+    }
+}
+
+impl Default for MockDatabase {
+    fn default() -> MockDatabase {
+        let mut db = MockDatabase {
+            events: Default::default(),
+            runtime: salsa::Runtime::default(),
+            id_maps: Default::default(),
+        };
+        db.query_mut(ra_db::SourceRootQuery)
+            .set(ra_db::WORKSPACE, Default::default());
+        db.query_mut(ra_db::CrateGraphQuery)
+            .set((), Default::default());
+        db.query_mut(ra_db::LibrariesQuery)
+            .set((), Default::default());
+        db
+    }
+}
+
+impl salsa::ParallelDatabase for MockDatabase {
+    fn snapshot(&self) -> salsa::Snapshot<MockDatabase> {
+        salsa::Snapshot::new(MockDatabase {
+            events: Default::default(),
+            runtime: self.runtime.snapshot(self),
+            id_maps: self.id_maps.clone(),
+        })
+    }
+}
+
+impl BaseDatabase for MockDatabase {}
+
+impl AsRef<LocationIntener<DefLoc, DefId>> for MockDatabase {
+    fn as_ref(&self) -> &LocationIntener<DefLoc, DefId> {
+        &self.id_maps.defs
+    }
+}
+
+impl AsRef<LocationIntener<SourceItemId, FnId>> for MockDatabase {
+    fn as_ref(&self) -> &LocationIntener<SourceItemId, FnId> {
+        &self.id_maps.fns
+    }
+}
+
+impl MockDatabase {
+    pub(crate) fn log(&self, f: impl FnOnce()) -> Vec<salsa::Event<MockDatabase>> {
+        *self.events.lock() = Some(Vec::new());
+        f();
+        let events = self.events.lock().take().unwrap();
+        events
+    }
+
+    pub(crate) fn log_executed(&self, f: impl FnOnce()) -> Vec<String> {
+        let events = self.log(f);
+        events
+            .into_iter()
+            .filter_map(|e| match e.kind {
+                // This pretty horrible, but `Debug` is the only way to inspect
+                // QueryDescriptor at the moment.
+                salsa::EventKind::WillExecute { descriptor } => Some(format!("{:?}", descriptor)),
+                _ => None,
+            })
+            .collect()
+    }
+}
+
+salsa::database_storage! {
+    pub(crate) struct MockDatabaseStorage for MockDatabase {
+        impl ra_db::FilesDatabase {
+            fn file_text() for ra_db::FileTextQuery;
+            fn file_source_root() for ra_db::FileSourceRootQuery;
+            fn source_root() for ra_db::SourceRootQuery;
+            fn libraries() for ra_db::LibrariesQuery;
+            fn crate_graph() for ra_db::CrateGraphQuery;
+        }
+        impl ra_db::SyntaxDatabase {
+            fn source_file() for ra_db::SourceFileQuery;
+            fn file_lines() for ra_db::FileLinesQuery;
+        }
+        impl db::HirDatabase {
+            fn module_tree() for db::ModuleTreeQuery;
+            fn fn_scopes() for db::FnScopesQuery;
+            fn file_items() for db::SourceFileItemsQuery;
+            fn file_item() for db::FileItemQuery;
+            fn input_module_items() for db::InputModuleItemsQuery;
+            fn item_map() for db::ItemMapQuery;
+            fn fn_syntax() for db::FnSyntaxQuery;
+            fn submodules() for db::SubmodulesQuery;
+        }
+    }
+}
diff --git a/crates/ra_hir/src/module/mod.rs b/crates/ra_hir/src/module/mod.rs
index a011fd53e..3ae83d8cb 100644
--- a/crates/ra_hir/src/module/mod.rs
+++ b/crates/ra_hir/src/module/mod.rs
@@ -26,8 +26,7 @@ pub use self::nameres::ModuleScope;
 pub struct Module {
     tree: Arc<ModuleTree>,
     source_root_id: SourceRootId,
-    //TODO: make private
-    pub module_id: ModuleId,
+    module_id: ModuleId,
 }
 
 impl Module {
@@ -122,7 +121,6 @@ impl Module {
     }
 
     /// `name` is `None` for the crate's root module
-    #[allow(unused)]
     pub fn name(&self) -> Option<SmolStr> {
         let link = self.module_id.parent_link(&self.tree)?;
         Some(link.name(&self.tree))
@@ -218,7 +216,7 @@ pub enum ModuleSource {
 
 /// An owned syntax node for a module. Unlike `ModuleSource`,
 /// this holds onto the AST for the whole file.
-pub enum ModuleSourceNode {
+pub(crate) enum ModuleSourceNode {
     SourceFile(ast::SourceFileNode),
     Module(ast::ModuleNode),
 }
@@ -338,7 +336,7 @@ impl ModuleSource {
         }
     }
 
-    pub fn resolve(self, db: &impl HirDatabase) -> ModuleSourceNode {
+    pub(crate) fn resolve(self, db: &impl HirDatabase) -> ModuleSourceNode {
         match self {
             ModuleSource::SourceFile(file_id) => {
                 let syntax = db.source_file(file_id);
diff --git a/crates/ra_hir/src/module/nameres.rs b/crates/ra_hir/src/module/nameres.rs
index 837a8d5ae..8529e16b3 100644
--- a/crates/ra_hir/src/module/nameres.rs
+++ b/crates/ra_hir/src/module/nameres.rs
@@ -44,7 +44,7 @@ pub struct ItemMap {
 
 #[derive(Debug, Default, PartialEq, Eq, Clone)]
 pub struct ModuleScope {
-    pub items: FxHashMap<SmolStr, Resolution>,
+    items: FxHashMap<SmolStr, Resolution>,
 }
 
 impl ModuleScope {
@@ -200,11 +200,11 @@ impl ModuleItem {
 }
 
 pub(crate) struct Resolver<'a, DB> {
-    pub db: &'a DB,
-    pub input: &'a FxHashMap<ModuleId, Arc<InputModuleItems>>,
-    pub source_root: SourceRootId,
-    pub module_tree: Arc<ModuleTree>,
-    pub result: ItemMap,
+    pub(crate) db: &'a DB,
+    pub(crate) input: &'a FxHashMap<ModuleId, Arc<InputModuleItems>>,
+    pub(crate) source_root: SourceRootId,
+    pub(crate) module_tree: Arc<ModuleTree>,
+    pub(crate) result: ItemMap,
 }
 
 impl<'a, DB> Resolver<'a, DB>
@@ -336,3 +336,99 @@ where
         f(module_items)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+
+    use salsa::Database;
+    use ra_db::FilesDatabase;
+    use ra_syntax::SmolStr;
+
+    use crate::{
+        self as hir,
+        db::HirDatabase,
+        mock::MockDatabase,
+};
+
+    fn item_map(fixture: &str) -> (Arc<hir::ItemMap>, hir::ModuleId) {
+        let (db, pos) = MockDatabase::with_position(fixture);
+        let source_root = db.file_source_root(pos.file_id);
+        let module = hir::Module::guess_from_position(&db, pos).unwrap().unwrap();
+        let module_id = module.module_id;
+        (db.item_map(source_root).unwrap(), module_id)
+    }
+
+    #[test]
+    fn test_item_map() {
+        let (item_map, module_id) = item_map(
+            "
+            //- /lib.rs
+            mod foo;
+
+            use crate::foo::bar::Baz;
+            <|>
+
+            //- /foo/mod.rs
+            pub mod bar;
+
+            //- /foo/bar.rs
+            pub struct Baz;
+        ",
+        );
+        let name = SmolStr::from("Baz");
+        let resolution = &item_map.per_module[&module_id].items[&name];
+        assert!(resolution.def_id.is_some());
+    }
+
+    #[test]
+    fn typing_inside_a_function_should_not_invalidate_item_map() {
+        let (mut db, pos) = MockDatabase::with_position(
+            "
+            //- /lib.rs
+            mod foo;<|>
+
+            use crate::foo::bar::Baz;
+
+            fn foo() -> i32 {
+                1 + 1
+            }
+            //- /foo/mod.rs
+            pub mod bar;
+
+            //- /foo/bar.rs
+            pub struct Baz;
+        ",
+        );
+        let source_root = db.file_source_root(pos.file_id);
+        {
+            let events = db.log_executed(|| {
+                db.item_map(source_root).unwrap();
+            });
+            assert!(format!("{:?}", events).contains("item_map"))
+        }
+
+        let new_text = "
+            mod foo;
+
+            use crate::foo::bar::Baz;
+
+            fn foo() -> i32 { 92 }
+        "
+        .to_string();
+
+        db.query_mut(ra_db::FileTextQuery)
+            .set(pos.file_id, Arc::new(new_text));
+
+        {
+            let events = db.log_executed(|| {
+                db.item_map(source_root).unwrap();
+            });
+            assert!(
+                !format!("{:?}", events).contains("_item_map"),
+                "{:#?}",
+                events
+            )
+        }
+    }
+}