diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 761da6c361d..b820e4664e3 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -374,9 +374,9 @@ fn resolve_external_dep( resolve_path_to_ns(&dep_directive, dep_module.krate, importing_crate, def_maps, path_references) } -// Issue an error if the given private function is being called from a non-child module, or -// if the given pub(crate) function is being called from another crate -fn can_reference_module_id( +// Returns false if the given private function is being called from a non-child module, or +// if the given pub(crate) function is being called from another crate. Otherwise returns true. +pub fn can_reference_module_id( def_maps: &BTreeMap, importing_crate: CrateId, current_module: LocalModuleId, diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 15b5be1690c..c61f92795ad 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -16,16 +16,19 @@ use noirc_errors::{Location, Span}; use noirc_frontend::{ ast::{ AsTraitPath, BlockExpression, CallExpression, ConstructorExpression, Expression, - ExpressionKind, ForLoopStatement, Ident, IfExpression, LValue, Lambda, LetStatement, - MemberAccessExpression, MethodCallExpression, NoirFunction, NoirStruct, NoirTraitImpl, - Path, PathKind, PathSegment, Pattern, Statement, StatementKind, TraitItem, TypeImpl, - UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, UnresolvedTypeData, UseTree, - UseTreeKind, + ExpressionKind, ForLoopStatement, Ident, IfExpression, ItemVisibility, LValue, Lambda, + LetStatement, MemberAccessExpression, MethodCallExpression, NoirFunction, NoirStruct, + NoirTraitImpl, Path, PathKind, PathSegment, Pattern, Statement, StatementKind, TraitItem, + TypeImpl, UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, UnresolvedTypeData, + UseTree, UseTreeKind, }, graph::{CrateId, Dependency}, hir::{ def_map::{CrateDefMap, LocalModuleId, ModuleId}, - resolution::path_resolver::{PathResolver, StandardPathResolver}, + resolution::{ + import::can_reference_module_id, + path_resolver::{PathResolver, StandardPathResolver}, + }, }, hir_def::traits::Trait, macros_api::{ModuleDefId, NodeInterner}, @@ -1238,29 +1241,33 @@ impl<'a> NodeFinder<'a> { if name_matches(name, prefix) { let per_ns = module_data.find_name(ident); - if let Some((module_def_id, _, _)) = per_ns.types { - if let Some(completion_item) = self.module_def_id_completion_item( - module_def_id, - name.clone(), - function_completion_kind, - function_kind, - requested_items, - ) { - self.completion_items.push(completion_item); - self.suggested_module_def_ids.insert(module_def_id); + if let Some((module_def_id, visibility, _)) = per_ns.types { + if is_visible(module_id, self.module_id, visibility, self.def_maps) { + if let Some(completion_item) = self.module_def_id_completion_item( + module_def_id, + name.clone(), + function_completion_kind, + function_kind, + requested_items, + ) { + self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(module_def_id); + } } } - if let Some((module_def_id, _, _)) = per_ns.values { - if let Some(completion_item) = self.module_def_id_completion_item( - module_def_id, - name.clone(), - function_completion_kind, - function_kind, - requested_items, - ) { - self.completion_items.push(completion_item); - self.suggested_module_def_ids.insert(module_def_id); + if let Some((module_def_id, visibility, _)) = per_ns.values { + if is_visible(module_id, self.module_id, visibility, self.def_maps) { + if let Some(completion_item) = self.module_def_id_completion_item( + module_def_id, + name.clone(), + function_completion_kind, + function_kind, + requested_items, + ) { + self.completion_items.push(completion_item); + self.suggested_module_def_ids.insert(module_def_id); + } } } } @@ -1384,6 +1391,21 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option, +) -> bool { + can_reference_module_id( + def_maps, + current_module_id.krate, + current_module_id.local_id, + target_module_id, + visibility, + ) +} + #[cfg(test)] mod completion_name_matches_tests { use crate::requests::completion::name_matches; diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 07ac8885bd5..59e007c5a70 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -182,7 +182,8 @@ mod completion_tests { async fn test_use_function() { let src = r#" mod foo { - fn bar(x: i32) -> u64 { 0 } + pub fn bar(x: i32) -> u64 { 0 } + fn bar_is_private(x: i32) -> u64 { 0 } } use foo::>|< "#; @@ -1703,7 +1704,7 @@ mod completion_tests { async fn test_completes_after_colon_in_the_middle_of_an_ident_middle_segment() { let src = r#" mod foo { - fn bar() {} + pub fn bar() {} } fn main() { @@ -1725,7 +1726,7 @@ mod completion_tests { async fn test_completes_at_function_call_name() { let src = r#" mod foo { - fn bar() {} + pub fn bar() {} } fn main() {