chore: generalise FunctionVisibility to ItemVisibility (#4495)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This pulls out a renaming from #4491.

We have the concept of `FunctionVisibility` which determines whether a
function can be called from other modules in the same crate/other
crates.

If we're expanding the same concept of visibility to items other than
functions we need a generic name for this which doesn't mention
functions. I've then renamed it to `ModuleVisibility` to reflect that it
shows how this item is visible in other modules but I'm open to
suggestions.


## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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.

aztec_macros/src/lib.rs

@@ -10,18 +10,19 @@ use noirc_frontend::macros_api::parse_program;
 use noirc_frontend::macros_api::FieldElement;
 use noirc_frontend::macros_api::{
     BlockExpression, CallExpression, CastExpression, Distinctness, Expression, ExpressionKind,
-    ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility,
-    HirContext, HirExpression, HirLiteral, HirStatement, Ident, IndexExpression, LetStatement,
-    Literal, MemberAccessExpression, MethodCallExpression, NoirFunction, NoirStruct, Param, Path,
-    PathKind, Pattern, PrefixExpression, SecondaryAttribute, Signedness, Span, Statement,
-    StatementKind, StructType, Type, TypeImpl, UnaryOp, UnresolvedType, UnresolvedTypeData,
-    Visibility,
+    ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, HirContext, HirExpression,
+    HirLiteral, HirStatement, Ident, IndexExpression, LetStatement, Literal,
+    MemberAccessExpression, MethodCallExpression, NoirFunction, NoirStruct, Param, Path, PathKind,
+    Pattern, PrefixExpression, SecondaryAttribute, Signedness, Span, Statement, StatementKind,
+    StructType, Type, TypeImpl, UnaryOp, UnresolvedType, UnresolvedTypeData, Visibility,
 };
 use noirc_frontend::macros_api::{CrateId, FileId};
 use noirc_frontend::macros_api::{MacroError, MacroProcessor};
 use noirc_frontend::macros_api::{ModuleDefId, NodeInterner, SortedModule, StructId};
 use noirc_frontend::node_interner::{FuncId, TraitId, TraitImplId, TraitImplKind};
-use noirc_frontend::{BinaryOpKind, ConstrainKind, ConstrainStatement, InfixExpression, Lambda};
+use noirc_frontend::{
+    BinaryOpKind, ConstrainKind, ConstrainStatement, InfixExpression, ItemVisibility, Lambda,
+};
 pub struct AztecMacro;
 
 impl MacroProcessor for AztecMacro {
@@ -1100,7 +1101,7 @@ fn generate_selector_impl(structure: &NoirStruct) -> TypeImpl {
         &return_type,
     );
 
-    selector_fn_def.visibility = FunctionVisibility::Public;
+    selector_fn_def.visibility = ItemVisibility::Public;
 
     // Seems to be necessary on contract modules
     selector_fn_def.return_visibility = Visibility::Public;

compiler/noirc_frontend/src/ast/expression.rs

@@ -3,7 +3,7 @@ use std::fmt::Display;
 
 use crate::token::{Attributes, Token};
 use crate::{
-    Distinctness, FunctionVisibility, Ident, Path, Pattern, Recoverable, Statement, StatementKind,
+    Distinctness, Ident, ItemVisibility, Path, Pattern, Recoverable, Statement, StatementKind,
     UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
 };
 use acvm::FieldElement;
@@ -378,7 +378,7 @@ pub struct FunctionDefinition {
     pub is_unconstrained: bool,
 
     /// Indicate if this function was defined with the 'pub' keyword
-    pub visibility: FunctionVisibility,
+    pub visibility: ItemVisibility,
 
     pub generics: UnresolvedGenerics,
     pub parameters: Vec<Param>,
@@ -677,7 +677,7 @@ impl FunctionDefinition {
             is_open: false,
             is_internal: false,
             is_unconstrained: false,
-            visibility: FunctionVisibility::Private,
+            visibility: ItemVisibility::Private,
             generics: generics.clone(),
             parameters: p,
             body: body.clone(),

compiler/noirc_frontend/src/ast/mod.rs

@@ -354,8 +354,8 @@ impl UnresolvedTypeExpression {
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-/// Represents whether the function can be called outside its module/crate
-pub enum FunctionVisibility {
+/// Represents whether the definition can be referenced outside its module/crate
+pub enum ItemVisibility {
     Public,
     Private,
     PublicCrate,

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

@@ -394,7 +394,7 @@ impl<'a> ModCollector<'a> {
 
                         let modifiers = FunctionModifiers {
                             name: name.to_string(),
-                            visibility: crate::FunctionVisibility::Public,
+                            visibility: crate::ItemVisibility::Public,
                             // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
                             attributes: crate::token::Attributes::empty(),
                             is_unconstrained: false,

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

@@ -1,16 +1,11 @@
 use super::{namespace::PerNs, ModuleDefId, ModuleId};
 use crate::{
     node_interner::{FuncId, TraitId},
-    Ident,
+    Ident, ItemVisibility,
 };
 use std::collections::{hash_map::Entry, HashMap};
 
-type Scope = HashMap<Option<TraitId>, (ModuleDefId, Visibility, bool /*is_prelude*/)>;
-
-#[derive(Debug, PartialEq, Eq, Copy, Clone)]
-pub enum Visibility {
-    Public,
-}
+type Scope = HashMap<Option<TraitId>, (ModuleDefId, ItemVisibility, bool /*is_prelude*/)>;
 
 #[derive(Default, Debug, PartialEq, Eq)]
 pub struct ItemScope {
@@ -55,12 +50,12 @@ impl ItemScope {
                         Err((old_ident.clone(), name))
                     }
                 } else {
-                    trait_hashmap.insert(trait_id, (mod_def, Visibility::Public, is_prelude));
+                    trait_hashmap.insert(trait_id, (mod_def, ItemVisibility::Public, is_prelude));
                     Ok(())
                 }
             } else {
                 let mut trait_hashmap = HashMap::new();
-                trait_hashmap.insert(trait_id, (mod_def, Visibility::Public, is_prelude));
+                trait_hashmap.insert(trait_id, (mod_def, ItemVisibility::Public, is_prelude));
                 map.insert(name, trait_hashmap);
                 Ok(())
             }

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

@@ -1,15 +1,16 @@
-use super::{item_scope::Visibility, ModuleDefId};
+use super::ModuleDefId;
+use crate::ItemVisibility;
 
 // This works exactly the same as in r-a, just simplified
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
 pub struct PerNs {
-    pub types: Option<(ModuleDefId, Visibility, bool)>,
-    pub values: Option<(ModuleDefId, Visibility, bool)>,
+    pub types: Option<(ModuleDefId, ItemVisibility, bool)>,
+    pub values: Option<(ModuleDefId, ItemVisibility, bool)>,
 }
 
 impl PerNs {
     pub fn types(t: ModuleDefId) -> PerNs {
-        PerNs { types: Some((t, Visibility::Public, false)), values: None }
+        PerNs { types: Some((t, ItemVisibility::Public, false)), values: None }
     }
 
     pub fn take_types(self) -> Option<ModuleDefId> {
@@ -24,7 +25,7 @@ impl PerNs {
         self.types.map(|it| it.0).into_iter().chain(self.values.map(|it| it.0))
     }
 
-    pub fn iter_items(self) -> impl Iterator<Item = (ModuleDefId, Visibility, bool)> {
+    pub fn iter_items(self) -> impl Iterator<Item = (ModuleDefId, ItemVisibility, bool)> {
         self.types.into_iter().chain(self.values)
     }
 

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

@@ -38,8 +38,8 @@ use crate::{
 };
 use crate::{
     ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition,
-    FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param,
-    Path, PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind,
+    FunctionReturnType, Generics, ItemVisibility, LValue, NoirStruct, NoirTypeAlias, Param, Path,
+    PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind,
     UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
     UnresolvedTypeExpression, Visibility, ERROR_IDENT,
 };
@@ -237,7 +237,7 @@ impl<'a> Resolver<'a> {
             is_open: false,
             is_internal: false,
             is_unconstrained: false,
-            visibility: FunctionVisibility::Public, // Trait functions are always public
+            visibility: ItemVisibility::Public, // Trait functions are always public
             generics: generics.clone(),
             parameters: vecmap(parameters, |(name, typ)| Param {
                 visibility: Visibility::Private,
@@ -1320,7 +1320,7 @@ impl<'a> Resolver<'a> {
         &mut self,
         func: FuncId,
         span: Span,
-        visibility: FunctionVisibility,
+        visibility: ItemVisibility,
     ) {
         let function_module = self.interner.function_module(func);
         let current_module = self.path_resolver.module_id();
@@ -1330,8 +1330,8 @@ impl<'a> Resolver<'a> {
         let current_module = current_module.local_id;
         let name = self.interner.function_name(&func).to_string();
         match visibility {
-            FunctionVisibility::Public => (),
-            FunctionVisibility::Private => {
+            ItemVisibility::Public => (),
+            ItemVisibility::Private => {
                 if !same_crate
                     || !self.module_descendent_of_target(
                         krate,
@@ -1342,7 +1342,7 @@ impl<'a> Resolver<'a> {
                     self.errors.push(ResolverError::PrivateFunctionCalled { span, name });
                 }
             }
-            FunctionVisibility::PublicCrate => {
+            ItemVisibility::PublicCrate => {
                 if !same_crate {
                     self.errors.push(ResolverError::NonCrateFunctionCalled { span, name });
                 }
@@ -1451,9 +1451,7 @@ impl<'a> Resolver<'a> {
                                     self.interner.add_function_dependency(current_item, id);
                                 }
 
-                                if self.interner.function_visibility(id)
-                                    != FunctionVisibility::Public
-                                {
+                                if self.interner.function_visibility(id) != ItemVisibility::Public {
                                     let span = hir_ident.location.span;
                                     self.check_can_reference_function(
                                         id,

compiler/noirc_frontend/src/lib.rs

@@ -56,14 +56,13 @@ pub mod macros_api {
     pub use crate::hir::def_map::ModuleDefId;
     pub use crate::{
         hir::Context as HirContext, BlockExpression, CallExpression, CastExpression, Distinctness,
-        Expression, ExpressionKind, FunctionReturnType, Ident, IndexExpression, LetStatement,
-        Literal, MemberAccessExpression, MethodCallExpression, NoirFunction, Path, PathKind,
-        Pattern, Statement, UnresolvedType, UnresolvedTypeData, Visibility,
+        Expression, ExpressionKind, FunctionReturnType, Ident, IndexExpression, ItemVisibility,
+        LetStatement, Literal, MemberAccessExpression, MethodCallExpression, NoirFunction, Path,
+        PathKind, Pattern, Statement, UnresolvedType, UnresolvedTypeData, Visibility,
     };
     pub use crate::{
-        ForLoopStatement, ForRange, FunctionDefinition, FunctionVisibility, ImportStatement,
-        NoirStruct, Param, PrefixExpression, Signedness, StatementKind, StructType, Type, TypeImpl,
-        UnaryOp,
+        ForLoopStatement, ForRange, FunctionDefinition, ImportStatement, NoirStruct, Param,
+        PrefixExpression, Signedness, StatementKind, StructType, Type, TypeImpl, UnaryOp,
     };
 
     /// Methods to process the AST before and after type checking

compiler/noirc_frontend/src/node_interner.rs

@@ -28,7 +28,7 @@ use crate::hir_def::{
 };
 use crate::token::{Attributes, SecondaryAttribute};
 use crate::{
-    BinaryOpKind, ContractFunctionType, FunctionDefinition, FunctionVisibility, Generics, Shared,
+    BinaryOpKind, ContractFunctionType, FunctionDefinition, Generics, ItemVisibility, Shared,
     TypeAlias, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind,
 };
 
@@ -236,7 +236,7 @@ pub struct FunctionModifiers {
     pub name: String,
 
     /// Whether the function is `pub` or not.
-    pub visibility: FunctionVisibility,
+    pub visibility: ItemVisibility,
 
     pub attributes: Attributes,
 
@@ -259,7 +259,7 @@ impl FunctionModifiers {
     pub fn new() -> Self {
         Self {
             name: String::new(),
-            visibility: FunctionVisibility::Public,
+            visibility: ItemVisibility::Public,
             attributes: Attributes::empty(),
             is_unconstrained: false,
             is_internal: None,
@@ -799,7 +799,7 @@ impl NodeInterner {
     ///
     /// The underlying function_visibilities map is populated during def collection,
     /// so this function can be called anytime afterward.
-    pub fn function_visibility(&self, func: FuncId) -> FunctionVisibility {
+    pub fn function_visibility(&self, func: FuncId) -> ItemVisibility {
         self.function_modifiers[&func].visibility
     }
 

compiler/noirc_frontend/src/parser/parser/function.rs

@@ -8,7 +8,7 @@ use crate::parser::labels::ParsingRuleLabel;
 use crate::parser::spanned;
 use crate::token::{Keyword, Token};
 use crate::{
-    Distinctness, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident, NoirFunction,
+    Distinctness, FunctionDefinition, FunctionReturnType, Ident, ItemVisibility, NoirFunction,
     Param, Visibility,
 };
 
@@ -53,24 +53,24 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunct
 }
 
 /// visibility_modifier: 'pub(crate)'? 'pub'? ''
-fn visibility_modifier() -> impl NoirParser<FunctionVisibility> {
+fn visibility_modifier() -> impl NoirParser<ItemVisibility> {
     let is_pub_crate = (keyword(Keyword::Pub)
         .then_ignore(just(Token::LeftParen))
         .then_ignore(keyword(Keyword::Crate))
         .then_ignore(just(Token::RightParen)))
-    .map(|_| FunctionVisibility::PublicCrate);
+    .map(|_| ItemVisibility::PublicCrate);
 
-    let is_pub = keyword(Keyword::Pub).map(|_| FunctionVisibility::Public);
+    let is_pub = keyword(Keyword::Pub).map(|_| ItemVisibility::Public);
 
-    let is_private = empty().map(|_| FunctionVisibility::Private);
+    let is_private = empty().map(|_| ItemVisibility::Private);
 
     choice((is_pub_crate, is_pub, is_private))
 }
 
 /// function_modifiers: 'unconstrained'? (visibility)? 'open'?
 ///
 /// returns (is_unconstrained, visibility, is_open) for whether each keyword was present
-fn function_modifiers() -> impl NoirParser<(bool, FunctionVisibility, bool)> {
+fn function_modifiers() -> impl NoirParser<(bool, ItemVisibility, bool)> {
     keyword(Keyword::Unconstrained)
         .or_not()
         .then(visibility_modifier())

compiler/noirc_frontend/src/parser/parser/traits.rs

@@ -11,7 +11,7 @@ use crate::{
         ParserErrorReason, TopLevelStatement,
     },
     token::{Keyword, Token},
-    Expression, FunctionVisibility, NoirTrait, NoirTraitImpl, TraitBound, TraitImplItem, TraitItem,
+    Expression, ItemVisibility, NoirTrait, NoirTraitImpl, TraitBound, TraitImplItem, TraitItem,
     UnresolvedTraitConstraint, UnresolvedType,
 };
 
@@ -123,12 +123,12 @@ fn trait_implementation_body() -> impl NoirParser<Vec<TraitImplItem>> {
         if f.def().is_internal
             || f.def().is_unconstrained
             || f.def().is_open
-            || f.def().visibility != FunctionVisibility::Private
+            || f.def().visibility != ItemVisibility::Private
         {
             emit(ParserError::with_reason(ParserErrorReason::TraitImplFunctionModifiers, span));
         }
         // Trait impl functions are always public
-        f.def_mut().visibility = FunctionVisibility::Public;
+        f.def_mut().visibility = ItemVisibility::Public;
         TraitImplItem::Function(f)
     });