From 734e68da4ceb1b15b3430302f233d4700d694728 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 7 Mar 2020 23:03:56 +0100 Subject: Handle visibility in method call completion --- crates/ra_hir/src/code_model.rs | 8 ++++++ crates/ra_hir_def/src/data.rs | 12 ++++++--- crates/ra_ide/src/completion/complete_dot.rs | 38 +++++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 2944926e6..f93b43fb6 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -571,6 +571,14 @@ impl Function { } } +impl HasVisibility for Function { + fn visibility(&self, db: &impl HirDatabase) -> Visibility { + let function_data = db.function_data(self.id); + let visibility = &function_data.visibility; + visibility.resolve(db, &self.id.resolver(db)) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Const { pub(crate) id: ConstId, diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 9fc43f3fb..8b343af9d 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -7,13 +7,16 @@ use hir_expand::{ AstId, InFile, }; use ra_prof::profile; -use ra_syntax::ast::{self, AstNode, ImplItem, ModuleItemOwner, NameOwner, TypeAscriptionOwner}; +use ra_syntax::ast::{ + self, AstNode, ImplItem, ModuleItemOwner, NameOwner, TypeAscriptionOwner, VisibilityOwner, +}; use crate::{ db::DefDatabase, path::{path, GenericArgs, Path}, src::HasSource, type_ref::{Mutability, TypeBound, TypeRef}, + visibility::RawVisibility, AssocContainerId, AssocItemId, ConstId, ConstLoc, Expander, FunctionId, FunctionLoc, HasModule, ImplId, Intern, Lookup, ModuleId, StaticId, TraitId, TypeAliasId, TypeAliasLoc, }; @@ -26,6 +29,7 @@ pub struct FunctionData { /// True if the first param is `self`. This is relevant to decide whether this /// can be called as a method. pub has_self_param: bool, + pub visibility: RawVisibility, } impl FunctionData { @@ -72,7 +76,9 @@ impl FunctionData { ret_type }; - let sig = FunctionData { name, params, ret_type, has_self_param }; + let visibility = RawVisibility::from_ast(db, src.map(|s| s.visibility())); + + let sig = FunctionData { name, params, ret_type, has_self_param, visibility }; Arc::new(sig) } } @@ -230,7 +236,7 @@ impl ConstData { Arc::new(ConstData::new(&node)) } - fn new(node: &N) -> ConstData { + 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 } diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index 9145aa183..acada48ae 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -57,7 +57,10 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: &T let mut seen_methods = FxHashSet::default(); let traits_in_scope = ctx.scope().traits_in_scope(); receiver.iterate_method_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, func| { - if func.has_self_param(ctx.db) && seen_methods.insert(func.name(ctx.db)) { + if func.has_self_param(ctx.db) + && ctx.scope().module().map_or(true, |m| func.is_visible_from(ctx.db, m)) + && seen_methods.insert(func.name(ctx.db)) + { acc.add_function(ctx, func); } None::<()> @@ -307,6 +310,39 @@ mod tests { ); } + #[test] + fn test_method_completion_private() { + assert_debug_snapshot!( + do_ref_completion( + r" + struct A {} + mod m { + impl super::A { + fn private_method(&self) {} + pub(super) fn the_method(&self) {} + } + } + fn foo(a: A) { + a.<|> + } + ", + ), + @r###" + [ + CompletionItem { + label: "the_method()", + source_range: [256; 256), + delete: [256; 256), + insert: "the_method()$0", + kind: Method, + lookup: "the_method", + detail: "pub(super) fn the_method(&self)", + }, + ] + "### + ); + } + #[test] fn test_trait_method_completion() { assert_debug_snapshot!( -- cgit v1.2.3 From d9c77c54534fcde7c432c6e11746d636d972a20b Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 8 Mar 2020 10:51:40 +0100 Subject: Handle visibility for path completion (not in all cases yet) --- crates/ra_hir/src/code_model.rs | 12 +++++++- crates/ra_ide/src/completion/complete_path.rs | 44 ++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index f93b43fb6..4d1e8f921 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -204,10 +204,20 @@ impl Module { } /// Returns a `ModuleScope`: a set of items, visible in this module. - pub fn scope(self, db: &impl HirDatabase) -> 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 + } else { + Some((name, filtered)) + } + } else { + Some((name, def)) + }) .map(|(name, def)| (name.clone(), def.into())) .collect() } diff --git a/crates/ra_ide/src/completion/complete_path.rs b/crates/ra_ide/src/completion/complete_path.rs index 1a9699466..f99b1c2c4 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}; +use hir::{Adt, PathResolution, ScopeDef, HasVisibility}; use ra_syntax::AstNode; use test_utils::tested_by; @@ -15,9 +15,10 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { Some(PathResolution::Def(def)) => def, _ => return, }; + let context_module = ctx.scope().module(); match def { hir::ModuleDef::Module(module) => { - let module_scope = module.scope(ctx.db); + let module_scope = module.scope(ctx.db, context_module); for (name, def) in module_scope { if ctx.use_item_syntax.is_some() { if let ScopeDef::Unknown = def { @@ -53,7 +54,7 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) { ty.iterate_path_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, item| { match item { hir::AssocItem::Function(func) => { - if !func.has_self_param(ctx.db) { + if !func.has_self_param(ctx.db) && context_module.map_or(true, |m| func.is_visible_from(ctx.db, m)) { acc.add_function(ctx, func); } } @@ -169,6 +170,41 @@ mod tests { ); } + #[test] + fn path_visibility() { + assert_debug_snapshot!( + do_reference_completion( + r" + use self::my::<|>; + + mod my { + struct Bar; + pub struct Foo; + pub use Bar as PublicBar; + } + " + ), + @r###" + [ + CompletionItem { + label: "Foo", + source_range: [31; 31), + delete: [31; 31), + insert: "Foo", + kind: Struct, + }, + CompletionItem { + label: "PublicBar", + source_range: [31; 31), + delete: [31; 31), + insert: "PublicBar", + kind: Struct, + }, + ] + "### + ); + } + #[test] fn completes_use_item_starting_with_self() { assert_debug_snapshot!( @@ -177,7 +213,7 @@ mod tests { use self::m::<|>; mod m { - struct Bar; + pub struct Bar; } " ), -- cgit v1.2.3 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(-) 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