diff --git a/crates/ra_arena/src/map.rs b/crates/ra_arena/src/map.rs index 2f09d677f1..be80edaf38 100644 --- a/crates/ra_arena/src/map.rs +++ b/crates/ra_arena/src/map.rs @@ -29,6 +29,10 @@ impl ArenaMap { self.v.get(Self::to_idx(id)).and_then(|it| it.as_ref()) } + pub fn get_mut(&mut self, id: ID) -> Option<&mut T> { + self.v.get_mut(Self::to_idx(id)).and_then(|it| it.as_mut()) + } + pub fn values(&self) -> impl Iterator { self.v.iter().filter_map(|o| o.as_ref()) } diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index 6739627b4e..a469ad4770 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs @@ -165,11 +165,11 @@ impl Module { /// Returns a `ModuleScope`: a set of items, visible in this module. pub fn scope(&self, db: &impl HirDatabase) -> ModuleScope { - self.scope_impl(db) + db.item_map(self.krate)[self.module_id].clone() } pub fn resolve_path(&self, db: &impl HirDatabase, path: &Path) -> PerNs { - self.resolve_path_impl(db, path) + db.item_map(self.krate).resolve_path(db, *self, path) } pub fn problems(&self, db: &impl HirDatabase) -> Vec<(TreeArc, Problem)> { diff --git a/crates/ra_hir/src/code_model_impl/module.rs b/crates/ra_hir/src/code_model_impl/module.rs index 6419d3934c..480ec27bf5 100644 --- a/crates/ra_hir/src/code_model_impl/module.rs +++ b/crates/ra_hir/src/code_model_impl/module.rs @@ -2,10 +2,10 @@ use ra_db::FileId; use ra_syntax::{ast, SyntaxNode, TreeArc}; use crate::{ - Module, ModuleSource, Problem, ModuleDef, - Crate, Name, Path, PathKind, PerNs, + Module, ModuleSource, Problem, + Crate, Name, module_tree::ModuleId, - nameres::{ModuleScope, lower::ImportId}, + nameres::{lower::ImportId}, db::HirDatabase, }; @@ -90,74 +90,6 @@ impl Module { Some(self.with_module_id(parent_id)) } - /// Returns a `ModuleScope`: a set of items, visible in this module. - pub(crate) fn scope_impl(&self, db: &impl HirDatabase) -> ModuleScope { - let item_map = db.item_map(self.krate); - item_map.per_module[&self.module_id].clone() - } - - pub(crate) fn resolve_path_impl(&self, db: &impl HirDatabase, path: &Path) -> PerNs { - let mut curr_per_ns: PerNs = PerNs::types(match path.kind { - PathKind::Crate => self.crate_root(db).into(), - PathKind::Self_ | PathKind::Plain => self.clone().into(), - PathKind::Super => { - if let Some(p) = self.parent(db) { - p.into() - } else { - return PerNs::none(); - } - } - PathKind::Abs => { - // TODO: absolute use is not supported - return PerNs::none(); - } - }); - - for segment in path.segments.iter() { - let curr = match curr_per_ns.as_ref().take_types() { - Some(r) => r, - None => { - // we still have path segments left, but the path so far - // didn't resolve in the types namespace => no resolution - // (don't break here because curr_per_ns might contain - // something in the value namespace, and it would be wrong - // to return that) - return PerNs::none(); - } - }; - // resolve segment in curr - - curr_per_ns = match curr { - ModuleDef::Module(m) => { - let scope = m.scope(db); - match scope.get(&segment.name) { - Some(r) => r.def_id.clone(), - None => PerNs::none(), - } - } - ModuleDef::Enum(e) => { - // enum variant - let matching_variant = e - .variants(db) - .into_iter() - .find(|(n, _variant)| n == &segment.name); - - match matching_variant { - Some((_n, variant)) => PerNs::both(variant.into(), (*e).into()), - None => PerNs::none(), - } - } - _ => { - // could be an inherent method call in UFCS form - // (`Struct::method`), or some other kind of associated - // item... Which we currently don't handle (TODO) - PerNs::none() - } - }; - } - curr_per_ns - } - pub(crate) fn problems_impl( &self, db: &impl HirDatabase, diff --git a/crates/ra_hir/src/marks.rs b/crates/ra_hir/src/marks.rs index f4d0c3e596..338ed0516a 100644 --- a/crates/ra_hir/src/marks.rs +++ b/crates/ra_hir/src/marks.rs @@ -1,3 +1,4 @@ -use test_utils::mark; - -mark!(name_res_works_for_broken_modules); +test_utils::marks!( + name_res_works_for_broken_modules + item_map_enum_importing +); diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index a3bc989580..639726b5ee 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -19,6 +19,8 @@ pub(crate) mod lower; use std::sync::Arc; use ra_db::CrateId; +use ra_arena::map::ArenaMap; +use test_utils::tested_by; use rustc_hash::{FxHashMap, FxHashSet}; use crate::{ @@ -27,16 +29,21 @@ use crate::{ HirDatabase, Crate, Name, module_tree::{ModuleId, ModuleTree}, -//FIXME: deglobify - nameres::lower::*, + nameres::lower::{ImportId, LoweredModule, ImportData}, }; /// `ItemMap` is the result of name resolution. It contains, for each /// module, the set of visible items. -// FIXME: currenty we compute item map per source-root. We should do it per crate instead. #[derive(Default, Debug, PartialEq, Eq)] pub struct ItemMap { - pub per_module: FxHashMap, + per_module: ArenaMap, +} + +impl std::ops::Index for ItemMap { + type Output = ModuleScope; + fn index(&self, id: ModuleId) -> &ModuleScope { + &self.per_module[id] + } } #[derive(Debug, Default, PartialEq, Eq, Clone)] @@ -58,7 +65,7 @@ impl ModuleScope { #[derive(Debug, Clone, PartialEq, Eq)] pub struct Resolution { /// None for unresolved - pub def_id: PerNs, + pub def: PerNs, /// ident by which this is imported into local scope. pub import: Option, } @@ -210,11 +217,11 @@ where let krate = Crate::new(crate_id); for dep in krate.dependencies(self.db) { if let Some(module) = dep.krate.root_module(self.db) { - let def_id = module.into(); + let def = module.into(); self.add_module_item( &mut module_items, dep.name.clone(), - PerNs::types(def_id), + PerNs::types(def), ); } } @@ -226,7 +233,7 @@ where module_items.items.insert( segment.name.clone(), Resolution { - def_id: PerNs::none(), + def: PerNs::none(), import: Some(import_id), }, ); @@ -234,11 +241,8 @@ where } } // Populate explicitly declared items, except modules - for (name, &def_id) in input.declarations.iter() { - let resolution = Resolution { - def_id, - import: None, - }; + for (name, &def) in input.declarations.iter() { + let resolution = Resolution { def, import: None }; module_items.items.insert(name.clone(), resolution); } @@ -254,16 +258,8 @@ where self.result.per_module.insert(module_id, module_items); } - fn add_module_item( - &self, - module_items: &mut ModuleScope, - name: Name, - def_id: PerNs, - ) { - let resolution = Resolution { - def_id, - import: None, - }; + fn add_module_item(&self, module_items: &mut ModuleScope, name: Name, def: PerNs) { + let resolution = Resolution { def, import: None }; module_items.items.insert(name, resolution); } @@ -273,7 +269,7 @@ where // already done continue; } - if self.resolve_import(module_id, import_id, import_data) { + if self.resolve_import(module_id, import_id, import_data) == ReachedFixedPoint::Yes { log::debug!("import {:?} resolved (or definite error)", import_id); self.processed_imports.insert((module_id, import_id)); } @@ -285,118 +281,148 @@ where module_id: ModuleId, import_id: ImportId, import: &ImportData, - ) -> bool { + ) -> ReachedFixedPoint { log::debug!("resolving import: {:?}", import); if import.is_glob { - return false; + return ReachedFixedPoint::Yes; }; - - let mut curr: ModuleId = match import.path.kind { - PathKind::Plain | PathKind::Self_ => module_id, - PathKind::Super => { - match module_id.parent(&self.module_tree) { - Some(it) => it, - None => { - // TODO: error - log::debug!("super path in root module"); - return true; // this can't suddenly resolve if we just resolve some other imports - } - } - } - PathKind::Crate => module_id.crate_root(&self.module_tree), - PathKind::Abs => { - // TODO: absolute use is not supported for now - return false; - } + let original_module = Module { + krate: self.krate, + module_id, }; + let (def, reached_fixedpoint) = + self.result + .resolve_path_fp(self.db, original_module, &import.path); - for (i, segment) in import.path.segments.iter().enumerate() { - let is_last = i == import.path.segments.len() - 1; - - let def_id = match self.result.per_module[&curr].items.get(&segment.name) { - Some(res) if !res.def_id.is_none() => res.def_id, - _ => { - log::debug!("path segment {:?} not found", segment.name); - return false; - } - }; - - if !is_last { - let type_def_id = if let Some(d) = def_id.take(Namespace::Types) { - d - } else { - log::debug!( - "path segment {:?} resolved to value only, but is not last", - segment.name - ); - return false; + if reached_fixedpoint == ReachedFixedPoint::Yes { + let last_segment = import.path.segments.last().unwrap(); + self.update(module_id, |items| { + let res = Resolution { + def, + import: Some(import_id), }; - curr = match type_def_id { - ModuleDef::Module(module) => { - if module.krate == self.krate { - module.module_id - } else { - let path = Path { - segments: import.path.segments[i + 1..].iter().cloned().collect(), - kind: PathKind::Crate, - }; - log::debug!("resolving {:?} in other source root", path); - let def_id = module.resolve_path(self.db, &path); - if !def_id.is_none() { - let last_segment = path.segments.last().unwrap(); - self.update(module_id, |items| { - let res = Resolution { - def_id, - import: Some(import_id), - }; - items.items.insert(last_segment.name.clone(), res); - }); - log::debug!( - "resolved import {:?} ({:?}) cross-source root to {:?}", - last_segment.name, - import, - def_id, - ); - return true; - } else { - log::debug!("rest of path did not resolve in other source root"); - return true; - } - } - } - _ => { - log::debug!( - "path segment {:?} resolved to non-module {:?}, but is not last", - segment.name, - type_def_id, - ); - return true; // this resolved to a non-module, so the path won't ever resolve - } - } - } else { - log::debug!( - "resolved import {:?} ({:?}) within source root to {:?}", - segment.name, - import, - def_id, - ); - self.update(module_id, |items| { - let res = Resolution { - def_id, - import: Some(import_id), - }; - items.items.insert(segment.name.clone(), res); - }) - } + items.items.insert(last_segment.name.clone(), res); + }); + log::debug!( + "resolved import {:?} ({:?}) cross-source root to {:?}", + last_segment.name, + import, + def, + ); } - true + reached_fixedpoint } fn update(&mut self, module_id: ModuleId, f: impl FnOnce(&mut ModuleScope)) { - let module_items = self.result.per_module.get_mut(&module_id).unwrap(); + let module_items = self.result.per_module.get_mut(module_id).unwrap(); f(module_items) } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReachedFixedPoint { + Yes, + No, +} + +impl ItemMap { + pub(crate) fn resolve_path( + &self, + db: &impl HirDatabase, + original_module: Module, + path: &Path, + ) -> PerNs { + self.resolve_path_fp(db, original_module, path).0 + } + + // Returns Yes if we are sure that additions to `ItemMap` wouldn't change + // the result. + fn resolve_path_fp( + &self, + db: &impl HirDatabase, + original_module: Module, + path: &Path, + ) -> (PerNs, ReachedFixedPoint) { + let mut curr_per_ns: PerNs = PerNs::types(match path.kind { + PathKind::Crate => original_module.crate_root(db).into(), + PathKind::Self_ | PathKind::Plain => original_module.into(), + PathKind::Super => { + if let Some(p) = original_module.parent(db) { + p.into() + } else { + log::debug!("super path in root module"); + return (PerNs::none(), ReachedFixedPoint::Yes); + } + } + PathKind::Abs => { + // TODO: absolute use is not supported + return (PerNs::none(), ReachedFixedPoint::Yes); + } + }); + + for (i, segment) in path.segments.iter().enumerate() { + let curr = match curr_per_ns.as_ref().take_types() { + Some(r) => r, + None => { + // we still have path segments left, but the path so far + // didn't resolve in the types namespace => no resolution + // (don't break here because curr_per_ns might contain + // something in the value namespace, and it would be wrong + // to return that) + return (PerNs::none(), ReachedFixedPoint::No); + } + }; + // resolve segment in curr + + curr_per_ns = match curr { + ModuleDef::Module(module) => { + if module.krate != original_module.krate { + let path = Path { + segments: path.segments[i..].iter().cloned().collect(), + kind: PathKind::Crate, + }; + log::debug!("resolving {:?} in other crate", path); + let def = module.resolve_path(db, &path); + return (def, ReachedFixedPoint::Yes); + } + + match self[module.module_id].items.get(&segment.name) { + Some(res) if !res.def.is_none() => res.def, + _ => { + log::debug!("path segment {:?} not found", segment.name); + return (PerNs::none(), ReachedFixedPoint::No); + } + } + } + ModuleDef::Enum(e) => { + // enum variant + tested_by!(item_map_enum_importing); + let matching_variant = e + .variants(db) + .into_iter() + .find(|(n, _variant)| n == &segment.name); + + match matching_variant { + Some((_n, variant)) => PerNs::both(variant.into(), (*e).into()), + None => PerNs::none(), + } + } + _ => { + // could be an inherent method call in UFCS form + // (`Struct::method`), or some other kind of associated + // item... Which we currently don't handle (TODO) + log::debug!( + "path segment {:?} resolved to non-module {:?}, but is not last", + segment.name, + curr, + ); + return (PerNs::none(), ReachedFixedPoint::Yes); + } + }; + } + (curr_per_ns, ReachedFixedPoint::Yes) + } +} + #[cfg(test)] mod tests; diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs index 9322bf08ce..c033bebe87 100644 --- a/crates/ra_hir/src/nameres/tests.rs +++ b/crates/ra_hir/src/nameres/tests.rs @@ -20,7 +20,7 @@ fn item_map(fixture: &str) -> (Arc, ModuleId) { } fn check_module_item_map(map: &ItemMap, module_id: ModuleId, expected: &str) { - let mut lines = map.per_module[&module_id] + let mut lines = map[module_id] .items .iter() .map(|(name, res)| format!("{}: {}", name, dump_resolution(res))) @@ -37,8 +37,8 @@ fn check_module_item_map(map: &ItemMap, module_id: ModuleId, expected: &str) { fn dump_resolution(resolution: &Resolution) -> &'static str { match ( - resolution.def_id.types.is_some(), - resolution.def_id.values.is_some(), + resolution.def.types.is_some(), + resolution.def.values.is_some(), ) { (true, true) => "t v", (true, false) => "t", @@ -215,6 +215,27 @@ fn item_map_using_self() { ); } +#[test] +fn item_map_enum_importing() { + covers!(item_map_enum_importing); + let (item_map, module_id) = item_map( + " + //- /lib.rs + enum E { V } + use self::E::V; + <|> + ", + ); + check_module_item_map( + &item_map, + module_id, + " + E: t + V: t v + ", + ); +} + #[test] fn item_map_across_crates() { let (mut db, sr) = MockDatabase::with_files( diff --git a/crates/ra_ide_api/src/completion/completion_item.rs b/crates/ra_ide_api/src/completion/completion_item.rs index 3ba6c33ee6..b2b0477661 100644 --- a/crates/ra_ide_api/src/completion/completion_item.rs +++ b/crates/ra_ide_api/src/completion/completion_item.rs @@ -209,10 +209,7 @@ impl Builder { ctx: &CompletionContext, resolution: &hir::Resolution, ) -> Builder { - let def = resolution - .def_id - .take_types() - .or(resolution.def_id.take_values()); + let def = resolution.def.take_types().or(resolution.def.take_values()); let def = match def { None => return self, Some(it) => it, diff --git a/crates/ra_ide_api/src/marks.rs b/crates/ra_ide_api/src/marks.rs index b4a726ef00..dc5b2702ae 100644 --- a/crates/ra_ide_api/src/marks.rs +++ b/crates/ra_ide_api/src/marks.rs @@ -1,3 +1 @@ -use test_utils::mark; - -mark!(inserts_parens_for_function_calls); +test_utils::marks!(inserts_parens_for_function_calls); diff --git a/crates/test_utils/src/marks.rs b/crates/test_utils/src/marks.rs index 79ffedf69b..ee47b52198 100644 --- a/crates/test_utils/src/marks.rs +++ b/crates/test_utils/src/marks.rs @@ -46,11 +46,13 @@ macro_rules! covers { } #[macro_export] -macro_rules! mark { - ($ident:ident) => { +macro_rules! marks { + ($($ident:ident)*) => { + $( #[allow(bad_style)] pub(crate) static $ident: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + )* }; }