Merge #268

268: WIP: resolve imports across crates r=matklad a=matklad



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

7 files changed, 281 insertions, 144 deletions

diff --git a/crates/ra_hir/src/krate.rs b/crates/ra_hir/src/krate.rs
new file mode 100644
index 000000000..1196dcef1
--- /dev/null
+++ b/crates/ra_hir/src/krate.rs
@@ -0,0 +1,48 @@
+use ra_syntax::SmolStr;
+pub use ra_db::CrateId;
+
+use crate::{HirDatabase, Module, Cancelable};
+
+/// hir::Crate describes a single crate. It's the main inteface with which
+/// crate's dependencies interact. Mostly, it should be just a proxy for the
+/// root module.
+#[derive(Debug)]
+pub struct Crate {
+    crate_id: CrateId,
+}
+
+#[derive(Debug)]
+pub struct CrateDependency {
+    pub krate: Crate,
+    pub name: SmolStr,
+}
+
+impl Crate {
+    pub(crate) fn new(crate_id: CrateId) -> Crate {
+        Crate { crate_id }
+    }
+    pub fn dependencies(&self, db: &impl HirDatabase) -> Vec<CrateDependency> {
+        let crate_graph = db.crate_graph();
+        crate_graph
+            .dependencies(self.crate_id)
+            .map(|dep| {
+                let krate = Crate::new(dep.crate_id());
+                let name = dep.name.clone();
+                CrateDependency { krate, name }
+            })
+            .collect()
+    }
+    pub fn root_module(&self, db: &impl HirDatabase) -> Cancelable<Option<Module>> {
+        let crate_graph = db.crate_graph();
+        let file_id = crate_graph.crate_root(self.crate_id);
+        let source_root_id = db.file_source_root(file_id);
+        let module_tree = db.module_tree(source_root_id)?;
+        // FIXME: teach module tree about crate roots instead of guessing
+        let (module_id, _) = ctry!(module_tree
+            .modules_with_sources()
+            .find(|(_, src)| src.file_id() == file_id));
+
+        let module = Module::new(db, source_root_id, module_id)?;
+        Ok(Some(module))
+    }
+}
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index f50b922af..578fde259 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -18,12 +18,14 @@ pub mod db;
 #[cfg(test)]
 mod mock;
 mod query_definitions;
-mod function;
-mod module;
 mod path;
 mod arena;
 pub mod source_binder;
 
+mod krate;
+mod module;
+mod function;
+
 use std::ops::Index;
 
 use ra_syntax::{SyntaxNodeRef, SyntaxNode};
