From 31af774254d1fb4e9105ba050546db16b9b54fb6 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 21 Feb 2020 15:34:06 +0100 Subject: Refactor how builtins are resolved This fixes autocompletion suggesting e.g. self::usize. --- crates/ra_hir_def/src/item_scope.rs | 33 +++---------- crates/ra_hir_def/src/nameres/path_resolution.rs | 63 +++++++++++++----------- crates/ra_hir_def/src/resolver.rs | 13 +++-- crates/ra_ide/src/completion/complete_path.rs | 13 ++--- crates/ra_ide/src/marks.rs | 1 - 5 files changed, 55 insertions(+), 68 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index 6e958ca75..5e943b780 100644 --- a/crates/ra_hir_def/src/item_scope.rs +++ b/crates/ra_hir_def/src/item_scope.rs @@ -30,7 +30,7 @@ pub struct ItemScope { legacy_macros: FxHashMap, } -static BUILTIN_SCOPE: Lazy> = Lazy::new(|| { +pub(crate) static BUILTIN_SCOPE: Lazy> = Lazy::new(|| { BuiltinType::ALL .iter() .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into(), Visibility::Public))) @@ -40,9 +40,9 @@ static BUILTIN_SCOPE: Lazy> = Lazy::new(|| { /// Shadow mode for builtin type which can be shadowed by module. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(crate) enum BuiltinShadowMode { - // Prefer Module + /// Prefer user-defined modules (or other types) over builtins. Module, - // Prefer Other Types + /// Prefer builtins over user-defined modules (but not other types). Other, } @@ -51,7 +51,7 @@ pub(crate) enum BuiltinShadowMode { impl ItemScope { pub fn entries<'a>(&'a self) -> impl Iterator + 'a { //FIXME: shadowing - self.visible.iter().chain(BUILTIN_SCOPE.iter()).map(|(n, def)| (n, *def)) + self.visible.iter().map(|(n, def)| (n, *def)) } pub fn entries_without_primitives<'a>( @@ -79,29 +79,8 @@ impl ItemScope { } /// Get a name from current module scope, legacy macros are not included - pub(crate) fn get(&self, name: &Name, shadow: BuiltinShadowMode) -> PerNs { - match shadow { - BuiltinShadowMode::Module => self - .visible - .get(name) - .or_else(|| BUILTIN_SCOPE.get(name)) - .copied() - .unwrap_or_else(PerNs::none), - BuiltinShadowMode::Other => { - let item = self.visible.get(name).copied(); - if let Some(def) = item { - if let Some(ModuleDefId::ModuleId(_)) = def.take_types() { - return BUILTIN_SCOPE - .get(name) - .copied() - .or(item) - .unwrap_or_else(PerNs::none); - } - } - - item.or_else(|| BUILTIN_SCOPE.get(name).copied()).unwrap_or_else(PerNs::none) - } - } + pub(crate) fn get(&self, name: &Name) -> PerNs { + self.visible.get(name).copied().unwrap_or_else(PerNs::none) } pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> { diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index fd6422d60..c058e70aa 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -18,6 +18,7 @@ use test_utils::tested_by; use crate::{ db::DefDatabase, + item_scope::BUILTIN_SCOPE, nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, @@ -103,15 +104,6 @@ impl CrateDefMap { path: &ModPath, shadow: BuiltinShadowMode, ) -> ResolvePathResult { - // if it is not the last segment, we prefer the module to the builtin - let prefer_module = |index| { - if index == path.segments.len() - 1 { - shadow - } else { - BuiltinShadowMode::Module - } - }; - let mut segments = path.segments.iter().enumerate(); let mut curr_per_ns: PerNs = match path.kind { PathKind::DollarCrate(krate) => { @@ -140,20 +132,29 @@ impl CrateDefMap { if self.edition == Edition::Edition2015 && (path.kind == PathKind::Abs || mode == ResolveMode::Import) => { - let (idx, segment) = match segments.next() { + let (_, segment) = match segments.next() { Some((idx, segment)) => (idx, segment), None => return ResolvePathResult::empty(ReachedFixedPoint::Yes), }; log::debug!("resolving {:?} in crate root (+ extern prelude)", segment); - self.resolve_name_in_crate_root_or_extern_prelude(&segment, prefer_module(idx)) + self.resolve_name_in_crate_root_or_extern_prelude(&segment) } PathKind::Plain => { - let (idx, segment) = match segments.next() { + let (_, segment) = match segments.next() { Some((idx, segment)) => (idx, segment), None => return ResolvePathResult::empty(ReachedFixedPoint::Yes), }; + // The first segment may be a builtin type. If the path has more + // than one segment, we first try resolving it as a module + // anyway. + // FIXME: If the next segment doesn't resolve in the module and + // BuiltinShadowMode wasn't Module, then we need to try + // resolving it as a builtin. + let prefer_module = + if path.segments.len() == 1 { shadow } else { BuiltinShadowMode::Module }; + log::debug!("resolving {:?} in module", segment); - self.resolve_name_in_module(db, original_module, &segment, prefer_module(idx)) + self.resolve_name_in_module(db, original_module, &segment, prefer_module) } PathKind::Super(lvl) => { let m = successors(Some(original_module), |m| self.modules[*m].parent) @@ -216,7 +217,7 @@ impl CrateDefMap { } // Since it is a qualified path here, it should not contains legacy macros - self[module.local_id].scope.get(&segment, prefer_module(i)) + self[module.local_id].scope.get(&segment) } ModuleDefId::AdtId(AdtId::EnumId(e)) => { // enum variant @@ -275,33 +276,35 @@ impl CrateDefMap { .scope .get_legacy_macro(name) .map_or_else(PerNs::none, |m| PerNs::macros(m, Visibility::Public)); - let from_scope = self[module].scope.get(name, shadow); + let from_scope = self[module].scope.get(name); + let from_builtin = BUILTIN_SCOPE.get(name).copied().unwrap_or_else(PerNs::none); + let from_scope_or_builtin = match shadow { + BuiltinShadowMode::Module => from_scope.or(from_builtin), + BuiltinShadowMode::Other => { + if let Some(ModuleDefId::ModuleId(_)) = from_scope.take_types() { + from_builtin.or(from_scope) + } else { + from_scope.or(from_builtin) + } + } + }; let from_extern_prelude = self .extern_prelude .get(name) .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public)); - let from_prelude = self.resolve_in_prelude(db, name, shadow); + let from_prelude = self.resolve_in_prelude(db, name); - from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude) + from_legacy_macro.or(from_scope_or_builtin).or(from_extern_prelude).or(from_prelude) } - fn resolve_name_in_crate_root_or_extern_prelude( - &self, - name: &Name, - shadow: BuiltinShadowMode, - ) -> PerNs { - let from_crate_root = self[self.root].scope.get(name, shadow); + fn resolve_name_in_crate_root_or_extern_prelude(&self, name: &Name) -> PerNs { + let from_crate_root = self[self.root].scope.get(name); let from_extern_prelude = self.resolve_name_in_extern_prelude(name); from_crate_root.or(from_extern_prelude) } - fn resolve_in_prelude( - &self, - db: &impl DefDatabase, - name: &Name, - shadow: BuiltinShadowMode, - ) -> PerNs { + fn resolve_in_prelude(&self, db: &impl DefDatabase, name: &Name) -> PerNs { if let Some(prelude) = self.prelude { let keep; let def_map = if prelude.krate == self.krate { @@ -311,7 +314,7 @@ impl CrateDefMap { keep = db.crate_def_map(prelude.krate); &keep }; - def_map[prelude.local_id].scope.get(name, shadow) + def_map[prelude.local_id].scope.get(name) } else { PerNs::none() } diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs index 5365b80e2..9dd4fa555 100644 --- a/crates/ra_hir_def/src/resolver.rs +++ b/crates/ra_hir_def/src/resolver.rs @@ -15,7 +15,7 @@ use crate::{ db::DefDatabase, expr::{ExprId, PatId}, generics::GenericParams, - item_scope::BuiltinShadowMode, + item_scope::{BuiltinShadowMode, BUILTIN_SCOPE}, nameres::CrateDefMap, path::{ModPath, PathKind}, per_ns::PerNs, @@ -193,7 +193,7 @@ impl Resolver { return Some((res, idx)); } Scope::LocalItemsScope(body) => { - let def = body.item_scope.get(first_name, BuiltinShadowMode::Other); + let def = body.item_scope.get(first_name); if let Some(res) = to_type_ns(def) { return Some((res, None)); } @@ -335,8 +335,10 @@ impl Resolver { }; } Scope::LocalItemsScope(body) => { - let def = body.item_scope.get(first_name, BuiltinShadowMode::Other); - if let Some(res) = to_value_ns(def) { + // we don't bother looking in the builtin scope here because there are no builtin values + let def = to_value_ns(body.item_scope.get(first_name)); + + if let Some(res) = def { return Some(ResolveValueResult::ValueNs(res)); } } @@ -476,6 +478,9 @@ impl Scope { m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| { f(name.clone(), ScopeDef::PerNs(PerNs::types(def, Visibility::Public))); }); + BUILTIN_SCOPE.iter().for_each(|(name, &def)| { + f(name.clone(), ScopeDef::PerNs(def)); + }); if let Some(prelude) = m.crate_def_map.prelude { let prelude_def_map = db.crate_def_map(prelude.krate); prelude_def_map[prelude.local_id].scope.entries().for_each(|(name, def)| { diff --git a/crates/ra_ide/src/completion/complete_path.rs b/crates/ra_ide/src/completion/complete_path.rs index af24e9f48..2d7f09a6c 100644 --- a/crates/ra_ide/src/completion/complete_path.rs +++ b/crates/ra_ide/src/completion/complete_path.rs @@ -1,4 +1,4 @@ -//! FIXME: write short doc here +//! Completion of paths, including when writing a single name. use hir::{Adt, PathResolution, ScopeDef}; use ra_syntax::AstNode; @@ -20,10 +20,6 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { let module_scope = module.scope(ctx.db); for (name, def) in module_scope { if ctx.use_item_syntax.is_some() { - if let hir::ScopeDef::ModuleDef(hir::ModuleDef::BuiltinType(..)) = def { - tested_by!(dont_complete_primitive_in_use); - continue; - } if let ScopeDef::Unknown = def { if let Some(name_ref) = ctx.name_ref_syntax.as_ref() { if name_ref.syntax().text() == name.to_string().as_str() { @@ -125,11 +121,16 @@ mod tests { #[test] fn dont_complete_primitive_in_use() { - covers!(dont_complete_primitive_in_use); let completions = do_completion(r"use self::<|>;", CompletionKind::BuiltinType); assert!(completions.is_empty()); } + #[test] + fn dont_complete_primitive_in_module_scope() { + let completions = do_completion(r"fn foo() { self::<|> }", CompletionKind::BuiltinType); + assert!(completions.is_empty()); + } + #[test] fn completes_primitives() { let completions = diff --git a/crates/ra_ide/src/marks.rs b/crates/ra_ide/src/marks.rs index 5bf4d2062..bcb67e373 100644 --- a/crates/ra_ide/src/marks.rs +++ b/crates/ra_ide/src/marks.rs @@ -10,6 +10,5 @@ test_utils::marks!( goto_def_for_field_init_shorthand call_info_bad_offset dont_complete_current_use - dont_complete_primitive_in_use test_resolve_parent_module_on_module_decl ); -- cgit v1.2.3