fix: add missing visibility for auto-import names (#6205)

# Description

## Problem

Even though we started tracking visibility for many items, this
visibility wasn't registered for auto-import.

## Summary

## Additional Context

I was thinking that maybe registering things for auto-import isn't
necessary. We could traverse all types and all functions in all modules
and check if they match... but maybe that'll be much slower (right now
we group all items by name so maybe iterating that is faster). In any
case we could explore that other option later on.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -1385,7 +1385,7 @@ impl<'context> Elaborator<'context> {
         }
 
         if let Some(name) = name {
-            self.interner.register_global(global_id, name, self.module_id());
+            self.interner.register_global(global_id, name, global.visibility, self.module_id());
         }
 
         self.local_module = old_module;

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -111,6 +111,7 @@ pub struct UnresolvedGlobal {
     pub module_id: LocalModuleId,
     pub global_id: GlobalId,
     pub stmt_def: LetStatement,
+    pub visibility: ItemVisibility,
 }
 
 pub struct ModuleAttribute {

compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -357,7 +357,12 @@ impl<'a> ModCollector<'a> {
             if context.def_interner.is_in_lsp_mode() {
                 let parent_module_id = ModuleId { krate, local_id: self.module_id };
                 let name = name.to_string();
-                context.def_interner.register_type_alias(type_alias_id, name, parent_module_id);
+                context.def_interner.register_type_alias(
+                    type_alias_id,
+                    name,
+                    visibility,
+                    parent_module_id,
+                );
             }
         }
         errors
@@ -591,7 +596,12 @@ impl<'a> ModCollector<'a> {
 
             if context.def_interner.is_in_lsp_mode() {
                 let parent_module_id = ModuleId { krate, local_id: self.module_id };
-                context.def_interner.register_trait(trait_id, name.to_string(), parent_module_id);
+                context.def_interner.register_trait(
+                    trait_id,
+                    name.to_string(),
+                    visibility,
+                    parent_module_id,
+                );
             }
 
             self.def_collector.items.traits.insert(trait_id, unresolved);
@@ -879,7 +889,7 @@ fn push_child_module(
         );
 
         if interner.is_in_lsp_mode() {
-            interner.register_module(mod_id, mod_name.0.contents.clone());
+            interner.register_module(mod_id, visibility, mod_name.0.contents.clone());
         }
     }
 
@@ -1221,7 +1231,7 @@ pub(crate) fn collect_global(
 
     interner.set_doc_comments(ReferenceId::Global(global_id), doc_comments);
 
-    let global = UnresolvedGlobal { file_id, module_id, global_id, stmt_def: global };
+    let global = UnresolvedGlobal { file_id, module_id, global_id, stmt_def: global, visibility };
     (global, error)
 }
 

compiler/noirc_frontend/src/locations.rs

@@ -278,20 +278,23 @@ impl NodeInterner {
             .next()
     }
 
-    pub(crate) fn register_module(&mut self, id: ModuleId, name: String) {
-        let visibility = ItemVisibility::Public;
+    pub(crate) fn register_module(
+        &mut self,
+        id: ModuleId,
+        visibility: ItemVisibility,
+        name: String,
+    ) {
         self.register_name_for_auto_import(name, ModuleDefId::ModuleId(id), visibility, None);
     }
 
     pub(crate) fn register_global(
         &mut self,
         id: GlobalId,
         name: String,
+        visibility: ItemVisibility,
         parent_module_id: ModuleId,
     ) {
         self.add_definition_location(ReferenceId::Global(id), Some(parent_module_id));
-
-        let visibility = ItemVisibility::Public;
         self.register_name_for_auto_import(name, ModuleDefId::GlobalId(id), visibility, None);
     }
 
@@ -306,22 +309,25 @@ impl NodeInterner {
         self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None);
     }
 
-    pub(crate) fn register_trait(&mut self, id: TraitId, name: String, parent_module_id: ModuleId) {
+    pub(crate) fn register_trait(
+        &mut self,
+        id: TraitId,
+        name: String,
+        visibility: ItemVisibility,
+        parent_module_id: ModuleId,
+    ) {
         self.add_definition_location(ReferenceId::Trait(id), Some(parent_module_id));
-
-        let visibility = ItemVisibility::Public;
         self.register_name_for_auto_import(name, ModuleDefId::TraitId(id), visibility, None);
     }
 
     pub(crate) fn register_type_alias(
         &mut self,
         id: TypeAliasId,
         name: String,
+        visibility: ItemVisibility,
         parent_module_id: ModuleId,
     ) {
         self.add_definition_location(ReferenceId::Alias(id), Some(parent_module_id));
-
-        let visibility = ItemVisibility::Public;
         self.register_name_for_auto_import(name, ModuleDefId::TypeAliasId(id), visibility, None);
     }
 

tooling/lsp/src/requests/code_action/import_or_qualify.rs

@@ -186,7 +186,7 @@ fn foo(x: SomeTypeInBar) {}"#;
         let src = r#"
         mod foo {
             mod bar {
-                mod some_module_in_bar {}
+                pub mod some_module_in_bar {}
             }
         }
 
@@ -198,7 +198,7 @@ fn foo(x: SomeTypeInBar) {}"#;
         let expected = r#"
         mod foo {
             mod bar {
-                mod some_module_in_bar {}
+                pub mod some_module_in_bar {}
             }
         }
 
@@ -216,7 +216,7 @@ fn foo(x: SomeTypeInBar) {}"#;
 
         let src = r#"mod foo {
     mod bar {
-        mod some_module_in_bar {}
+        pub(crate) mod some_module_in_bar {}
     }
 }
 
@@ -228,7 +228,7 @@ fn main() {
 
 mod foo {
     mod bar {
-        mod some_module_in_bar {}
+        pub(crate) mod some_module_in_bar {}
     }
 }
 

tooling/lsp/src/requests/completion/tests.rs

@@ -1575,7 +1575,7 @@ mod completion_tests {
     async fn test_auto_import_suggests_modules_too() {
         let src = r#"
             mod foo {
-                mod barbaz {
+                pub mod barbaz {
                     fn hello_world() {}
                 }
             }
@@ -2271,4 +2271,56 @@ mod completion_tests {
         )
         .await;
     }
+
+    #[test]
+    async fn test_does_not_auto_import_private_global() {
+        let src = r#"mod moo {
+            global foobar = 1;
+        }
+
+        fn main() {
+            fooba>|<
+        }"#;
+
+        assert_completion(src, Vec::new()).await;
+    }
+
+    #[test]
+    async fn test_does_not_auto_import_private_type_alias() {
+        let src = r#"mod moo {
+            type foobar = i32;
+        }
+
+        fn main() {
+            fooba>|<
+        }"#;
+
+        assert_completion(src, Vec::new()).await;
+    }
+
+    #[test]
+    async fn test_does_not_auto_import_private_trait() {
+        let src = r#"mod moo {
+            trait Foobar {}
+        }
+
+        fn main() {
+            Fooba>|<
+        }"#;
+
+        assert_completion(src, Vec::new()).await;
+    }
+
+    #[test]
+    async fn test_does_not_auto_import_private_module() {
+        let src = r#"mod moo {
+            mod foobar {}
+        }
+
+        fn main() {
+            fooba>|<
+        }"#;
+
+        assert_completion(src, Vec::new()).await;
+    }
 }