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