From 22b412f1a9d245cc3d3774dab9408e3a39a52025 Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 14:25:19 +0100
Subject: find_path WIP

---
 crates/ra_hir_def/src/find_path.rs | 44 ++++++++++++++++++++++++++++++++++++++
 crates/ra_hir_def/src/lib.rs       |  1 +
 crates/ra_hir_def/src/test_db.rs   | 13 +++++++++++
 3 files changed, 58 insertions(+)
 create mode 100644 crates/ra_hir_def/src/find_path.rs

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
new file mode 100644
index 000000000..1ddf5fca6
--- /dev/null
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -0,0 +1,44 @@
+//! An algorithm to find a path to refer to a certain item.
+
+use crate::{ModuleDefId, path::ModPath, ModuleId};
+
+pub fn find_path(item: ModuleDefId, from: ModuleId) -> ModPath {
+    todo!()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use ra_db::{fixture::WithFixture, SourceDatabase};
+    use crate::{db::DefDatabase, test_db::TestDB};
+    use ra_syntax::ast::AstNode;
+    use hir_expand::hygiene::Hygiene;
+
+    /// `code` needs to contain a cursor marker; checks that `find_path` for the
+    /// item the `path` refers to returns that same path when called from the
+    /// module the cursor is in.
+    fn check_found_path(code: &str, path: &str) {
+        let (db, pos) = TestDB::with_position(code);
+        let module = db.module_for_file(pos.file_id);
+        let parsed_path_file = ra_syntax::SourceFile::parse(&format!("use {};", path));
+        let ast_path = parsed_path_file.syntax_node().descendants().find_map(ra_syntax::ast::Path::cast).unwrap();
+        let mod_path = ModPath::from_src(ast_path, &Hygiene::new_unhygienic()).unwrap();
+
+        let crate_def_map = db.crate_def_map(module.krate);
+        let resolved = crate_def_map.resolve_path(&db, module.local_id, &mod_path, crate::item_scope::BuiltinShadowMode::Module).0.take_types().unwrap();
+
+        let found_path = find_path(resolved, module);
+
+        assert_eq!(mod_path, found_path);
+    }
+
+    #[test]
+    fn same_module() {
+        let code = r#"
+//- /main.rs
+struct S;
+<|>
+"#;
+        check_found_path(code, "S");
+    }
+}
diff --git a/crates/ra_hir_def/src/lib.rs b/crates/ra_hir_def/src/lib.rs
index 61f044ecf..ebc12e891 100644
--- a/crates/ra_hir_def/src/lib.rs
+++ b/crates/ra_hir_def/src/lib.rs
@@ -37,6 +37,7 @@ pub mod src;
 pub mod child_by_source;
 
 pub mod visibility;
+pub mod find_path;
 
 #[cfg(test)]
 mod test_db;
diff --git a/crates/ra_hir_def/src/test_db.rs b/crates/ra_hir_def/src/test_db.rs
index 54e3a84bd..a403f183f 100644
--- a/crates/ra_hir_def/src/test_db.rs
+++ b/crates/ra_hir_def/src/test_db.rs
@@ -6,6 +6,7 @@ use std::{
 };
 
 use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, RelativePath};
+use crate::db::DefDatabase;
 
 #[salsa::database(
     ra_db::SourceDatabaseExtStorage,
@@ -54,6 +55,18 @@ impl FileLoader for TestDB {
 }
 
 impl TestDB {
+    pub fn module_for_file(&self, file_id: FileId) -> crate::ModuleId {
+        for &krate in self.relevant_crates(file_id).iter() {
+            let crate_def_map = self.crate_def_map(krate);
+            for (local_id, data) in crate_def_map.modules.iter() {
+                if data.origin.file_id() == Some(file_id) {
+                    return crate::ModuleId { krate, local_id };
+                }
+            }
+        }
+        panic!("Can't find module for file")
+    }
+
     pub fn log(&self, f: impl FnOnce()) -> Vec<salsa::Event<TestDB>> {
         *self.events.lock().unwrap() = Some(Vec::new());
         f();
-- 
cgit v1.2.3


From 2c50f996b68e9a24a564de728ffcc13d896afc1c Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 17:31:15 +0100
Subject: more WIP

---
 crates/ra_hir_def/src/find_path.rs  | 102 +++++++++++++++++++++++++++++++-----
 crates/ra_hir_def/src/item_scope.rs |  32 +++++++++++
 2 files changed, 122 insertions(+), 12 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 1ddf5fca6..cc686ea6a 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -1,18 +1,35 @@
 //! An algorithm to find a path to refer to a certain item.
 
-use crate::{ModuleDefId, path::ModPath, ModuleId};
+use crate::{
+    db::DefDatabase,
+    item_scope::ItemInNs,
+    path::{ModPath, PathKind},
+    ModuleId,
+};
 
-pub fn find_path(item: ModuleDefId, from: ModuleId) -> ModPath {
+pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> ModPath {
+    // 1. Find all locations that the item could be imported from (i.e. that are visible)
+    //    - this needs to consider other crates, for reexports from transitive dependencies
+    //    - filter by visibility
+    // 2. For each of these, go up the module tree until we find an
+    //    item/module/crate that is already in scope (including because it is in
+    //    the prelude, and including aliases!)
+    // 3. Then select the one that gives the shortest path
+    let def_map = db.crate_def_map(from.krate);
+    let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
+    if let Some((name, _)) = from_scope.reverse_get(item) {
+        return ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]);
+    }
     todo!()
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use ra_db::{fixture::WithFixture, SourceDatabase};
-    use crate::{db::DefDatabase, test_db::TestDB};
-    use ra_syntax::ast::AstNode;
+    use crate::test_db::TestDB;
     use hir_expand::hygiene::Hygiene;
+    use ra_db::fixture::WithFixture;
+    use ra_syntax::ast::AstNode;
 
     /// `code` needs to contain a cursor marker; checks that `find_path` for the
     /// item the `path` refers to returns that same path when called from the
@@ -21,13 +38,26 @@ mod tests {
         let (db, pos) = TestDB::with_position(code);
         let module = db.module_for_file(pos.file_id);
         let parsed_path_file = ra_syntax::SourceFile::parse(&format!("use {};", path));
-        let ast_path = parsed_path_file.syntax_node().descendants().find_map(ra_syntax::ast::Path::cast).unwrap();
+        let ast_path = parsed_path_file
+            .syntax_node()
+            .descendants()
+            .find_map(ra_syntax::ast::Path::cast)
+            .unwrap();
         let mod_path = ModPath::from_src(ast_path, &Hygiene::new_unhygienic()).unwrap();
 
         let crate_def_map = db.crate_def_map(module.krate);
-        let resolved = crate_def_map.resolve_path(&db, module.local_id, &mod_path, crate::item_scope::BuiltinShadowMode::Module).0.take_types().unwrap();
+        let resolved = crate_def_map
+            .resolve_path(
+                &db,
+                module.local_id,
+                &mod_path,
+                crate::item_scope::BuiltinShadowMode::Module,
+            )
+            .0
+            .take_types()
+            .unwrap();
 
-        let found_path = find_path(resolved, module);
+        let found_path = find_path(&db, ItemInNs::Types(resolved), module);
 
         assert_eq!(mod_path, found_path);
     }
@@ -35,10 +65,58 @@ mod tests {
     #[test]
     fn same_module() {
         let code = r#"
-//- /main.rs
-struct S;
-<|>
-"#;
+            //- /main.rs
+            struct S;
+            <|>
+        "#;
         check_found_path(code, "S");
     }
+
+    #[test]
+    fn sub_module() {
+        let code = r#"
+            //- /main.rs
+            mod foo {
+                pub struct S;
+            }
+            <|>
+        "#;
+        check_found_path(code, "foo::S");
+    }
+
+    #[test]
+    fn same_crate() {
+        let code = r#"
+            //- /main.rs
+            mod foo;
+            struct S;
+            //- /foo.rs
+            <|>
+        "#;
+        check_found_path(code, "crate::S");
+    }
+
+    #[test]
+    fn different_crate() {
+        let code = r#"
+            //- /main.rs crate:main deps:std
+            <|>
+            //- /std.rs crate:std
+            pub struct S;
+        "#;
+        check_found_path(code, "std::S");
+    }
+
+    #[test]
+    fn same_crate_reexport() {
+        let code = r#"
+            //- /main.rs
+            mod bar {
+                mod foo { pub(super) struct S; }
+                pub(crate) use foo::*;
+            }
+            <|>
+        "#;
+        check_found_path(code, "bar::S");
+    }
 }
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index fe7bb9779..f88502d78 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -104,6 +104,15 @@ impl ItemScope {
         }
     }
 
+    pub(crate) fn reverse_get(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
+        for (name, per_ns) in &self.visible {
+            if let Some(vis) = item.match_with(*per_ns) {
+                return Some((name, vis));
+            }
+        }
+        None
+    }
+
     pub(crate) fn traits<'a>(&'a self) -> impl Iterator<Item = TraitId> + 'a {
         self.visible.values().filter_map(|def| match def.take_types() {
             Some(ModuleDefId::TraitId(t)) => Some(t),
@@ -173,3 +182,26 @@ impl PerNs {
         }
     }
 }
