6 files changed, 84 insertions, 30 deletions

diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs
index 510c5e064..30db48f86 100644
--- a/crates/ra_hir_def/src/diagnostics.rs
+++ b/crates/ra_hir_def/src/diagnostics.rs
@@ -3,7 +3,6 @@
 use std::any::Any;
 
 use hir_expand::diagnostics::Diagnostic;
-use ra_db::RelativePathBuf;
 use ra_syntax::{ast, AstPtr, SyntaxNodePtr};
 
 use hir_expand::{HirFileId, InFile};
@@ -12,7 +11,7 @@ use hir_expand::{HirFileId, InFile};
 pub struct UnresolvedModule {
     pub file: HirFileId,
     pub decl: AstPtr<ast::Module>,
-    pub candidate: RelativePathBuf,
+    pub candidate: String,
 }
 
 impl Diagnostic for UnresolvedModule {
diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index a7f59e028..06701a830 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -159,10 +159,16 @@ fn find_path_inner(
         let crate_graph = db.crate_graph();
         let extern_paths = crate_graph[from.krate].dependencies.iter().filter_map(|dep| {
             let import_map = db.import_map(dep.crate_id);
-            import_map.path_of(item).map(|modpath| {
-                let mut modpath = modpath.clone();
-                modpath.segments.insert(0, dep.as_name());
-                modpath
+            import_map.import_info_for(item).and_then(|info| {
+                // Determine best path for containing module and append last segment from `info`.
+                let mut path = find_path_inner(
+                    db,
+                    ItemInNs::Types(ModuleDefId::ModuleId(info.container)),
+                    from,
+                    best_path_len - 1,
+                )?;
+                path.segments.push(info.path.segments.last().unwrap().clone());
+                Some(path)
             })
         });
 
@@ -299,8 +305,8 @@ mod tests {
     /// `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);
+    fn check_found_path(ra_fixture: &str, path: &str) {
+        let (db, pos) = TestDB::with_position(ra_fixture);
         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
@@ -420,7 +426,6 @@ mod tests {
 
     #[test]
     fn different_crate_renamed() {
-        // Even if a local path exists, if the item is defined externally, prefer an external path.
         let code = r#"
             //- /main.rs crate:main deps:std
             extern crate std as std_renamed;
@@ -428,7 +433,45 @@ mod tests {
             //- /std.rs crate:std
             pub struct S;
         "#;
-        check_found_path(code, "std::S");
+        check_found_path(code, "std_renamed::S");
+    }
+
+    #[test]
+    fn partially_imported() {
+        // Tests that short paths are used even for external items, when parts of the path are
+        // already in scope.
+        check_found_path(
+            r#"
+            //- /main.rs crate:main deps:ra_syntax
+
+            use ra_syntax::ast;
+            <|>
+
+            //- /lib.rs crate:ra_syntax
+            pub mod ast {
+                pub enum ModuleItem {
+                    A, B, C,
+                }
+            }
+        "#,
+            "ast::ModuleItem",
+        );
+
+        check_found_path(
+            r#"
+            //- /main.rs crate:main deps:ra_syntax
+
+            <|>
+
+            //- /lib.rs crate:ra_syntax
+            pub mod ast {
+                pub enum ModuleItem {
+                    A, B, C,
+                }
+            }
+        "#,
+            "ra_syntax::ast::ModuleItem",
+        );
     }
 
     #[test]
diff --git a/crates/ra_hir_def/src/import_map.rs b/crates/ra_hir_def/src/import_map.rs
index 36b4fdd81..68e20d06b 100644
--- a/crates/ra_hir_def/src/import_map.rs
+++ b/crates/ra_hir_def/src/import_map.rs
@@ -17,6 +17,15 @@ use crate::{
 
 type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;
 
+/// Item import details stored in the `ImportMap`.
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct ImportInfo {
+    /// A path that can be used to import the item, relative to the crate's root.
+    pub path: ModPath,
+    /// The module containing this item.
+    pub container: ModuleId,
+}
+
 /// A map from publicly exported items to the path needed to import/name them from a downstream
 /// crate.
 ///
@@ -26,7 +35,7 @@ type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;
 /// Note that all paths are relative to the containing crate's root, so the crate name still needs
 /// to be prepended to the `ModPath` before the path is valid.
 pub struct ImportMap {
-    map: FxIndexMap<ItemInNs, ModPath>,
+    map: FxIndexMap<ItemInNs, ImportInfo>,
 
     /// List of keys stored in `map`, sorted lexicographically by their `ModPath`. Indexed by the
     /// values returned by running `fst`.
@@ -78,12 +87,12 @@ impl ImportMap {
                     let path = mk_path();
                     match import_map.entry(item) {
                         Entry::Vacant(entry) => {
-                            entry.insert(path);
+                            entry.insert(ImportInfo { path, container: module });
                         }
                         Entry::Occupied(mut entry) => {
                             // If the new path is shorter, prefer that one.
-                            if path.len() < entry.get().len() {
-                                *entry.get_mut() = path;
+                            if path.len() < entry.get().path.len() {
+                                *entry.get_mut() = ImportInfo { path, container: module };
                             } else {
                                 continue;
                             }
@@ -119,7 +128,7 @@ impl ImportMap {
             let start = last_batch_start;
             last_batch_start = idx + 1;
 
-            let key = fst_path(&importables[start].1);
+            let key = fst_path(&importables[start].1.path);
 
             builder.insert(key, start as u64).unwrap();
         }
@@ -132,6 +141,10 @@ impl ImportMap {
 
     /// Returns the `ModPath` needed to import/mention `item`, relative to this crate's root.
     pub fn path_of(&self, item: ItemInNs) -> Option<&ModPath> {
+        Some(&self.map.get(&item)?.path)
+    }
+
+    pub fn import_info_for(&self, item: ItemInNs) -> Option<&ImportInfo> {
         self.map.get(&item)
     }
 }
@@ -150,13 +163,13 @@ impl fmt::Debug for ImportMap {
         let mut importable_paths: Vec<_> = self
             .map
             .iter()
-            .map(|(item, modpath)| {
+            .map(|(item, info)| {
                 let ns = match item {
                     ItemInNs::Types(_) => "t",
                     ItemInNs::Values(_) => "v",
                     ItemInNs::Macros(_) => "m",
                 };
-                format!("- {} ({})", modpath, ns)
+                format!("- {} ({})", info.path, ns)
             })
             .collect();
 
@@ -171,9 +184,9 @@ fn fst_path(path: &ModPath) -> String {
     s
 }
 
-fn cmp((_, lhs): &(&ItemInNs, &ModPath), (_, rhs): &(&ItemInNs, &ModPath)) -> Ordering {
-    let lhs_str = fst_path(lhs);
-    let rhs_str = fst_path(rhs);
+fn cmp((_, lhs): &(&ItemInNs, &ImportInfo), (_, rhs): &(&ItemInNs, &ImportInfo)) -> Ordering {
+    let lhs_str = fst_path(&lhs.path);
+    let rhs_str = fst_path(&rhs.path);
     lhs_str.cmp(&rhs_str)
 }
 
@@ -243,7 +256,7 @@ pub fn search_dependencies<'a>(
             let importables = &import_map.importables[indexed_value.value as usize..];
 
             // Path shared by the importable items in this group.
-            let path = &import_map.map[&importables[0]];
+            let path = &import_map.map[&importables[0]].path;
 
             if query.anchor_end {
                 // Last segment must match query.
@@ -256,14 +269,14 @@ pub fn search_dependencies<'a>(
             // Add the items from this `ModPath` group. Those are all subsequent items in
             // `importables` whose paths match `path`.
             let iter = importables.iter().copied().take_while(|item| {
-                let item_path = &import_map.map[item];
+                let item_path = &import_map.map[item].path;
                 fst_path(item_path) == fst_path(path)
             });
 
             if query.case_sensitive {
                 // FIXME: This does not do a subsequence match.
                 res.extend(iter.filter(|item| {
-                    let item_path = &import_map.map[item];
+                    let item_path = &import_map.map[item].path;
                     item_path.to_string().contains(&query.query)
                 }));
             } else {
diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs
index f279c2ad4..b3e5f491a 100644
--- a/crates/ra_hir_def/src/nameres.rs
+++ b/crates/ra_hir_def/src/nameres.rs
@@ -296,7 +296,6 @@ pub enum ModuleSource {
 
 mod diagnostics {
     use hir_expand::diagnostics::DiagnosticSink;
-    use ra_db::RelativePathBuf;
     use ra_syntax::{ast, AstPtr};
 
     use crate::{db::DefDatabase, diagnostics::UnresolvedModule, nameres::LocalModuleId, AstId};
@@ -306,7 +305,7 @@ mod diagnostics {
         UnresolvedModule {
             module: LocalModuleId,
             declaration: AstId<ast::Module>,
-            candidate: RelativePathBuf,
+            candidate: String,
         },
     }
 
diff --git a/crates/ra_hir_def/src/nameres/mod_resolution.rs b/crates/ra_hir_def/src/nameres/mod_resolution.rs
index cede4a6fc..19fe0615a 100644
--- a/crates/ra_hir_def/src/nameres/mod_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/mod_resolution.rs
@@ -44,7 +44,7 @@ impl ModDir {
         file_id: HirFileId,
         name: &Name,
         attr_path: Option<&SmolStr>,
-    ) -> Result<(FileId, ModDir), RelativePathBuf> {
+    ) -> Result<(FileId, ModDir), String> {
         let file_id = file_id.original_file(db.upcast());
 
         let mut candidate_files = Vec::new();
@@ -52,11 +52,11 @@ impl ModDir {
             Some(attr_path) => {
                 let base =
                     if self.root_non_dir_owner { self.path.parent().unwrap() } else { &self.path };
-                candidate_files.push(base.join(attr_path))
+                candidate_files.push(base.join(attr_path).to_string())
             }
             None => {
-                candidate_files.push(self.path.join(&format!("{}.rs", name)));
-                candidate_files.push(self.path.join(&format!("{}/mod.rs", name)));
+                candidate_files.push(self.path.join(&format!("{}.rs", name)).to_string());
+                candidate_files.push(self.path.join(&format!("{}/mod.rs", name)).to_string());
             }
         };
 
diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs
index ba16442bd..190d6d98d 100644
--- a/crates/ra_hir_def/src/path.rs
+++ b/crates/ra_hir_def/src/path.rs
@@ -154,7 +154,7 @@ pub enum GenericArg {
 
 impl Path {
     /// Converts an `ast::Path` to `Path`. Works with use trees.
-    /// DEPRECATED: It does not handle `$crate` from macro call.
+    #[deprecated = "Doesn't handle hygiene, don't add new calls, remove old ones"]
     pub fn from_ast(path: ast::Path) -> Option<Path> {
         lower::lower_path(path, &Hygiene::new_unhygienic())
     }