Fix importables: add submodule imports and handle crate reexports (#7647)

Author: piotmag769

crates/cairo-lang-semantic/src/db.rs

@@ -1649,7 +1649,7 @@ pub trait SemanticGroup:
         module_id: ModuleFileId,
     ) -> Option<Arc<OrderedHashMap<ImportableId, String>>>;
     /// Returns all visible importables in a module, alongside a visible use path to the trait.
-    /// `user_module_file_id` is the module from which the importables are should be visible. If
+    /// `user_module_file_id` is the module from which the importables should be visible. If
     /// `include_parent` is true, the parent module of `module_id` is also considered.
     #[salsa::invoke(lsp_helpers::visible_importables_in_module)]
     fn visible_importables_in_module(
@@ -1659,7 +1659,7 @@ pub trait SemanticGroup:
         include_parent: bool,
     ) -> Arc<[(ImportableId, String)]>;
     /// Returns all visible importables in a crate, alongside a visible use path to the trait.
-    /// `user_module_file_id` is the module from which the importables are should be visible.
+    /// `user_module_file_id` is the module from which the importables should be visible.
     #[salsa::invoke(lsp_helpers::visible_importables_in_crate)]
     fn visible_importables_in_crate(
         &self,

crates/cairo-lang-semantic/src/lsp_helpers.rs

@@ -18,7 +18,6 @@ use crate::db::SemanticGroup;
 use crate::expr::inference::InferenceId;
 use crate::items::functions::GenericFunctionId;
 use crate::items::us::SemanticUseEx;
-use crate::items::visibility::peek_visible_in;
 use crate::keyword::SELF_PARAM_KW;
 use crate::resolve::{ResolvedGenericItem, Resolver};
 use crate::types::TypeHead;
@@ -124,20 +123,11 @@ fn visible_importables_in_module_ex(
     }
 
     let resolver = Resolver::new(db, user_module_file_id, InferenceId::NoContext);
-    let ignore_visibility = resolver.ignore_visibility_checks(module_id);
+
     // Check if an item in the current module is visible from the user module.
     let is_visible = |item_name: SmolStr| {
-        if ignore_visibility {
-            Some(true)
-        } else {
-            let item_info = db.module_item_info_by_name(module_id, item_name).ok()??;
-            Some(peek_visible_in(
-                db.upcast(),
-                item_info.visibility,
-                module_id,
-                user_module_file_id.0,
-            ))
-        }
+        let item_info = db.module_item_info_by_name(module_id, item_name).ok()??;
+        Some(resolver.is_item_visible(module_id, &item_info, user_module_file_id.0))
     };
     visited_modules.insert(module_id);
     let mut modules_to_visit = vec![];
@@ -148,7 +138,15 @@ fn visible_importables_in_module_ex(
         }
         let resolved_item = db.use_resolved_item(use_id).ok()?;
         let (resolved_item, name) = match resolved_item {
-            ResolvedGenericItem::Module(ModuleId::CrateRoot(_)) => continue,
+            ResolvedGenericItem::Module(ModuleId::CrateRoot(crate_id)) => {
+                result.extend_from_slice(
+                    &db.visible_importables_in_crate(
+                        crate_id,
+                        ModuleFileId(module_id, FileIndex(0)),
+                    )[..],
+                );
+                continue;
+            }
             ResolvedGenericItem::Module(inner_module_id @ ModuleId::Submodule(module)) => {
                 modules_to_visit.push(inner_module_id);
 
@@ -163,7 +161,6 @@ fn visible_importables_in_module_ex(
             ResolvedGenericItem::GenericFunction(GenericFunctionId::Extern(item_id)) => {
                 (ImportableId::ExternFunction(item_id), item_id.name(db.upcast()))
             }
-            ResolvedGenericItem::GenericFunction(GenericFunctionId::Impl(_item_id)) => continue,
             ResolvedGenericItem::GenericType(GenericTypeId::Struct(item_id)) => {
                 (ImportableId::Struct(item_id), item_id.name(db.upcast()))
             }
@@ -188,7 +185,9 @@ fn visible_importables_in_module_ex(
             ResolvedGenericItem::Impl(item_id) => {
                 (ImportableId::Impl(item_id), item_id.name(db.upcast()))
             }
-            ResolvedGenericItem::Variable(_) | ResolvedGenericItem::TraitItem(_) => continue,
+            ResolvedGenericItem::Variable(_)
+            | ResolvedGenericItem::TraitItem(_)
+            | ResolvedGenericItem::GenericFunction(GenericFunctionId::Impl(_)) => continue,
         };
 
         result.push((resolved_item, name.to_string()));
@@ -197,6 +196,7 @@ fn visible_importables_in_module_ex(
         if !is_visible(submodule_id.name(db.upcast()))? {
             continue;
         }
+        result.push((ImportableId::Submodule(submodule_id), submodule_id.name(db).to_string()));
         modules_to_visit.push(ModuleId::Submodule(submodule_id));
     }
 
@@ -281,14 +281,8 @@ pub fn visible_importables_from_module(
     module_file_id: ModuleFileId,
 ) -> Option<Arc<OrderedHashMap<ImportableId, String>>> {
     let module_id = module_file_id.0;
-    let mut current_top_module = module_id;
-    while let ModuleId::Submodule(submodule_id) = current_top_module {
-        current_top_module = submodule_id.parent_module(db.upcast());
-    }
-    let current_crate_id = match current_top_module {
-        ModuleId::CrateRoot(crate_id) => crate_id,
-        ModuleId::Submodule(_) => unreachable!("current module is not a top-level module"),
-    };
+
+    let current_crate_id = module_id.owning_crate(db);
     let edition = db.crate_config(current_crate_id)?.settings.edition;
     let prelude_submodule_name = edition.prelude_submodule_name();
     let core_prelude_submodule = core_submodule(db, "prelude");
@@ -300,7 +294,7 @@ pub fn visible_importables_from_module(
     module_visible_importables.extend_from_slice(
         &db.visible_importables_in_module(prelude_submodule, prelude_submodule_file_id, false)[..],
     );
-    // Collect importables from all visible crates, including the current crate.
+    // Collect importables from all dependency crates, including the current crate and corelib.
     let settings = db.crate_config(current_crate_id).map(|c| c.settings).unwrap_or_default();
     for crate_id in chain!(
         [current_crate_id],
@@ -322,7 +316,7 @@ pub fn visible_importables_from_module(
         .extend_from_slice(&db.visible_importables_in_module(module_id, module_file_id, true)[..]);
 
     // Deduplicate importables, preferring shorter paths.
-    // This is the reason for searching in the crates before the current module- to prioritize
+    // This is the reason for searching in the crates before the current module - to prioritize
     // shorter, canonical paths prefixed with `crate::` over paths using `super::` or local
     // imports.
     let mut result: OrderedHashMap<ImportableId, String> = OrderedHashMap::default();

crates/cairo-lang-semantic/src/resolve/mod.rs

@@ -1912,7 +1912,7 @@ impl<'db> Resolver<'db> {
     }
 
     /// Checks if an item is visible from the current module.
-    fn is_item_visible(
+    pub fn is_item_visible(
         &self,
         containing_module_id: ModuleId,
         item_info: &ModuleItemInfo,