diff --git a/Cargo.lock b/Cargo.lock index 7fea2137a0..2373e79d82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -460,6 +460,7 @@ dependencies = [ "hir_def", "hir_expand", "hir_ty", + "indexmap", "itertools", "log", "once_cell", diff --git a/crates/hir/Cargo.toml b/crates/hir/Cargo.toml index 1924906201..b9561e6f9e 100644 --- a/crates/hir/Cargo.toml +++ b/crates/hir/Cargo.toml @@ -16,6 +16,7 @@ arrayvec = "0.7" itertools = "0.10.0" smallvec = "1.4.0" once_cell = "1" +indexmap = "1.7" stdx = { path = "../stdx", version = "0.0.0" } syntax = { path = "../syntax", version = "0.0.0" } diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index cc320227f8..354fb276a5 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -923,31 +923,28 @@ impl<'a> SemanticsScope<'a> { } pub fn process_all_names(&self, f: &mut dyn FnMut(Name, ScopeDef)) { - let resolver = &self.resolver; - - resolver.process_all_names(self.db.upcast(), &mut |name, def| { - let def = match def { - resolver::ScopeDef::PerNs(it) => { - let items = ScopeDef::all_items(it); - for item in items { - f(name.clone(), item); + let scope = self.resolver.names_in_scope(self.db.upcast()); + for (name, entries) in scope { + for entry in entries { + let def = match entry { + resolver::ScopeDef::ModuleDef(it) => ScopeDef::ModuleDef(it.into()), + resolver::ScopeDef::MacroDef(it) => ScopeDef::MacroDef(it.into()), + resolver::ScopeDef::Unknown => ScopeDef::Unknown, + resolver::ScopeDef::ImplSelfType(it) => ScopeDef::ImplSelfType(it.into()), + resolver::ScopeDef::AdtSelfType(it) => ScopeDef::AdtSelfType(it.into()), + resolver::ScopeDef::GenericParam(id) => ScopeDef::GenericParam(id.into()), + resolver::ScopeDef::Local(pat_id) => { + let parent = self.resolver.body_owner().unwrap(); + ScopeDef::Local(Local { parent, pat_id }) } - return; - } - resolver::ScopeDef::ImplSelfType(it) => ScopeDef::ImplSelfType(it.into()), - resolver::ScopeDef::AdtSelfType(it) => ScopeDef::AdtSelfType(it.into()), - resolver::ScopeDef::GenericParam(id) => ScopeDef::GenericParam(id.into()), - resolver::ScopeDef::Local(pat_id) => { - let parent = resolver.body_owner().unwrap(); - ScopeDef::Local(Local { parent, pat_id }) - } - resolver::ScopeDef::Label(label_id) => { - let parent = resolver.body_owner().unwrap(); - ScopeDef::Label(Label { parent, label_id }) - } - }; - f(name, def) - }) + resolver::ScopeDef::Label(label_id) => { + let parent = self.resolver.body_owner().unwrap(); + ScopeDef::Label(Label { parent, label_id }) + } + }; + f(name.clone(), def) + } + } } /// Resolve a path as-if it was written at the given scope. This is diff --git a/crates/hir_def/src/resolver.rs b/crates/hir_def/src/resolver.rs index cd1a6c6864..0cf28acf7f 100644 --- a/crates/hir_def/src/resolver.rs +++ b/crates/hir_def/src/resolver.rs @@ -6,7 +6,9 @@ use hir_expand::{ name::{name, Name}, MacroDefId, }; +use indexmap::IndexMap; use rustc_hash::FxHashSet; +use smallvec::SmallVec; use crate::{ body::scope::{ExprScopes, ScopeId}, @@ -348,10 +350,50 @@ impl Resolver { item_map.resolve_path(db, module, path, BuiltinShadowMode::Other).0.take_macros() } - pub fn process_all_names(&self, db: &dyn DefDatabase, f: &mut dyn FnMut(Name, ScopeDef)) { + /// Returns a set of names available in the current scope. + /// + /// Note that this is a somewhat fuzzy concept -- internally, the compiler + /// doesn't necessary follow a strict scoping discipline. Rathe, it just + /// tells for each ident what it resolves to. + /// + /// A good example is something like `str::from_utf8`. From scopes point of + /// view, this code is erroneous -- both `str` module and `str` type occupy + /// the same type namespace. + /// + /// We don't try to model that super-correctly -- this functionality is + /// primarily exposed for completions. + /// + /// Note that in Rust one name can be bound to several items: + /// + /// ``` + /// macro_rules! t { () => (()) } + /// type t = t!(); + /// const t: t = t!() + /// ``` + /// + /// That's why we return a multimap. + /// + /// The shadowing is accounted for: in + /// + /// ``` + /// let x = 92; + /// { + /// let x = 92; + /// $0 + /// } + /// ``` + /// + /// there will be only one entry for `x` in the result. + /// + /// The result is ordered *roughly* from the innermost scope to the + /// outermost: when the name is introduced in two namespaces in two scopes, + /// we use the position of the first scope. + pub fn names_in_scope(&self, db: &dyn DefDatabase) -> IndexMap> { + let mut res = ScopeNames::default(); for scope in self.scopes() { - scope.process_names(db, f); + scope.process_names(db, &mut res); } + res.map } pub fn traits_in_scope(&self, db: &dyn DefDatabase) -> FxHashSet { @@ -434,8 +476,11 @@ impl Resolver { } } +#[derive(Debug, PartialEq, Eq)] pub enum ScopeDef { - PerNs(PerNs), + ModuleDef(ModuleDefId), + MacroDef(MacroDefId), + Unknown, ImplSelfType(ImplId), AdtSelfType(AdtId), GenericParam(GenericParamId), @@ -444,8 +489,7 @@ pub enum ScopeDef { } impl Scope { - fn process_names(&self, db: &dyn DefDatabase, f: &mut dyn FnMut(Name, ScopeDef)) { - let mut seen = FxHashSet::default(); + fn process_names(&self, db: &dyn DefDatabase, acc: &mut ScopeNames) { match self { Scope::ModuleScope(m) => { // FIXME: should we provide `self` here? @@ -456,58 +500,53 @@ impl Scope { // }), // ); m.def_map[m.module_id].scope.entries().for_each(|(name, def)| { - f(name.clone(), ScopeDef::PerNs(def)); + acc.add_per_ns(name, def); }); - m.def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| { - let scope = PerNs::macros(macro_, Visibility::Public); - seen.insert((name.clone(), scope)); - f(name.clone(), ScopeDef::PerNs(scope)); + m.def_map[m.module_id].scope.legacy_macros().for_each(|(name, mac)| { + acc.add(name, ScopeDef::MacroDef(mac)); }); m.def_map.extern_prelude().for_each(|(name, &def)| { - f(name.clone(), ScopeDef::PerNs(PerNs::types(def, Visibility::Public))); + acc.add(name, ScopeDef::ModuleDef(def)); }); BUILTIN_SCOPE.iter().for_each(|(name, &def)| { - f(name.clone(), ScopeDef::PerNs(def)); + acc.add_per_ns(name, def); }); if let Some(prelude) = m.def_map.prelude() { let prelude_def_map = prelude.def_map(db); - prelude_def_map[prelude.local_id].scope.entries().for_each(|(name, def)| { - let seen_tuple = (name.clone(), def); - if !seen.contains(&seen_tuple) { - f(seen_tuple.0, ScopeDef::PerNs(def)); - } - }); + for (name, def) in prelude_def_map[prelude.local_id].scope.entries() { + acc.add_per_ns(name, def) + } } } Scope::GenericParams { params, def: parent } => { let parent = *parent; for (local_id, param) in params.types.iter() { - if let Some(ref name) = param.name { + if let Some(name) = ¶m.name { let id = TypeParamId { parent, local_id }; - f(name.clone(), ScopeDef::GenericParam(id.into())) + acc.add(name, ScopeDef::GenericParam(id.into())) } } for (local_id, param) in params.consts.iter() { let id = ConstParamId { parent, local_id }; - f(param.name.clone(), ScopeDef::GenericParam(id.into())) + acc.add(¶m.name, ScopeDef::GenericParam(id.into())) } for (local_id, param) in params.lifetimes.iter() { let id = LifetimeParamId { parent, local_id }; - f(param.name.clone(), ScopeDef::GenericParam(id.into())) + acc.add(¶m.name, ScopeDef::GenericParam(id.into())) } } Scope::ImplDefScope(i) => { - f(name![Self], ScopeDef::ImplSelfType(*i)); + acc.add(&name![Self], ScopeDef::ImplSelfType(*i)); } Scope::AdtScope(i) => { - f(name![Self], ScopeDef::AdtSelfType(*i)); + acc.add(&name![Self], ScopeDef::AdtSelfType(*i)); } Scope::ExprScope(scope) => { if let Some((label, name)) = scope.expr_scopes.label(scope.scope_id) { - f(name, ScopeDef::Label(label)) + acc.add(&name, ScopeDef::Label(label)) } scope.expr_scopes.entries(scope.scope_id).iter().for_each(|e| { - f(e.name().clone(), ScopeDef::Local(e.pat())); + acc.add_local(e.name(), e.pat()); }); } } @@ -651,6 +690,47 @@ fn to_type_ns(per_ns: PerNs) -> Option { Some(res) } +#[derive(Default)] +struct ScopeNames { + map: IndexMap>, +} + +impl ScopeNames { + fn add(&mut self, name: &Name, def: ScopeDef) { + let set = self.map.entry(name.clone()).or_default(); + if !set.contains(&def) { + set.push(def) + } + } + fn add_per_ns(&mut self, name: &Name, def: PerNs) { + if let Some(ty) = &def.types { + self.add(name, ScopeDef::ModuleDef(ty.0)) + } + if let Some(val) = &def.values { + self.add(name, ScopeDef::ModuleDef(val.0)) + } + if let Some(mac) = &def.macros { + self.add(name, ScopeDef::MacroDef(mac.0)) + } + if def.is_none() { + self.add(name, ScopeDef::Unknown) + } + } + fn add_local(&mut self, name: &Name, pat: PatId) { + let set = self.map.entry(name.clone()).or_default(); + // XXX: hack, account for local (and only local) shadowing. + // + // This should be somewhat more principled and take namespaces into + // accounts, but, alas, scoping rules are a hoax. `str` type and `str` + // module can be both available in the same scope. + if set.iter().any(|it| matches!(it, &ScopeDef::Local(_))) { + cov_mark::hit!(shadowing_shows_single_completion); + return; + } + set.push(ScopeDef::Local(pat)) + } +} + pub trait HasResolver: Copy { /// Builds a resolver for type references inside this def. fn resolver(self, db: &dyn DefDatabase) -> Resolver; diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs index 0af282d83d..d81cb5391c 100644 --- a/crates/ide_completion/src/completions/unqualified_path.rs +++ b/crates/ide_completion/src/completions/unqualified_path.rs @@ -304,25 +304,4 @@ pub mod prelude { "#]], ); } - - #[test] - fn local_variable_shadowing() { - // FIXME: this isn't actually correct, should emit `x` only once. - check( - r#" -fn main() { - let x = 92; - { - let x = 92; - x$0; - } -} -"#, - expect![[r#" - lc x i32 - lc x i32 - fn main() fn() - "#]], - ); - } } diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 6585b1cc8a..23be915bbc 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -1348,10 +1348,10 @@ fn foo() { lc foo [type+local] ev Foo::A(…) [type_could_unify] ev Foo::B [type_could_unify] + fn foo() [] en Foo [] fn baz() [] fn bar() [] - fn foo() [] "#]], ); } diff --git a/crates/ide_completion/src/tests/expression.rs b/crates/ide_completion/src/tests/expression.rs index cf22fb20ab..5c8dccc780 100644 --- a/crates/ide_completion/src/tests/expression.rs +++ b/crates/ide_completion/src/tests/expression.rs @@ -118,7 +118,6 @@ impl Unit { un Union ev TupleV(…) (u32) ct CONST - ma makro!(…) #[macro_export] macro_rules! makro me self.foo() fn(self) "##]], ); @@ -155,6 +154,8 @@ impl Unit { #[test] fn shadowing_shows_single_completion() { + cov_mark::check!(shadowing_shows_single_completion); + check_empty( r#" fn foo() { @@ -165,7 +166,6 @@ fn foo() { } } "#, - // FIXME: should be only one bar here expect![[r#" kw unsafe kw match @@ -182,7 +182,6 @@ fn foo() { kw super kw crate lc bar i32 - lc bar i32 fn foo() fn() bt u32 "#]], diff --git a/crates/ide_completion/src/tests/item.rs b/crates/ide_completion/src/tests/item.rs index ad87bdc751..aa298fa0bf 100644 --- a/crates/ide_completion/src/tests/item.rs +++ b/crates/ide_completion/src/tests/item.rs @@ -29,7 +29,6 @@ impl Tra$0 st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ) @@ -53,7 +52,6 @@ impl Trait for Str$0 st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ) diff --git a/crates/ide_completion/src/tests/item_list.rs b/crates/ide_completion/src/tests/item_list.rs index bd8df49e41..f355d37afc 100644 --- a/crates/ide_completion/src/tests/item_list.rs +++ b/crates/ide_completion/src/tests/item_list.rs @@ -67,7 +67,6 @@ fn in_source_file_item_list() { kw crate md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ) } @@ -172,7 +171,6 @@ fn in_impl_assoc_item_list() { kw crate md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ) } @@ -206,7 +204,6 @@ fn in_trait_assoc_item_list() { kw crate md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } @@ -243,7 +240,6 @@ impl Test for () { kw crate md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } diff --git a/crates/ide_completion/src/tests/pattern.rs b/crates/ide_completion/src/tests/pattern.rs index 5791921e44..10647e490e 100644 --- a/crates/ide_completion/src/tests/pattern.rs +++ b/crates/ide_completion/src/tests/pattern.rs @@ -122,7 +122,6 @@ fn foo() { bn TupleV TupleV($1)$0 ev TupleV ct CONST - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } @@ -143,7 +142,6 @@ fn foo() { st Tuple st Unit ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } @@ -163,7 +161,6 @@ fn foo(a$0) { st Tuple st Unit ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } diff --git a/crates/ide_completion/src/tests/predicate.rs b/crates/ide_completion/src/tests/predicate.rs index d43e4b50b1..163080307d 100644 --- a/crates/ide_completion/src/tests/predicate.rs +++ b/crates/ide_completion/src/tests/predicate.rs @@ -28,7 +28,6 @@ struct Foo<'lt, T, const C: usize> where $0 {} st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ); @@ -47,7 +46,6 @@ struct Foo<'lt, T, const C: usize> where T: $0 {} tt Trait md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } @@ -67,7 +65,6 @@ struct Foo<'lt, T, const C: usize> where 'lt: $0 {} tt Trait md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } @@ -85,7 +82,6 @@ struct Foo<'lt, T, const C: usize> where for<'a> T: $0 {} tt Trait md module ma makro!(…) #[macro_export] macro_rules! makro - ma makro!(…) #[macro_export] macro_rules! makro "##]], ); } @@ -109,7 +105,6 @@ struct Foo<'lt, T, const C: usize> where for<'a> $0 {} st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ); @@ -136,7 +131,6 @@ impl Record { st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ); diff --git a/crates/ide_completion/src/tests/type_pos.rs b/crates/ide_completion/src/tests/type_pos.rs index 88146357ca..b6cf8945e2 100644 --- a/crates/ide_completion/src/tests/type_pos.rs +++ b/crates/ide_completion/src/tests/type_pos.rs @@ -31,7 +31,6 @@ struct Foo<'lt, T, const C: usize> { st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ) @@ -60,7 +59,6 @@ struct Foo<'lt, T, const C: usize>(f$0); st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ) @@ -85,7 +83,6 @@ fn x<'lt, T, const C: usize>() -> $0 st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ); @@ -113,7 +110,6 @@ fn foo<'lt, T, const C: usize>() { st Unit ma makro!(…) #[macro_export] macro_rules! makro un Union - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ); @@ -164,7 +160,6 @@ fn foo<'lt, T: Trait2<$0>, const CONST_PARAM: usize>(_: T) {} tt Trait2 un Union ct CONST - ma makro!(…) #[macro_export] macro_rules! makro bt u32 "##]], ); diff --git a/docs/dev/style.md b/docs/dev/style.md index d5340e2b8e..3edbaf89d8 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -839,6 +839,7 @@ crate -> krate enum -> enum_ fn -> func impl -> imp +macro -> mac mod -> module struct -> strukt trait -> trait_