+
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub enum ItemInNs {
+    Types(ModuleDefId),
+    Values(ModuleDefId),
+    Macros(MacroDefId),
+}
+
+impl ItemInNs {
+    fn match_with(self, per_ns: PerNs) -> Option<Visibility> {
+        match self {
+            ItemInNs::Types(def) => {
+                per_ns.types.filter(|(other_def, _)| *other_def == def).map(|(_, vis)| vis)
+            },
+            ItemInNs::Values(def) => {
+                per_ns.values.filter(|(other_def, _)| *other_def == def).map(|(_, vis)| vis)
+            },
+            ItemInNs::Macros(def) => {
+                per_ns.macros.filter(|(other_def, _)| *other_def == def).map(|(_, vis)| vis)
+            },
+        }
+    }
+}
-- 
cgit v1.2.3


From b62292e8f9e28410741059ebb25133b8e1e8638a Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 21:18:43 +0100
Subject: basics working

---
 crates/ra_hir_def/src/find_path.rs | 118 +++++++++++++++++++++++++++++++++++--
 1 file changed, 113 insertions(+), 5 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index cc686ea6a..6772330e0 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -4,10 +4,18 @@ use crate::{
     db::DefDatabase,
     item_scope::ItemInNs,
     path::{ModPath, PathKind},
-    ModuleId,
+    ModuleId, ModuleDefId,
 };
+use hir_expand::name::Name;
 
-pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> ModPath {
+// TODO handle prelude
+// TODO handle enum variants
+// TODO don't import from super imports? or at least deprioritize
+// TODO use super?
+// TODO use shortest path
+// TODO performance / memoize
+
+pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
     // 1. Find all locations that the item could be imported from (i.e. that are visible)
     //    - this needs to consider other crates, for reexports from transitive dependencies
     //    - filter by visibility
@@ -15,12 +23,63 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> ModPa
     //    item/module/crate that is already in scope (including because it is in
     //    the prelude, and including aliases!)
     // 3. Then select the one that gives the shortest path
+
+    // Base cases:
+
+    // - if the item is already in scope, return the name under which it is
     let def_map = db.crate_def_map(from.krate);
     let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
     if let Some((name, _)) = from_scope.reverse_get(item) {
-        return ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]);
+        return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
+    }
+
+    // - if the item is the crate root, return `crate`
+    if item == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { krate: from.krate, local_id: def_map.root })) {
+        return Some(ModPath::from_simple_segments(PathKind::Crate, Vec::new()));
+    }
+
+    // - if the item is the crate root of a dependency crate, return the name from the extern prelude
+    for (name, def_id) in &def_map.extern_prelude {
+        if item == ItemInNs::Types(*def_id) {
+            return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
+        }
+    }
+
+    // - if the item is in the prelude, return the name from there
+    // TODO check prelude
+
+    // Recursive case:
+    // - if the item is an enum variant, refer to it via the enum
+
+    // - otherwise, look for modules containing (reexporting) it and import it from one of those
+    let importable_locations = find_importable_locations(db, item, from);
+    // XXX going in order for now
+    for (module_id, name) in importable_locations {
+        // TODO prevent infinite loops
+        let mut path = match find_path(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from) {
+            None => continue,
+            Some(path) => path,
+        };
+        path.segments.push(name);
+        return Some(path);
+    }
+    None
+}
+
+fn find_importable_locations(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Vec<(ModuleId, Name)> {
+    let crate_graph = db.crate_graph();
+    let mut result = Vec::new();
+    for krate in Some(from.krate).into_iter().chain(crate_graph.dependencies(from.krate).map(|dep| dep.crate_id)) {
+        let def_map = db.crate_def_map(krate);
+        for (local_id, data) in def_map.modules.iter() {
+            if let Some((name, vis)) = data.scope.reverse_get(item) {
+                if vis.is_visible_from(db, from) {
+                    result.push((ModuleId { krate, local_id }, name.clone()));
+                }
+            }
+        }
     }
-    todo!()
+    result
 }
 
 #[cfg(test)]
@@ -59,7 +118,7 @@ mod tests {
 
         let found_path = find_path(&db, ItemInNs::Types(resolved), module);
 
-        assert_eq!(mod_path, found_path);
+        assert_eq!(found_path, Some(mod_path));
     }
 
     #[test]
@@ -84,6 +143,17 @@ mod tests {
         check_found_path(code, "foo::S");
     }
 
+    #[test]
+    fn crate_root() {
+        let code = r#"
+            //- /main.rs
+            mod foo;
+            //- /foo.rs
+            <|>
+        "#;
+        check_found_path(code, "crate");
+    }
+
     #[test]
     fn same_crate() {
         let code = r#"
@@ -107,6 +177,18 @@ mod tests {
         check_found_path(code, "std::S");
     }
 
+    #[test]
+    fn different_crate_renamed() {
+        let code = r#"
+            //- /main.rs crate:main deps:std
+            extern crate std as std_renamed;
+            <|>
+            //- /std.rs crate:std
+            pub struct S;
+        "#;
+        check_found_path(code, "std_renamed::S");
+    }
+
     #[test]
     fn same_crate_reexport() {
         let code = r#"
@@ -119,4 +201,30 @@ mod tests {
         "#;
         check_found_path(code, "bar::S");
     }
+
+    #[test]
+    fn same_crate_reexport_rename() {
+        let code = r#"
+            //- /main.rs
+            mod bar {
+                mod foo { pub(super) struct S; }
+                pub(crate) use foo::S as U;
+            }
+            <|>
+        "#;
+        check_found_path(code, "bar::U");
+    }
+
+    #[test]
+    fn prelude() {
+        let code = r#"
+            //- /main.rs crate:main deps:std
+            <|>
+            //- /std.rs crate:std
+            pub mod prelude { pub struct S; }
+            #[prelude_import]
+            pub use prelude::*;
+        "#;
+        check_found_path(code, "S");
+    }
 }
-- 
cgit v1.2.3


From 1ea2b475a99b982829e543616a7dc2694e749e70 Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 21:33:25 +0100
Subject: handle most cases

---
 crates/ra_hir_def/src/find_path.rs  | 70 +++++++++++++++++++++++++++++++------
 crates/ra_hir_def/src/item_scope.rs |  8 +++++
 2 files changed, 67 insertions(+), 11 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 6772330e0..afcf04280 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -8,22 +8,12 @@ use crate::{
 };
 use hir_expand::name::Name;
 
-// TODO handle prelude
-// TODO handle enum variants
 // TODO don't import from super imports? or at least deprioritize
 // TODO use super?
 // TODO use shortest path
 // TODO performance / memoize
 
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
-    // 1. Find all locations that the item could be imported from (i.e. that are visible)
-    //    - this needs to consider other crates, for reexports from transitive dependencies
-    //    - filter by visibility
-    // 2. For each of these, go up the module tree until we find an
-    //    item/module/crate that is already in scope (including because it is in
-    //    the prelude, and including aliases!)
-    // 3. Then select the one that gives the shortest path
-
     // Base cases:
 
     // - if the item is already in scope, return the name under which it is
@@ -46,10 +36,28 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
     }
 
     // - if the item is in the prelude, return the name from there
-    // TODO check prelude
+    if let Some(prelude_module) = def_map.prelude {
+        let prelude_def_map = db.crate_def_map(prelude_module.krate);
+        let prelude_scope: &crate::item_scope::ItemScope = &prelude_def_map.modules[prelude_module.local_id].scope;
+        if let Some((name, vis)) = prelude_scope.reverse_get(item) {
+            if vis.is_visible_from(db, from) {
+                return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
+            }
+        }
+    }
 
     // Recursive case:
     // - if the item is an enum variant, refer to it via the enum
+    if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() {
+        if let Some(mut path) = find_path(db, ItemInNs::Types(variant.parent.into()), from) {
+            let data = db.enum_data(variant.parent);
+            path.segments.push(data.variants[variant.local_id].name.clone());
+            return Some(path);
+        }
+        // If this doesn't work, it seems we have no way of referring to the
+        // enum; that's very weird, but there might still be a reexport of the
+        // variant somewhere
+    }
 
     // - otherwise, look for modules containing (reexporting) it and import it from one of those
     let importable_locations = find_importable_locations(db, item, from);
@@ -131,6 +139,16 @@ mod tests {
         check_found_path(code, "S");
     }
 
+    #[test]
+    fn enum_variant() {
+        let code = r#"
+            //- /main.rs
+            enum E { A }
+            <|>
+        "#;
+        check_found_path(code, "E::A");
+    }
+
     #[test]
     fn sub_module() {
         let code = r#"
@@ -215,6 +233,19 @@ mod tests {
         check_found_path(code, "bar::U");
     }
 
+    #[test]
+    fn different_crate_reexport() {
+        let code = r#"
+            //- /main.rs crate:main deps:std
+            <|>
+            //- /std.rs crate:std deps:core
+            pub use core::S;
+            //- /core.rs crate:core
+            pub struct S;
+        "#;
+        check_found_path(code, "std::S");
+    }
+
     #[test]
     fn prelude() {
         let code = r#"
@@ -227,4 +258,21 @@ mod tests {
         "#;
         check_found_path(code, "S");
     }
+
+    #[test]
+    fn enum_variant_from_prelude() {
+        let code = r#"
+            //- /main.rs crate:main deps:std
+            <|>
+            //- /std.rs crate:std
+            pub mod prelude {
+                pub enum Option<T> { Some(T), None }
+                pub use Option::*;
+            }
+            #[prelude_import]
+            pub use prelude::*;
+        "#;
+        check_found_path(code, "None");
+        check_found_path(code, "Some");
+    }
 }
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index f88502d78..71afdb235 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -204,4 +204,12 @@ impl ItemInNs {
             },
         }
     }
