From a9527210d5005a79f7302792825f1f55e16d731d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Sm=C3=B3=C5=82ka?= <jp.smolka@gmail.com>
Date: Thu, 14 Nov 2024 19:19:37 +0100
Subject: [PATCH] LS: Limit module import completion to visible members (#6566)

---
 .../src/ide/completion/completions.rs         | 19 ++++--
 .../tests/e2e/completions.rs                  |  1 +
 .../test_data/completions/module_items.txt    | 58 +++++++++++++++++++
 3 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 crates/cairo-lang-language-server/tests/test_data/completions/module_items.txt

diff --git a/crates/cairo-lang-language-server/src/ide/completion/completions.rs b/crates/cairo-lang-language-server/src/ide/completion/completions.rs
index 287ba9c60e3..19d46a3d430 100644
--- a/crates/cairo-lang-language-server/src/ide/completion/completions.rs
+++ b/crates/cairo-lang-language-server/src/ide/completion/completions.rs
@@ -126,17 +126,24 @@ pub fn colon_colon_completions(
         .resolve_concrete_path(&mut diagnostics, segments, NotFoundItemType::Identifier)
         .ok()?;
 
+    let current_module_id = module_file_id.0;
+
     Some(match item {
         ResolvedConcreteItem::Module(module_id) => db
             .module_items(module_id)
             .ok()?
             .iter()
-            .map(|item| CompletionItem {
-                label: item.name(db.upcast()).to_string(),
-                kind: ResolvedGenericItem::from_module_item(db, *item)
-                    .ok()
-                    .map(resolved_generic_item_completion_kind),
-                ..CompletionItem::default()
+            .filter_map(|item| {
+                let resolved_item = ResolvedGenericItem::from_module_item(db, *item).ok()?;
+                let item_info = db.module_item_info_by_name(module_id, item.name(db)).ok()??;
+
+                peek_visible_in(db, item_info.visibility, module_id, current_module_id).then(|| {
+                    CompletionItem {
+                        label: item.name(db.upcast()).to_string(),
+                        kind: Some(resolved_generic_item_completion_kind(resolved_item)),
+                        ..CompletionItem::default()
+                    }
+                })
             })
             .collect(),
         ResolvedConcreteItem::Trait(item) => db
diff --git a/crates/cairo-lang-language-server/tests/e2e/completions.rs b/crates/cairo-lang-language-server/tests/e2e/completions.rs
index 1a0f79190ea..acf7967e1e9 100644
--- a/crates/cairo-lang-language-server/tests/e2e/completions.rs
+++ b/crates/cairo-lang-language-server/tests/e2e/completions.rs
@@ -11,6 +11,7 @@ cairo_lang_test_utils::test_file_test!(
     {
         methods_text_edits: "methods_text_edits.txt",
         structs: "structs.txt",
+        module_items: "module_items.txt",
     },
     test_completions_text_edits
 
diff --git a/crates/cairo-lang-language-server/tests/test_data/completions/module_items.txt b/crates/cairo-lang-language-server/tests/test_data/completions/module_items.txt
new file mode 100644
index 00000000000..8704e4c189a
--- /dev/null
+++ b/crates/cairo-lang-language-server/tests/test_data/completions/module_items.txt
@@ -0,0 +1,58 @@
+//! > Test completing public members of a module.
+
+//! > test_runner_name
+test_completions_text_edits
+
+//! > cairo_project.toml
+[crate_roots]
+hello = "src"
+
+[config.global]
+edition = "2024_07"
+
+//! > cairo_code
+mod helper_module {
+    pub trait Trait1<T> {
+        fn some_method(self: @T);
+    }
+
+    pub const CONST: felt252 = 0x0;
+
+    fn foo() {}
+    pub fn bar() {}
+}
+
+mod not_exporting_module {
+    const CONST: u32 = 0;
+    fn foo() {}
+    fn bar() {}
+}
+
+mod nested_module {
+    pub mod inner {}
+}
+
+use helper_module::<caret>;
+use non_exporting_module::<caret>;
+use nested_module::<caret>;
+use non_existent_module::<caret>;
+
+//! > Completions #0
+use helper_module::<caret>;
+--------------------------
+Completion: Trait1
+--------------------------
+Completion: CONST
+--------------------------
+Completion: bar
+
+//! > Completions #1
+use non_exporting_module::<caret>;
+
+//! > Completions #2
+use nested_module::<caret>;
+--------------------------
+Completion: inner
+
+//! > Completions #3
+use non_existent_module::<caret>;