feat: detect unconstructed structs

compiler/noirc_frontend/src/ast/type_alias.rs

@@ -1,4 +1,4 @@
-use super::{Ident, UnresolvedGenerics, UnresolvedType};
+use super::{Ident, ItemVisibility, UnresolvedGenerics, UnresolvedType};
 use iter_extended::vecmap;
 use noirc_errors::Span;
 use std::fmt::Display;
@@ -9,6 +9,7 @@ pub struct NoirTypeAlias {
     pub name: Ident,
     pub generics: UnresolvedGenerics,
     pub typ: UnresolvedType,
+    pub visibility: ItemVisibility,
     pub span: Span,
 }
 
@@ -17,9 +18,10 @@ impl NoirTypeAlias {
         name: Ident,
         generics: UnresolvedGenerics,
         typ: UnresolvedType,
+        visibility: ItemVisibility,
         span: Span,
     ) -> NoirTypeAlias {
-        NoirTypeAlias { name, generics, typ, span }
+        NoirTypeAlias { name, generics, typ, visibility, span }
     }
 }
 

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -532,6 +532,8 @@ impl<'context> Elaborator<'context> {
             }
         };
 
+        self.mark_struct_as_constructed(r#type.clone());
+
         let turbofish_span = last_segment.turbofish_span();
 
         let struct_generics = self.resolve_struct_turbofish_generics(
@@ -561,6 +563,12 @@ impl<'context> Elaborator<'context> {
         (expr, Type::Struct(struct_type, generics))
     }
 
+    pub(super) fn mark_struct_as_constructed(&mut self, struct_type: Shared<StructType>) {
+        let struct_type = struct_type.borrow();
+        let parent_module_id = struct_type.id.parent_module_id(self.def_maps);
+        self.interner.usage_tracker.mark_as_used(parent_module_id, &struct_type.name);
+    }
+
     /// Resolve all the fields of a struct constructor expression.
     /// Ensures all fields are present, none are repeated, and all
     /// are part of the struct.

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -95,7 +95,7 @@ pub struct Elaborator<'context> {
 
     pub(crate) interner: &'context mut NodeInterner,
 
-    def_maps: &'context mut DefMaps,
+    pub(crate) def_maps: &'context mut DefMaps,
 
     file: FileId,
 
@@ -756,6 +756,10 @@ impl<'context> Elaborator<'context> {
                 type_span,
             );
 
+            if is_entry_point {
+                self.mark_parameter_type_as_used(&typ);
+            }
+
             let pattern = self.elaborate_pattern_and_store_ids(
                 pattern,
                 typ.clone(),
@@ -833,6 +837,57 @@ impl<'context> Elaborator<'context> {
         self.current_item = None;
     }
 
+    fn mark_parameter_type_as_used(&mut self, typ: &Type) {
+        match typ {
+            Type::Array(_n, typ) => self.mark_parameter_type_as_used(typ),
+            Type::Slice(typ) => self.mark_parameter_type_as_used(typ),
+            Type::Tuple(types) => {
+                for typ in types {
+                    self.mark_parameter_type_as_used(typ);
+                }
+            }
+            Type::Struct(struct_type, generics) => {
+                self.mark_struct_as_constructed(struct_type.clone());
+                for generic in generics {
+                    self.mark_parameter_type_as_used(generic);
+                }
+            }
+            Type::Alias(alias_type, generics) => {
+                self.mark_parameter_type_as_used(&alias_type.borrow().get_type(generics));
+            }
+            Type::MutableReference(typ) => {
+                self.mark_parameter_type_as_used(typ);
+            }
+            Type::InfixExpr(left, _op, right) => {
+                self.mark_parameter_type_as_used(left);
+                self.mark_parameter_type_as_used(right);
+            }
+            Type::FieldElement
+            | Type::Integer(..)
+            | Type::Bool
+            | Type::String(_)
+            | Type::FmtString(_, _)
+            | Type::Unit
+            | Type::Quoted(..)
+            | Type::Constant(_)
+            | Type::TraitAsType(..)
+            | Type::TypeVariable(..)
+            | Type::NamedGeneric(..)
+            | Type::Function(..)
+            | Type::Forall(..)
+            | Type::Error => (),
+        }
+
+        if let Type::Alias(alias_type, generics) = typ {
+            self.mark_parameter_type_as_used(&alias_type.borrow().get_type(generics));
+            return;
+        }
+
+        if let Type::Struct(struct_type, _generics) = typ {
+            self.mark_struct_as_constructed(struct_type.clone());
+        }
+    }
+
     fn run_function_lints(&mut self, func: &FuncMeta, modifiers: &FunctionModifiers) {
         self.run_lint(|_| lints::inlining_attributes(func, modifiers).map(Into::into));
         self.run_lint(|_| lints::missing_pub(func, modifiers).map(Into::into));
@@ -1165,13 +1220,100 @@ impl<'context> Elaborator<'context> {
         self.file = alias.file_id;
         self.local_module = alias.module_id;
 
+        let name = &alias.type_alias_def.name;
+        let visibility = alias.type_alias_def.visibility;
+        let span = alias.type_alias_def.typ.span;
+
         let generics = self.add_generics(&alias.type_alias_def.generics);
         self.current_item = Some(DependencyId::Alias(alias_id));
         let typ = self.resolve_type(alias.type_alias_def.typ);
+
+        if visibility != ItemVisibility::Private {
+            self.check_aliased_type_is_not_more_private(name, visibility, &typ, span);
+        }
+
         self.interner.set_type_alias(alias_id, typ, generics);
         self.generics.clear();
     }
 
+    fn check_aliased_type_is_not_more_private(
+        &mut self,
+        name: &Ident,
+        visibility: ItemVisibility,
+        typ: &Type,
+        span: Span,
+    ) {
+        match typ {
+            Type::Struct(struct_type, generics) => {
+                let struct_type = struct_type.borrow();
+                let struct_module_id = struct_type.id.module_id();
+
+                // We only check this in types in the same crate. If it's in a different crate
+                // then it's either accessible (all good) or it's not, in which case a different
+                // error will happen somewhere else, but no need to error again here.
+                if struct_module_id.krate == self.crate_id {
+                    // Find the struct in the parent module so we can know its visibility
+                    let parent_module_id = struct_type.id.parent_module_id(self.def_maps);
+                    let parent_module_data = self.get_module(parent_module_id);
+                    let per_ns = parent_module_data.find_name(&struct_type.name);
+                    if let Some((_, aliased_visibility, _)) = per_ns.types {
+                        if aliased_visibility < visibility {
+                            self.push_err(ResolverError::TypeIsMorePrivateThenItem {
+                                typ: struct_type.name.to_string(),
+                                item: name.to_string(),
+                                span,
+                            });
+                        }
+                    }
+                }
+
+                for generic in generics {
+                    self.check_aliased_type_is_not_more_private(name, visibility, generic, span);
+                }
+            }
+            Type::Tuple(types) => {
+                for typ in types {
+                    self.check_aliased_type_is_not_more_private(name, visibility, typ, span);
+                }
+            }
+            Type::Alias(alias_type, generics) => {
+                self.check_aliased_type_is_not_more_private(
+                    name,
+                    visibility,
+                    &alias_type.borrow().get_type(generics),
+                    span,
+                );
+            }
+            Type::Function(args, return_type, env, _) => {
+                for arg in args {
+                    self.check_aliased_type_is_not_more_private(name, visibility, arg, span);
+                }
+                self.check_aliased_type_is_not_more_private(name, visibility, return_type, span);
+                self.check_aliased_type_is_not_more_private(name, visibility, env, span);
+            }
+            Type::MutableReference(typ) | Type::Array(_, typ) | Type::Slice(typ) => {
+                self.check_aliased_type_is_not_more_private(name, visibility, typ, span);
+            }
+            Type::InfixExpr(left, _op, right) => {
+                self.check_aliased_type_is_not_more_private(name, visibility, left, span);
+                self.check_aliased_type_is_not_more_private(name, visibility, right, span);
+            }
+            Type::FieldElement
+            | Type::Integer(..)
+            | Type::Bool
+            | Type::String(..)
+            | Type::FmtString(..)
+            | Type::Unit
+            | Type::Quoted(..)
+            | Type::TypeVariable(..)
+            | Type::Forall(..)
+            | Type::TraitAsType(..)
+            | Type::Constant(..)
+            | Type::NamedGeneric(..)
+            | Type::Error => (),
+        }
+    }
+
     fn collect_struct_definitions(&mut self, structs: &BTreeMap<StructId, UnresolvedStruct>) {
         // This is necessary to avoid cloning the entire struct map
         // when adding checks after each struct field is resolved.

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

@@ -30,7 +30,6 @@ use crate::{
             InterpreterError, Value,
         },
         def_collector::dc_crate::CollectedItems,
-        def_map::ModuleId,
     },
     hir_def::function::FunctionBody,
     lexer::Lexer,
@@ -513,14 +512,7 @@ fn struct_def_module(
 ) -> IResult<Value> {
     let self_argument = check_one_argument(arguments, location)?;
     let struct_id = get_struct(self_argument)?;
-    let struct_module_id = struct_id.module_id();
-
-    // A struct's module is its own module. To get the module where its defined we need
-    // to look for its parent.
-    let module_data = interpreter.elaborator.get_module(struct_module_id);
-    let parent_local_id = module_data.parent.expect("Expected struct module parent to exist");
-    let parent = ModuleId { krate: struct_module_id.krate, local_id: parent_local_id };
-
+    let parent = struct_id.parent_module_id(interpreter.elaborator.def_maps);
     Ok(Value::ModuleDefinition(parent))
 }
 

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

@@ -508,7 +508,7 @@ impl DefCollector {
                 let ident = ident.clone();
                 let error = CompilationError::ResolverError(ResolverError::UnusedItem {
                     ident,
-                    item_type: unused_item.item_type(),
+                    item: *unused_item,
                 });
                 (error, module.location.file)
             })

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

@@ -314,6 +314,7 @@ impl<'a> ModCollector<'a> {
             let doc_comments = type_alias.doc_comments;
             let type_alias = type_alias.item;
             let name = type_alias.name.clone();
+            let visibility = type_alias.visibility;
 
             // And store the TypeId -> TypeAlias mapping somewhere it is reachable
             let unresolved = UnresolvedTypeAlias {
@@ -335,8 +336,19 @@ impl<'a> ModCollector<'a> {
             context.def_interner.set_doc_comments(ReferenceId::Alias(type_alias_id), doc_comments);
 
             // Add the type alias to scope so its path can be looked up later
-            let result = self.def_collector.def_map.modules[self.module_id.0]
-                .declare_type_alias(name.clone(), type_alias_id);
+            let result = self.def_collector.def_map.modules[self.module_id.0].declare_type_alias(
+                name.clone(),
+                visibility,
+                type_alias_id,
+            );
+
+            let parent_module_id = ModuleId { krate, local_id: self.module_id };
+            context.def_interner.usage_tracker.add_unused_item(
+                parent_module_id,
+                name.clone(),
+                UnusedItem::TypeAlias(type_alias_id),
+                visibility,
+            );
 
             if let Err((first_def, second_def)) = result {
                 let err = DefCollectorErrorKind::Duplicate {
@@ -533,7 +545,11 @@ impl<'a> ModCollector<'a> {
                     TraitItem::Type { name } => {
                         if let Err((first_def, second_def)) = self.def_collector.def_map.modules
                             [trait_id.0.local_id.0]
-                            .declare_type_alias(name.clone(), TypeAliasId::dummy_id())
+                            .declare_type_alias(
+                                name.clone(),
+                                ItemVisibility::Public,
+                                TypeAliasId::dummy_id(),
+                            )
                         {
                             let error = DefCollectorErrorKind::Duplicate {
                                 typ: DuplicateType::TraitAssociatedType,

compiler/noirc_frontend/src/hir/def_map/mod.rs

@@ -45,12 +45,16 @@ impl ModuleId {
     pub fn dummy_id() -> ModuleId {
         ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() }
     }
-}
 
-impl ModuleId {
     pub fn module(self, def_maps: &DefMaps) -> &ModuleData {
         &def_maps[&self.krate].modules()[self.local_id.0]
     }
+
+    /// Returns this module's parent, if there's any.
+    pub fn parent(self, def_maps: &DefMaps) -> Option<ModuleId> {
+        let module_data = &def_maps[&self.krate].modules()[self.local_id.0];
+        module_data.parent.map(|local_id| ModuleId { krate: self.krate, local_id })
+    }
 }
 
 pub type DefMaps = BTreeMap<CrateId, CrateDefMap>;

compiler/noirc_frontend/src/hir/def_map/module_data.rs

@@ -112,9 +112,10 @@ impl ModuleData {
     pub fn declare_type_alias(
         &mut self,
         name: Ident,
+        visibility: ItemVisibility,
         id: TypeAliasId,
     ) -> Result<(), (Ident, Ident)> {
-        self.declare(name, ItemVisibility::Public, id.into(), None)
+        self.declare(name, visibility, id.into(), None)
     }
 
     pub fn declare_trait(