diff --git a/compiler/noirc_frontend/src/ast/type_alias.rs b/compiler/noirc_frontend/src/ast/type_alias.rs index 3228765170e..b279d86f19e 100644 --- a/compiler/noirc_frontend/src/ast/type_alias.rs +++ b/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 } } } diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index c36231bcf3b..9dd76885d61 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/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) { // This is necessary to avoid cloning the entire struct map // when adding checks after each struct field is resolved. diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index f50a0608fab..132b62f84b7 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/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, diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 645d8650c7e..5bbff85079e 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/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( diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 1e7f29527e2..cd3acbdc0f1 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/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, + ) + }, } } } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 0eb49ae975a..3f9a1d7a08b 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -290,14 +290,21 @@ fn contract( fn type_alias_definition() -> impl NoirParser { 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 { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index cb291902ae2..c8e94d2c3e0 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/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; + pub struct Foo {} + pub type Bar = Foo; fn main() {} "#; diff --git a/compiler/noirc_frontend/src/tests/unused_items.rs b/compiler/noirc_frontend/src/tests/unused_items.rs index da0a40d93cf..13787dd6955 100644 --- a/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/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 { value: T } + struct Foo {} + pub type Bar = Generic; + 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"); +} diff --git a/compiler/noirc_frontend/src/usage_tracker.rs b/compiler/noirc_frontend/src/usage_tracker.rs index b6f41dc72f2..23c97370b46 100644 --- a/compiler/noirc_frontend/src/usage_tracker.rs +++ b/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", } } } diff --git a/docs/docs/noir/concepts/data_types/index.md b/docs/docs/noir/concepts/data_types/index.md index 3eadb2dc8a4..11f51e2b65a 100644 --- a/docs/docs/noir/concepts/data_types/index.md +++ b/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. diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index 1d787ebcdc1..21d80e76bfc 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -36,7 +36,7 @@ use crate::hash::poseidon2::Poseidon2Hasher; // A derive function is one that given a struct definition can // create us a quoted trait impl from it. -type DeriveFunction = fn(StructDefinition) -> Quoted; +pub type DeriveFunction = fn(StructDefinition) -> Quoted; // We'll keep a global HANDLERS map to keep track of the derive handler for each trait comptime mut global HANDLERS: UHashMap> = UHashMap::default();