find_path: return shorter paths for external items

If a containing module is already in scope, there's no need to
use the full path to the item.
This commit is contained in:
Jonas Schievink 2020-06-12 13:01:20 +02:00
parent 36353bb182
commit 0231e4ac77
2 changed files with 77 additions and 21 deletions

View File

@ -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]

View File

@ -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 {