Merge 2e29b92 into e8bbce7

Author: asterite

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/mod.rs

@@ -1131,13 +1131,106 @@ 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 {
+                    // Go to the struct's parent module
+                    let module_data = self.get_module(struct_module_id);
+                    let parent_local_id =
+                        module_data.parent.expect("Expected struct module parent to exist");
+                    let parent_module_id =
+                        ModuleId { krate: struct_module_id.krate, local_id: parent_local_id };
+                    let parent_module_data = self.get_module(parent_module_id);
+
+                    // Find the struct in the parent module so we can know its visibility
+                    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/def_collector/dc_mod.rs

@@ -306,6 +306,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 {
@@ -327,8 +328,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 {
@@ -526,7 +538,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/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(

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -130,6 +130,8 @@ pub enum ResolverError {
     MutatingComptimeInNonComptimeContext { name: String, span: Span },
     #[error("Failed to parse `{statement}` as an expression")]
     InvalidInternedStatementInExpr { statement: String, span: Span },
+    #[error("Type `{typ}` is more private than item `{item}`")]
+    TypeIsMorePrivateThenItem { typ: String, item: String, span: Span },
 }
 
 impl ResolverError {
@@ -529,6 +531,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                     *span,
                 )
             },
+            ResolverError::TypeIsMorePrivateThenItem { typ, item, span } => {
+                Diagnostic::simple_warning(
+                    format!("Type `{typ}` is more private than item `{item}`"),
+                    String::new(),
+                    *span,
+                )
+            },
         }
     }
 }

compiler/noirc_frontend/src/parser/parser.rs

@@ -290,14 +290,21 @@ fn contract(
 fn type_alias_definition() -> impl NoirParser<TopLevelStatementKind> {
     use self::Keyword::Type;
 
-    let p = ignore_then_commit(keyword(Type), ident());
-    let p = then_commit(p, function::generics());
-    let p = then_commit_ignore(p, just(Token::Assign));
-    let p = then_commit(p, parse_type());
-
-    p.map_with_span(|((name, generics), typ), span| {
-        TopLevelStatementKind::TypeAlias(NoirTypeAlias { name, generics, typ, span })
-    })
+    item_visibility()
+        .then_ignore(keyword(Type))
+        .then(ident())
+        .then(function::generics())
+        .then_ignore(just(Token::Assign))
+        .then(parse_type())
+        .map_with_span(|(((visibility, name), generics), typ), span| {
+            TopLevelStatementKind::TypeAlias(NoirTypeAlias {
+                name,
+                generics,
+                typ,
+                visibility,
+                span,
+            })
+        })
 }
 
 fn self_parameter() -> impl NoirParser<Param> {

compiler/noirc_frontend/src/tests.rs

@@ -2659,8 +2659,8 @@ fn incorrect_generic_count_on_struct_impl() {
 #[test]
 fn incorrect_generic_count_on_type_alias() {
     let src = r#"
-    struct Foo {}
-    type Bar = Foo<i32>;
+    pub struct Foo {}
+    pub type Bar = Foo<i32>;
     fn main() {}
     "#;
 

compiler/noirc_frontend/src/tests/unused_items.rs

@@ -171,3 +171,77 @@ fn silences_unused_variable_warning() {
     "#;
     assert_no_errors(src);
 }
+
+#[test]
+fn errors_on_unused_type_alias() {
+    let src = r#"
+    type Foo = Field;
+    type Bar = Field;
+    pub fn bar(_: Bar) {}
+    fn main() {}
+    "#;
+
+    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, "type alias");
+}
+
+#[test]
+fn errors_if_type_alias_aliases_more_private_type() {
+    let src = r#"
+    struct Foo {}
+    pub type Bar = Foo;
+    pub fn no_unused_warnings(_b: Bar) {
+        let _ = Foo {};
+    }
+    fn main() {}
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem {
+        typ, item, ..
+    }) = &errors[0].0
+    else {
+        panic!("Expected an unused item error");
+    };
+
+    assert_eq!(typ, "Foo");
+    assert_eq!(item, "Bar");
+}
+
+#[test]
+fn errors_if_type_alias_aliases_more_private_type_in_generic() {
+    let src = r#"
+    pub struct Generic<T> { value: T }
+    struct Foo {}
+    pub type Bar = Generic<Foo>;
+    pub fn no_unused_warnings(_b: Bar) {
+        let _ = Foo {};
+        let _ = Generic { value: 1 };
+    }
+    fn main() {}
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem {
+        typ, item, ..
+    }) = &errors[0].0
+    else {
+        panic!("Expected an unused item error");
+    };
+
+    assert_eq!(typ, "Foo");
+    assert_eq!(item, "Bar");
+}

compiler/noirc_frontend/src/usage_tracker.rs

@@ -4,7 +4,7 @@ use crate::{
     ast::{Ident, ItemVisibility},
     hir::def_map::ModuleId,
     macros_api::StructId,
-    node_interner::{FuncId, TraitId},
+    node_interner::{FuncId, TraitId, TypeAliasId},
 };
 
 #[derive(Debug)]
@@ -13,6 +13,7 @@ pub enum UnusedItem {
     Function(FuncId),
     Struct(StructId),
     Trait(TraitId),
+    TypeAlias(TypeAliasId),
 }
 
 impl UnusedItem {
@@ -22,6 +23,7 @@ impl UnusedItem {
             UnusedItem::Function(_) => "function",
             UnusedItem::Struct(_) => "struct",
             UnusedItem::Trait(_) => "trait",
+            UnusedItem::TypeAlias(_) => "type alias",
         }
     }
 }

docs/docs/noir/concepts/data_types/index.md

@@ -105,6 +105,14 @@ type Bad2 = Bad1;
 //   ^^^^^^^^^^^ 'Bad2' recursively depends on itself: Bad2 -> Bad1 -> Bad2
 ```
 
+By default, like functions, type aliases are private to the module the exist in. You can use `pub`
+to make the type alias public or `pub(crate)` to make it public to just its crate:
+
+```rust
+// This type alias is now public
+pub type Id = u8;
+```
+
 ## Wildcard Type
 Noir can usually infer the type of the variable from the context, so specifying the type of a variable is only required when it cannot be inferred. However, specifying a complex type can be tedious, especially when it has multiple generic arguments. Often some of the generic types can be inferred from the context, and Noir only needs a hint to properly infer the other types. We can partially specify a variable's type by using `_` as a marker, indicating where we still want the compiler to infer the type.