fix: suggest traits via visible reexports if they are not directly visible

compiler/noirc_driver/src/lib.rs

@@ -419,6 +419,10 @@ pub fn check_crate(
     crate_id: CrateId,
     options: &CompileOptions,
 ) -> CompilationResult<()> {
+    if options.disable_comptime_printing {
+        context.disable_comptime_printing();
+    }
+
     let diagnostics = CrateDefMap::collect_defs(crate_id, context, options.frontend_options());
     let crate_files = context.crate_files(&crate_id);
     let warnings_and_errors: Vec<CustomDiagnostic> = diagnostics

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -1924,7 +1924,7 @@ impl<'context> Elaborator<'context> {
                 for (field_index, field) in fields.iter().enumerate() {
                     let location = field.name.location();
                     let reference_id = ReferenceId::StructMember(*type_id, field_index);
-                    self.interner.add_definition_location(reference_id, location, None);
+                    self.interner.add_definition_location(reference_id, location);
                 }
             }
 
@@ -1999,7 +1999,6 @@ impl<'context> Elaborator<'context> {
                 UnresolvedType { typ: UnresolvedTypeData::Resolved(self_type_id), location };
 
             datatype.borrow_mut().init_variants();
-            let module_id = ModuleId { krate: self.crate_id, local_id: typ.module_id };
             self.resolving_ids.insert(*type_id);
 
             for (i, variant) in typ.enum_def.variants.iter().enumerate() {
@@ -2025,7 +2024,7 @@ impl<'context> Elaborator<'context> {
 
                 let reference_id = ReferenceId::EnumVariant(*type_id, i);
                 let location = variant.item.name.location();
-                self.interner.add_definition_location(reference_id, location, Some(module_id));
+                self.interner.add_definition_location(reference_id, location);
             }
 
             self.resolving_ids.remove(type_id);
@@ -2072,13 +2071,7 @@ impl<'context> Elaborator<'context> {
         self.elaborate_comptime_global(global_id);
 
         if let Some(name) = name {
-            self.interner.register_global(
-                global_id,
-                name,
-                location,
-                global.visibility,
-                self.module_id(),
-            );
+            self.interner.register_global(global_id, name, location, global.visibility);
         }
 
         self.local_module = old_module;

compiler/noirc_frontend/src/elaborator/types.rs

@@ -16,7 +16,7 @@ use crate::{
     elaborator::UnstableFeature,
     hir::{
         def_collector::dc_crate::CompilationError,
-        def_map::fully_qualified_module_path,
+        def_map::{ModuleDefId, fully_qualified_module_path},
         resolution::{errors::ResolverError, import::PathResolutionError},
         type_check::{
             NoMatchingImplFoundError, Source, TypeCheckError,
@@ -32,6 +32,7 @@ use crate::{
         stmt::HirStatement,
         traits::{NamedType, ResolvedTraitBound, Trait, TraitConstraint},
     },
+    modules::{get_ancestor_module_reexport, module_def_id_is_visible},
     node_interner::{
         DependencyId, ExprId, FuncId, GlobalValue, ImplSearchErrorKind, TraitId, TraitImplKind,
         TraitItemId,
@@ -2317,7 +2318,62 @@ impl Elaborator<'_> {
     }
 
     pub(crate) fn fully_qualified_trait_path(&self, trait_: &Trait) -> String {
-        fully_qualified_module_path(self.def_maps, self.crate_graph, &trait_.crate_id, trait_.id.0)
+        let module_def_id = ModuleDefId::TraitId(trait_.id);
+        let visibility = trait_.visibility;
+        let defining_module = None;
+        let trait_is_visible = module_def_id_is_visible(
+            module_def_id,
+            self.module_id(),
+            visibility,
+            defining_module,
+            self.interner,
+            self.def_maps,
+            &self.crate_graph[self.crate_id].dependencies,
+        );
+
+        if !trait_is_visible {
+            let dependencies = &self.crate_graph[self.crate_id].dependencies;
+
+            for reexport in self.interner.get_trait_reexports(trait_.id) {
+                let reexport_is_visible = module_def_id_is_visible(
+                    module_def_id,
+                    self.module_id(),
+                    reexport.visibility,
+                    Some(reexport.module_id),
+                    self.interner,
+                    self.def_maps,
+                    dependencies,
+                );
+                if reexport_is_visible {
+                    let module_path = fully_qualified_module_path(
+                        self.def_maps,
+                        self.crate_graph,
+                        &self.crate_id,
+                        reexport.module_id,
+                    );
+                    return format!("{module_path}::{}", reexport.name);
+                }
+            }
+
+            if let Some(reexport) = get_ancestor_module_reexport(
+                module_def_id,
+                visibility,
+                self.module_id(),
+                self.interner,
+                self.def_maps,
+                dependencies,
+            ) {
+                let module_path = fully_qualified_module_path(
+                    self.def_maps,
+                    self.crate_graph,
+                    &self.crate_id,
+                    reexport.module_id,
+                );
+                return format!("{module_path}::{}::{}", reexport.name, trait_.name);
+            }
+        }
+
+        fully_qualified_module_path(self.def_maps, self.crate_graph, &self.crate_id, trait_.id.0)
     }
 }
 

compiler/noirc_frontend/src/hir/comptime/display.rs

@@ -480,7 +480,7 @@ impl Display for ValuePrinter<'_, '_> {
                 write!(f, "{}", self.interner.function_name(function_id))
             }
             Value::ModuleDefinition(module_id) => {
-                if let Some(attributes) = self.interner.try_module_attributes(module_id) {
+                if let Some(attributes) = self.interner.try_module_attributes(*module_id) {
                     write!(f, "{}", &attributes.name)
                 } else {
                     write!(f, "(crate root)")

compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

@@ -2932,7 +2932,7 @@ fn module_name(
 ) -> IResult<Value> {
     let self_argument = check_one_argument(arguments, location)?;
     let module_id = get_module(self_argument)?;
-    let name = &interner.module_attributes(&module_id).name;
+    let name = &interner.module_attributes(module_id).name;
     let token = Token::Ident(name.clone());
     let token = LocatedToken::new(token, location);
     let tokens = Rc::new(vec![token]);

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

@@ -109,6 +109,7 @@ pub struct UnresolvedTraitImpl {
 #[derive(Clone)]
 pub struct UnresolvedTypeAlias {
     pub file_id: FileId,
+    pub crate_id: CrateId,
     pub module_id: LocalModuleId,
     pub type_alias_def: NoirTypeAlias,
 }
@@ -439,9 +440,7 @@ impl DefCollector {
                                 visibility,
                             );
 
-                            if context.def_interner.is_in_lsp_mode()
-                                && visibility != ItemVisibility::Private
-                            {
+                            if visibility != ItemVisibility::Private {
                                 context.def_interner.register_name_for_auto_import(
                                     name.to_string(),
                                     module_def_id,

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

@@ -382,6 +382,7 @@ impl ModCollector<'_> {
             // And store the TypeId -> TypeAlias mapping somewhere it is reachable
             let unresolved = UnresolvedTypeAlias {
                 file_id: self.file_id,
+                crate_id: krate,
                 module_id: self.module_id,
                 type_alias_def: type_alias,
             };
@@ -424,15 +425,8 @@ impl ModCollector<'_> {
             self.def_collector.items.type_aliases.insert(type_alias_id, unresolved);
 
             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,
-                    location,
-                    visibility,
-                    parent_module_id,
-                );
+                context.def_interner.register_type_alias(type_alias_id, name, location, visibility);
             }
         }
         errors
@@ -556,10 +550,7 @@ impl ModCollector<'_> {
                             .push_function_definition(func_id, modifiers, trait_id.0, location);
 
                         let referenced = ReferenceId::Function(func_id);
-                        let module_id = Some(trait_id.0);
-                        context
-                            .def_interner
-                            .add_definition_location(referenced, location, module_id);
+                        context.def_interner.add_definition_location(referenced, location);
 
                         if !trait_item.doc_comments.is_empty() {
                             context.def_interner.set_doc_comments(
@@ -698,13 +689,11 @@ impl ModCollector<'_> {
             );
 
             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(),
                     location,
                     visibility,
-                    parent_module_id,
                 );
             }
 
@@ -1000,14 +989,7 @@ fn push_child_module(
         );
 
         if interner.is_in_lsp_mode() {
-            let parent_module_id = ModuleId { krate: def_map.krate(), local_id: parent };
-            interner.register_module(
-                mod_id,
-                location,
-                visibility,
-                mod_name.to_string(),
-                parent_module_id,
-            );
+            interner.register_module(mod_id, location, visibility, mod_name.to_string());
         }
     }
 
@@ -1201,7 +1183,7 @@ pub fn collect_struct(
     }
 
     if interner.is_in_lsp_mode() {
-        interner.register_type(id, name.to_string(), location, visibility, parent_module_id);
+        interner.register_type(id, name.to_string(), location, visibility);
     }
 
     Some((id, unresolved))
@@ -1305,7 +1287,7 @@ pub fn collect_enum(
     }
 
     if interner.is_in_lsp_mode() {
-        interner.register_type(id, name.to_string(), location, visibility, parent_module_id);
+        interner.register_type(id, name.to_string(), location, visibility);
     }
 
     Some((id, unresolved))

compiler/noirc_frontend/src/hir_def/types.rs

@@ -9,7 +9,10 @@ use acvm::{AcirField, FieldElement};
 
 use crate::{
     ast::{IntegerBitSize, ItemVisibility},
-    hir::type_check::{TypeCheckError, generics::TraitGenerics},
+    hir::{
+        def_map::ModuleId,
+        type_check::{TypeCheckError, generics::TraitGenerics},
+    },
     hir_def::types::{self},
     node_interner::{NodeInterner, TraitAssociatedTypeId, TraitId, TypeAliasId},
     signed_field::{AbsU128, SignedField},
@@ -690,6 +693,7 @@ pub struct TypeAlias {
     pub generics: Generics,
     pub visibility: ItemVisibility,
     pub location: Location,
+    pub module_id: ModuleId,
 }
 
 impl std::hash::Hash for TypeAlias {
@@ -730,8 +734,9 @@ impl TypeAlias {
         typ: Type,
         generics: Generics,
         visibility: ItemVisibility,
+        module_id: ModuleId,
     ) -> TypeAlias {
-        TypeAlias { id, typ, name, location, generics, visibility }
+        TypeAlias { id, typ, name, location, generics, visibility, module_id }
     }
 
     pub fn set_type_and_generics(&mut self, new_typ: Type, new_generics: Generics) {

compiler/noirc_frontend/src/locations.rs

@@ -81,7 +81,7 @@ pub struct AutoImportEntry {
 impl NodeInterner {
     pub fn reference_location(&self, reference: ReferenceId) -> Location {
         match reference {
-            ReferenceId::Module(id) => self.module_attributes(&id).location,
+            ReferenceId::Module(id) => self.module_attributes(id).location,
             ReferenceId::Function(id) => self.function_modifiers(&id).name_location,
             ReferenceId::Type(id) => {
                 let typ = self.get_type(id);
@@ -119,10 +119,6 @@ impl NodeInterner {
         }
     }
 
-    pub fn reference_module(&self, reference: ReferenceId) -> Option<&ModuleId> {
-        self.reference_modules.get(&reference)
-    }
-
     pub(crate) fn add_module_def_id_reference(
         &mut self,
         def_id: ModuleDefId,
@@ -233,17 +229,13 @@ impl NodeInterner {
         &mut self,
         referenced: ReferenceId,
         referenced_location: Location,
-        module_id: Option<ModuleId>,
     ) {
         if !self.lsp_mode {
             return;
         }
 
         let referenced_index = self.get_or_insert_reference(referenced);
         self.location_indices.add_location(referenced_location, referenced_index);
-        if let Some(module_id) = module_id {
-            self.reference_modules.insert(referenced, module_id);
-        }
     }
 
     #[tracing::instrument(skip(self), ret)]
@@ -350,9 +342,8 @@ impl NodeInterner {
         location: Location,
         visibility: ItemVisibility,
         name: String,
-        parent_module_id: ModuleId,
     ) {
-        self.add_definition_location(ReferenceId::Module(id), location, Some(parent_module_id));
+        self.add_definition_location(ReferenceId::Module(id), location);
         self.register_name_for_auto_import(name, ModuleDefId::ModuleId(id), visibility, None);
     }
 
@@ -362,9 +353,8 @@ impl NodeInterner {
         name: String,
         location: Location,
         visibility: ItemVisibility,
-        parent_module_id: ModuleId,
     ) {
-        self.add_definition_location(ReferenceId::Global(id), location, Some(parent_module_id));
+        self.add_definition_location(ReferenceId::Global(id), location);
         self.register_name_for_auto_import(name, ModuleDefId::GlobalId(id), visibility, None);
     }
 
@@ -374,9 +364,8 @@ impl NodeInterner {
         name: String,
         location: Location,
         visibility: ItemVisibility,
-        parent_module_id: ModuleId,
     ) {
-        self.add_definition_location(ReferenceId::Type(id), location, Some(parent_module_id));
+        self.add_definition_location(ReferenceId::Type(id), location);
         self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None);
     }
 
@@ -386,9 +375,8 @@ impl NodeInterner {
         name: String,
         location: Location,
         visibility: ItemVisibility,
-        parent_module_id: ModuleId,
     ) {
-        self.add_definition_location(ReferenceId::Trait(id), location, Some(parent_module_id));
+        self.add_definition_location(ReferenceId::Trait(id), location);
         self.register_name_for_auto_import(name, ModuleDefId::TraitId(id), visibility, None);
     }
 
@@ -398,9 +386,8 @@ impl NodeInterner {
         name: String,
         location: Location,
         visibility: ItemVisibility,
-        parent_module_id: ModuleId,
     ) {
-        self.add_definition_location(ReferenceId::Alias(id), location, Some(parent_module_id));
+        self.add_definition_location(ReferenceId::Alias(id), location);
         self.register_name_for_auto_import(name, ModuleDefId::TypeAliasId(id), visibility, None);
     }