Merge 9ed684e into 44cf9a2

noir-lang · Sep 4, 2024 · 0a8e2ba · 0a8e2ba
2 parents 44cf9a2 + 9ed684e
commit 0a8e2ba
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 38 deletions.
diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs
@@ -269,10 +269,7 @@ impl<'context> Elaborator<'context> {
                 let module = self.module_id();
                 self.interner.push_function(id, &function.def, module, location);
 
-                if self.interner.is_in_lsp_mode()
-                    && !function.def.is_test()
-                    && !function.def.is_private()
-                {
+                if self.interner.is_in_lsp_mode() && !function.def.is_test() {
                     self.interner.register_function(id, &function.def);
                 }
 

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -239,10 +239,7 @@ impl<'a> ModCollector<'a> {
             let location = Location::new(function.span(), self.file_id);
             context.def_interner.push_function(func_id, &function.def, module, location);
 
-            if context.def_interner.is_in_lsp_mode()
-                && !function.def.is_test()
-                && !function.def.is_private()
-            {
+            if context.def_interner.is_in_lsp_mode() && !function.def.is_test() {
                 context.def_interner.register_function(func_id, &function.def);
             }
 

diff --git a/tooling/lsp/src/modules.rs b/tooling/lsp/src/modules.rs
@@ -47,10 +47,11 @@ pub(crate) fn module_full_path(
     current_module_id: ModuleId,
     current_module_parent_id: Option<ModuleId>,
     interner: &NodeInterner,
+    def_maps: &BTreeMap<CrateId, CrateDefMap>,
 ) -> Option<String> {
     let full_path;
     if let ModuleDefId::ModuleId(module_id) = module_def_id {
-        if !is_visible(visibility, current_module_id, module_id) {
+        if !is_visible(module_id, current_module_id, visibility, def_maps) {
             return None;
         }
 
@@ -61,7 +62,7 @@ pub(crate) fn module_full_path(
             return None;
         };
 
-        if !is_visible(visibility, current_module_id, parent_module) {
+        if !is_visible(parent_module, current_module_id, visibility, def_maps) {
             return None;
         }
 

diff --git a/tooling/lsp/src/requests/code_action/import_or_qualify.rs b/tooling/lsp/src/requests/code_action/import_or_qualify.rs
@@ -52,6 +52,7 @@ impl<'a> CodeActionFinder<'a> {
                         self.module_id,
                         current_module_parent_id,
                         self.interner,
+                        self.def_maps,
                     ) else {
                         continue;
                     };

diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs
@@ -22,10 +22,7 @@ use noirc_frontend::{
         UnresolvedGenerics, UnresolvedType, UseTree, UseTreeKind, Visitor,
     },
     graph::{CrateId, Dependency},
-    hir::{
-        def_map::{CrateDefMap, LocalModuleId, ModuleId},
-        resolution::import::can_reference_module_id,
-    },
+    hir::def_map::{CrateDefMap, LocalModuleId, ModuleId},
     hir_def::traits::Trait,
     macros_api::{ModuleDefId, NodeInterner},
     node_interner::ReferenceId,
@@ -34,7 +31,7 @@ use noirc_frontend::{
 };
 use sort_text::underscore_sort_text;
 
-use crate::{requests::to_lsp_location, utils, LspState};
+use crate::{requests::to_lsp_location, utils, visibility::is_visible, LspState};
 
 use super::process_request;
 
@@ -1263,21 +1260,6 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option<ModuleDe
     }
 }
 
-fn is_visible(
-    target_module_id: ModuleId,
-    current_module_id: ModuleId,
-    visibility: ItemVisibility,
-    def_maps: &BTreeMap<CrateId, CrateDefMap>,
-) -> 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/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs
@@ -53,6 +53,7 @@ impl<'a> NodeFinder<'a> {
                         self.module_id,
                         current_module_parent_id,
                         self.interner,
+                        self.def_maps,
                     ) else {
                         continue;
                     };

diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs
@@ -1863,4 +1863,29 @@ mod completion_tests {
             Some("(use bar::foobar)".to_string()),
         );
     }
+
+    #[test]
+    async fn test_auto_import_suggests_private_function_if_visibile() {
+        let src = r#"
+            mod foo {
+                fn qux() {
+                  barba>|<
+                }
+            }
+
+            fn barbaz() {}
+
+            fn main() {}
+        "#;
+
+        let items = get_completions(src).await;
+        assert_eq!(items.len(), 1);
+
+        let item = &items[0];
+        assert_eq!(item.label, "barbaz()");
+        assert_eq!(
+            item.label_details.as_ref().unwrap().detail,
+            Some("(use super::barbaz)".to_string()),
+        );
+    }
 }
diff --git a/tooling/lsp/src/visibility.rs b/tooling/lsp/src/visibility.rs
@@ -1,13 +1,25 @@
-use noirc_frontend::{ast::ItemVisibility, hir::def_map::ModuleId};
+use std::collections::BTreeMap;
+
+use noirc_frontend::{
+    ast::ItemVisibility,
+    graph::CrateId,
+    hir::{
+        def_map::{CrateDefMap, ModuleId},
+        resolution::import::can_reference_module_id,
+    },
+};
 
 pub(super) fn is_visible(
+    target_module_id: ModuleId,
+    current_module_id: ModuleId,
     visibility: ItemVisibility,
-    current_module: ModuleId,
-    target_module: ModuleId,
+    def_maps: &BTreeMap<CrateId, CrateDefMap>,
 ) -> bool {
-    match visibility {
-        ItemVisibility::Public => true,
-        ItemVisibility::Private => false,
-        ItemVisibility::PublicCrate => current_module.krate == target_module.krate,
-    }
+    can_reference_module_id(
+        def_maps,
+        current_module_id.krate,
+        current_module_id.local_id,
+        target_module_id,
+        visibility,
+    )
 }