feat(traits): added checks for duplicated trait associated items (typ…

…es, consts, functions) (noir-lang#2927)

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

@@ -468,7 +468,7 @@ fn collect_trait_impl_methods(
 
             if overrides.len() > 1 {
                 let error = DefCollectorErrorKind::Duplicate {
-                    typ: DuplicateType::Function,
+                    typ: DuplicateType::TraitAssociatedFunction,
                     first_def: overrides[0].2.name_ident().clone(),
                     second_def: overrides[1].2.name_ident().clone(),
                 };
@@ -542,7 +542,7 @@ fn collect_trait_impl(
 
             if let Some(struct_type) = get_struct_type(&typ) {
                 errors.extend(take_errors(trait_impl.file_id, resolver));
-                let current_def_map = def_maps.get_mut(&crate_id).unwrap();
+                let current_def_map = def_maps.get_mut(&struct_type.borrow().id.krate()).unwrap();
                 match add_method_to_struct_namespace(
                     current_def_map,
                     struct_type,
@@ -801,11 +801,7 @@ fn resolve_trait_methods(
                 .iter()
                 .filter(|(_, _, q)| q.name() == name.0.contents)
                 .collect();
-            let default_impl = if !default_impl_list.is_empty() {
-                if default_impl_list.len() > 1 {
-                    // TODO(nickysn): Add check for method duplicates in the trait and emit proper error messages. This is planned in a future PR.
-                    panic!("Too many functions with the same name!");
-                }
+            let default_impl = if default_impl_list.len() == 1 {
                 Some(Box::new(default_impl_list[0].2.clone()))
             } else {
                 None

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

@@ -6,7 +6,7 @@ use noirc_errors::Location;
 use crate::{
     graph::CrateId,
     hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
-    node_interner::TraitId,
+    node_interner::{TraitId, TypeAliasId},
     parser::{SortedModule, SubModule},
     FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl,
     NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl,
@@ -360,27 +360,77 @@ impl<'a> ModCollector<'a> {
                 trait_id: None,
             };
             for trait_item in &trait_definition.items {
-                // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
-                if let TraitItem::Function {
-                    name,
-                    generics,
-                    parameters,
-                    return_type,
-                    where_clause,
-                    body: Some(body),
-                } = trait_item
-                {
-                    let func_id = context.def_interner.push_empty_fn();
-
-                    let impl_method = NoirFunction::normal(FunctionDefinition::normal(
+                match trait_item {
+                    TraitItem::Function {
                         name,
                         generics,
                         parameters,
-                        body,
-                        where_clause,
                         return_type,
-                    ));
-                    unresolved_functions.push_fn(self.module_id, func_id, impl_method);
+                        where_clause,
+                        body,
+                    } => {
+                        let func_id = context.def_interner.push_empty_fn();
+                        match self.def_collector.def_map.modules[id.0.local_id.0]
+                            .declare_function(name.clone(), func_id)
+                        {
+                            Ok(()) => {
+                                // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
+                                if let Some(body) = body {
+                                    let impl_method =
+                                        NoirFunction::normal(FunctionDefinition::normal(
+                                            name,
+                                            generics,
+                                            parameters,
+                                            body,
+                                            where_clause,
+                                            return_type,
+                                        ));
+                                    unresolved_functions.push_fn(
+                                        self.module_id,
+                                        func_id,
+                                        impl_method,
+                                    );
+                                }
+                            }
+                            Err((first_def, second_def)) => {
+                                let error = DefCollectorErrorKind::Duplicate {
+                                    typ: DuplicateType::TraitAssociatedFunction,
+                                    first_def,
+                                    second_def,
+                                };
+                                errors.push((error.into(), self.file_id));
+                            }
+                        }
+                    }
+                    TraitItem::Constant { name, .. } => {
+                        let stmt_id = context.def_interner.push_empty_global();
+
+                        if let Err((first_def, second_def)) = self.def_collector.def_map.modules
+                            [id.0.local_id.0]
+                            .declare_global(name.clone(), stmt_id)
+                        {
+                            let error = DefCollectorErrorKind::Duplicate {
+                                typ: DuplicateType::TraitAssociatedConst,
+                                first_def,
+                                second_def,
+                            };
+                            errors.push((error.into(), self.file_id));
+                        }
+                    }
+                    TraitItem::Type { name } => {
+                        // TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()
+                        if let Err((first_def, second_def)) = self.def_collector.def_map.modules
+                            [id.0.local_id.0]
+                            .declare_type_alias(name.clone(), TypeAliasId::dummy_id())
+                        {
+                            let error = DefCollectorErrorKind::Duplicate {
+                                typ: DuplicateType::TraitAssociatedType,
+                                first_def,
+                                second_def,
+                            };
+                            errors.push((error.into(), self.file_id));
+                        }
+                    }
                 }
             }
 

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

@@ -18,6 +18,9 @@ pub enum DuplicateType {
     Import,
     Trait,
     TraitImplementation,
+    TraitAssociatedType,
+    TraitAssociatedConst,
+    TraitAssociatedFunction,
 }
 
 #[derive(Error, Debug, Clone)]
@@ -79,6 +82,9 @@ impl fmt::Display for DuplicateType {
             DuplicateType::Trait => write!(f, "trait definition"),
             DuplicateType::TraitImplementation => write!(f, "trait implementation"),
             DuplicateType::Import => write!(f, "import"),
+            DuplicateType::TraitAssociatedType => write!(f, "trait associated type"),
+            DuplicateType::TraitAssociatedConst => write!(f, "trait associated constant"),
+            DuplicateType::TraitAssociatedFunction => write!(f, "trait associated function"),
         }
     }
 }

tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_items_1"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/compile_failure/dup_trait_items_1/src/main.nr

@@ -0,0 +1,7 @@
+trait MyTrait {
+  fn SomeFunc();
+  fn SomeFunc();
+}
+
+fn main() {
+}

tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_items_2"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/compile_failure/dup_trait_items_2/src/main.nr

@@ -0,0 +1,7 @@
+trait MyTrait {
+  let SomeConst: u32;
+  let SomeConst: Field;
+}
+
+fn main() {
+}

tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_items_3"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/compile_failure/dup_trait_items_3/src/main.nr

@@ -0,0 +1,7 @@
+trait MyTrait {
+  type SomeType;
+  type SomeType;
+}
+
+fn main() {
+}

tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_items_4"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/compile_failure/dup_trait_items_4/src/main.nr

@@ -0,0 +1,7 @@
+trait MyTrait {
+  let MyItem: u32;
+  fn MyItem();
+}
+
+fn main() {
+}

tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_items_5"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/compile_failure/dup_trait_items_5/src/main.nr

@@ -0,0 +1,7 @@
+trait MyTrait {
+  fn MyItem();
+  let MyItem: u32;
+}
+
+fn main() {
+}

tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_items_6"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/compile_failure/dup_trait_items_6/src/main.nr

@@ -0,0 +1,15 @@
+trait MyTrait {
+  fn SomeFunc() { };
+  fn SomeFunc() { };
+}
+
+struct MyStruct {
+}
+
+impl MyTrait for MyStruct {
+  fn SomeFunc() {
+  }
+}
+
+fn main() {
+}

tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "trait_allowed_item_name_matches"
+type = "bin"
+authors = [""]
+compiler_version = "0.15.0"
+
+[dependencies]

tooling/nargo_cli/tests/execution_success/trait_allowed_item_name_matches/src/main.nr

@@ -0,0 +1,26 @@
+trait Trait1 {
+  // types and consts with the same name are allowed
+  type Tralala;
+  let Tralala: u32;
+}
+
+trait Trait2 {
+  // consts and types with the same name are allowed
+  let Tralala: u32;
+  type Tralala;
+}
+
+trait Trait3 {
+  // types and functions with the same name are allowed
+  type Tralala;
+  fn Tralala();
+}
+
+trait Trait4 {
+  // functions and types with the same name are allowed
+  fn Tralala();
+  type Tralala;
+}
+
+fn main() {
+}