fix: allow names to collide in the values/types namespaces

compiler/noirc_frontend/src/elaborator/enums.rs

@@ -27,7 +27,7 @@ use crate::{
     token::Attributes,
 };
 
-use super::Elaborator;
+use super::{Elaborator, path_resolution::PathResolutionTarget};
 
 const WILDCARD_PATTERN: &str = "_";
 
@@ -419,7 +419,7 @@ impl Elaborator<'_> {
                 // user is trying to resolve to a non-local item.
                 let shadow_existing = path.is_ident().then_some(last_ident);
 
-                match self.resolve_path_or_error(path) {
+                match self.resolve_path_or_error(path, PathResolutionTarget::Value) {
                     Ok(resolution) => self.path_resolution_to_constructor(
                         resolution,
                         shadow_existing,
@@ -588,7 +588,7 @@ impl Elaborator<'_> {
             ExpressionKind::Variable(path) => {
                 let location = path.location;
 
-                match self.resolve_path_or_error(path) {
+                match self.resolve_path_or_error(path, PathResolutionTarget::Value) {
                     // Use None for `name` here - we don't want to define a variable if this
                     // resolves to an existing item.
                     Ok(resolution) => self.path_resolution_to_constructor(

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -70,7 +70,9 @@ use noirc_errors::{Located, Location};
 pub(crate) use options::ElaboratorOptions;
 pub use options::{FrontendOptions, UnstableFeature};
 pub use path_resolution::Turbofish;
-use path_resolution::{PathResolution, PathResolutionItem, PathResolutionMode};
+use path_resolution::{
+    PathResolution, PathResolutionItem, PathResolutionMode, PathResolutionTarget,
+};
 use types::bind_ordered_generics;
 
 use self::traits::check_trait_impl_method_matches_declaration;
@@ -788,16 +790,18 @@ impl<'context> Elaborator<'context> {
     }
 
     pub fn resolve_module_by_path(&mut self, path: Path) -> Option<ModuleId> {
-        match self.resolve_path(path.clone()) {
-            Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors }) => {
-                if errors.is_empty() { Some(module_id) } else { None }
+        match self.resolve_path_as_type(path.clone()) {
+            Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors })
+                if errors.is_empty() =>
+            {
+                Some(module_id)
             }
             _ => None,
         }
     }
 
     fn resolve_trait_by_path(&mut self, path: Path) -> Option<TraitId> {
-        let error = match self.resolve_path(path.clone()) {
+        let error = match self.resolve_path_as_type(path.clone()) {
             Ok(PathResolution { item: PathResolutionItem::Trait(trait_id), errors }) => {
                 for error in errors {
                     self.push_err(error);
@@ -863,11 +867,9 @@ impl<'context> Elaborator<'context> {
     ) -> Vec<ResolvedGeneric> {
         let mut added_generics = Vec::new();
 
-        let Ok(item) = self.resolve_path_or_error(bound.trait_path.clone()) else {
-            return Vec::new();
-        };
-
-        let PathResolutionItem::Trait(trait_id) = item else {
+        let Ok(PathResolutionItem::Trait(trait_id)) =
+            self.resolve_path_or_error(bound.trait_path.clone(), PathResolutionTarget::Type)
+        else {
             return Vec::new();
         };
 

compiler/noirc_frontend/src/elaborator/path_resolution.rs

@@ -24,12 +24,15 @@ pub(crate) struct PathResolution {
 /// All possible items that result from resolving a Path.
 /// Note that this item doesn't include the last turbofish in a Path,
 /// only intermediate ones, if any.
-#[derive(Debug, Clone)]
-pub enum PathResolutionItem {
+#[derive(Debug)]
+pub(crate) enum PathResolutionItem {
+    // These are types
     Module(ModuleId),
     Type(TypeId),
     TypeAlias(TypeAliasId),
     Trait(TraitId),
+
+    // These are values
     Global(GlobalId),
     ModuleFunction(FuncId),
     Method(TypeId, Option<Turbofish>, FuncId),
@@ -39,7 +42,7 @@ pub enum PathResolutionItem {
 }
 
 impl PathResolutionItem {
-    pub fn function_id(&self) -> Option<FuncId> {
+    pub(crate) fn function_id(&self) -> Option<FuncId> {
         match self {
             PathResolutionItem::ModuleFunction(func_id)
             | PathResolutionItem::Method(_, _, func_id)
@@ -54,14 +57,7 @@ impl PathResolutionItem {
         }
     }
 
-    pub fn module_id(&self) -> Option<ModuleId> {
-        match self {
-            Self::Module(module_id) => Some(*module_id),
-            _ => None,
-        }
-    }
-
-    pub fn description(&self) -> &'static str {
+    pub(crate) fn description(&self) -> &'static str {
         match self {
             PathResolutionItem::Module(..) => "module",
             PathResolutionItem::Type(..) => "type",
@@ -84,7 +80,7 @@ pub struct Turbofish {
 }
 
 /// Any item that can appear before the last segment in a path.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 enum IntermediatePathResolutionItem {
     SelfType,
     Module,
@@ -142,27 +138,43 @@ pub(super) enum PathResolutionMode {
     MarkAsUsed,
 }
 
+/// Depenending on where a path appears in the source code it should either resolve to a type
+/// or a value. For example, in `let x: Foo::Bar = Foo::Bar {}` both `Foo::Bar` should resolve to
+/// types, never values. On the other hand, in `Foo::Bar()` `Foo::Bar` should resolve to a value,
+/// typically a function.
+///
+/// When using any of the `resolve` methods in this module, items in the target namespace
+/// will be returned first if another one exists in the other namespace.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub(super) enum PathResolutionTarget {
+    Type,
+    Value,
+}
+
 impl Elaborator<'_> {
     pub(super) fn resolve_path_or_error(
         &mut self,
         path: Path,
+        target: PathResolutionTarget,
     ) -> Result<PathResolutionItem, ResolverError> {
-        self.resolve_path_or_error_inner(path, PathResolutionMode::MarkAsReferenced)
+        self.resolve_path_or_error_inner(path, target, PathResolutionMode::MarkAsReferenced)
     }
 
     pub(super) fn use_path_or_error(
         &mut self,
         path: Path,
+        target: PathResolutionTarget,
     ) -> Result<PathResolutionItem, ResolverError> {
-        self.resolve_path_or_error_inner(path, PathResolutionMode::MarkAsUsed)
+        self.resolve_path_or_error_inner(path, target, PathResolutionMode::MarkAsUsed)
     }
 
     pub(super) fn resolve_path_or_error_inner(
         &mut self,
         path: Path,
+        target: PathResolutionTarget,
         mode: PathResolutionMode,
     ) -> Result<PathResolutionItem, ResolverError> {
-        let path_resolution = self.resolve_path_inner(path, mode)?;
+        let path_resolution = self.resolve_path_inner(path, target, mode)?;
 
         for error in path_resolution.errors {
             self.push_err(error);
@@ -171,12 +183,16 @@ impl Elaborator<'_> {
         Ok(path_resolution.item)
     }
 
-    pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult {
-        self.resolve_path_inner(path, PathResolutionMode::MarkAsReferenced)
+    pub(super) fn resolve_path_as_type(&mut self, path: Path) -> PathResolutionResult {
+        self.resolve_path_inner(
+            path,
+            PathResolutionTarget::Type,
+            PathResolutionMode::MarkAsReferenced,
+        )
     }
 
-    pub(super) fn use_path(&mut self, path: Path) -> PathResolutionResult {
-        self.resolve_path_inner(path, PathResolutionMode::MarkAsUsed)
+    pub(super) fn use_path_as_type(&mut self, path: Path) -> PathResolutionResult {
+        self.resolve_path_inner(path, PathResolutionTarget::Type, PathResolutionMode::MarkAsUsed)
     }
 
     /// Resolves a path in the current module.
@@ -186,6 +202,7 @@ impl Elaborator<'_> {
     pub(super) fn resolve_path_inner(
         &mut self,
         mut path: Path,
+        target: PathResolutionTarget,
         mode: PathResolutionMode,
     ) -> PathResolutionResult {
         let mut module_id = self.module_id();
@@ -207,7 +224,7 @@ impl Elaborator<'_> {
             }
         }
 
-        self.resolve_path_in_module(path, module_id, intermediate_item, mode)
+        self.resolve_path_in_module(path, module_id, intermediate_item, target, mode)
     }
 
     /// Resolves a path in `current_module`.
@@ -217,6 +234,7 @@ impl Elaborator<'_> {
         path: Path,
         importing_module: ModuleId,
         intermediate_item: IntermediatePathResolutionItem,
+        target: PathResolutionTarget,
         mode: PathResolutionMode,
     ) -> PathResolutionResult {
         let references_tracker = if self.interner.is_in_lsp_mode() {
@@ -226,7 +244,14 @@ impl Elaborator<'_> {
         };
         let (path, module_id, _) =
             resolve_path_kind(path, importing_module, self.def_maps, references_tracker)?;
-        self.resolve_name_in_module(path, module_id, importing_module, intermediate_item, mode)
+        self.resolve_name_in_module(
+            path,
+            module_id,
+            importing_module,
+            intermediate_item,
+            target,
+            mode,
+        )
     }
 
     /// Resolves a Path assuming we are inside `starting_module`.
@@ -237,6 +262,7 @@ impl Elaborator<'_> {
         starting_module: ModuleId,
         importing_module: ModuleId,
         mut intermediate_item: IntermediatePathResolutionItem,
+        target: PathResolutionTarget,
         mode: PathResolutionMode,
     ) -> PathResolutionResult {
         // There is a possibility that the import path is empty. In that case, early return.
@@ -403,9 +429,51 @@ impl Elaborator<'_> {
             current_ns = found_ns;
         }
 
-        let (module_def_id, visibility, _) =
-            current_ns.values.or(current_ns.types).expect("Found empty namespace");
+        let (target_ns, fallback_ns) = match target {
+            PathResolutionTarget::Type => (current_ns.types, current_ns.values),
+            PathResolutionTarget::Value => (current_ns.values, current_ns.types),
+        };
 
+        let item = target_ns
+            .map(|(module_def_id, visibility, ..)| {
+                self.per_ns_item_to_path_resolution_item(
+                    path.clone(),
+                    importing_module,
+                    intermediate_item.clone(),
+                    current_module_id,
+                    &mut errors,
+                    module_def_id,
+                    visibility,
+                )
+            })
+            .unwrap_or_else(|| {
+                let (module_def_id, visibility, ..) =
+                    fallback_ns.expect("A namespace should never be empty");
+                self.per_ns_item_to_path_resolution_item(
+                    path.clone(),
+                    importing_module,
+                    intermediate_item,
+                    current_module_id,
+                    &mut errors,
+                    module_def_id,
+                    visibility,
+                )
+            });
+
+        Ok(PathResolution { item, errors })
+    }
+
+    #[allow(clippy::too_many_arguments)]
+    fn per_ns_item_to_path_resolution_item(
+        &mut self,
+        path: Path,
+        importing_module: ModuleId,
+        intermediate_item: IntermediatePathResolutionItem,
+        current_module_id: ModuleId,
+        errors: &mut Vec<PathResolutionError>,
+        module_def_id: ModuleDefId,
+        visibility: crate::ast::ItemVisibility,
+    ) -> PathResolutionItem {
         let name = path.last_ident();
         let is_self_type = name.is_self_type_name();
         let location = name.location();
@@ -427,7 +495,7 @@ impl Elaborator<'_> {
             errors.push(PathResolutionError::Private(name.clone()));
         }
 
-        Ok(PathResolution { item, errors })
+        item
     }
 
     fn self_type_module_id(&self) -> Option<ModuleId> {

compiler/noirc_frontend/src/elaborator/patterns.rs

@@ -874,7 +874,7 @@ impl Elaborator<'_> {
         }
     }
 
-    pub fn get_ident_from_path(
+    pub(crate) fn get_ident_from_path(
         &mut self,
         path: Path,
     ) -> ((HirIdent, usize), Option<PathResolutionItem>) {

compiler/noirc_frontend/src/elaborator/scope.rs

@@ -15,7 +15,7 @@ use crate::{Type, TypeAlias};
 
 use super::path_resolution::{PathResolutionItem, PathResolutionMode};
 use super::types::SELF_TYPE_NAME;
-use super::{Elaborator, ResolverMeta};
+use super::{Elaborator, PathResolutionTarget, ResolverMeta};
 
 type Scope = GenericScope<String, ResolverMeta>;
 type ScopeTree = GenericScopeTree<String, ResolverMeta>;
@@ -82,7 +82,7 @@ impl Elaborator<'_> {
         path: Path,
     ) -> Result<(DefinitionId, PathResolutionItem), ResolverError> {
         let location = path.location;
-        let item = self.use_path_or_error(path)?;
+        let item = self.use_path_or_error(path, PathResolutionTarget::Value)?;
 
         if let Some(function) = item.function_id() {
             return Ok((self.interner.function_definition_id(function), item));
@@ -137,7 +137,7 @@ impl Elaborator<'_> {
     /// Lookup a given trait by name/path.
     pub fn lookup_trait_or_error(&mut self, path: Path) -> Option<&mut Trait> {
         let location = path.location;
-        match self.resolve_path_or_error(path) {
+        match self.resolve_path_or_error(path, PathResolutionTarget::Type) {
             Ok(item) => {
                 if let PathResolutionItem::Trait(trait_id) = item {
                     Some(self.get_trait_mut(trait_id))
@@ -164,7 +164,7 @@ impl Elaborator<'_> {
         mode: PathResolutionMode,
     ) -> Option<Shared<DataType>> {
         let location = path.location;
-        match self.resolve_path_or_error_inner(path, mode) {
+        match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) {
             Ok(item) => {
                 if let PathResolutionItem::Type(struct_id) = item {
                     Some(self.get_type(struct_id))
@@ -195,7 +195,7 @@ impl Elaborator<'_> {
         }
 
         let location = path.location;
-        match self.use_path_or_error(path) {
+        match self.use_path_or_error(path, PathResolutionTarget::Type) {
             Ok(PathResolutionItem::Type(struct_id)) => {
                 let struct_type = self.get_type(struct_id);
                 let generics = struct_type.borrow().instantiate(self.interner);
@@ -226,7 +226,7 @@ impl Elaborator<'_> {
         path: Path,
         mode: PathResolutionMode,
     ) -> Option<Shared<TypeAlias>> {
-        match self.resolve_path_or_error_inner(path, mode) {
+        match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) {
             Ok(PathResolutionItem::TypeAlias(type_alias_id)) => {
                 Some(self.interner.get_type_alias(type_alias_id))
             }