From 877fda04c5a37241b09f155847d7e27b20875b63 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 13:53:43 +0100 Subject: [PATCH 01/19] Add test --- .../src/assists/add_missing_impl_members.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index bc49e71fec6..f0dfe778090 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -400,6 +400,29 @@ impl Foo for S { ) } + #[test] + fn test_qualify_path_1() { + check_assist( + add_missing_impl_members, + " +mod foo { + struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + #[test] fn test_empty_trait() { check_assist_not_applicable( From 22b412f1a9d245cc3d3774dab9408e3a39a52025 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 14:25:19 +0100 Subject: [PATCH 02/19] 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 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 00000000000..1ddf5fca6b6 --- /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 61f044ecf0b..ebc12e891d7 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 54e3a84bdfb..a403f183f57 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> { *self.events.lock().unwrap() = Some(Vec::new()); f(); From 2c50f996b68e9a24a564de728ffcc13d896afc1c Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 17:31:15 +0100 Subject: [PATCH 03/19] 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(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 1ddf5fca6b6..cc686ea6a5d 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 fe7bb9779c8..f88502d7886 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 + '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 { + 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) + }, + } + } +} From b62292e8f9e28410741059ebb25133b8e1e8638a Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 21:18:43 +0100 Subject: [PATCH 04/19] basics working --- crates/ra_hir_def/src/find_path.rs | 118 +++++++++++++++++++++++++++-- crates/ra_hir_expand/src/name.rs | 4 + 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index cc686ea6a5d..6772330e099 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 { // 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()])); } - todo!() + + // - 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())); + } + } + } + } + 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"); + } } diff --git a/crates/ra_hir_expand/src/name.rs b/crates/ra_hir_expand/src/name.rs index b3fa1efbab5..a4d912b99db 100644 --- a/crates/ra_hir_expand/src/name.rs +++ b/crates/ra_hir_expand/src/name.rs @@ -37,6 +37,10 @@ impl Name { Name(Repr::TupleField(idx)) } + pub fn for_crate_dependency(dep: &ra_db::Dependency) -> Name { + Name::new_text(dep.name.clone()) + } + /// Shortcut to create inline plain text name const fn new_inline_ascii(text: &[u8]) -> Name { Name::new_text(SmolStr::new_inline_from_ascii(text.len(), text)) From 1ea2b475a99b982829e543616a7dc2694e749e70 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 21:33:25 +0100 Subject: [PATCH 05/19] 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(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 6772330e099..afcf0428072 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 { - // 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 { 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 f88502d7886..71afdb235fe 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 { + match self { + ItemInNs::Types(t) => Some(t), + ItemInNs::Values(v) => Some(v), + ItemInNs::Macros(_) => None, + } + } } From df9d3bd25e9e80a7c55f6a786ccccdcca4a7eb03 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 22:22:12 +0100 Subject: [PATCH 06/19] Use shortest path --- crates/ra_hir_def/src/find_path.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index afcf0428072..f7fd0c00a6f 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 { @@ -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"); + } } From 947eec7b87c4e385176e53acf4577df5fbb566cd Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 22:40:50 +0100 Subject: [PATCH 07/19] Use super, don't use private imports --- crates/ra_hir_def/src/find_path.rs | 50 ++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index f7fd0c00a6f..be34ee66261 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 { @@ -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"); + } } From b1325488ec4c1b965e2e9a0b8b6dec1c8342498b Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 22:51:37 +0100 Subject: [PATCH 08/19] Use query for importable locations --- crates/ra_hir_def/src/db.rs | 7 +++ crates/ra_hir_def/src/find_path.rs | 96 ++++++++++++++++++++--------- crates/ra_hir_def/src/item_scope.rs | 8 +-- crates/ra_hir_def/src/test_db.rs | 2 +- 4 files changed, 78 insertions(+), 35 deletions(-) diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs index da273eb1155..0c00627b597 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; + + #[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 { diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index be34ee66261..09f3bf87d0f 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 { // 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,35 +89,60 @@ 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.into() +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index 71afdb235fe..87c50b34f8e 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 a403f183f57..1568820e9af 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, From 38cd9f0c947981388af652c8691574675673c768 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 23:07:47 +0100 Subject: [PATCH 09/19] Handle cycles --- crates/ra_hir_def/src/find_path.rs | 59 +++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 09f3bf87d0f..3df2f2c0994 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 { + find_path_inner(db, item, from, 15) +} + +fn find_path_inner(db: &impl DefDatabase, item: ItemInNs, from: ModuleId, max_len: usize) -> Option { // 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"); + } } From 2906d188c2442d67b3d3b1bf532553e5adc9020a Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 30 Dec 2019 23:08:40 +0100 Subject: [PATCH 10/19] Cleanup --- crates/ra_hir_def/src/find_path.rs | 38 ++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 3df2f2c0994..166728153c5 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 { - 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 { - // Base cases: - +fn find_path_inner( + db: &impl DefDatabase, + item: ItemInNs, + from: ModuleId, + max_len: usize, +) -> Option { 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( From 460fa71c5528d95d34465a4db6853dc8c992b80b Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 31 Dec 2019 13:01:00 +0100 Subject: [PATCH 11/19] Use `self` --- crates/ra_hir_def/src/find_path.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 166728153c5..69cdfc943a4 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 { 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#" From 4d75430e912491c19fb1a7b1a95ee812f6a8a124 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 31 Dec 2019 16:17:08 +0100 Subject: [PATCH 12/19] Qualify some paths in 'add missing impl members' --- .../src/assists/add_missing_impl_members.rs | 49 ++++++++++++++++++- crates/ra_hir/src/code_model.rs | 13 +++++ crates/ra_hir/src/from_id.rs | 16 ++++++ crates/ra_hir/src/source_binder.rs | 4 ++ crates/ra_hir_def/src/path.rs | 44 ++++++++++++++++- crates/ra_hir_def/src/resolver.rs | 5 ++ 6 files changed, 128 insertions(+), 3 deletions(-) diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index f0dfe778090..2b072686991 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -139,6 +139,12 @@ fn add_missing_impl_members_inner( ctx.add_assist(AssistId(assist_id), label, |edit| { let n_existing_items = impl_item_list.impl_items().count(); + let module = hir::SourceAnalyzer::new( + db, + hir::InFile::new(file_id.into(), impl_node.syntax()), + None, + ) + .module(); let substs = get_syntactic_substs(impl_node).unwrap_or_default(); let generic_def: hir::GenericDef = trait_.into(); let substs_by_param: HashMap<_, _> = generic_def @@ -150,6 +156,10 @@ fn add_missing_impl_members_inner( .collect(); let items = missing_items .into_iter() + .map(|it| match module { + Some(module) => qualify_paths(db, hir::InFile::new(file_id.into(), it), module), + None => it, + }) .map(|it| { substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) }) @@ -227,6 +237,41 @@ fn substitute_type_params( } } +use hir::PathResolution; + +// TODO handle partial paths, with generic args +// TODO handle value ns? + +fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { + let path_replacements = node + .value + .syntax() + .descendants() + .filter_map(ast::Path::cast) + .filter_map(|p| { + let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); + let resolution = analyzer.resolve_path(db, &p)?; + match resolution { + PathResolution::Def(def) => { + let found_path = from.find_path(db, def)?; + Some((p, found_path.to_ast())) + } + PathResolution::Local(_) + | PathResolution::TypeParam(_) + | PathResolution::SelfType(_) => None, + PathResolution::Macro(_) => None, + PathResolution::AssocItem(_) => None, + } + }) + .collect::>(); + + if path_replacements.is_empty() { + node.value + } else { + edit::replace_descendants(&node.value, path_replacements.into_iter()) + } +} + /// Given an `ast::ImplBlock`, resolves the target trait (the one being /// implemented) to a `ast::TraitDef`. fn resolve_target_trait_def( @@ -406,14 +451,14 @@ impl Foo for S { add_missing_impl_members, " mod foo { - struct Bar; + pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; impl foo::Foo for S { <|> }", " mod foo { - struct Bar; + pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index cc42068a100..4da3db0d6b8 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -227,6 +227,19 @@ impl Module { pub(crate) fn with_module_id(self, module_id: LocalModuleId) -> Module { Module::new(self.krate(), module_id) } + + pub fn find_path( + self, + db: &impl DefDatabase, + item: ModuleDef, + ) -> Option { + // FIXME expose namespace choice + hir_def::find_path::find_path( + db, + hir_def::item_scope::ItemInNs::Types(item.into()), + self.into(), + ) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/crates/ra_hir/src/from_id.rs b/crates/ra_hir/src/from_id.rs index 75a1a777221..c16c17072b7 100644 --- a/crates/ra_hir/src/from_id.rs +++ b/crates/ra_hir/src/from_id.rs @@ -91,6 +91,22 @@ impl From for ModuleDef { } } +impl From for ModuleDefId { + fn from(id: ModuleDef) -> Self { + match id { + ModuleDef::Module(it) => ModuleDefId::ModuleId(it.into()), + ModuleDef::Function(it) => ModuleDefId::FunctionId(it.into()), + ModuleDef::Adt(it) => ModuleDefId::AdtId(it.into()), + ModuleDef::EnumVariant(it) => ModuleDefId::EnumVariantId(it.into()), + ModuleDef::Const(it) => ModuleDefId::ConstId(it.into()), + ModuleDef::Static(it) => ModuleDefId::StaticId(it.into()), + ModuleDef::Trait(it) => ModuleDefId::TraitId(it.into()), + ModuleDef::TypeAlias(it) => ModuleDefId::TypeAliasId(it.into()), + ModuleDef::BuiltinType(it) => ModuleDefId::BuiltinType(it), + } + } +} + impl From for DefWithBodyId { fn from(def: DefWithBody) -> Self { match def { diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index 2c422af8bd6..71339565f06 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -205,6 +205,10 @@ impl SourceAnalyzer { } } + pub fn module(&self) -> Option { + Some(crate::code_model::Module { id: self.resolver.module_id()? }) + } + fn expr_id(&self, expr: &ast::Expr) -> Option { let src = InFile { file_id: self.file_id, value: expr }; self.body_source_map.as_ref()?.node_expr(src) diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs index 82cfa67a9a4..7dd1939b988 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 `::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 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 5d16dd0871e..40d0cb58808 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 { + let (def_map, local_id) = self.module()?; + Some(ModuleId { krate: def_map.krate, local_id }) + } + pub fn krate(&self) -> Option { self.module().map(|t| t.0.krate) } From 5cb1f7132277e16ec4eecafbc274563c4d27158e Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 1 Jan 2020 23:08:22 +0100 Subject: [PATCH 13/19] More failing tests --- .../src/assists/add_missing_impl_members.rs | 127 +++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 2b072686991..dd62b1b78a5 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -239,9 +239,11 @@ fn substitute_type_params( use hir::PathResolution; -// TODO handle partial paths, with generic args +// TODO handle generic args +// TODO handle associated item paths // TODO handle value ns? +// FIXME extract this to a general utility as well fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { let path_replacements = node .value @@ -249,6 +251,10 @@ fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: .descendants() .filter_map(ast::Path::cast) .filter_map(|p| { + if p.segment().and_then(|s| s.param_list()).is_some() { + // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway + return None; + } let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); let resolution = analyzer.resolve_path(db, &p)?; match resolution { @@ -468,6 +474,125 @@ impl foo::Foo for S { ); } + #[test] + fn test_qualify_path_generic() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_and_substitute_param() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_associated_item() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + impl Bar { type Assoc = u32; } + trait Foo { fn foo(&self, bar: Bar::Assoc); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + impl Bar { type Assoc = u32; } + trait Foo { fn foo(&self, bar: Bar::Assoc); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar::Assoc) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_nested() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub struct Bar; + pub struct Baz; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub struct Bar; + pub struct Baz; + trait Foo { fn foo(&self, bar: Bar); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: foo::Bar) { unimplemented!() } +}", + ); + } + + #[test] + fn test_qualify_path_fn_trait_notation() { + check_assist( + add_missing_impl_members, + " +mod foo { + pub trait Fn { type Output; } + trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); } +} +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + pub trait Fn { type Output; } + trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); } +} +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { unimplemented!() } +}", + ); + } + #[test] fn test_empty_trait() { check_assist_not_applicable( From 4545f289a991ec3888896aac0e0bcbfac9061e80 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 3 Jan 2020 19:58:56 +0100 Subject: [PATCH 14/19] Handle type args --- .../src/assists/add_missing_impl_members.rs | 21 +++++++++++-------- crates/ra_syntax/src/ast/make.rs | 7 +++++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index dd62b1b78a5..7b50fb422fc 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -156,13 +156,13 @@ fn add_missing_impl_members_inner( .collect(); let items = missing_items .into_iter() + .map(|it| { + substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) + }) .map(|it| match module { Some(module) => qualify_paths(db, hir::InFile::new(file_id.into(), it), module), None => it, }) - .map(|it| { - substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) - }) .map(|it| match it { ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)), _ => it, @@ -239,11 +239,9 @@ fn substitute_type_params( use hir::PathResolution; -// TODO handle generic args -// TODO handle associated item paths -// TODO handle value ns? - // FIXME extract this to a general utility as well +// FIXME handle value ns? +// FIXME this doesn't 'commute' with `substitute_type_params`, since type params in newly generated type arg lists don't resolve. Currently we can avoid this problem, but it's worth thinking about a solution fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { let path_replacements = node .value @@ -255,12 +253,17 @@ fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } + // FIXME check if some ancestor is already being replaced, if so skip this let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); let resolution = analyzer.resolve_path(db, &p)?; match resolution { PathResolution::Def(def) => { let found_path = from.find_path(db, def)?; - Some((p, found_path.to_ast())) + let args = p + .segment() + .and_then(|s| s.type_arg_list()) + .map(|arg_list| qualify_paths(db, node.with_value(arg_list), from)); + Some((p, make::path_with_type_arg_list(found_path.to_ast(), args))) } PathResolution::Local(_) | PathResolution::TypeParam(_) @@ -535,7 +538,7 @@ impl foo::Foo for S { <|> }", " mod foo { pub struct Bar; - impl Bar { type Assoc = u32; } + impl Bar { type Assoc = u32; } trait Foo { fn foo(&self, bar: Bar::Assoc); } } struct S; diff --git a/crates/ra_syntax/src/ast/make.rs b/crates/ra_syntax/src/ast/make.rs index 04a5408fefa..68d64a0cc42 100644 --- a/crates/ra_syntax/src/ast/make.rs +++ b/crates/ra_syntax/src/ast/make.rs @@ -21,6 +21,13 @@ pub fn path_qualified(qual: ast::Path, name_ref: ast::NameRef) -> ast::Path { fn path_from_text(text: &str) -> ast::Path { ast_from_text(text) } +pub fn path_with_type_arg_list(path: ast::Path, args: Option) -> ast::Path { + if let Some(args) = args { + ast_from_text(&format!("const X: {}{}", path.syntax(), args.syntax())) + } else { + path + } +} pub fn record_field(name: ast::NameRef, expr: Option) -> ast::RecordField { return match expr { From def124e932f02f5961d26af6cc03f696f389205f Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 3 Jan 2020 23:05:58 +0100 Subject: [PATCH 15/19] Fix file ID when qualifying paths; add another failing test --- .../src/assists/add_missing_impl_members.rs | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 7b50fb422fc..22f1157ccdc 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -134,8 +134,9 @@ fn add_missing_impl_members_inner( return None; } - let file_id = ctx.frange.file_id; let db = ctx.db; + let file_id = ctx.frange.file_id; + let trait_file_id = trait_.source(db).file_id; ctx.add_assist(AssistId(assist_id), label, |edit| { let n_existing_items = impl_item_list.impl_items().count(); @@ -157,10 +158,10 @@ fn add_missing_impl_members_inner( let items = missing_items .into_iter() .map(|it| { - substitute_type_params(db, hir::InFile::new(file_id.into(), it), &substs_by_param) + substitute_type_params(db, hir::InFile::new(trait_file_id, it), &substs_by_param) }) .map(|it| match module { - Some(module) => qualify_paths(db, hir::InFile::new(file_id.into(), it), module), + Some(module) => qualify_paths(db, hir::InFile::new(trait_file_id, it), module), None => it, }) .map(|it| match it { @@ -259,6 +260,7 @@ fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: match resolution { PathResolution::Def(def) => { let found_path = from.find_path(db, def)?; + // TODO fix type arg replacements being qualified let args = p .segment() .and_then(|s| s.type_arg_list()) @@ -523,6 +525,32 @@ impl foo::Foo for S { ); } + #[test] + fn test_substitute_param_no_qualify() { + // when substituting params, the substituted param should not be qualified! + check_assist( + add_missing_impl_members, + " +mod foo { + trait Foo { fn foo(&self, bar: T); } + pub struct Param; +} +struct Param; +struct S; +impl foo::Foo for S { <|> }", + " +mod foo { + trait Foo { fn foo(&self, bar: T); } + pub struct Param; +} +struct Param; +struct S; +impl foo::Foo for S { + <|>fn foo(&self, bar: Param) { unimplemented!() } +}", + ); + } + #[test] fn test_qualify_path_associated_item() { check_assist( From 12905e5b58f22df026ef30afa6f0bdf7319cbddd Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 5 Jan 2020 21:32:18 +0100 Subject: [PATCH 16/19] Some more refactoring --- .../src/assists/add_missing_impl_members.rs | 14 ++++++-------- crates/ra_hir_expand/src/lib.rs | 10 ++++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 22f1157ccdc..942b34dc161 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -207,25 +207,23 @@ fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> } // FIXME: This should be a general utility (not even just for assists) -fn substitute_type_params( +fn substitute_type_params( db: &impl HirDatabase, node: hir::InFile, substs: &HashMap, ) -> N { let type_param_replacements = node - .value - .syntax() - .descendants() - .filter_map(ast::TypeRef::cast) + .clone() + .descendants::() .filter_map(|n| { - let path = match &n { + let path = match &n.value { ast::TypeRef::PathType(path_type) => path_type.path()?, _ => return None, }; - let analyzer = hir::SourceAnalyzer::new(db, node.with_value(n.syntax()), None); + let analyzer = hir::SourceAnalyzer::new(db, n.syntax(), None); let resolution = analyzer.resolve_path(db, &path)?; match resolution { - hir::PathResolution::TypeParam(tp) => Some((n, substs.get(&tp)?.clone())), + hir::PathResolution::TypeParam(tp) => Some((n.value, substs.get(&tp)?.clone())), _ => None, } }) diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index 2fa5d51402b..51c5f9623c2 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -322,3 +322,13 @@ impl InFile { }) } } + +impl InFile { + pub fn descendants(self) -> impl Iterator> { + self.value.syntax().descendants().filter_map(T::cast).map(move |n| self.with_value(n)) + } + + pub fn syntax(&self) -> InFile<&SyntaxNode> { + self.with_value(self.value.syntax()) + } +} From 15fc643e05bf8273e378243edbfb3be7aea7ce11 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:26:18 +0100 Subject: [PATCH 17/19] Fix ordering problem between qualifying paths and substituting params --- .../src/assists/add_missing_impl_members.rs | 121 +----------- crates/ra_assists/src/ast_transform.rs | 178 ++++++++++++++++++ crates/ra_assists/src/lib.rs | 1 + crates/ra_ide/src/expand_macro.rs | 7 +- crates/ra_syntax/src/algo.rs | 6 +- crates/ra_syntax/src/ast/edit.rs | 8 +- crates/ra_syntax/src/ast/make.rs | 11 +- 7 files changed, 206 insertions(+), 126 deletions(-) create mode 100644 crates/ra_assists/src/ast_transform.rs diff --git a/crates/ra_assists/src/assists/add_missing_impl_members.rs b/crates/ra_assists/src/assists/add_missing_impl_members.rs index 942b34dc161..bf1136193d3 100644 --- a/crates/ra_assists/src/assists/add_missing_impl_members.rs +++ b/crates/ra_assists/src/assists/add_missing_impl_members.rs @@ -1,12 +1,13 @@ -use std::collections::HashMap; - -use hir::{db::HirDatabase, HasSource}; +use hir::{db::HirDatabase, HasSource, InFile}; use ra_syntax::{ ast::{self, edit, make, AstNode, NameOwner}, SmolStr, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{ + ast_transform::{self, AstTransform, QualifyPaths, SubstituteTypeParams}, + Assist, AssistCtx, AssistId, +}; #[derive(PartialEq)] enum AddMissingImplMembersMode { @@ -146,24 +147,11 @@ fn add_missing_impl_members_inner( None, ) .module(); - let substs = get_syntactic_substs(impl_node).unwrap_or_default(); - let generic_def: hir::GenericDef = trait_.into(); - let substs_by_param: HashMap<_, _> = generic_def - .params(db) - .into_iter() - // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky - .skip(1) - .zip(substs.into_iter()) - .collect(); + let ast_transform = QualifyPaths::new(db, module) + .or(SubstituteTypeParams::for_trait_impl(db, trait_, impl_node)); let items = missing_items .into_iter() - .map(|it| { - substitute_type_params(db, hir::InFile::new(trait_file_id, it), &substs_by_param) - }) - .map(|it| match module { - Some(module) => qualify_paths(db, hir::InFile::new(trait_file_id, it), module), - None => it, - }) + .map(|it| ast_transform::apply(&*ast_transform, InFile::new(trait_file_id, it))) .map(|it| match it { ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)), _ => it, @@ -188,99 +176,6 @@ fn add_body(fn_def: ast::FnDef) -> ast::FnDef { } } -// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the -// trait ref, and then go from the types in the substs back to the syntax) -// FIXME: This should be a general utility (not even just for assists) -fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> { - let target_trait = impl_block.target_trait()?; - let path_type = match target_trait { - ast::TypeRef::PathType(path) => path, - _ => return None, - }; - let type_arg_list = path_type.path()?.segment()?.type_arg_list()?; - let mut result = Vec::new(); - for type_arg in type_arg_list.type_args() { - let type_arg: ast::TypeArg = type_arg; - result.push(type_arg.type_ref()?); - } - Some(result) -} - -// FIXME: This should be a general utility (not even just for assists) -fn substitute_type_params( - db: &impl HirDatabase, - node: hir::InFile, - substs: &HashMap, -) -> N { - let type_param_replacements = node - .clone() - .descendants::() - .filter_map(|n| { - let path = match &n.value { - ast::TypeRef::PathType(path_type) => path_type.path()?, - _ => return None, - }; - let analyzer = hir::SourceAnalyzer::new(db, n.syntax(), None); - let resolution = analyzer.resolve_path(db, &path)?; - match resolution { - hir::PathResolution::TypeParam(tp) => Some((n.value, substs.get(&tp)?.clone())), - _ => None, - } - }) - .collect::>(); - - if type_param_replacements.is_empty() { - node.value - } else { - edit::replace_descendants(&node.value, type_param_replacements.into_iter()) - } -} - -use hir::PathResolution; - -// FIXME extract this to a general utility as well -// FIXME handle value ns? -// FIXME this doesn't 'commute' with `substitute_type_params`, since type params in newly generated type arg lists don't resolve. Currently we can avoid this problem, but it's worth thinking about a solution -fn qualify_paths(db: &impl HirDatabase, node: hir::InFile, from: hir::Module) -> N { - let path_replacements = node - .value - .syntax() - .descendants() - .filter_map(ast::Path::cast) - .filter_map(|p| { - if p.segment().and_then(|s| s.param_list()).is_some() { - // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway - return None; - } - // FIXME check if some ancestor is already being replaced, if so skip this - let analyzer = hir::SourceAnalyzer::new(db, node.with_value(p.syntax()), None); - let resolution = analyzer.resolve_path(db, &p)?; - match resolution { - PathResolution::Def(def) => { - let found_path = from.find_path(db, def)?; - // TODO fix type arg replacements being qualified - let args = p - .segment() - .and_then(|s| s.type_arg_list()) - .map(|arg_list| qualify_paths(db, node.with_value(arg_list), from)); - Some((p, make::path_with_type_arg_list(found_path.to_ast(), args))) - } - PathResolution::Local(_) - | PathResolution::TypeParam(_) - | PathResolution::SelfType(_) => None, - PathResolution::Macro(_) => None, - PathResolution::AssocItem(_) => None, - } - }) - .collect::>(); - - if path_replacements.is_empty() { - node.value - } else { - edit::replace_descendants(&node.value, path_replacements.into_iter()) - } -} - /// Given an `ast::ImplBlock`, resolves the target trait (the one being /// implemented) to a `ast::TraitDef`. fn resolve_target_trait_def( diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs new file mode 100644 index 00000000000..84666158781 --- /dev/null +++ b/crates/ra_assists/src/ast_transform.rs @@ -0,0 +1,178 @@ +//! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined. +use std::collections::HashMap; + +use hir::{db::HirDatabase, InFile, PathResolution}; +use ra_syntax::ast::{self, make, AstNode}; + +pub trait AstTransform<'a> { + fn get_substitution( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option; + + fn chain_before(self, other: Box + 'a>) -> Box + 'a>; + fn or + 'a>(self, other: T) -> Box + 'a> + where + Self: Sized + 'a, + { + self.chain_before(Box::new(other)) + } +} + +struct NullTransformer; + +impl<'a> AstTransform<'a> for NullTransformer { + fn get_substitution( + &self, + _node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + None + } + fn chain_before(self, other: Box + 'a>) -> Box + 'a> { + other + } +} + +pub struct SubstituteTypeParams<'a, DB: HirDatabase> { + db: &'a DB, + substs: HashMap, + previous: Box + 'a>, +} + +impl<'a, DB: HirDatabase> SubstituteTypeParams<'a, DB> { + pub fn for_trait_impl( + db: &'a DB, + trait_: hir::Trait, + impl_block: ast::ImplBlock, + ) -> SubstituteTypeParams<'a, DB> { + let substs = get_syntactic_substs(impl_block).unwrap_or_default(); + let generic_def: hir::GenericDef = trait_.into(); + let substs_by_param: HashMap<_, _> = generic_def + .params(db) + .into_iter() + // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky + .skip(1) + .zip(substs.into_iter()) + .collect(); + return SubstituteTypeParams { + db, + substs: substs_by_param, + previous: Box::new(NullTransformer), + }; + + fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> { + let target_trait = impl_block.target_trait()?; + let path_type = match target_trait { + ast::TypeRef::PathType(path) => path, + _ => return None, + }; + let type_arg_list = path_type.path()?.segment()?.type_arg_list()?; + let mut result = Vec::new(); + for type_arg in type_arg_list.type_args() { + let type_arg: ast::TypeArg = type_arg; + result.push(type_arg.type_ref()?); + } + Some(result) + } + } + fn get_substitution_inner( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + let type_ref = ast::TypeRef::cast(node.value.clone())?; + let path = match &type_ref { + ast::TypeRef::PathType(path_type) => path_type.path()?, + _ => return None, + }; + let analyzer = hir::SourceAnalyzer::new(self.db, node, None); + let resolution = analyzer.resolve_path(self.db, &path)?; + match resolution { + hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()), + _ => None, + } + } +} + +impl<'a, DB: HirDatabase> AstTransform<'a> for SubstituteTypeParams<'a, DB> { + fn get_substitution( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + self.get_substitution_inner(node).or_else(|| self.previous.get_substitution(node)) + } + fn chain_before(self, other: Box + 'a>) -> Box + 'a> { + Box::new(SubstituteTypeParams { previous: other, ..self }) + } +} + +pub struct QualifyPaths<'a, DB: HirDatabase> { + db: &'a DB, + from: Option, + previous: Box + 'a>, +} + +impl<'a, DB: HirDatabase> QualifyPaths<'a, DB> { + pub fn new(db: &'a DB, from: Option) -> Self { + Self { db, from, previous: Box::new(NullTransformer) } + } + + fn get_substitution_inner( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + // FIXME handle value ns? + let from = self.from?; + let p = ast::Path::cast(node.value.clone())?; + if p.segment().and_then(|s| s.param_list()).is_some() { + // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway + return None; + } + let analyzer = hir::SourceAnalyzer::new(self.db, node, None); + let resolution = analyzer.resolve_path(self.db, &p)?; + match resolution { + PathResolution::Def(def) => { + let found_path = from.find_path(self.db, def)?; + let args = p + .segment() + .and_then(|s| s.type_arg_list()) + .map(|arg_list| apply(self, node.with_value(arg_list))); + Some(make::path_with_type_arg_list(found_path.to_ast(), args).syntax().clone()) + } + PathResolution::Local(_) + | PathResolution::TypeParam(_) + | PathResolution::SelfType(_) => None, + PathResolution::Macro(_) => None, + PathResolution::AssocItem(_) => None, + } + } +} + +pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: InFile) -> N { + let syntax = node.value.syntax(); + let result = ra_syntax::algo::replace_descendants(syntax, &|element| match element { + ra_syntax::SyntaxElement::Node(n) => { + let replacement = transformer.get_substitution(node.with_value(&n))?; + Some(replacement.into()) + } + _ => None, + }); + N::cast(result).unwrap() +} + +impl<'a, DB: HirDatabase> AstTransform<'a> for QualifyPaths<'a, DB> { + fn get_substitution( + &self, + node: InFile<&ra_syntax::SyntaxNode>, + ) -> Option { + self.get_substitution_inner(node).or_else(|| self.previous.get_substitution(node)) + } + fn chain_before(self, other: Box + 'a>) -> Box + 'a> { + Box::new(QualifyPaths { previous: other, ..self }) + } +} + +// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the +// trait ref, and then go from the types in the substs back to the syntax) +// FIXME: This should be a general utility (not even just for assists) + +// FIXME: This should be a general utility (not even just for assists) diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 98fb20b227f..712ff6f6a5f 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -11,6 +11,7 @@ mod marks; mod doc_tests; #[cfg(test)] mod test_db; +pub mod ast_transform; use hir::db::HirDatabase; use ra_db::FileRange; diff --git a/crates/ra_ide/src/expand_macro.rs b/crates/ra_ide/src/expand_macro.rs index bdbc31704b0..0f7b6e875f7 100644 --- a/crates/ra_ide/src/expand_macro.rs +++ b/crates/ra_ide/src/expand_macro.rs @@ -7,8 +7,7 @@ use rustc_hash::FxHashMap; use ra_syntax::{ algo::{find_node_at_offset, replace_descendants}, - ast::{self}, - AstNode, NodeOrToken, SyntaxKind, SyntaxNode, WalkEvent, T, + ast, AstNode, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, WalkEvent, T, }; pub struct ExpandedMacro { @@ -43,7 +42,7 @@ fn expand_macro_recur( let mut expanded: SyntaxNode = db.parse_or_expand(macro_file_id)?; let children = expanded.descendants().filter_map(ast::MacroCall::cast); - let mut replaces = FxHashMap::default(); + let mut replaces: FxHashMap = FxHashMap::default(); for child in children.into_iter() { let node = hir::InFile::new(macro_file_id, &child); @@ -59,7 +58,7 @@ fn expand_macro_recur( } } - Some(replace_descendants(&expanded, &replaces)) + Some(replace_descendants(&expanded, &|n| replaces.get(n).cloned())) } // FIXME: It would also be cool to share logic here and in the mbe tests, diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs index 2b2b295f9d1..30a479f015f 100644 --- a/crates/ra_syntax/src/algo.rs +++ b/crates/ra_syntax/src/algo.rs @@ -184,17 +184,17 @@ pub fn replace_children( /// to create a type-safe abstraction on top of it instead. pub fn replace_descendants( parent: &SyntaxNode, - map: &FxHashMap, + map: &impl Fn(&SyntaxElement) -> Option, ) -> SyntaxNode { // FIXME: this could be made much faster. let new_children = parent.children_with_tokens().map(|it| go(map, it)).collect::>(); return with_children(parent, new_children); fn go( - map: &FxHashMap, + map: &impl Fn(&SyntaxElement) -> Option, element: SyntaxElement, ) -> NodeOrToken { - if let Some(replacement) = map.get(&element) { + if let Some(replacement) = map(&element) { return match replacement { NodeOrToken::Node(it) => NodeOrToken::Node(it.green().clone()), NodeOrToken::Token(it) => NodeOrToken::Token(it.green().clone()), diff --git a/crates/ra_syntax/src/ast/edit.rs b/crates/ra_syntax/src/ast/edit.rs index ae5d639276e..b736098acc0 100644 --- a/crates/ra_syntax/src/ast/edit.rs +++ b/crates/ra_syntax/src/ast/edit.rs @@ -236,8 +236,8 @@ pub fn replace_descendants( ) -> N { let map = replacement_map .map(|(from, to)| (from.syntax().clone().into(), to.syntax().clone().into())) - .collect::>(); - let new_syntax = algo::replace_descendants(parent.syntax(), &map); + .collect::>(); + let new_syntax = algo::replace_descendants(parent.syntax(), &|n| map.get(n).cloned()); N::cast(new_syntax).unwrap() } @@ -292,7 +292,7 @@ impl IndentLevel { ) }) .collect(); - algo::replace_descendants(&node, &replacements) + algo::replace_descendants(&node, &|n| replacements.get(n).cloned()) } pub fn decrease_indent(self, node: N) -> N { @@ -320,7 +320,7 @@ impl IndentLevel { ) }) .collect(); - algo::replace_descendants(&node, &replacements) + algo::replace_descendants(&node, &|n| replacements.get(n).cloned()) } } diff --git a/crates/ra_syntax/src/ast/make.rs b/crates/ra_syntax/src/ast/make.rs index 68d64a0cc42..9781b748f1d 100644 --- a/crates/ra_syntax/src/ast/make.rs +++ b/crates/ra_syntax/src/ast/make.rs @@ -2,7 +2,7 @@ //! of smaller pieces. use itertools::Itertools; -use crate::{ast, AstNode, SourceFile}; +use crate::{algo, ast, AstNode, SourceFile}; pub fn name(text: &str) -> ast::Name { ast_from_text(&format!("mod {};", text)) @@ -23,7 +23,14 @@ fn path_from_text(text: &str) -> ast::Path { } pub fn path_with_type_arg_list(path: ast::Path, args: Option) -> ast::Path { if let Some(args) = args { - ast_from_text(&format!("const X: {}{}", path.syntax(), args.syntax())) + let syntax = path.syntax(); + // FIXME: remove existing type args + let new_syntax = algo::insert_children( + syntax, + crate::algo::InsertPosition::Last, + &mut Some(args).into_iter().map(|n| n.syntax().clone().into()), + ); + ast::Path::cast(new_syntax).unwrap() } else { path } From 4496e2a06a91e5825f355ce730af802643e8018a Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:40:45 +0100 Subject: [PATCH 18/19] Apply review suggestions --- crates/ra_assists/src/ast_transform.rs | 15 ++++++++------- crates/ra_hir/src/code_model.rs | 4 +++- crates/ra_hir/src/source_binder.rs | 2 +- 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 +++++++------- crates/ra_hir_expand/src/name.rs | 4 ---- 9 files changed, 35 insertions(+), 47 deletions(-) diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index 84666158781..ace59f29066 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -60,6 +60,8 @@ impl<'a, DB: HirDatabase> SubstituteTypeParams<'a, DB> { previous: Box::new(NullTransformer), }; + // FIXME: It would probably be nicer if we could get this via HIR (i.e. get the + // trait ref, and then go from the types in the substs back to the syntax) fn get_syntactic_substs(impl_block: ast::ImplBlock) -> Option> { let target_trait = impl_block.target_trait()?; let path_type = match target_trait { @@ -131,12 +133,12 @@ impl<'a, DB: HirDatabase> QualifyPaths<'a, DB> { let resolution = analyzer.resolve_path(self.db, &p)?; match resolution { PathResolution::Def(def) => { - let found_path = from.find_path(self.db, def)?; + let found_path = from.find_use_path(self.db, def)?; let args = p .segment() .and_then(|s| s.type_arg_list()) .map(|arg_list| apply(self, node.with_value(arg_list))); - Some(make::path_with_type_arg_list(found_path.to_ast(), args).syntax().clone()) + Some(make::path_with_type_arg_list(path_to_ast(found_path), args).syntax().clone()) } PathResolution::Local(_) | PathResolution::TypeParam(_) @@ -171,8 +173,7 @@ impl<'a, DB: HirDatabase> AstTransform<'a> for QualifyPaths<'a, DB> { } } -// FIXME: It would probably be nicer if we could get this via HIR (i.e. get the -// trait ref, and then go from the types in the substs back to the syntax) -// FIXME: This should be a general utility (not even just for assists) - -// FIXME: This should be a general utility (not even just for assists) +fn path_to_ast(path: hir::ModPath) -> ast::Path { + let parse = ast::SourceFile::parse(&path.to_string()); + parse.tree().syntax().descendants().find_map(ast::Path::cast).unwrap() +} diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 4da3db0d6b8..df9c151e5aa 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -228,7 +228,9 @@ impl Module { Module::new(self.krate(), module_id) } - pub fn find_path( + /// Finds a path that can be used to refer to the given item from within + /// this module, if possible. + pub fn find_use_path( self, db: &impl DefDatabase, item: ModuleDef, diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index 71339565f06..a2a9d968cb4 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -206,7 +206,7 @@ impl SourceAnalyzer { } pub fn module(&self) -> Option { - Some(crate::code_model::Module { id: self.resolver.module_id()? }) + Some(crate::code_model::Module { id: self.resolver.module()? }) } fn expr_id(&self, expr: &ast::Expr) -> Option { diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs index 0c00627b597..da273eb1155 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; - - #[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 { diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 69cdfc943a4..f7dc8acb7f3 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 87c50b34f8e..d74a1cef215 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 { 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 7dd1939b988..9f93a54244d 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 `::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 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 40d0cb58808..f7bac580112 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 { 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 { - 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 { - let (def_map, local_id) = self.module()?; + pub fn module(&self) -> Option { + let (def_map, local_id) = self.module_scope()?; Some(ModuleId { krate: def_map.krate, local_id }) } pub fn krate(&self) -> Option { - self.module().map(|t| t.0.krate) + self.module_scope().map(|t| t.0.krate) } pub fn where_predicates_in_scope<'a>( diff --git a/crates/ra_hir_expand/src/name.rs b/crates/ra_hir_expand/src/name.rs index a4d912b99db..b3fa1efbab5 100644 --- a/crates/ra_hir_expand/src/name.rs +++ b/crates/ra_hir_expand/src/name.rs @@ -37,10 +37,6 @@ impl Name { Name(Repr::TupleField(idx)) } - pub fn for_crate_dependency(dep: &ra_db::Dependency) -> Name { - Name::new_text(dep.name.clone()) - } - /// Shortcut to create inline plain text name const fn new_inline_ascii(text: &[u8]) -> Name { Name::new_text(SmolStr::new_inline_from_ascii(text.len(), text)) From ccb75f7c979b56bc62b61fadd81903e11a7f5d74 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:59:57 +0100 Subject: [PATCH 19/19] Use FxHashMap --- Cargo.lock | 1 + crates/ra_assists/Cargo.toml | 1 + crates/ra_assists/src/ast_transform.rs | 6 +++--- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ba9a201b91e..90c505f401b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -872,6 +872,7 @@ dependencies = [ "ra_hir 0.1.0", "ra_syntax 0.1.0", "ra_text_edit 0.1.0", + "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "test_utils 0.1.0", ] diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml index 434e6656ccc..50be8d9bc28 100644 --- a/crates/ra_assists/Cargo.toml +++ b/crates/ra_assists/Cargo.toml @@ -10,6 +10,7 @@ doctest = false [dependencies] format-buf = "1.0.0" join_to_string = "0.1.3" +rustc-hash = "1.0" itertools = "0.8.0" ra_syntax = { path = "../ra_syntax" } diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index ace59f29066..cbddc50acb6 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -1,5 +1,5 @@ //! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined. -use std::collections::HashMap; +use rustc_hash::FxHashMap; use hir::{db::HirDatabase, InFile, PathResolution}; use ra_syntax::ast::{self, make, AstNode}; @@ -35,7 +35,7 @@ impl<'a> AstTransform<'a> for NullTransformer { pub struct SubstituteTypeParams<'a, DB: HirDatabase> { db: &'a DB, - substs: HashMap, + substs: FxHashMap, previous: Box + 'a>, } @@ -47,7 +47,7 @@ impl<'a, DB: HirDatabase> SubstituteTypeParams<'a, DB> { ) -> SubstituteTypeParams<'a, DB> { let substs = get_syntactic_substs(impl_block).unwrap_or_default(); let generic_def: hir::GenericDef = trait_.into(); - let substs_by_param: HashMap<_, _> = generic_def + let substs_by_param: FxHashMap<_, _> = generic_def .params(db) .into_iter() // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky