From 05e1c7b1972a87abe6d352b5d0cd8a58e2b7adc7 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 8 Mar 2020 15:11:57 +0100 Subject: Handle visibility for assoc item path completion as well --- crates/ra_hir/src/code_model.rs | 48 +++++++++++++++---- crates/ra_hir_def/src/data.rs | 31 ++++++++----- crates/ra_ide/src/completion/complete_path.rs | 67 ++++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 22 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 4d1e8f921..911c809fd 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -204,19 +204,25 @@ impl Module { } /// Returns a `ModuleScope`: a set of items, visible in this module. - pub fn scope(self, db: &impl HirDatabase, visible_from: Option) -> Vec<(Name, ScopeDef)> { + pub fn scope( + self, + db: &impl HirDatabase, + visible_from: Option, + ) -> Vec<(Name, ScopeDef)> { db.crate_def_map(self.id.krate)[self.id.local_id] .scope .entries() - .filter_map(|(name, def)| if let Some(m) = visible_from { - let filtered = def.filter_visibility(|vis| vis.is_visible_from(db, m.id)); - if filtered.is_none() && !def.is_none() { - None + .filter_map(|(name, def)| { + if let Some(m) = visible_from { + let filtered = def.filter_visibility(|vis| vis.is_visible_from(db, m.id)); + if filtered.is_none() && !def.is_none() { + None + } else { + Some((name, filtered)) + } } else { - Some((name, filtered)) + Some((name, def)) } - } else { - Some((name, def)) }) .map(|(name, def)| (name.clone(), def.into())) .collect() @@ -608,6 +614,14 @@ impl Const { } } +impl HasVisibility for Const { + fn visibility(&self, db: &impl HirDatabase) -> Visibility { + let function_data = db.const_data(self.id); + let visibility = &function_data.visibility; + visibility.resolve(db, &self.id.resolver(db)) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Static { pub(crate) id: StaticId, @@ -682,6 +696,14 @@ impl TypeAlias { } } +impl HasVisibility for TypeAlias { + fn visibility(&self, db: &impl HirDatabase) -> Visibility { + let function_data = db.type_alias_data(self.id); + let visibility = &function_data.visibility; + visibility.resolve(db, &self.id.resolver(db)) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct MacroDef { pub(crate) id: MacroDefId, @@ -769,6 +791,16 @@ impl AssocItem { } } +impl HasVisibility for AssocItem { + fn visibility(&self, db: &impl HirDatabase) -> Visibility { + match self { + AssocItem::Function(f) => f.visibility(db), + AssocItem::Const(c) => c.visibility(db), + AssocItem::TypeAlias(t) => t.visibility(db), + } + } +} + #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] pub enum GenericDef { Function(Function), diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 8b343af9d..a72eb5369 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -97,6 +97,7 @@ fn desugar_future_path(orig: TypeRef) -> Path { pub struct TypeAliasData { pub name: Name, pub type_ref: Option, + pub visibility: RawVisibility, } impl TypeAliasData { @@ -104,10 +105,11 @@ impl TypeAliasData { db: &impl DefDatabase, typ: TypeAliasId, ) -> Arc { - let node = typ.lookup(db).source(db).value; - let name = node.name().map_or_else(Name::missing, |n| n.as_name()); - let type_ref = node.type_ref().map(TypeRef::from_ast); - Arc::new(TypeAliasData { name, type_ref }) + let node = typ.lookup(db).source(db); + let name = node.value.name().map_or_else(Name::missing, |n| n.as_name()); + let type_ref = node.value.type_ref().map(TypeRef::from_ast); + let visibility = RawVisibility::from_ast(db, node.map(|n| n.visibility())); + Arc::new(TypeAliasData { name, type_ref, visibility }) } } @@ -223,23 +225,28 @@ pub struct ConstData { /// const _: () = (); pub name: Option, pub type_ref: TypeRef, + pub visibility: RawVisibility, } impl ConstData { pub(crate) fn const_data_query(db: &impl DefDatabase, konst: ConstId) -> Arc { - let node = konst.lookup(db).source(db).value; - Arc::new(ConstData::new(&node)) + let node = konst.lookup(db).source(db); + Arc::new(ConstData::new(db, node)) } pub(crate) fn static_data_query(db: &impl DefDatabase, konst: StaticId) -> Arc { - let node = konst.lookup(db).source(db).value; - Arc::new(ConstData::new(&node)) + let node = konst.lookup(db).source(db); + Arc::new(ConstData::new(db, node)) } - fn new(node: &N) -> ConstData { - let name = node.name().map(|n| n.as_name()); - let type_ref = TypeRef::from_ast_opt(node.ascribed_type()); - ConstData { name, type_ref } + fn new( + db: &impl DefDatabase, + node: InFile, + ) -> ConstData { + let name = node.value.name().map(|n| n.as_name()); + let type_ref = TypeRef::from_ast_opt(node.value.ascribed_type()); + let visibility = RawVisibility::from_ast(db, node.map(|n| n.visibility())); + ConstData { name, type_ref, visibility } } } diff --git a/crates/ra_ide/src/completion/complete_path.rs b/crates/ra_ide/src/completion/complete_path.rs index f99b1c2c4..d2c758571 100644 --- a/crates/ra_ide/src/completion/complete_path.rs +++ b/crates/ra_ide/src/completion/complete_path.rs @@ -1,6 +1,6 @@ //! Completion of paths, including when writing a single name. -use hir::{Adt, PathResolution, ScopeDef, HasVisibility}; +use hir::{Adt, HasVisibility, PathResolution, ScopeDef}; use ra_syntax::AstNode; use test_utils::tested_by; @@ -52,9 +52,12 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { if let Some(krate) = krate { let traits_in_scope = ctx.scope().traits_in_scope(); ty.iterate_path_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, item| { + if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + return None; + } match item { hir::AssocItem::Function(func) => { - if !func.has_self_param(ctx.db) && context_module.map_or(true, |m| func.is_visible_from(ctx.db, m)) { + if !func.has_self_param(ctx.db) { acc.add_function(ctx, func); } } @@ -65,6 +68,9 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { }); ty.iterate_impl_items(ctx.db, krate, |item| { + if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + return None; + } match item { hir::AssocItem::Function(_) | hir::AssocItem::Const(_) => {} hir::AssocItem::TypeAlias(ty) => acc.add_type_alias(ctx, ty), @@ -75,6 +81,9 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { } hir::ModuleDef::Trait(t) => { for item in t.items(ctx.db) { + if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + continue; + } match item { hir::AssocItem::Function(func) => { if !func.has_self_param(ctx.db) { @@ -537,6 +546,60 @@ mod tests { ); } + #[test] + fn associated_item_visibility() { + assert_debug_snapshot!( + do_reference_completion( + " + //- /lib.rs + struct S; + + mod m { + impl super::S { + pub(super) fn public_method() { } + fn private_method() { } + pub(super) type PublicType = u32; + type PrivateType = u32; + pub(super) const PUBLIC_CONST: u32 = 1; + const PRIVATE_CONST: u32 = 1; + } + } + + fn foo() { let _ = S::<|> } + " + ), + @r###" + [ + CompletionItem { + label: "PUBLIC_CONST", + source_range: [302; 302), + delete: [302; 302), + insert: "PUBLIC_CONST", + kind: Const, + detail: "pub(super) const PUBLIC_CONST: u32 = 1;", + }, + CompletionItem { + label: "PublicType", + source_range: [302; 302), + delete: [302; 302), + insert: "PublicType", + kind: TypeAlias, + detail: "pub(super) type PublicType = u32;", + }, + CompletionItem { + label: "public_method()", + source_range: [302; 302), + delete: [302; 302), + insert: "public_method()$0", + kind: Function, + lookup: "public_method", + detail: "pub(super) fn public_method()", + }, + ] + "### + ); + } + #[test] fn completes_enum_associated_method() { assert_debug_snapshot!( -- cgit v1.2.3