Name clash in star imports should result in error when the name is used

sway-core/src/language/call_path.rs

@@ -319,13 +319,17 @@ impl CallPath {
                     .cloned()
                 {
                     Some(path)
-                } else if let Some((path, _)) = m
+                } else if let Some(paths_and_decls) = m
                     .current_items()
                     .use_glob_synonyms
                     .get(&self.suffix)
                     .cloned()
                 {
-                    Some(path)
+                    if paths_and_decls.len() == 1 {
+                        Some(paths_and_decls[0].0.clone())
+                    } else {
+                        None
+                    }
                 } else {
                     None
                 }

sway-core/src/language/ty/declaration/declaration.rs

@@ -127,6 +127,23 @@ impl PartialEqWithEngines for TyDecl {
                 TyDecl::EnumDecl(EnumDecl { decl_id: lid, .. }),
                 TyDecl::EnumDecl(EnumDecl { decl_id: rid, .. }),
             ) => decl_engine.get(lid).eq(&decl_engine.get(rid), ctx),
+            (
+                TyDecl::EnumVariantDecl(EnumVariantDecl {
+                    enum_ref: l_enum,
+                    variant_name: ln,
+                    ..
+                }),
+                TyDecl::EnumVariantDecl(EnumVariantDecl {
+                    enum_ref: r_enum,
+                    variant_name: rn,
+                    ..
+                }),
+            ) => {
+                ln == rn
+                    && decl_engine
+                        .get_enum(l_enum)
+                        .eq(&decl_engine.get_enum(r_enum), ctx)
+            }
             (
                 TyDecl::ImplTrait(ImplTrait { decl_id: lid, .. }),
                 TyDecl::ImplTrait(ImplTrait { decl_id: rid, .. }),

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

@@ -1,6 +1,6 @@
 use crate::{
     decl_engine::{parsed_id::ParsedDeclId, *},
-    engine_threading::Engines,
+    engine_threading::{Engines, PartialEqWithEngines, PartialEqWithEnginesContext},
     language::{
         parsed::{Declaration, FunctionDeclaration},
         ty::{self, StructAccessInfo, TyDecl, TyStorageDecl},
@@ -38,7 +38,7 @@ impl ResolvedFunctionDecl {
 pub(super) type ParsedSymbolMap = im::OrdMap<Ident, Declaration>;
 pub(super) type SymbolMap = im::OrdMap<Ident, ty::TyDecl>;
 type SourceIdent = Ident;
-pub(super) type GlobSynonyms = im::HashMap<Ident, (ModulePathBuf, ty::TyDecl)>;
+pub(super) type GlobSynonyms = im::HashMap<Ident, Vec<(ModulePathBuf, ty::TyDecl)>>;
 pub(super) type ItemSynonyms = im::HashMap<Ident, (Option<SourceIdent>, ModulePathBuf, ty::TyDecl)>;
 
 /// Represents a lexical scope integer-based identifier, which can be used to reference
@@ -75,6 +75,12 @@ pub struct Items {
     /// path `foo::bar::Baz`.
     ///
     /// use_glob_synonyms contains symbols imported using star imports (`use foo::*`.).
+    ///
+    /// When star importing from multiple modules the same name may be imported more than once. This
+    /// is not an error, but it is an error to use the name without a module path. To represent
+    /// this, use_glob_synonyms maps identifiers to a vector of (module path, type declaration)
+    /// tuples.
+    ///
     /// use_item_synonyms contains symbols imported using item imports (`use foo::bar`).
     ///
     /// For aliased item imports `use ::foo::bar::Baz as Wiz` the map key is `Wiz`. `Baz` is stored
@@ -304,6 +310,54 @@ impl Items {
         Ok(())
     }
 
+    // Insert a symbol into use_glob_synonyms if the symbol has not yet been inserted
+    pub(crate) fn insert_glob_use_symbol(
+        &mut self,
+        engines: &Engines,
+        symbol: Ident,
+        src_path: ModulePathBuf,
+        decl: &ty::TyDecl,
+    ) {
+        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());
+                }
+                None => {
+                    // New decl for this name. Add it to the end
+                    cur_decls.push((src_path.to_vec(), decl.clone()));
+                }
+            }
+        } else {
+            let new_vec = vec![(src_path.to_vec(), decl.clone())];
+            self.use_glob_synonyms.insert(symbol, new_vec);
+        }
+    }
+
     pub(crate) fn check_symbol(&self, name: &Ident) -> Result<ResolvedDeclaration, CompileError> {
         self.symbols
             .get(name)

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

@@ -278,13 +278,14 @@ impl Namespace {
             .cloned()
             .chain(Some(mod_name.clone()))
             .collect();
-        let parent_mod_path = std::mem::replace(&mut self.mod_path, submod_path);
+        let parent_mod_path = std::mem::replace(&mut self.mod_path, submod_path.clone());
         // self.module() now refers to a different module, so refetch
         let new_module = self.module_mut(engines);
         new_module.name = Some(mod_name);
         new_module.span = Some(module_span);
         new_module.visibility = visibility;
         new_module.is_external = false;
+        new_module.mod_path = submod_path;
         SubmoduleNamespace {
             namespace: self,
             parent_mod_path,

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

@@ -81,12 +81,15 @@ impl Root {
             .current_items_mut()
             .implemented_traits
             .extend(implemented_traits, engines);
-        for symbol_and_decl in symbols_and_decls {
-            dst_mod
-                .current_items_mut()
-                .use_glob_synonyms
-                .insert(symbol_and_decl.0, (src.to_vec(), symbol_and_decl.1));
-        }
+
+        symbols_and_decls.iter().for_each(|(symbol, decl)| {
+            dst_mod.current_items_mut().insert_glob_use_symbol(
+                engines,
+                symbol.clone(),
+                src.to_vec(),
+                decl,
+            )
+        });
 
         Ok(())
     }
@@ -338,20 +341,22 @@ impl Root {
 
                     for variant_decl in enum_decl.variants.iter() {
                         let variant_name = &variant_decl.name;
+                        let decl = TyDecl::EnumVariantDecl(ty::EnumVariantDecl {
+                            enum_ref: enum_ref.clone(),
+                            variant_name: variant_name.clone(),
+                            variant_decl_span: variant_decl.span.clone(),
+                        });
 
                         // import it this way.
-                        let dst_mod = self.module.lookup_submodule_mut(handler, engines, dst)?;
-                        dst_mod.current_items_mut().use_glob_synonyms.insert(
-                            variant_name.clone(),
-                            (
+                        self.module
+                            .lookup_submodule_mut(handler, engines, dst)?
+                            .current_items_mut()
+                            .insert_glob_use_symbol(
+                                engines,
+                                variant_name.clone(),
                                 src.to_vec(),
-                                TyDecl::EnumVariantDecl(ty::EnumVariantDecl {
-                                    enum_ref: enum_ref.clone(),
-                                    variant_name: variant_name.clone(),
-                                    variant_decl_span: variant_decl.span.clone(),
-                                }),
-                            ),
-                        );
+                                &decl,
+                            );
                     }
                 } else {
                     return Err(handler.emit_err(CompileError::Internal(
@@ -396,8 +401,10 @@ impl Root {
 
         // collect all declared and reexported symbols from the source module
         let mut all_symbols_and_decls = vec![];
-        for (symbol, (_, decl)) in src_mod.current_items().use_glob_synonyms.iter() {
-            all_symbols_and_decls.push((symbol.clone(), decl.clone()));
+        for (symbol, decls) in src_mod.current_items().use_glob_synonyms.iter() {
+            decls
+                .iter()
+                .for_each(|(_, decl)| all_symbols_and_decls.push((symbol.clone(), decl.clone())));
         }
         for (symbol, (_, _, decl)) in src_mod.current_items().use_item_synonyms.iter() {
             all_symbols_and_decls.push((symbol.clone(), decl.clone()));
@@ -428,8 +435,14 @@ impl Root {
         for (symbol, (_, mod_path, decl)) in use_item_synonyms {
             symbols_paths_and_decls.push((symbol, get_path(mod_path), decl));
         }
-        for (symbol, (mod_path, decl)) in use_glob_synonyms {
-            symbols_paths_and_decls.push((symbol, get_path(mod_path), decl));
+        for (symbol, decls) in use_glob_synonyms {
+            decls.iter().for_each(|(mod_path, decl)| {
+                symbols_paths_and_decls.push((
+                    symbol.clone(),
+                    get_path(mod_path.clone()),
+                    decl.clone(),
+                ))
+            });
         }
 
         let dst_mod = self.module.lookup_submodule_mut(handler, engines, dst)?;
@@ -441,16 +454,15 @@ impl Root {
         let mut try_add = |symbol, path, decl: ty::TyDecl| {
             dst_mod
                 .current_items_mut()
-                .use_glob_synonyms
-                .insert(symbol, (path, decl));
+                .insert_glob_use_symbol(engines, symbol, path, &decl);
         };
 
         for (symbol, decl) in all_symbols_and_decls {
-            try_add(symbol, src.to_vec(), decl);
+            try_add(symbol.clone(), src.to_vec(), decl);
         }
 
         for (symbol, path, decl) in symbols_paths_and_decls {
-            try_add(symbol.clone(), path, decl.clone());
+            try_add(symbol.clone(), path, decl);
         }
 
         Ok(())
@@ -813,8 +825,33 @@ impl Root {
             return Ok(ResolvedDeclaration::Typed(decl.clone()));
         }
         // Check glob imports
-        if let Some((_, decl)) = module.current_items().use_glob_synonyms.get(symbol) {
-            return Ok(ResolvedDeclaration::Typed(decl.clone()));
+        if let Some(decls) = module.current_items().use_glob_synonyms.get(symbol) {
+            if decls.len() == 1 {
+                return Ok(ResolvedDeclaration::Typed(decls[0].1.clone()));
+            } else if decls.is_empty() {
+                return Err(handler.emit_err(CompileError::Internal(
+                    "The name {symbol} was bound in a star import, but no corresponding module paths were found",
+                    symbol.span(),
+                )));
+            } else {
+                // Symbol not found
+                return Err(handler.emit_err(CompileError::SymbolWithMultipleBindings {
+                    name: symbol.clone(),
+                    path_1: decls[0]
+                        .0
+                        .iter()
+                        .map(|x| x.as_str())
+                        .collect::<Vec<_>>()
+                        .join("::"),
+                    path_2: decls[1]
+                        .0
+                        .iter()
+                        .map(|x| x.as_str())
+                        .collect::<Vec<_>>()
+                        .join("::"),
+                    span: symbol.span(),
+                }));
+            }
         }
         // Symbol not found
         Err(handler.emit_err(CompileError::SymbolNotFound {

sway-error/src/error.rs

@@ -426,6 +426,13 @@ pub enum CompileError {
     DeclIsNotATypeAlias { actually: String, span: Span },
     #[error("Could not find symbol \"{name}\" in this scope.")]
     SymbolNotFound { name: Ident, span: Span },
+    #[error("Found multiple bindings for \"{name}\" in this scope: {path_1}::{name} and {path_2}::{name} are both valid.")]
+    SymbolWithMultipleBindings {
+        name: Ident,
+        path_1: String,
+        path_2: String,
+        span: Span,
+    },
     #[error("Symbol \"{name}\" is private.")]
     ImportPrivateSymbol { name: Ident, span: Span },
     #[error("Module \"{name}\" is private.")]
@@ -1019,6 +1026,7 @@ impl Spanned for CompileError {
             NotIndexable { span, .. } => span.clone(),
             FieldAccessOnNonStruct { span, .. } => span.clone(),
             SymbolNotFound { span, .. } => span.clone(),
+            SymbolWithMultipleBindings { span, .. } => span.clone(),
             ImportPrivateSymbol { span, .. } => span.clone(),
             ImportPrivateModule { span, .. } => span.clone(),
             NoElseBranch { span, .. } => span.clone(),

test/src/e2e_vm_tests/test_programs/should_fail/language/import_star_name_clash/Forc.lock

@@ -0,0 +1,13 @@
+[[package]]
+name = "core"
+source = "path+from-root-95C7DDCD726BD0EA"
+
+[[package]]
+name = "import_star_name_clash"
+source = "member"
+dependencies = ["std"]
+
+[[package]]
+name = "std"
+source = "path+from-root-95C7DDCD726BD0EA"
+dependencies = ["core"]

test/src/e2e_vm_tests/test_programs/should_fail/language/import_star_name_clash/Forc.toml

@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+license = "Apache-2.0"
+name = "import_star_name_clash"
+entry = "main.sw"
+
+[dependencies]
+std = { path = "../../../../../../../sway-lib-std" }

test/src/e2e_vm_tests/test_programs/should_fail/language/import_star_name_clash/json_abi_oracle.json

@@ -0,0 +1,25 @@
+{
+  "configurables": [],
+  "functions": [
+    {
+      "attributes": null,
+      "inputs": [],
+      "name": "main",
+      "output": {
+        "name": "",
+        "type": 0,
+        "typeArguments": null
+      }
+    }
+  ],
+  "loggedTypes": [],
+  "messagesTypes": [],
+  "types": [
+    {
+      "components": [],
+      "type": "()",
+      "typeId": 0,
+      "typeParameters": null
+    }
+  ]
+}

test/src/e2e_vm_tests/test_programs/should_fail/language/import_star_name_clash/json_abi_oracle_new_encoding.json

@@ -0,0 +1,26 @@
+{
+  "configurables": [],
+  "encoding": "1",
+  "functions": [
+    {
+      "attributes": null,
+      "inputs": [],
+      "name": "main",
+      "output": {
+        "name": "",
+        "type": 0,
+        "typeArguments": null
+      }
+    }
+  ],
+  "loggedTypes": [],
+  "messagesTypes": [],
+  "types": [
+    {
+      "components": [],
+      "type": "()",
+      "typeId": 0,
+      "typeParameters": null
+    }
+  ]
+}