feat: allow visibility modifiers in struct definitions (#6054)

# Description

## Problem

Part of #4515
Part of #4775

## Summary

Allows marking structs as pub or pub(crate). Also will now report unused
structs.

## Additional Context

Let me know if some of the structs I marked as `pub` shouldn't actually
be `pub`. I didn't mark some as pub, I think only the ones used for
tests.

## Documentation

Check one:
- [ ] No documentation needed.
- [x] 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.

compiler/noirc_frontend/src/ast/structure.rs

@@ -6,13 +6,14 @@ use crate::token::SecondaryAttribute;
 use iter_extended::vecmap;
 use noirc_errors::Span;
 
-use super::Documented;
+use super::{Documented, ItemVisibility};
 
 /// Ast node for a struct
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct NoirStruct {
     pub name: Ident,
     pub attributes: Vec<SecondaryAttribute>,
+    pub visibility: ItemVisibility,
     pub generics: UnresolvedGenerics,
     pub fields: Vec<Documented<StructField>>,
     pub span: Span,

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

@@ -976,7 +976,17 @@ pub fn collect_struct(
     }
 
     // Add the struct to scope so its path can be looked up later
-    let result = def_map.modules[module_id.0].declare_struct(name.clone(), id);
+    let visibility = unresolved.struct_def.visibility;
+    let result = def_map.modules[module_id.0].declare_struct(name.clone(), visibility, id);
+
+    let parent_module_id = ModuleId { krate, local_id: module_id };
+
+    interner.usage_tracker.add_unused_item(
+        parent_module_id,
+        name.clone(),
+        UnusedItem::Struct(id),
+        visibility,
+    );
 
     if let Err((first_def, second_def)) = result {
         let error = DefCollectorErrorKind::Duplicate {
@@ -988,8 +998,7 @@ pub fn collect_struct(
     }
 
     if interner.is_in_lsp_mode() {
-        let parent_module_id = ModuleId { krate, local_id: module_id };
-        interner.register_struct(id, name.to_string(), parent_module_id);
+        interner.register_struct(id, name.to_string(), visibility, parent_module_id);
     }
 
     Some((id, unresolved))

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

@@ -100,8 +100,13 @@ impl ModuleData {
         self.declare(name, ItemVisibility::Public, id.into(), None)
     }
 
-    pub fn declare_struct(&mut self, name: Ident, id: StructId) -> Result<(), (Ident, Ident)> {
-        self.declare(name, ItemVisibility::Public, ModuleDefId::TypeId(id), None)
+    pub fn declare_struct(
+        &mut self,
+        name: Ident,
+        visibility: ItemVisibility,
+        id: StructId,
+    ) -> Result<(), (Ident, Ident)> {
+        self.declare(name, visibility, ModuleDefId::TypeId(id), None)
     }
 
     pub fn declare_type_alias(

compiler/noirc_frontend/src/locations.rs

@@ -298,11 +298,10 @@ impl NodeInterner {
         &mut self,
         id: StructId,
         name: String,
+        visibility: ItemVisibility,
         parent_module_id: ModuleId,
     ) {
         self.add_definition_location(ReferenceId::Struct(id), Some(parent_module_id));
-
-        let visibility = ItemVisibility::Public;
         self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None);
     }
 

compiler/noirc_frontend/src/parser/parser/structs.rs

@@ -1,6 +1,7 @@
 use chumsky::prelude::*;
 
 use crate::ast::{Documented, NoirStruct, StructField};
+use crate::parser::parser::visibility::item_visibility;
 use crate::{
     parser::{
         parser::{
@@ -30,13 +31,21 @@ pub(super) fn struct_definition() -> impl NoirParser<TopLevelStatementKind> {
         .or(just(Semicolon).to(Vec::new()));
 
     attributes()
+        .then(item_visibility())
         .then_ignore(keyword(Struct))
         .then(ident())
         .then(function::generics())
         .then(fields)
-        .validate(|(((attributes, name), generics), fields), span, emit| {
+        .validate(|((((attributes, visibility), name), generics), fields), span, emit| {
             let attributes = validate_secondary_attributes(attributes, span, emit);
-            TopLevelStatementKind::Struct(NoirStruct { name, attributes, generics, fields, span })
+            TopLevelStatementKind::Struct(NoirStruct {
+                name,
+                attributes,
+                visibility,
+                generics,
+                fields,
+                span,
+            })
         })
 }
 

compiler/noirc_frontend/src/tests.rs

@@ -1565,11 +1565,11 @@ fn struct_numeric_generic_in_function() {
 #[test]
 fn struct_numeric_generic_in_struct() {
     let src = r#"
-    struct Foo {
+    pub struct Foo {
         inner: u64
     }
 
-    struct Bar<let N: Foo> { }
+    pub struct Bar<let N: Foo> { }
     "#;
     let errors = get_program_errors(src);
     assert_eq!(errors.len(), 1);
@@ -1658,7 +1658,7 @@ fn numeric_generic_in_function_signature() {
 #[test]
 fn numeric_generic_as_struct_field_type() {
     let src = r#"
-    struct Foo<let N: u32> {
+    pub struct Foo<let N: u32> {
         a: Field,
         b: N,
     }
@@ -1674,7 +1674,7 @@ fn numeric_generic_as_struct_field_type() {
 #[test]
 fn normal_generic_as_array_length() {
     let src = r#"
-    struct Foo<N> {
+    pub struct Foo<N> {
         a: Field,
         b: [Field; N],
     }
@@ -1719,11 +1719,11 @@ fn numeric_generic_as_param_type() {
 #[test]
 fn numeric_generic_used_in_nested_type_fail() {
     let src = r#"
-    struct Foo<let N: u32> {
+    pub struct Foo<let N: u32> {
         a: Field,
         b: Bar<N>,
     }
-    struct Bar<N> {
+    pub struct Bar<N> {
         inner: N
     }
     "#;
@@ -1738,11 +1738,11 @@ fn numeric_generic_used_in_nested_type_fail() {
 #[test]
 fn normal_generic_used_in_nested_array_length_fail() {
     let src = r#"
-    struct Foo<N> {
+    pub struct Foo<N> {
         a: Field,
         b: Bar<N>,
     }
-    struct Bar<let N: u32> {
+    pub struct Bar<let N: u32> {
         inner: [Field; N]
     }
     "#;
@@ -1756,11 +1756,11 @@ fn numeric_generic_used_in_nested_type_pass() {
     // The order of these structs should not be changed to make sure
     // that we are accurately resolving all struct generics before struct fields
     let src = r#"
-    struct NestedNumeric<let N: u32> {
+    pub struct NestedNumeric<let N: u32> {
         a: Field,
         b: InnerNumeric<N>
     }
-    struct InnerNumeric<let N: u32> {
+    pub struct InnerNumeric<let N: u32> {
         inner: [u64; N],
     }    
     "#;
@@ -2721,7 +2721,7 @@ fn bit_not_on_untyped_integer() {
 #[test]
 fn duplicate_struct_field() {
     let src = r#"
-    struct Foo {
+    pub struct Foo {
         x: i32,
         x: i32,
     }
@@ -2742,8 +2742,8 @@ fn duplicate_struct_field() {
     assert_eq!(first_def.to_string(), "x");
     assert_eq!(second_def.to_string(), "x");
 
-    assert_eq!(first_def.span().start(), 26);
-    assert_eq!(second_def.span().start(), 42);
+    assert_eq!(first_def.span().start(), 30);
+    assert_eq!(second_def.span().start(), 46);
 }
 
 #[test]
@@ -3423,6 +3423,30 @@ fn errors_on_unused_function() {
     assert_eq!(*item_type, "function");
 }
 
+#[test]
+fn errors_on_unused_struct() {
+    let src = r#"
+    struct Foo {}
+    struct Bar {}
+
+    fn main() {
+        let _ = Bar {};
+    }
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
+        &errors[0].0
+    else {
+        panic!("Expected an unused item error");
+    };
+
+    assert_eq!(ident.to_string(), "Foo");
+    assert_eq!(*item_type, "struct");
+}
+
 #[test]
 fn constrained_reference_to_unconstrained() {
     let src = r#"

compiler/noirc_frontend/src/usage_tracker.rs

@@ -3,20 +3,23 @@ use std::collections::HashMap;
 use crate::{
     ast::{Ident, ItemVisibility},
     hir::def_map::ModuleId,
+    macros_api::StructId,
     node_interner::FuncId,
 };
 
 #[derive(Debug)]
 pub enum UnusedItem {
     Import,
     Function(FuncId),
+    Struct(StructId),
 }
 
 impl UnusedItem {
     pub fn item_type(&self) -> &'static str {
         match self {
             UnusedItem::Import => "import",
             UnusedItem::Function(_) => "function",
+            UnusedItem::Struct(_) => "struct",
         }
     }
 }

docs/docs/noir/concepts/data_types/structs.md

@@ -68,3 +68,15 @@ fn get_octopus() -> Animal {
 
 The new variables can be bound with names different from the original struct field names, as
 showcased in the `legs --> feet` binding in the example above.
+
+By default, like functions, structs are private to the module the exist in. You can use `pub`
+to make the struct public or `pub(crate)` to make it public to just its crate:
+
+```rust
+// This struct is now public
+pub struct Animal {
+    hands: Field,
+    legs: Field,
+    eyes: u8,
+}
+```

noir_stdlib/src/bigint.nr

@@ -12,7 +12,7 @@ global secpr1_fq = &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
                      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF];
 global secpr1_fr = &[81, 37, 99, 252, 194, 202, 185, 243, 132, 158, 23, 167, 173, 250, 230, 188, 255, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 255, 255, 255, 255];
 // docs:start:big_int_definition
-struct BigInt {
+pub struct BigInt {
     pointer: u32,
     modulus: u32,
 }
@@ -49,7 +49,7 @@ trait BigField {
     fn to_le_bytes(self) -> [u8];
 }
 
-struct Secpk1Fq {
+pub struct Secpk1Fq {
    array: [u8;32],
 }
 
@@ -106,7 +106,7 @@ impl Eq for Secpk1Fq {
     }
 }
 
-struct Secpk1Fr {
+pub struct Secpk1Fr {
     array: [u8;32],
 }
 
@@ -163,7 +163,7 @@ impl Eq for Secpk1Fr {
     }
 }
 
-struct Bn254Fr {
+pub struct Bn254Fr {
     array: [u8;32],
 }
 
@@ -220,7 +220,7 @@ impl Eq for Bn254Fr {
     }
 }
 
-struct Bn254Fq {
+pub struct Bn254Fq {
     array: [u8;32],
 }
 
@@ -277,7 +277,7 @@ impl Eq for Bn254Fq {
     }
 }
 
-struct Secpr1Fq {
+pub struct Secpr1Fq {
     array: [u8;32],
 }
 
@@ -334,7 +334,7 @@ impl Eq for Secpr1Fq {
     }
 }
 
-struct Secpr1Fr {
+pub struct Secpr1Fr {
     array: [u8;32],
 }
 

noir_stdlib/src/cmp.nr

@@ -150,7 +150,7 @@ impl Eq for Ordering {
 
 // Noir doesn't have enums yet so we emulate (Lt | Eq | Gt) with a struct
 // that has 3 public functions for constructing the struct.
-struct Ordering {
+pub struct Ordering {
     result: Field,
 }
 

noir_stdlib/src/collections/bounded_vec.nr

@@ -22,7 +22,7 @@ use crate::{cmp::Eq, convert::From};
 /// assert(vector.len() == 5);
 /// assert(vector.max_len() == 10);
 /// ```
-struct BoundedVec<T, let MaxLen: u32> {
+pub struct BoundedVec<T, let MaxLen: u32> {
     storage: [T; MaxLen],
     len: u32,
 }

noir_stdlib/src/collections/map.nr

@@ -30,7 +30,7 @@ global MAX_LOAD_FACTOR_DEN0MINATOR = 4;
 /// 
 /// let two = map.get(1).unwrap();
 /// ```
-struct HashMap<K, V, let N: u32, B> {
+pub struct HashMap<K, V, let N: u32, B> {
     _table: [Slot<K, V>; N],
 
     /// Amount of valid elements in the map.

noir_stdlib/src/collections/umap.nr

@@ -11,7 +11,7 @@ use crate::hash::{Hash, Hasher, BuildHasher};
 //
 // Compared to the constrained HashMap type, UHashMap can grow automatically
 // as needed and is more efficient since it can break out of loops early.
-struct UHashMap<K, V, B> {
+pub struct UHashMap<K, V, B> {
     _table: [Slot<K, V>],
 
     // Amount of valid elements in the map.

noir_stdlib/src/collections/vec.nr

@@ -1,4 +1,4 @@
-struct Vec<T> { 
+pub struct Vec<T> { 
     slice: [T]
 }
 // A mutable vector type implemented as a wrapper around immutable slices.

noir_stdlib/src/ec/consts/te.nr

@@ -1,7 +1,7 @@
 use crate::ec::tecurve::affine::Point as TEPoint;
 use crate::ec::tecurve::affine::Curve as TECurve;
 
-struct BabyJubjub {
+pub struct BabyJubjub {
     curve: TECurve,
     base8: TEPoint,
     suborder: Field,