@@ -36,6 +38,7 @@ use crate::{
 
 pub use self::{
     path::{Path, PathKind},
+    krate::Crate,
     module::{Module, ModuleId, Problem, nameres::ItemMap},
     function::{Function, FnScopes},
 };
diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs
index e855df11d..b7193c4f3 100644
--- a/crates/ra_hir/src/mock.rs
+++ b/crates/ra_hir/src/mock.rs
@@ -2,7 +2,7 @@ use std::sync::Arc;
 
 use parking_lot::Mutex;
 use salsa::{self, Database};
-use ra_db::{LocationIntener, BaseDatabase, FilePosition, mock::FileMap, FileId, WORKSPACE};
+use ra_db::{LocationIntener, BaseDatabase, FilePosition, mock::FileMap, FileId, WORKSPACE, CrateGraph};
 use relative_path::RelativePathBuf;
 use test_utils::{parse_fixture, CURSOR_MARKER, extract_offset};
 
@@ -16,7 +16,24 @@ pub(crate) struct MockDatabase {
 }
 
 impl MockDatabase {
+    pub(crate) fn with_files(fixture: &str) -> (MockDatabase, FileMap) {
+        let (db, file_map, position) = MockDatabase::from_fixture(fixture);
+        assert!(position.is_none());
+        (db, file_map)
+    }
+
     pub(crate) fn with_position(fixture: &str) -> (MockDatabase, FilePosition) {
+        let (db, _, position) = MockDatabase::from_fixture(fixture);
+        let position = position.expect("expected a marker ( <|> )");
+        (db, position)
+    }
+
+    pub(crate) fn set_crate_graph(&mut self, crate_graph: CrateGraph) {
+        self.query_mut(ra_db::CrateGraphQuery)
+            .set((), Arc::new(crate_graph));
+    }
+
+    fn from_fixture(fixture: &str) -> (MockDatabase, FileMap, Option<FilePosition>) {
         let mut db = MockDatabase::default();
 
         let mut position = None;
@@ -32,11 +49,10 @@ impl MockDatabase {
                 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();
+        let source_root = file_map.clone().into_source_root();
         db.query_mut(ra_db::SourceRootQuery)
             .set(WORKSPACE, Arc::new(source_root));
-        (db, position)
+        (db, file_map, position)
     }
 
     fn add_file(&mut self, file_map: &mut FileMap, path: &str, text: &str) -> FileId {
diff --git a/crates/ra_hir/src/module.rs b/crates/ra_hir/src/module.rs
index e7a49f83a..c6bb76d56 100644
--- a/crates/ra_hir/src/module.rs
+++ b/crates/ra_hir/src/module.rs
@@ -12,7 +12,7 @@ use ra_db::{SourceRootId, FileId, Cancelable};
 use relative_path::RelativePathBuf;
 
 use crate::{
-    DefKind, DefLoc, DefId, Path, PathKind, HirDatabase, SourceItemId, SourceFileItemId,
+    DefKind, DefLoc, DefId, Path, PathKind, HirDatabase, SourceItemId, SourceFileItemId, Crate,
     arena::{Arena, Id},
 };
 
@@ -64,6 +64,15 @@ impl Module {
         })
     }
 
+    /// Returns the crate this module is part of.
+    pub fn krate(&self, db: &impl HirDatabase) -> Option<Crate> {
+        let root_id = self.module_id.crate_root(&self.tree);
+        let file_id = root_id.source(&self.tree).file_id();
+        let crate_graph = db.crate_graph();
+        let crate_id = crate_graph.crate_id_for_crate_root(file_id)?;
+        Some(Crate::new(crate_id))
+    }
+
     /// The root of the tree this module is part of
     pub fn crate_root(&self) -> Module {
         let root_id = self.module_id.crate_root(&self.tree);
diff --git a/crates/ra_hir/src/module/nameres.rs b/crates/ra_hir/src/module/nameres.rs
index 6511359d0..9afeade9e 100644
--- a/crates/ra_hir/src/module/nameres.rs
+++ b/crates/ra_hir/src/module/nameres.rs
@@ -31,7 +31,7 @@ use crate::{
     DefId, DefLoc, DefKind,
     SourceItemId, SourceFileItemId, SourceFileItems,
     Path, PathKind,
-    HirDatabase,
+    HirDatabase, Crate,
     module::{ModuleId, ModuleTree},
 };
 
@@ -200,34 +200,63 @@ impl ModuleItem {
 }
 
 pub(crate) struct Resolver<'a, DB> {
-    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,
+    db: &'a DB,
+    input: &'a FxHashMap<ModuleId, Arc<InputModuleItems>>,
+    source_root: SourceRootId,
+    module_tree: Arc<ModuleTree>,
+    result: ItemMap,
 }
 
 impl<'a, DB> Resolver<'a, DB>
 where
     DB: HirDatabase,
 {
+    pub(crate) fn new(
+        db: &'a DB,
+        input: &'a FxHashMap<ModuleId, Arc<InputModuleItems>>,
+        source_root: SourceRootId,
+        module_tree: Arc<ModuleTree>,
+    ) -> Resolver<'a, DB> {
+        Resolver {
+            db,
+            input,
+            source_root,
+            module_tree,
+            result: ItemMap::default(),
+        }
+    }
+
     pub(crate) fn resolve(mut self) -> Cancelable<ItemMap> {
         for (&module_id, items) in self.input.iter() {
-            self.populate_module(module_id, items)
+            self.populate_module(module_id, items)?;
         }
 
         for &module_id in self.input.keys() {
             self.db.check_canceled()?;
-            self.resolve_imports(module_id);
+            self.resolve_imports(module_id)?;
         }
         Ok(self.result)
     }
 
-    fn populate_module(&mut self, module_id: ModuleId, input: &InputModuleItems) {
+    fn populate_module(&mut self, module_id: ModuleId, input: &InputModuleItems) -> Cancelable<()> {
         let file_id = module_id.source(&self.module_tree).file_id();
 
         let mut module_items = ModuleScope::default();
 
+        // Populate extern crates prelude
+        {
+            let root_id = module_id.crate_root(&self.module_tree);
+            let file_id = root_id.source(&self.module_tree).file_id();
+            let crate_graph = self.db.crate_graph();
+            if let Some(crate_id) = crate_graph.crate_id_for_crate_root(file_id) {
+                let krate = Crate::new(crate_id);
+                for dep in krate.dependencies(self.db) {
+                    if let Some(module) = dep.krate.root_module(self.db)? {
+                        self.add_module_item(&mut module_items, dep.name, module.module_id);
+                    }
+                }
+            };
+        }
         for import in input.imports.iter() {
             if let Some(name) = import.path.segments.iter().last() {
                 if let ImportKind::Named(import) = import.kind {
@@ -241,10 +270,9 @@ where
                 }
             }
         }
-
+        // Populate explicitelly declared items, except modules
         for item in input.items.iter() {
             if item.kind == MODULE {
-                // handle submodules separatelly
                 continue;
             }
             let def_loc = DefLoc {
@@ -264,45 +292,50 @@ where
             module_items.items.insert(item.name.clone(), resolution);
         }
 
+        // Populate modules
         for (name, module_id) in module_id.children(&self.module_tree) {
-            let def_loc = DefLoc {
-                kind: DefKind::Module,
-                source_root_id: self.source_root,
-                module_id,
-                source_item_id: module_id.source(&self.module_tree).0,
-            };
-            let def_id = def_loc.id(self.db);
-            let resolution = Resolution {
-                def_id: Some(def_id),
-                import: None,
-            };
-            module_items.items.insert(name, resolution);
+            self.add_module_item(&mut module_items, name, module_id);
         }
 
         self.result.per_module.insert(module_id, module_items);
+        Ok(())
     }
 
-    fn resolve_imports(&mut self, module_id: ModuleId) {
+    fn add_module_item(&self, module_items: &mut ModuleScope, name: SmolStr, module_id: ModuleId) {
+        let def_loc = DefLoc {
+            kind: DefKind::Module,
+            source_root_id: self.source_root,
+            module_id,
+            source_item_id: module_id.source(&self.module_tree).0,
+        };
+        let def_id = def_loc.id(self.db);
+        let resolution = Resolution {
+            def_id: Some(def_id),
+            import: None,
+        };
+        module_items.items.insert(name, resolution);
+    }
+
+    fn resolve_imports(&mut self, module_id: ModuleId) -> Cancelable<()> {
         for import in self.input[&module_id].imports.iter() {
-            self.resolve_import(module_id, import);
+            self.resolve_import(module_id, import)?;
         }
+        Ok(())
     }
 
-    fn resolve_import(&mut self, module_id: ModuleId, import: &Import) {
+    fn resolve_import(&mut self, module_id: ModuleId, import: &Import) -> Cancelable<()> {
         let ptr = match import.kind {
-            ImportKind::Glob => return,
+            ImportKind::Glob => return Ok(()),
             ImportKind::Named(ptr) => ptr,
         };
 
         let mut curr = match import.path.kind {
-            // TODO: handle extern crates
-            PathKind::Plain => return,
-            PathKind::Self_ => module_id,
+            PathKind::Plain | PathKind::Self_ => module_id,
             PathKind::Super => {
                 match module_id.parent(&self.module_tree) {
                     Some(it) => it,
                     // TODO: error
-                    None => return,
+                    None => return Ok(()),
                 }
             }
             PathKind::Crate => module_id.crate_root(&self.module_tree),
@@ -312,10 +345,10 @@ where
             let is_last = i == import.path.segments.len() - 1;
 
             let def_id = match self.result.per_module[&curr].items.get(name) {
-                None => return,
+                None => return Ok(()),
                 Some(res) => match res.def_id {
                     Some(it) => it,
-                    None => return,
+                    None => return Ok(()),
                 },
             };
 
@@ -326,7 +359,7 @@ where
                         module_id,
                         ..
                     } => module_id,
-                    _ => return,
+                    _ => return Ok(()),
                 }
             } else {
                 self.update(module_id, |items| {
@@ -338,6 +371,7 @@ where
                 })
             }
         }
+        Ok(())
     }
 
     fn update(&mut self, module_id: ModuleId, f: impl FnOnce(&mut ModuleScope)) {
@@ -347,99 +381,4 @@ where
 }
 
 #[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::source_binder::module_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
-            )
-        }
-    }
-}
+mod tests;
diff --git a/crates/ra_hir/src/module/nameres/tests.rs b/crates/ra_hir/src/module/nameres/tests.rs
new file mode 100644
index 000000000..9ddc32dcd
--- /dev/null
+++ b/crates/ra_hir/src/module/nameres/tests.rs
@@ -0,0 +1,127 @@
+use std::sync::Arc;
+
+use salsa::Database;
+use ra_db::{FilesDatabase, CrateGraph};
+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::source_binder::module_from_position(&db, pos)
+        .unwrap()
+        .unwrap();
+    let module_id = module.module_id;
+    (db.item_map(source_root).unwrap(), module_id)
+}
+
+#[test]
+fn item_map_smoke_test() {
+    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 item_map_across_crates() {
+    let (mut db, files) = MockDatabase::with_files(
+        "
+        //- /main.rs
+        use test_crate::Baz;
+
+        //- /lib.rs
+        pub struct Baz;
+    ",
+    );
+    let main_id = files.file_id("/main.rs");
+    let lib_id = files.file_id("/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);
+
+    db.set_crate_graph(crate_graph);
+
+    let source_root = db.file_source_root(main_id);
+    let module = hir::source_binder::module_from_file_id(&db, main_id)
+        .unwrap()
+        .unwrap();
+    let module_id = module.module_id;
+    let item_map = db.item_map(source_root).unwrap();
+
+    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
+        )
+    }
+}
diff --git a/crates/ra_hir/src/query_definitions.rs b/crates/ra_hir/src/query_definitions.rs
index bb4457d07..37c4f9e4f 100644
--- a/crates/ra_hir/src/query_definitions.rs
+++ b/crates/ra_hir/src/query_definitions.rs
@@ -141,13 +141,8 @@ pub(super) fn item_map(
             Ok((id, items))
         })
         .collect::<Cancelable<FxHashMap<_, _>>>()?;
-    let resolver = Resolver {
-        db: db,
-        input: &input,
-        source_root,
-        module_tree,
-        result: ItemMap::default(),
-    };
+
+    let resolver = Resolver::new(db, &input, source_root, module_tree);
     let res = resolver.resolve()?;
     let elapsed = start.elapsed();
     log::info!("item_map: {:?}", elapsed);