+
+    pub fn as_module_def_id(self) -> Option<ModuleDefId> {
+        match self {
+            ItemInNs::Types(t) => Some(t),
+            ItemInNs::Values(v) => Some(v),
+            ItemInNs::Macros(_) => None,
+        }
+    }
 }
-- 
cgit v1.2.3


From df9d3bd25e9e80a7c55f6a786ccccdcca4a7eb03 Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 22:22:12 +0100
Subject: Use shortest path

---
 crates/ra_hir_def/src/find_path.rs | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index afcf04280..f7fd0c00a 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -10,7 +10,6 @@ use hir_expand::name::Name;
 
 // TODO don't import from super imports? or at least deprioritize
 // TODO use super?
-// TODO use shortest path
 // TODO performance / memoize
 
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
@@ -61,7 +60,7 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
 
     // - otherwise, look for modules containing (reexporting) it and import it from one of those
     let importable_locations = find_importable_locations(db, item, from);
-    // XXX going in order for now
+    let mut candidate_paths = Vec::new();
     for (module_id, name) in importable_locations {
         // TODO prevent infinite loops
         let mut path = match find_path(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from) {
@@ -69,9 +68,9 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
             Some(path) => path,
         };
         path.segments.push(name);
-        return Some(path);
+        candidate_paths.push(path);
     }
-    None
+    candidate_paths.into_iter().min_by_key(|path| path.segments.len())
 }
 
 fn find_importable_locations(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Vec<(ModuleId, Name)> {
@@ -275,4 +274,20 @@ mod tests {
         check_found_path(code, "None");
         check_found_path(code, "Some");
     }
+
+    #[test]
+    fn shortest_path() {
+        let code = r#"
+            //- /main.rs
+            pub mod foo;
+            pub mod baz;
+            struct S;
+            <|>
+            //- /foo.rs
+            pub mod bar { pub struct S; }
+            //- /baz.rs
+            pub use crate::foo::bar::S;
+        "#;
+        check_found_path(code, "baz::S");
+    }
 }
-- 
cgit v1.2.3


From 947eec7b87c4e385176e53acf4577df5fbb566cd Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 22:40:50 +0100
Subject: Use super, don't use private imports

---
 crates/ra_hir_def/src/find_path.rs | 50 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index f7fd0c00a..be34ee662 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -8,8 +8,6 @@ use crate::{
 };
 use hir_expand::name::Name;
 
-// TODO don't import from super imports? or at least deprioritize
-// TODO use super?
 // TODO performance / memoize
 
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
@@ -27,6 +25,13 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
         return Some(ModPath::from_simple_segments(PathKind::Crate, Vec::new()));
     }
 
+    // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
+    if let Some(parent_id) = def_map.modules[from.local_id].parent {
+        if item == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { krate: from.krate, local_id: parent_id })) {
+            return Some(ModPath::from_simple_segments(PathKind::Super(1), Vec::new()));
+        }
+    }
+
     // - if the item is the crate root of a dependency crate, return the name from the extern prelude
     for (name, def_id) in &def_map.extern_prelude {
         if item == ItemInNs::Types(*def_id) {
@@ -80,6 +85,19 @@ fn find_importable_locations(db: &impl DefDatabase, item: ItemInNs, from: Module
         let def_map = db.crate_def_map(krate);
         for (local_id, data) in def_map.modules.iter() {
             if let Some((name, vis)) = data.scope.reverse_get(item) {
+                let is_private = if let crate::visibility::Visibility::Module(private_to) = vis {
+                    private_to.local_id == local_id
+                } else { false };
+                let is_original_def = if let Some(module_def_id) = item.as_module_def_id() {
+                    data.scope.declarations().any(|it| it == module_def_id)
+                } else { false };
+                if is_private && !is_original_def {
+                    // Ignore private imports. these could be used if we are
+                    // in a submodule of this module, but that's usually not
+                    // what the user wants; and if this module can import
+                    // the item and we're a submodule of it, so can we.
+                    continue;
+                }
                 if vis.is_visible_from(db, from) {
                     result.push((ModuleId { krate, local_id }, name.clone()));
                 }
@@ -160,6 +178,20 @@ mod tests {
         check_found_path(code, "foo::S");
     }
 
+    #[test]
+    fn super_module() {
+        let code = r#"
+            //- /main.rs
+            mod foo;
+            //- /foo.rs
+            mod bar;
+            struct S;
+            //- /foo/bar.rs
+            <|>
+        "#;
+        check_found_path(code, "super::S");
+    }
+
     #[test]
     fn crate_root() {
         let code = r#"
@@ -290,4 +322,18 @@ mod tests {
         "#;
         check_found_path(code, "baz::S");
     }
+
+    #[test]
+    fn discount_private_imports() {
+        let code = r#"
+            //- /main.rs
+            mod foo;
+            pub mod bar { pub struct S; }
+            use bar::S;
+            //- /foo.rs
+            <|>
+        "#;
+        // crate::S would be shorter, but using private imports seems wrong
+        check_found_path(code, "crate::bar::S");
+    }
 }
-- 
cgit v1.2.3


From b1325488ec4c1b965e2e9a0b8b6dec1c8342498b Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 22:51:37 +0100
Subject: Use query for importable locations

---
 crates/ra_hir_def/src/db.rs         |  7 +++
 crates/ra_hir_def/src/find_path.rs  | 94 +++++++++++++++++++++++++------------
 crates/ra_hir_def/src/item_scope.rs |  8 ++--
 crates/ra_hir_def/src/test_db.rs    |  2 +-
 4 files changed, 77 insertions(+), 34 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs
index da273eb11..0c00627b5 100644
--- a/crates/ra_hir_def/src/db.rs
+++ b/crates/ra_hir_def/src/db.rs
@@ -107,6 +107,13 @@ pub trait DefDatabase: InternDatabase + AstDatabase {
     // Remove this query completely, in favor of `Attrs::docs` method
     #[salsa::invoke(Documentation::documentation_query)]
     fn documentation(&self, def: AttrDefId) -> Option<Documentation>;
+
+    #[salsa::invoke(crate::find_path::importable_locations_in_crate_query)]
+    fn importable_locations_in_crate(
+        &self,
+        item: crate::item_scope::ItemInNs,
+        krate: CrateId,
+    ) -> Arc<[(ModuleId, hir_expand::name::Name, crate::visibility::Visibility)]>;
 }
 
 fn crate_def_map(db: &impl DefDatabase, krate: CrateId) -> Arc<CrateDefMap> {
diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index be34ee662..09f3bf87d 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -4,12 +4,11 @@ use crate::{
     db::DefDatabase,
     item_scope::ItemInNs,
     path::{ModPath, PathKind},
-    ModuleId, ModuleDefId,
+    visibility::Visibility,
+    CrateId, ModuleDefId, ModuleId,
 };
 use hir_expand::name::Name;
 
-// TODO performance / memoize
-
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
     // Base cases:
 
@@ -21,13 +20,23 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
     }
 
     // - if the item is the crate root, return `crate`
-    if item == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { krate: from.krate, local_id: def_map.root })) {
+    if item
+        == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
+            krate: from.krate,
+            local_id: def_map.root,
+        }))
+    {
         return Some(ModPath::from_simple_segments(PathKind::Crate, Vec::new()));
     }
 
     // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
     if let Some(parent_id) = def_map.modules[from.local_id].parent {
-        if item == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { krate: from.krate, local_id: parent_id })) {
+        if item
+            == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
+                krate: from.krate,
+                local_id: parent_id,
+            }))
+        {
             return Some(ModPath::from_simple_segments(PathKind::Super(1), Vec::new()));
         }
     }
