Make module name mandatory and private (#6271)

## Description

Partial fix of #5498 .

So far the `name` field in `namespace::Module` has been public and of
type `Option<Ident>`. This makes little sense, since the name of a
module should always have a value, and should retain that value once it
is set. This PR fixes this problem.

The `visibility` and `span` fields have also been made private.
`visibility` can be read-only, but there is a single instance in which
the `span` field must be set later, so I have added a setter for that
field too.


## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: IGI-111 <igi-111@protonmail.com>

forc-pkg/src/pkg.rs

@@ -1600,18 +1600,19 @@ pub fn dependency_namespace(
 ) -> Result<namespace::Root, vec1::Vec1<CompileError>> {
     // TODO: Clean this up when config-time constants v1 are removed.
     let node_idx = &graph[node];
-    let name = Some(Ident::new_no_span(node_idx.name.clone()));
+    let name = Ident::new_no_span(node_idx.name.clone());
     let mut root_module = if let Some(contract_id_value) = contract_id_value {
-        namespace::default_with_contract_id(engines, name.clone(), contract_id_value, experimental)?
+        namespace::default_with_contract_id(
+            engines,
+            name.clone(),
+            Visibility::Public,
+            contract_id_value,
+            experimental,
+        )?
     } else {
-        namespace::Module::default()
+        namespace::Module::new(name, Visibility::Public, None)
     };
 
-    root_module.write(engines, |root_module| {
-        root_module.name.clone_from(&name);
-        root_module.visibility = Visibility::Public;
-    });
-
     // Add direct dependencies.
     let mut core_added = false;
     for edge in graph.edges_directed(node, Direction::Outgoing) {
@@ -1633,16 +1634,14 @@ pub fn dependency_namespace(
                 // Construct namespace with contract id
                 let contract_id_value = format!("0x{dep_contract_id}");
                 let node_idx = &graph[dep_node];
-                let name = Some(Ident::new_no_span(node_idx.name.clone()));
-                let mut module = namespace::default_with_contract_id(
+                let name = Ident::new_no_span(node_idx.name.clone());
+                namespace::default_with_contract_id(
                     engines,
                     name.clone(),
+                    Visibility::Private,
                     contract_id_value,
                     experimental,
-                )?;
-                module.name = name;
-                module.visibility = Visibility::Public;
-                module
+                )?
             }
         };
         dep_namespace.is_external = true;
@@ -2477,9 +2476,6 @@ pub fn build(
         }
 
         if let TreeType::Library = compiled.tree_type {
-            compiled.root_module.write(&engines, |root_module| {
-                root_module.name = Some(Ident::new_no_span(pkg.name.clone()));
-            });
             lib_namespace_map.insert(node, compiled.root_module);
         }
         source_map.insert_dependency(descriptor.manifest_file.dir());
@@ -2733,8 +2729,7 @@ pub fn check(
                     .namespace
                     .program_id(engines)
                     .read(engines, |m| m.clone());
-                module.name = Some(Ident::new_no_span(pkg.name.clone()));
-                module.span = Some(
+                module.set_span(
                     Span::new(
                         manifest.entry_string()?,
                         0,

sway-core/src/abi_generation/abi_str.rs

@@ -3,7 +3,7 @@ use sway_types::integer_bits::IntegerBits;
 use crate::{language::CallPath, Engines, TypeArgument, TypeId, TypeInfo};
 
 pub struct AbiStrContext {
-    pub program_name: Option<String>,
+    pub program_name: String,
     pub abi_with_callpaths: bool,
     pub abi_with_fully_specified_types: bool,
 }
@@ -182,9 +182,8 @@ fn call_path_display(ctx: &AbiStrContext, call_path: &CallPath) -> String {
         return call_path.suffix.as_str().to_string();
     }
     let mut buf = String::new();
-    let root_name = ctx.program_name.as_deref();
     for (index, prefix) in call_path.prefixes.iter().enumerate() {
-        if index == 0 && Some(prefix.as_str()) == root_name {
+        if index == 0 && prefix.as_str() == ctx.program_name {
             continue;
         }
         buf.push_str(prefix.as_str());

sway-core/src/abi_generation/fuel_abi.rs

@@ -26,7 +26,7 @@ impl<'a> AbiContext<'a> {
                 .root
                 .namespace
                 .program_id(engines)
-                .read(engines, |m| m.name.clone().map(|v| v.as_str().to_string())),
+                .read(engines, |m| m.name().to_string()),
             abi_with_callpaths: self.abi_with_callpaths,
             abi_with_fully_specified_types,
         }

sway-core/src/ir_generation/const_eval.rs

@@ -1316,12 +1316,11 @@ mod tests {
             },
         );
         let mut md_mgr = MetadataManager::default();
-        let mut core_lib = namespace::Root::from(namespace::Module {
-            name: Some(sway_types::Ident::new_no_span(
-                "assert_is_constant_test".to_string(),
-            )),
-            ..Default::default()
-        });
+        let mut core_lib = namespace::Root::from(namespace::Module::new(
+            sway_types::Ident::new_no_span("assert_is_constant_test".to_string()),
+            crate::Visibility::Private,
+            None,
+        ));
 
         let r = crate::compile_to_ast(
             &handler,

sway-core/src/language/call_path.rs

@@ -336,9 +336,7 @@ impl CallPath {
     /// before the identifier is added to the environment.
     pub fn ident_to_fullpath(suffix: Ident, namespace: &Namespace) -> CallPath {
         let mut res: Self = suffix.clone().into();
-        if let Some(ref pkg_name) = namespace.root_module().name {
-            res.prefixes.push(pkg_name.clone())
-        };
+        res.prefixes.push(namespace.root_module().name().clone());
         for mod_path in namespace.mod_path() {
             res.prefixes.push(mod_path.clone())
         }
@@ -404,9 +402,7 @@ impl CallPath {
             let mut prefixes: Vec<Ident> = vec![];
 
             if !is_external {
-                if let Some(pkg_name) = &namespace.root_module().name {
-                    prefixes.push(pkg_name.clone());
-                }
+                prefixes.push(namespace.root_module().name().clone());
 
                 if !is_absolute {
                     for mod_path in namespace.mod_path() {
@@ -439,9 +435,8 @@ impl CallPath {
                 }
             } else {
                 let mut prefixes: Vec<Ident> = vec![];
-                if let Some(pkg_name) = &namespace.root_module().read(engines, |m| m.name.clone()) {
-                    prefixes.push(pkg_name.clone());
-                }
+                prefixes.push(namespace.root_module().name().clone());
+
                 for mod_path in namespace.mod_path() {
                     prefixes.push(mod_path.clone());
                 }
@@ -473,7 +468,7 @@ impl CallPath {
         let converted = self.to_fullpath(engines, namespace);
 
         if let Some(first) = converted.prefixes.first() {
-            if namespace.root_module().read(engines, |m| m.name.clone()) == Some(first.clone()) {
+            if namespace.root_module().name() == first {
                 return converted.lshift();
             }
         }

sway-core/src/language/ty/expression/intrinsic_function.rs

@@ -118,7 +118,7 @@ impl CollectTypesMetadata for TyIntrinsicFunctionKind {
                 types_metadata.push(TypeMetadata::LoggedType(
                     LogId::new(logged_type.get_abi_type_str(
                         &AbiStrContext {
-                            program_name: Some(ctx.program_name.clone()),
+                            program_name: ctx.program_name.clone(),
                             abi_with_callpaths: true,
                             abi_with_fully_specified_types: true,
                         },

sway-core/src/semantic_analysis/ast_node/declaration/auto_impl.rs

@@ -401,8 +401,7 @@ where
         engines: &Engines,
         decl: &TyDecl,
     ) -> Option<(Option<TyAstNode>, Option<TyAstNode>)> {
-        if matches!(self.ctx.namespace.root().module.read(engines, |m| m.name.clone()).as_ref(), Some(x) if x.as_str() == "core")
-        {
+        if self.ctx.namespace.root().module.name().as_str() == "core" {
             return Some((None, None));
         }
 
@@ -437,8 +436,7 @@ where
         engines: &Engines,
         decl: &TyDecl,
     ) -> Option<(Option<TyAstNode>, Option<TyAstNode>)> {
-        if matches!(self.ctx.namespace.root().module.read(engines, |m| m.name.clone()).as_ref(), Some(x) if x.as_str() == "core")
-        {
+        if self.ctx.namespace.root().module.name().as_str() == "core" {
             return Some((None, None));
         }
 

sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs

@@ -2680,7 +2680,12 @@ mod tests {
         type_annotation: TypeId,
         experimental: ExperimentalFlags,
     ) -> Result<ty::TyExpression, ErrorEmitted> {
-        let mut root_module = namespace::Root::from(namespace::Module::default());
+        let root_module_name = sway_types::Ident::new_no_span("do_type_check_test".to_string());
+        let mut root_module = namespace::Root::from(namespace::Module::new(
+            root_module_name,
+            Visibility::Private,
+            None,
+        ));
         let mut namespace = Namespace::init_root(&mut root_module);
         let ctx = TypeCheckContext::from_namespace(&mut namespace, engines, experimental)
             .with_type_annotation(type_annotation);

sway-core/src/semantic_analysis/ast_node/expression/typed_expression/method_application.rs

@@ -829,7 +829,7 @@ pub(crate) fn resolve_method_name(
                 ctx.namespace().prepend_module_path(&call_path.prefixes)
             } else {
                 let mut module_path = call_path.prefixes.clone();
-                if let (Some(root_mod), Some(root_name)) = (
+                if let (Some(root_mod), root_name) = (
                     module_path.first().cloned(),
                     ctx.namespace().root_module_name().clone(),
                 ) {

sway-core/src/semantic_analysis/namespace/contract_helpers.rs

@@ -26,25 +26,34 @@ use super::{lexical_scope::SymbolMap, root::ResolvedDeclaration, Module, Root};
 /// `CONTRACT_ID`-containing modules: https://github.com/FuelLabs/sway/issues/3077
 pub fn default_with_contract_id(
     engines: &Engines,
-    name: Option<Ident>,
+    name: Ident,
+    visibility: Visibility,
     contract_id_value: String,
     experimental: crate::ExperimentalFlags,
 ) -> Result<Module, vec1::Vec1<CompileError>> {
     let handler = <_>::default();
-    default_with_contract_id_inner(&handler, engines, name, contract_id_value, experimental)
-        .map_err(|_| {
-            let (errors, warnings) = handler.consume();
-            assert!(warnings.is_empty());
+    default_with_contract_id_inner(
+        &handler,
+        engines,
+        name,
+        visibility,
+        contract_id_value,
+        experimental,
+    )
+    .map_err(|_| {
+        let (errors, warnings) = handler.consume();
+        assert!(warnings.is_empty());
 
-            // Invariant: `.value == None` => `!errors.is_empty()`.
-            vec1::Vec1::try_from_vec(errors).unwrap()
-        })
+        // Invariant: `.value == None` => `!errors.is_empty()`.
+        vec1::Vec1::try_from_vec(errors).unwrap()
+    })
 }
 
 fn default_with_contract_id_inner(
     handler: &Handler,
     engines: &Engines,
-    ns_name: Option<Ident>,
+    ns_name: Ident,
+    visibility: Visibility,
     contract_id_value: String,
     experimental: crate::ExperimentalFlags,
 ) -> Result<Module, ErrorEmitted> {
@@ -95,11 +104,9 @@ fn default_with_contract_id_inner(
         content: AstNodeContent::Declaration(Declaration::ConstantDeclaration(const_decl_id)),
         span: const_item_span.clone(),
     };
-    let mut root = Root::from(Module::default());
+    let mut root = Root::from(Module::new(ns_name.clone(), Visibility::Public, None));
     let mut ns = Namespace::init_root(&mut root);
     // This is pretty hacky but that's okay because of this code is being removed pretty soon
-    ns.root.module.name = ns_name;
-    ns.root.module.visibility = Visibility::Public;
     let type_check_ctx = TypeCheckContext::from_namespace(&mut ns, engines, experimental);
     let typed_node = TyAstNode::type_check(handler, type_check_ctx, &ast_node).unwrap();
     // get the decl out of the typed node:
@@ -118,7 +125,7 @@ fn default_with_contract_id_inner(
     };
     compiled_constants.insert(name, ResolvedDeclaration::Typed(typed_decl));
 
-    let mut ret = Module::default();
+    let mut ret = Module::new(ns_name, visibility, None);
     ret.current_lexical_scope_mut().items.symbols = compiled_constants;
     Ok(ret)
 }

sway-core/src/semantic_analysis/namespace/module.rs

@@ -36,11 +36,11 @@ pub struct Module {
     pub current_lexical_scope_id: LexicalScopeId,
     /// Name of the module, package name for root module, module name for other modules.
     /// Module name used is the same as declared in `mod name;`.
-    pub name: Option<Ident>,
+    name: Ident,
     /// Whether or not this is a `pub` module
-    pub visibility: Visibility,
+    visibility: Visibility,
     /// Empty span at the beginning of the file implementing the module
-    pub span: Option<Span>,
+    span: Option<Span>,
     /// Indicates whether the module is external to the current package. External modules are
     /// imported in the `Forc.toml` file.
     pub is_external: bool,
@@ -51,22 +51,57 @@ pub struct Module {
     pub(crate) mod_path: ModulePathBuf,
 }
 
-impl Default for Module {
-    fn default() -> Self {
+impl Module {
+    pub fn new(name: Ident, visibility: Visibility, span: Option<Span>) -> Self {
         Self {
-            visibility: Visibility::Private,
+            visibility,
             submodules: Default::default(),
             lexical_scopes: vec![LexicalScope::default()],
             current_lexical_scope_id: 0,
-            name: Default::default(),
-            span: Default::default(),
+            name,
+            span,
             is_external: Default::default(),
             mod_path: Default::default(),
         }
     }
-}
 
-impl Module {
+    // Specialized constructor for cloning Namespace::init. Should not be used for anything else
+    pub(super) fn new_submodule_from_init(
+        &self,
+        name: Ident,
+        visibility: Visibility,
+        span: Option<Span>,
+        is_external: bool,
+        mod_path: ModulePathBuf,
+    ) -> Self {
+        Self {
+            visibility,
+            submodules: self.submodules.clone(),
+            lexical_scopes: self.lexical_scopes.clone(),
+            current_lexical_scope_id: self.current_lexical_scope_id,
+            name,
+            span,
+            is_external,
+            mod_path,
+        }
+    }
+
+    pub fn name(&self) -> &Ident {
+        &self.name
+    }
+
+    pub fn visibility(&self) -> &Visibility {
+        &self.visibility
+    }
+
+    pub fn span(&self) -> &Option<Span> {
+        &self.span
+    }
+
+    pub fn set_span(&mut self, span: Span) {
+        self.span = Some(span);
+    }
+
     pub fn read<R>(&self, _engines: &crate::Engines, mut f: impl FnMut(&Module) -> R) -> R {
         f(self)
     }