From 2a727b3f7f7fb84ab88b0d08e1ab29ae012a8c4f Mon Sep 17 00:00:00 2001
From: Ary Borenszweig <asterite@gmail.com>
Date: Wed, 9 Oct 2024 10:00:50 -0300
Subject: [PATCH] feat: don't suggest private struct fields in LSP (#6256)

# Description

## Problem

Struct fields now have visibility, but LSP still suggests all fields
regardless of their visibility.

## Summary

Hides non-accessible struct fields using the same logic the compiler
applies to check if a struct field is visible.

## Additional Context

None.

## 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.
---
 tooling/lsp/src/requests/completion.rs       | 34 +++++++++++++-------
 tooling/lsp/src/requests/completion/tests.rs | 16 +++++++++
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs
index cd4440407ef..b1a207bc962 100644
--- a/tooling/lsp/src/requests/completion.rs
+++ b/tooling/lsp/src/requests/completion.rs
@@ -24,10 +24,12 @@ use noirc_frontend::{
         UseTreeKind, Visitor,
     },
     graph::{CrateId, Dependency},
-    hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId},
+    hir::{
+        def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId},
+        resolution::visibility::struct_field_is_visible,
+    },
     hir_def::traits::Trait,
-    node_interner::NodeInterner,
-    node_interner::ReferenceId,
+    node_interner::{NodeInterner, ReferenceId},
     parser::{Item, ItemKind, ParsedSubModule},
     token::{CustomAttribute, Token, Tokens},
     Kind, ParsedModule, StructType, Type, TypeBinding,
@@ -691,16 +693,24 @@ impl<'a> NodeFinder<'a> {
         prefix: &str,
         self_prefix: bool,
     ) {
-        for (field_index, (name, typ)) in struct_type.get_fields(generics).iter().enumerate() {
-            if name_matches(name, prefix) {
-                self.completion_items.push(self.struct_field_completion_item(
-                    name,
-                    typ,
-                    struct_type.id,
-                    field_index,
-                    self_prefix,
-                ));
+        for (field_index, (name, visibility, typ)) in
+            struct_type.get_fields_with_visibility(generics).iter().enumerate()
+        {
+            if !struct_field_is_visible(struct_type, *visibility, self.module_id, self.def_maps) {
+                continue;
             }
+
+            if !name_matches(name, prefix) {
+                continue;
+            }
+
+            self.completion_items.push(self.struct_field_completion_item(
+                name,
+                typ,
+                struct_type.id,
+                field_index,
+                self_prefix,
+            ));
         }
     }
 
diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs
index bc8bb75e10c..e9ebc8f1f13 100644
--- a/tooling/lsp/src/requests/completion/tests.rs
+++ b/tooling/lsp/src/requests/completion/tests.rs
@@ -1071,6 +1071,22 @@ mod completion_tests {
         assert_completion(src, vec![field_completion_item("bar", "i32")]).await;
     }
 
+    #[test]
+    async fn test_does_not_suggest_private_struct_field() {
+        let src = r#"
+            mod moo {
+                pub struct Some {
+                    property: i32,
+                }
+            }
+
+            fn foo(s: moo::Some) {
+                s.>|<
+            }
+        "#;
+        assert_completion(src, vec![]).await;
+    }
+
     #[test]
     async fn test_suggests_struct_impl_method() {
         let src = r#"