fix: Prevent overlapping associated types impls (#7047)

Co-authored-by: Tom French <[email protected]>

compiler/noirc_frontend/src/node_interner.rs

@@ -1722,8 +1722,18 @@ impl NodeInterner {
         let instantiated_object_type = object_type.substitute(&substitutions);
 
         let trait_generics = &trait_impl.borrow().trait_generics;
+
+        // Replace any associated types with fresh type variables so that we match
+        // any existing impl regardless of associated types if one already exists.
+        // E.g. if we already have an `impl Foo<Bar = i32> for Baz`, we should
+        // reject `impl Foo<Bar = u32> for Baz` if it were to be added.
         let associated_types = self.get_associated_types_for_impl(impl_id);
 
+        let associated_types = vecmap(associated_types, |named| {
+            let typ = self.next_type_variable();
+            NamedType { name: named.name.clone(), typ }
+        });
+
         // Ignoring overlapping `TraitImplKind::Assumed` impls here is perfectly fine.
         // It should never happen since impls are defined at global scope, but even
         // if they were, we should never prevent defining a new impl because a 'where'
@@ -1732,7 +1742,7 @@ impl NodeInterner {
             &instantiated_object_type,
             trait_id,
             trait_generics,
-            associated_types,
+            &associated_types,
         ) {
             let existing_impl = self.get_trait_implementation(existing);
             let existing_impl = existing_impl.borrow();

compiler/noirc_frontend/src/tests/traits.rs

@@ -1,4 +1,5 @@
 use crate::hir::def_collector::dc_crate::CompilationError;
+use crate::hir::def_collector::errors::DefCollectorErrorKind;
 use crate::hir::resolution::errors::ResolverError;
 use crate::hir::resolution::import::PathResolutionError;
 use crate::hir::type_check::TypeCheckError;
@@ -1237,6 +1238,64 @@ fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_on
     assert_eq!(trait_name, "private_mod::Foo");
 }
 
+#[test]
+fn error_on_duplicate_impl_with_associated_type() {
+    let src = r#"
+        trait Foo {
+            type Bar;
+        }
+
+        impl Foo for i32 {
+            type Bar = u32;
+        }
+
+        impl Foo for i32 {
+            type Bar = u8;
+        }
+
+        fn main() {}
+    "#;
+
+    // Expect "Impl for type `i32` overlaps with existing impl"
+    //    and "Previous impl defined here"
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 2);
+
+    use CompilationError::DefinitionError;
+    use DefCollectorErrorKind::*;
+    assert!(matches!(&errors[0].0, DefinitionError(OverlappingImpl { .. })));
+    assert!(matches!(&errors[1].0, DefinitionError(OverlappingImplNote { .. })));
+}
+
+#[test]
+fn error_on_duplicate_impl_with_associated_constant() {
+    let src = r#"
+        trait Foo {
+            let Bar: u32;
+        }
+
+        impl Foo for i32 {
+            let Bar = 5;
+        }
+
+        impl Foo for i32 {
+            let Bar = 6;
+        }
+
+        fn main() {}
+    "#;
+
+    // Expect "Impl for type `i32` overlaps with existing impl"
+    //    and "Previous impl defined here"
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 2);
+
+    use CompilationError::DefinitionError;
+    use DefCollectorErrorKind::*;
+    assert!(matches!(&errors[0].0, DefinitionError(OverlappingImpl { .. })));
+    assert!(matches!(&errors[1].0, DefinitionError(OverlappingImplNote { .. })));
+}
+
 // See https://github.com/noir-lang/noir/issues/6530
 #[test]
 fn regression_6530() {