@@ -42,7 +51,8 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
     // - if the item is in the prelude, return the name from there
     if let Some(prelude_module) = def_map.prelude {
         let prelude_def_map = db.crate_def_map(prelude_module.krate);
-        let prelude_scope: &crate::item_scope::ItemScope = &prelude_def_map.modules[prelude_module.local_id].scope;
+        let prelude_scope: &crate::item_scope::ItemScope =
+            &prelude_def_map.modules[prelude_module.local_id].scope;
         if let Some((name, vis)) = prelude_scope.reverse_get(item) {
             if vis.is_visible_from(db, from) {
                 return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
@@ -68,7 +78,8 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
     let mut candidate_paths = Vec::new();
     for (module_id, name) in importable_locations {
         // TODO prevent infinite loops
-        let mut path = match find_path(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from) {
+        let mut path = match find_path(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from)
+        {
             None => continue,
             Some(path) => path,
         };
@@ -78,33 +89,58 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
     candidate_paths.into_iter().min_by_key(|path| path.segments.len())
 }
 
-fn find_importable_locations(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Vec<(ModuleId, Name)> {
+fn find_importable_locations(
+    db: &impl DefDatabase,
+    item: ItemInNs,
+    from: ModuleId,
+) -> Vec<(ModuleId, Name)> {
     let crate_graph = db.crate_graph();
     let mut result = Vec::new();
-    for krate in Some(from.krate).into_iter().chain(crate_graph.dependencies(from.krate).map(|dep| dep.crate_id)) {
-        let def_map = db.crate_def_map(krate);
-        for (local_id, data) in def_map.modules.iter() {
-            if let Some((name, vis)) = data.scope.reverse_get(item) {
-                let is_private = if let crate::visibility::Visibility::Module(private_to) = vis {
-                    private_to.local_id == local_id
-                } else { false };
-                let is_original_def = if let Some(module_def_id) = item.as_module_def_id() {
-                    data.scope.declarations().any(|it| it == module_def_id)
-                } else { false };
-                if is_private && !is_original_def {
-                    // Ignore private imports. these could be used if we are
-                    // in a submodule of this module, but that's usually not
-                    // what the user wants; and if this module can import
-                    // the item and we're a submodule of it, so can we.
-                    continue;
-                }
-                if vis.is_visible_from(db, from) {
-                    result.push((ModuleId { krate, local_id }, name.clone()));
-                }
+    for krate in Some(from.krate)
+        .into_iter()
+        .chain(crate_graph.dependencies(from.krate).map(|dep| dep.crate_id))
+    {
+        result.extend(
+            db.importable_locations_in_crate(item, krate)
+                .iter()
+                .filter(|(_, _, vis)| vis.is_visible_from(db, from))
+                .map(|(m, n, _)| (*m, n.clone())),
+        );
+    }
+    result
+}
+
+pub(crate) fn importable_locations_in_crate_query(
+    db: &impl DefDatabase,
+    item: ItemInNs,
+    krate: CrateId,
+) -> std::sync::Arc<[(ModuleId, Name, Visibility)]> {
+    let def_map = db.crate_def_map(krate);
+    let mut result = Vec::new();
+    for (local_id, data) in def_map.modules.iter() {
+        if let Some((name, vis)) = data.scope.reverse_get(item) {
+            let is_private = if let Visibility::Module(private_to) = vis {
+                private_to.local_id == local_id
+            } else {
+                false
+            };
+            let is_original_def = if let Some(module_def_id) = item.as_module_def_id() {
+                data.scope.declarations().any(|it| it == module_def_id)
+            } else {
+                false
+            };
+            if is_private && !is_original_def {
+                // Ignore private imports. these could be used if we are
+                // in a submodule of this module, but that's usually not
+                // what the user wants; and if this module can import
+                // the item and we're a submodule of it, so can we.
+                // Also this keeps the cached data smaller.
+                continue;
             }
+            result.push((ModuleId { krate, local_id }, name.clone(), vis));
         }
     }
-    result
+    result.into()
 }
 
 #[cfg(test)]
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index 71afdb235..87c50b34f 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -183,7 +183,7 @@ impl PerNs {
     }
 }
 
-#[derive(Clone, Copy, PartialEq, Eq)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
 pub enum ItemInNs {
     Types(ModuleDefId),
     Values(ModuleDefId),
@@ -195,13 +195,13 @@ impl ItemInNs {
         match self {
             ItemInNs::Types(def) => {
                 per_ns.types.filter(|(other_def, _)| *other_def == def).map(|(_, vis)| vis)
-            },
+            }
             ItemInNs::Values(def) => {
                 per_ns.values.filter(|(other_def, _)| *other_def == def).map(|(_, vis)| vis)
-            },
+            }
             ItemInNs::Macros(def) => {
                 per_ns.macros.filter(|(other_def, _)| *other_def == def).map(|(_, vis)| vis)
-            },
+            }
         }
     }
 
diff --git a/crates/ra_hir_def/src/test_db.rs b/crates/ra_hir_def/src/test_db.rs
index a403f183f..1568820e9 100644
--- a/crates/ra_hir_def/src/test_db.rs
+++ b/crates/ra_hir_def/src/test_db.rs
@@ -5,8 +5,8 @@ use std::{
     sync::{Arc, Mutex},
 };
 
-use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, RelativePath};
 use crate::db::DefDatabase;
+use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, RelativePath};
 
 #[salsa::database(
     ra_db::SourceDatabaseExtStorage,
-- 
cgit v1.2.3


From 38cd9f0c947981388af652c8691574675673c768 Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 23:07:47 +0100
Subject: Handle cycles

---
 crates/ra_hir_def/src/find_path.rs | 59 ++++++++++++++++++++++++++++++++++----
 1 file changed, 54 insertions(+), 5 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 09f3bf87d..3df2f2c09 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -10,8 +10,16 @@ use crate::{
 use hir_expand::name::Name;
 
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
+    find_path_inner(db, item, from, 15)
+}
+
+fn find_path_inner(db: &impl DefDatabase, item: ItemInNs, from: ModuleId, max_len: usize) -> Option<ModPath> {
     // Base cases:
 
+    if max_len == 0 {
+        return None;
+    }
+
     // - if the item is already in scope, return the name under which it is
     let def_map = db.crate_def_map(from.krate);
     let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
@@ -75,18 +83,31 @@ pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Optio
 
     // - otherwise, look for modules containing (reexporting) it and import it from one of those
     let importable_locations = find_importable_locations(db, item, from);
-    let mut candidate_paths = Vec::new();
+    let mut best_path = None;
+    let mut best_path_len = max_len;
     for (module_id, name) in importable_locations {
-        // TODO prevent infinite loops
-        let mut path = match find_path(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from)
+        let mut path = match find_path_inner(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from, best_path_len - 1)
         {
             None => continue,
             Some(path) => path,
         };
         path.segments.push(name);
-        candidate_paths.push(path);
+        if path_len(&path) < best_path_len {
+            best_path_len = path_len(&path);
+            best_path = Some(path);
+        }
+    }
+    best_path
+}
+
+fn path_len(path: &ModPath) -> usize {
+    path.segments.len() + match path.kind {
+        PathKind::Plain => 0,
+        PathKind::Super(i) => i as usize,
+        PathKind::Crate => 1,
+        PathKind::Abs => 0,
+        PathKind::DollarCrate(_) => 1,
     }
-    candidate_paths.into_iter().min_by_key(|path| path.segments.len())
 }
 
 fn find_importable_locations(
@@ -96,6 +117,9 @@ fn find_importable_locations(
 ) -> Vec<(ModuleId, Name)> {
     let crate_graph = db.crate_graph();
     let mut result = Vec::new();
+    // We only look in the crate from which we are importing, and the direct
+    // dependencies. We cannot refer to names from transitive dependencies
+    // directly (only through reexports in direct dependencies).
     for krate in Some(from.krate)
         .into_iter()
         .chain(crate_graph.dependencies(from.krate).map(|dep| dep.crate_id))
@@ -110,6 +134,13 @@ fn find_importable_locations(
     result
 }
 
+/// Collects all locations from which we might import the item in a particular
+/// crate. These include the original definition of the item, and any
+/// non-private `use`s.
+///
+/// Note that the crate doesn't need to be the one in which the item is defined;
+/// it might be re-exported in other crates. We cache this as a query since we
+/// need to walk the whole def map for it.
 pub(crate) fn importable_locations_in_crate_query(
     db: &impl DefDatabase,
     item: ItemInNs,
@@ -372,4 +403,22 @@ mod tests {
         // crate::S would be shorter, but using private imports seems wrong
         check_found_path(code, "crate::bar::S");
     }
+
+    #[test]
+    fn import_cycle() {
+        let code = r#"
+            //- /main.rs
+            pub mod foo;
+            pub mod bar;
+            pub mod baz;
+            //- /bar.rs
+            <|>
+            //- /foo.rs
+            pub use super::baz;
+            pub struct S;
+            //- /baz.rs
+            pub use super::foo;
+        "#;
+        check_found_path(code, "crate::foo::S");
+    }
 }
-- 
cgit v1.2.3


From 2906d188c2442d67b3d3b1bf532553e5adc9020a Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Mon, 30 Dec 2019 23:08:40 +0100
Subject: Cleanup

---
 crates/ra_hir_def/src/find_path.rs | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 3df2f2c09..166728153 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -9,17 +9,24 @@ use crate::{
 };
 use hir_expand::name::Name;
 
+const MAX_PATH_LEN: usize = 15;
+
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
-    find_path_inner(db, item, from, 15)
+    find_path_inner(db, item, from, MAX_PATH_LEN)
 }
 
-fn find_path_inner(db: &impl DefDatabase, item: ItemInNs, from: ModuleId, max_len: usize) -> Option<ModPath> {
-    // Base cases:
-
+fn find_path_inner(
+    db: &impl DefDatabase,
+    item: ItemInNs,
+    from: ModuleId,
+    max_len: usize,
+) -> Option<ModPath> {
     if max_len == 0 {
         return None;
     }
 
+    // Base cases:
+
     // - if the item is already in scope, return the name under which it is
     let def_map = db.crate_def_map(from.krate);
     let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
@@ -86,8 +93,12 @@ fn find_path_inner(db: &impl DefDatabase, item: ItemInNs, from: ModuleId, max_le
     let mut best_path = None;
     let mut best_path_len = max_len;
     for (module_id, name) in importable_locations {
-        let mut path = match find_path_inner(db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from, best_path_len - 1)
-        {
+        let mut path = match find_path_inner(
+            db,
+            ItemInNs::Types(ModuleDefId::ModuleId(module_id)),
+            from,
+            best_path_len - 1,
+        ) {
             None => continue,
             Some(path) => path,
         };
@@ -101,13 +112,14 @@ fn find_path_inner(db: &impl DefDatabase, item: ItemInNs, from: ModuleId, max_le
 }
 
 fn path_len(path: &ModPath) -> usize {
-    path.segments.len() + match path.kind {
-        PathKind::Plain => 0,
-        PathKind::Super(i) => i as usize,
-        PathKind::Crate => 1,
-        PathKind::Abs => 0,
-        PathKind::DollarCrate(_) => 1,
-    }
+    path.segments.len()
+        + match path.kind {
+            PathKind::Plain => 0,
+            PathKind::Super(i) => i as usize,
+            PathKind::Crate => 1,
+            PathKind::Abs => 0,
+            PathKind::DollarCrate(_) => 1,
+        }
 }
 
 fn find_importable_locations(
-- 
cgit v1.2.3


From 460fa71c5528d95d34465a4db6853dc8c992b80b Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Tue, 31 Dec 2019 13:01:00 +0100
Subject: Use `self`

---
 crates/ra_hir_def/src/find_path.rs | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 166728153..69cdfc943 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -11,6 +11,10 @@ use hir_expand::name::Name;
 
 const MAX_PATH_LEN: usize = 15;
 
+// FIXME: handle local items
+
+/// Find a path that can be used to refer to a certain item. This can depend on
+/// *from where* you're referring to the item, hence the `from` parameter.
 pub fn find_path(db: &impl DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
     find_path_inner(db, item, from, MAX_PATH_LEN)
 }
@@ -44,6 +48,11 @@ fn find_path_inner(
         return Some(ModPath::from_simple_segments(PathKind::Crate, Vec::new()));
     }
 
+    // - if the item is the module we're in, use `self`
+    if item == ItemInNs::Types(from.into()) {
+        return Some(ModPath::from_simple_segments(PathKind::Super(0), Vec::new()));
+    }
+
     // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
     if let Some(parent_id) = def_map.modules[from.local_id].parent {
         if item
@@ -271,6 +280,17 @@ mod tests {
         check_found_path(code, "super::S");
     }
 
+    #[test]
+    fn self_module() {
+        let code = r#"
+            //- /main.rs
+            mod foo;
+            //- /foo.rs
+            <|>
+        "#;
+        check_found_path(code, "self");
+    }
+
     #[test]
     fn crate_root() {
         let code = r#"
-- 
cgit v1.2.3


From 4d75430e912491c19fb1a7b1a95ee812f6a8a124 Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Tue, 31 Dec 2019 16:17:08 +0100
Subject: Qualify some paths in 'add missing impl members'

---
 crates/ra_hir_def/src/path.rs     | 44 ++++++++++++++++++++++++++++++++++++++-
 crates/ra_hir_def/src/resolver.rs |  5 +++++
 2 files changed, 48 insertions(+), 1 deletion(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs
index 82cfa67a9..7dd1939b9 100644
--- a/crates/ra_hir_def/src/path.rs
+++ b/crates/ra_hir_def/src/path.rs
@@ -1,7 +1,7 @@
 //! A desugared representation of paths like `crate::foo` or `<Type as Trait>::bar`.
 mod lower;
 
-use std::{iter, sync::Arc};
+use std::{fmt::Display, iter, sync::Arc};
 
 use hir_expand::{
     hygiene::Hygiene,
@@ -78,6 +78,12 @@ impl ModPath {
         }
         self.segments.first()
     }
+
+    pub fn to_ast(&self) -> ast::Path {
+        use ast::AstNode;
+        let parse = ast::SourceFile::parse(&self.to_string());
+        parse.tree().syntax().descendants().find_map(ast::Path::cast).unwrap()
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -248,6 +254,42 @@ impl From<Name> for ModPath {
     }
 }
 
+impl Display for ModPath {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut first_segment = true;
+        let mut add_segment = |s| {
+            if !first_segment {
+                f.write_str("::")?;
+            }
+            first_segment = false;
+            f.write_str(s)?;
+            Ok(())
+        };
+        match self.kind {
+            PathKind::Plain => {}
+            PathKind::Super(n) => {
+                if n == 0 {
+                    add_segment("self")?;
+                }
+                for _ in 0..n {
+                    add_segment("super")?;
+                }
+            }
+            PathKind::Crate => add_segment("crate")?,
+            PathKind::Abs => add_segment("")?,
+            PathKind::DollarCrate(_) => add_segment("$crate")?,
+        }
+        for segment in &self.segments {
+            if !first_segment {
+                f.write_str("::")?;
+            }
+            first_segment = false;
+            write!(f, "{}", segment)?;
+        }
+        Ok(())
+    }
+}
+
 pub use hir_expand::name as __name;
 
 #[macro_export]
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 5d16dd087..40d0cb588 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -411,6 +411,11 @@ impl Resolver {
         })
     }
 
+    pub fn module_id(&self) -> Option<ModuleId> {
+        let (def_map, local_id) = self.module()?;
+        Some(ModuleId { krate: def_map.krate, local_id })
+    }
+
     pub fn krate(&self) -> Option<CrateId> {
         self.module().map(|t| t.0.krate)
     }
-- 
cgit v1.2.3


From 4496e2a06a91e5825f355ce730af802643e8018a Mon Sep 17 00:00:00 2001
From: Florian Diebold <florian.diebold@freiheit.com>
Date: Fri, 10 Jan 2020 18:40:45 +0100
Subject: Apply review suggestions

---
 crates/ra_hir_def/src/db.rs         |  7 -------
 crates/ra_hir_def/src/find_path.rs  | 17 ++++++++---------
 crates/ra_hir_def/src/item_scope.rs |  5 ++---
 crates/ra_hir_def/src/path.rs       | 14 ++++++--------
 crates/ra_hir_def/src/resolver.rs   | 14 +++++++-------
 5 files changed, 23 insertions(+), 34 deletions(-)

(limited to 'crates/ra_hir_def/src')

diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs
index 0c00627b5..da273eb11 100644
--- a/crates/ra_hir_def/src/db.rs
+++ b/crates/ra_hir_def/src/db.rs
@@ -107,13 +107,6 @@ pub trait DefDatabase: InternDatabase + AstDatabase {
     // Remove this query completely, in favor of `Attrs::docs` method
     #[salsa::invoke(Documentation::documentation_query)]
     fn documentation(&self, def: AttrDefId) -> Option<Documentation>;
-
-    #[salsa::invoke(crate::find_path::importable_locations_in_crate_query)]
-    fn importable_locations_in_crate(
-        &self,
-        item: crate::item_scope::ItemInNs,
-        krate: CrateId,
-    ) -> Arc<[(ModuleId, hir_expand::name::Name, crate::visibility::Visibility)]>;
 }
 
 fn crate_def_map(db: &impl DefDatabase, krate: CrateId) -> Arc<CrateDefMap> {
diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 69cdfc943..f7dc8acb7 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -34,7 +34,7 @@ fn find_path_inner(
     // - if the item is already in scope, return the name under which it is
     let def_map = db.crate_def_map(from.krate);
     let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
-    if let Some((name, _)) = from_scope.reverse_get(item) {
+    if let Some((name, _)) = from_scope.name_of(item) {
         return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
     }
 
@@ -77,7 +77,7 @@ fn find_path_inner(
         let prelude_def_map = db.crate_def_map(prelude_module.krate);
         let prelude_scope: &crate::item_scope::ItemScope =
             &prelude_def_map.modules[prelude_module.local_id].scope;
-        if let Some((name, vis)) = prelude_scope.reverse_get(item) {
+        if let Some((name, vis)) = prelude_scope.name_of(item) {
             if vis.is_visible_from(db, from) {
                 return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
             }
@@ -146,7 +146,7 @@ fn find_importable_locations(
         .chain(crate_graph.dependencies(from.krate).map(|dep| dep.crate_id))
     {
         result.extend(
-            db.importable_locations_in_crate(item, krate)
+            importable_locations_in_crate(db, item, krate)
                 .iter()
                 .filter(|(_, _, vis)| vis.is_visible_from(db, from))
                 .map(|(m, n, _)| (*m, n.clone())),
@@ -160,17 +160,16 @@ fn find_importable_locations(
 /// non-private `use`s.
 ///
 /// Note that the crate doesn't need to be the one in which the item is defined;
-/// it might be re-exported in other crates. We cache this as a query since we
-/// need to walk the whole def map for it.
-pub(crate) fn importable_locations_in_crate_query(
+/// it might be re-exported in other crates.
+fn importable_locations_in_crate(
     db: &impl DefDatabase,
     item: ItemInNs,
     krate: CrateId,
-) -> std::sync::Arc<[(ModuleId, Name, Visibility)]> {
+) -> Vec<(ModuleId, Name, Visibility)> {
     let def_map = db.crate_def_map(krate);
     let mut result = Vec::new();
     for (local_id, data) in def_map.modules.iter() {
-        if let Some((name, vis)) = data.scope.reverse_get(item) {
+        if let Some((name, vis)) = data.scope.name_of(item) {
             let is_private = if let Visibility::Module(private_to) = vis {
                 private_to.local_id == local_id
             } else {
@@ -192,7 +191,7 @@ pub(crate) fn importable_locations_in_crate_query(
             result.push((ModuleId { krate, local_id }, name.clone(), vis));
         }
     }
-    result.into()
+    result
 }
 
 #[cfg(test)]
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index 87c50b34f..d74a1cef2 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -104,7 +104,7 @@ impl ItemScope {
         }
     }
 
-    pub(crate) fn reverse_get(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
+    pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
         for (name, per_ns) in &self.visible {
             if let Some(vis) = item.match_with(*per_ns) {
                 return Some((name, vis));
@@ -207,8 +207,7 @@ impl ItemInNs {
 
     pub fn as_module_def_id(self) -> Option<ModuleDefId> {
         match self {
-            ItemInNs::Types(t) => Some(t),
-            ItemInNs::Values(v) => Some(v),
+            ItemInNs::Types(id) | ItemInNs::Values(id) => Some(id),
             ItemInNs::Macros(_) => None,
         }
     }
diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs
index 7dd1939b9..9f93a5424 100644
--- a/crates/ra_hir_def/src/path.rs
+++ b/crates/ra_hir_def/src/path.rs
@@ -1,7 +1,11 @@
 //! A desugared representation of paths like `crate::foo` or `<Type as Trait>::bar`.
 mod lower;
 
-use std::{fmt::Display, iter, sync::Arc};
+use std::{
+    fmt::{self, Display},
+    iter,
+    sync::Arc,
+};
 
 use hir_expand::{
     hygiene::Hygiene,
@@ -78,12 +82,6 @@ impl ModPath {
         }
         self.segments.first()
     }
-
-    pub fn to_ast(&self) -> ast::Path {
-        use ast::AstNode;
-        let parse = ast::SourceFile::parse(&self.to_string());
-        parse.tree().syntax().descendants().find_map(ast::Path::cast).unwrap()
-    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -255,7 +253,7 @@ impl From<Name> for ModPath {
 }
 
 impl Display for ModPath {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut first_segment = true;
         let mut add_segment = |s| {
             if !first_segment {
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 40d0cb588..f7bac5801 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -128,7 +128,7 @@ impl Resolver {
         path: &ModPath,
         shadow: BuiltinShadowMode,
     ) -> PerNs {
-        let (item_map, module) = match self.module() {
+        let (item_map, module) = match self.module_scope() {
             Some(it) => it,
             None => return PerNs::none(),
         };
@@ -239,7 +239,7 @@ impl Resolver {
     ) -> Option<Visibility> {
         match visibility {
             RawVisibility::Module(_) => {
-                let (item_map, module) = match self.module() {
+                let (item_map, module) = match self.module_scope() {
                     Some(it) => it,
                     None => return None,
                 };
@@ -379,7 +379,7 @@ impl Resolver {
         db: &impl DefDatabase,
         path: &ModPath,
     ) -> Option<MacroDefId> {
-        let (item_map, module) = self.module()?;
+        let (item_map, module) = self.module_scope()?;
         item_map.resolve_path(db, module, &path, BuiltinShadowMode::Other).0.take_macros()
     }
 
@@ -403,7 +403,7 @@ impl Resolver {
         traits
     }
 
-    fn module(&self) -> Option<(&CrateDefMap, LocalModuleId)> {
+    fn module_scope(&self) -> Option<(&CrateDefMap, LocalModuleId)> {
         self.scopes.iter().rev().find_map(|scope| match scope {
             Scope::ModuleScope(m) => Some((&*m.crate_def_map, m.module_id)),
 
@@ -411,13 +411,13 @@ impl Resolver {
         })
     }
 
-    pub fn module_id(&self) -> Option<ModuleId> {
-        let (def_map, local_id) = self.module()?;
+    pub fn module(&self) -> Option<ModuleId> {
+        let (def_map, local_id) = self.module_scope()?;
         Some(ModuleId { krate: def_map.krate, local_id })
     }
 
     pub fn krate(&self) -> Option<CrateId> {
-        self.module().map(|t| t.0.krate)
+        self.module_scope().map(|t| t.0.krate)
     }
 
     pub fn where_predicates_in_scope<'a>(
-- 
cgit v1.2.3