Feature: Reexport using pub use (#6116)

## Description

Fixes #3113 . Closes #2767 as Won't Fix, since one must use `pub use` to
reexport items.

This PR Introduce reexports through the use of `pub use`. The
implementation now keeps track of the visibility of a `use` statement,
and that visibility is in turn taken into account when items are
imported from the module where the `use` statement resides.

This feature allows `std::prelude` and `core::prelude` to be handled
correctly, instead of the special-case handling that we have been using
so far. The method `star_import_with_reexports` has therefore been
merged with `star_import`, and the `prelude.sw` files have accordingly
been changed to use `pub use` instead of `use`.

Note that `sway-lib-std/lib.sw` has a spurious import of `core::*`,
which has no effect, so I have removed that import.

This change also allows us to remove a hacky solution in
`lexical_scope.rs`. The hack was introduced because the special-case
handling of preludes meant that the preludes would contain multiple
copies of the same `std` and `core` items. Now that we no longer need to
treat the preludes specially, we can remove the hack.

I have gone a little overboard in adding tests, partly because I wanted
to make sure I hadn't missed some corner cases, but also because our
tests of the import logic has poor code coverage. I have found the
following issues that I don't think belongs in this PR, but which need
to be handled at some point:

- Path resolution is broken. The tests
`should_pass/language/reexport/simple_path_access` and
`should_fail/language/reexport/simple_path_access` are supposed to test
that you can access reexported using a path into the reexporting module,
i.e., without importing them using a `use` statement. Both tests are
disabled because path resolution is broken. I plan to fix that as part
of #5498 . A simlar problem exists in the test
`should_pass/language/reexport/reexport_paths` where an item is imported
via `std::prelude`.
- There is an issue with the import and reexport of enum variants, which
is illustrated in
`should_pass/language/reexport/reexport_paths_external_lib`. In short,
`pub use ext_3_lib::Items3_Variants::U` elevates the name `U` to the
top-level namespace in a module, and this makes `U` inaccessible as a
variant of `Items3_Variants`. I'm not sure how this is supposed to work,
but since it's related to all imports whether they are reexported or not
I have left it as a separate issue #6233 .
- Aliasing of enum variants (`use lib::Enum::X as MyX`) doesn't work
properly. In some cases `MyX` is available in the way you would expect,
and in other cases it is not, and sometimes it is available but not
recognized as being a variant of `lib::Enum`. The test
`should_pass/language/reexport/aliases` has a number of tests of these
types of things. #6123 tracks the issue.
- Aliased traits are not handled correctly by the dead code analysis.
The test `should_pass/language/reexport/aliases` generates warnings
about unimplemented traits that are actually implemented, but since they
are aliased in imports the DCA doesn't catch them. #6234 tracks the
issue.

Re. documentation: The whole import system is completely undocumented in
the Sway book, and I plan to write up a draft how it works when I'm done
fixing (the majority of) the many issue we have there. At the very least
I'd like to postpone the documentation efforts until path resolution
works correctly, since that is key to how the import system works.


## 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: Joshua Batty <joshpbatty@gmail.com>
Co-authored-by: João Matos <joao@tritao.eu>

forc-pkg/src/pkg.rs

@@ -1665,20 +1665,22 @@ pub fn dependency_namespace(
     let mut root = namespace::Root::from(root_module);
 
     if core_added {
-        let _ = root.star_import_with_reexports(
+        let _ = root.star_import(
             &Handler::default(),
             engines,
             &[CORE, PRELUDE].map(|s| Ident::new_no_span(s.into())),
             &[],
+            Visibility::Private,
         );
     }
 
     if has_std_dep(graph, node) {
-        let _ = root.star_import_with_reexports(
+        let _ = root.star_import(
             &Handler::default(),
             engines,
             &[STD, PRELUDE].map(|s| Ident::new_no_span(s.into())),
             &[],
+            Visibility::Private,
         );
     }
 

sway-core/src/language/call_path.rs

@@ -369,7 +369,7 @@ impl CallPath {
             if let Some(mod_path) = namespace.program_id(engines).read(engines, |m| {
                 if m.current_items().symbols().contains_key(&self.suffix) {
                     None
-                } else if let Some((_, path, _)) = m
+                } else if let Some((_, path, _, _)) = m
                     .current_items()
                     .use_item_synonyms
                     .get(&self.suffix)

sway-core/src/language/parsed/use_statement.rs

@@ -1,4 +1,4 @@
-use crate::parsed::Span;
+use crate::{language::Visibility, parsed::Span};
 use sway_types::ident::Ident;
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -17,5 +17,8 @@ pub struct UseStatement {
     // If `is_absolute` is true, then this use statement is an absolute path from
     // the project root namespace. If not, then it is relative to the current namespace.
     pub is_absolute: bool,
+    // If `reexport` is Visibility::Public, then this use statement reexports its imported binding.
+    // If not, then the import binding is private to the importing module.
+    pub reexport: Visibility,
     pub alias: Option<Ident>,
 }

sway-core/src/semantic_analysis/ast_node/mod.rs

@@ -167,16 +167,21 @@ fn collect_use_statement(
         ImportType::Star => {
             // try a standard starimport first
             let star_import_handler = Handler::default();
-            let import = ctx.star_import(&star_import_handler, engines, &path);
+            let import = ctx.star_import(&star_import_handler, engines, &path, stmt.reexport);
             if import.is_ok() {
                 handler.append(star_import_handler);
                 import
             } else {
                 // if it doesn't work it could be an enum star import
                 if let Some((enum_name, path)) = path.split_last() {
                     let variant_import_handler = Handler::default();
-                    let variant_import =
-                        ctx.variant_star_import(&variant_import_handler, engines, path, enum_name);
+                    let variant_import = ctx.variant_star_import(
+                        &variant_import_handler,
+                        engines,
+                        path,
+                        enum_name,
+                        stmt.reexport,
+                    );
                     if variant_import.is_ok() {
                         handler.append(variant_import_handler);
                         variant_import
@@ -190,12 +195,20 @@ fn collect_use_statement(
                 }
             }
         }
-        ImportType::SelfImport(_) => ctx.self_import(handler, engines, &path, stmt.alias.clone()),
+        ImportType::SelfImport(_) => {
+            ctx.self_import(handler, engines, &path, stmt.alias.clone(), stmt.reexport)
+        }
         ImportType::Item(ref s) => {
             // try a standard item import first
             let item_import_handler = Handler::default();
-            let import =
-                ctx.item_import(&item_import_handler, engines, &path, s, stmt.alias.clone());
+            let import = ctx.item_import(
+                &item_import_handler,
+                engines,
+                &path,
+                s,
+                stmt.alias.clone(),
+                stmt.reexport,
+            );
 
             if import.is_ok() {
                 handler.append(item_import_handler);
@@ -211,6 +224,7 @@ fn collect_use_statement(
                         enum_name,
                         s,
                         stmt.alias.clone(),
+                        stmt.reexport,
                     );
                     if variant_import.is_ok() {
                         handler.append(variant_import_handler);
@@ -252,16 +266,20 @@ fn handle_use_statement(
         ImportType::Star => {
             // try a standard starimport first
             let star_import_handler = Handler::default();
-            let import = ctx.star_import(&star_import_handler, &path);
+            let import = ctx.star_import(&star_import_handler, &path, stmt.reexport);
             if import.is_ok() {
                 handler.append(star_import_handler);
                 import
             } else {
                 // if it doesn't work it could be an enum star import
                 if let Some((enum_name, path)) = path.split_last() {
                     let variant_import_handler = Handler::default();
-                    let variant_import =
-                        ctx.variant_star_import(&variant_import_handler, path, enum_name);
+                    let variant_import = ctx.variant_star_import(
+                        &variant_import_handler,
+                        path,
+                        enum_name,
+                        stmt.reexport,
+                    );
                     if variant_import.is_ok() {
                         handler.append(variant_import_handler);
                         variant_import
@@ -275,11 +293,19 @@ fn handle_use_statement(
                 }
             }
         }
-        ImportType::SelfImport(_) => ctx.self_import(handler, &path, stmt.alias.clone()),
+        ImportType::SelfImport(_) => {
+            ctx.self_import(handler, &path, stmt.alias.clone(), stmt.reexport)
+        }
         ImportType::Item(ref s) => {
             // try a standard item import first
             let item_import_handler = Handler::default();
-            let import = ctx.item_import(&item_import_handler, &path, s, stmt.alias.clone());
+            let import = ctx.item_import(
+                &item_import_handler,
+                &path,
+                s,
+                stmt.alias.clone(),
+                stmt.reexport,
+            );
 
             if import.is_ok() {
                 handler.append(item_import_handler);
@@ -294,6 +320,7 @@ fn handle_use_statement(
                         enum_name,
                         s,
                         stmt.alias.clone(),
+                        stmt.reexport,
                     );
                     if variant_import.is_ok() {
                         handler.append(variant_import_handler);

sway-core/src/semantic_analysis/collection_context.rs

@@ -108,11 +108,12 @@ impl SymbolCollectionContext {
         handler: &Handler,
         engines: &Engines,
         src: &ModulePath,
+        visibility: Visibility,
     ) -> Result<(), ErrorEmitted> {
         let mod_path = self.namespace().mod_path.clone();
         self.namespace_mut()
             .root
-            .star_import(handler, engines, src, &mod_path)
+            .star_import(handler, engines, src, &mod_path, visibility)
     }
 
     /// Short-hand for performing a [Module::variant_star_import] with `mod_path` as the destination.
@@ -122,11 +123,12 @@ impl SymbolCollectionContext {
         engines: &Engines,
         src: &ModulePath,
         enum_name: &Ident,
+        visibility: Visibility,
     ) -> Result<(), ErrorEmitted> {
         let mod_path = self.namespace().mod_path.clone();
         self.namespace_mut()
             .root
-            .variant_star_import(handler, engines, src, &mod_path, enum_name)
+            .variant_star_import(handler, engines, src, &mod_path, enum_name, visibility)
     }
 
     /// Short-hand for performing a [Module::self_import] with `mod_path` as the destination.
@@ -136,11 +138,12 @@ impl SymbolCollectionContext {
         engines: &Engines,
         src: &ModulePath,
         alias: Option<Ident>,
+        visibility: Visibility,
     ) -> Result<(), ErrorEmitted> {
         let mod_path = self.namespace().mod_path.clone();
         self.namespace_mut()
             .root
-            .self_import(handler, engines, src, &mod_path, alias)
+            .self_import(handler, engines, src, &mod_path, alias, visibility)
     }
 
     /// Short-hand for performing a [Module::item_import] with `mod_path` as the destination.
@@ -151,11 +154,12 @@ impl SymbolCollectionContext {
         src: &ModulePath,
         item: &Ident,
         alias: Option<Ident>,
+        visibility: Visibility,
     ) -> Result<(), ErrorEmitted> {
         let mod_path = self.namespace().mod_path.clone();
         self.namespace_mut()
             .root
-            .item_import(handler, engines, src, item, &mod_path, alias)
+            .item_import(handler, engines, src, item, &mod_path, alias, visibility)
     }
 
     /// Short-hand for performing a [Module::variant_import] with `mod_path` as the destination.
@@ -168,6 +172,7 @@ impl SymbolCollectionContext {
         enum_name: &Ident,
         variant_name: &Ident,
         alias: Option<Ident>,
+        visibility: Visibility,
     ) -> Result<(), ErrorEmitted> {
         let mod_path = self.namespace().mod_path.clone();
         self.namespace_mut().root.variant_import(
@@ -178,6 +183,7 @@ impl SymbolCollectionContext {
             variant_name,
             &mod_path,
             alias,
+            visibility,
         )
     }
 }

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

@@ -4,7 +4,7 @@ use crate::{
     language::{
         parsed::{Declaration, FunctionDeclaration},
         ty::{self, StructAccessInfo, TyDecl, TyStorageDecl},
-        CallPath,
+        CallPath, Visibility,
     },
     namespace::*,
     semantic_analysis::{ast_node::ConstShadowingMode, GenericShadowingMode},
@@ -36,10 +36,20 @@ impl ResolvedFunctionDecl {
 }
 
 pub(super) type SymbolMap = im::OrdMap<Ident, ResolvedDeclaration>;
+
 type SourceIdent = Ident;
-pub(super) type GlobSynonyms = im::HashMap<Ident, Vec<(ModulePathBuf, ResolvedDeclaration)>>;
-pub(super) type ItemSynonyms =
-    im::HashMap<Ident, (Option<SourceIdent>, ModulePathBuf, ResolvedDeclaration)>;
+
+pub(super) type GlobSynonyms =
+    im::HashMap<Ident, Vec<(ModulePathBuf, ResolvedDeclaration, Visibility)>>;
+pub(super) type ItemSynonyms = im::HashMap<
+    Ident,
+    (
+        Option<SourceIdent>,
+        ModulePathBuf,
+        ResolvedDeclaration,
+        Visibility,
+    ),
+>;
 
 /// Represents a lexical scope integer-based identifier, which can be used to reference
 /// specific a lexical scope.
@@ -464,7 +474,7 @@ impl Items {
             );
         }
 
-        if let Some((ident, (imported_ident, _, decl))) =
+        if let Some((ident, (imported_ident, _, decl, _))) =
             self.use_item_synonyms.get_key_value(&name)
         {
             append_shadowing_error(
@@ -494,43 +504,30 @@ impl Items {
         symbol: Ident,
         src_path: ModulePathBuf,
         decl: &ResolvedDeclaration,
+        visibility: Visibility,
     ) {
         if let Some(cur_decls) = self.use_glob_synonyms.get_mut(&symbol) {
             // Name already bound. Check if the decl is already imported
             let ctx = PartialEqWithEnginesContext::new(engines);
-            match cur_decls.iter().position(|(cur_path, cur_decl)| {
-                cur_decl.eq(decl, &ctx)
-        // For some reason the equality check is not sufficient. In some cases items that
-        // are actually identical fail the eq check, so we have to add heuristics for these
-        // cases.
-        //
-            // These edge occur because core and std preludes are not reexported correctly. Once
-        // reexports are implemented we can handle the preludes correctly, and then these
-        // edge cases should go away.
-        // See https://github.com/FuelLabs/sway/issues/3113
-        //
-        // As a heuristic we replace any bindings from std and core if the new binding is
-        // also from std or core.  This does not work if the user has declared an item with
-        // the same name as an item in one of the preludes, but this is an edge case that we
-        // will have to live with for now.
-                    || ((cur_path[0].as_str() == "core" || cur_path[0].as_str() == "std")
-                        && (src_path[0].as_str() == "core" || src_path[0].as_str() == "std"))
-            }) {
-                Some(index) => {
-                    // The name is already bound to this decl, but
-                    // we need to replace the binding to make the paths work out.
-                    // This appears to be an issue with the core prelude, and will probably no
-                    // longer be necessary once reexports are implemented:
-                    // https://github.com/FuelLabs/sway/issues/3113
-                    cur_decls[index] = (src_path.to_vec(), decl.clone());
+            match cur_decls
+                .iter()
+                .position(|(_cur_path, cur_decl, _cur_visibility)| cur_decl.eq(decl, &ctx))
+            {
+                Some(index) if matches!(visibility, Visibility::Public) => {
+                    // The name is already bound to this decl. If the new symbol is more visible
+                    // than the old one, then replace the old one.
+                    cur_decls[index] = (src_path.to_vec(), decl.clone(), visibility);
+                }
+                Some(_) => {
+                    // Same binding as the existing one. Do nothing.
                 }
                 None => {
                     // New decl for this name. Add it to the end
-                    cur_decls.push((src_path.to_vec(), decl.clone()));
+                    cur_decls.push((src_path.to_vec(), decl.clone(), visibility));
                 }
             }
         } else {
-            let new_vec = vec![(src_path.to_vec(), decl.clone())];
+            let new_vec = vec![(src_path.to_vec(), decl.clone(), visibility)];
             self.use_glob_synonyms.insert(symbol, new_vec);
         }
     }