Check for type aliases when implementing traits (#6875)

## Description

Fixes #6329 .

During insertion of `impl`s into the trait map we used to forget to
check for alias types, which meant that it was possible to have multiple
implementations of a trait for the same type, one for the original type
and one for the alias.

#6626 fixed this bug, but the error message was unhelpful in that it
reported the error on the original type without mentioning the alias.

This PR improves the error message, and also improves the error message
for aliased types when they contain multiple declarations of the same
member.

## 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: IGI-111 <igi-111@protonmail.com>
Co-authored-by: Joshua Batty <joshpbatty@gmail.com>

Author: 3 people

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

@@ -237,7 +237,7 @@ impl TraitMap {
         is_extending_existing_impl: IsExtendingExistingImpl,
         engines: &Engines,
     ) -> Result<(), ErrorEmitted> {
-        let type_id = engines.te().get_unaliased_type_id(type_id);
+        let unaliased_type_id = engines.te().get_unaliased_type_id(type_id);
 
         handler.scope(|handler| {
             let mut trait_items: TraitItems = HashMap::new();
@@ -267,7 +267,7 @@ impl TraitMap {
                 }
             }
 
-            let trait_impls = self.get_impls_mut(engines, type_id);
+            let trait_impls = self.get_impls_mut(engines, unaliased_type_id);
 
             // check to see if adding this trait will produce a conflicting definition
             for TraitEntry {
@@ -295,11 +295,11 @@ impl TraitMap {
 
                 let unify_checker = UnifyCheck::non_generic_constraint_subset(engines);
 
-                // Types are subset if the `type_id` that we want to insert can unify with the
+                // Types are subset if the `unaliased_type_id` that we want to insert can unify with the
                 // existing `map_type_id`. In addition we need to additionally check for the case of
                 // `&mut <type>` and `&<type>`.
-                let types_are_subset = unify_checker.check(type_id, *map_type_id)
-                    && is_unified_type_subset(engines.te(), type_id, *map_type_id);
+                let types_are_subset = unify_checker.check(unaliased_type_id, *map_type_id)
+                    && is_unified_type_subset(engines.te(), unaliased_type_id, *map_type_id);
 
                 /// `left` can unify into `right`. Additionally we need to check subset condition in case of
                 /// [TypeInfo::Ref] types.  Although `&mut <type>` can unify with `&<type>`
@@ -368,6 +368,9 @@ impl TraitMap {
                     handler.emit_err(CompileError::ConflictingImplsForTraitAndType {
                         trait_name: trait_name.to_string_with_args(engines, &trait_type_args),
                         type_implementing_for: engines.help_out(type_id).to_string(),
+                        type_implementing_for_unaliased: engines
+                            .help_out(unaliased_type_id)
+                            .to_string(),
                         existing_impl_span: existing_impl_span.clone(),
                         second_impl_span: impl_span.clone(),
                     });
@@ -382,43 +385,61 @@ impl TraitMap {
                             ResolvedTraitImplItem::Parsed(_item) => todo!(),
                             ResolvedTraitImplItem::Typed(item) => match item {
                                 ty::TyTraitItem::Fn(decl_ref) => {
-                                    if map_trait_items.get(name).is_some() {
+                                    if let Some(existing_item) = map_trait_items.get(name) {
                                         handler.emit_err(
                                             CompileError::DuplicateDeclDefinedForType {
                                                 decl_kind: "method".into(),
                                                 decl_name: decl_ref.name().to_string(),
                                                 type_implementing_for: engines
                                                     .help_out(type_id)
                                                     .to_string(),
-                                                span: decl_ref.name().span(),
+                                                type_implementing_for_unaliased: engines
+                                                    .help_out(unaliased_type_id)
+                                                    .to_string(),
+                                                existing_impl_span: existing_item
+                                                    .span(engines)
+                                                    .clone(),
+                                                second_impl_span: decl_ref.name().span(),
                                             },
                                         );
                                     }
                                 }
                                 ty::TyTraitItem::Constant(decl_ref) => {
-                                    if map_trait_items.get(name).is_some() {
+                                    if let Some(existing_item) = map_trait_items.get(name) {
                                         handler.emit_err(
                                             CompileError::DuplicateDeclDefinedForType {
                                                 decl_kind: "constant".into(),
                                                 decl_name: decl_ref.name().to_string(),
                                                 type_implementing_for: engines
                                                     .help_out(type_id)
                                                     .to_string(),
-                                                span: decl_ref.name().span(),
+                                                type_implementing_for_unaliased: engines
+                                                    .help_out(unaliased_type_id)
+                                                    .to_string(),
+                                                existing_impl_span: existing_item
+                                                    .span(engines)
+                                                    .clone(),
+                                                second_impl_span: decl_ref.name().span(),
                                             },
                                         );
                                     }
                                 }
                                 ty::TyTraitItem::Type(decl_ref) => {
-                                    if map_trait_items.get(name).is_some() {
+                                    if let Some(existing_item) = map_trait_items.get(name) {
                                         handler.emit_err(
                                             CompileError::DuplicateDeclDefinedForType {
                                                 decl_kind: "type".into(),
                                                 decl_name: decl_ref.name().to_string(),
                                                 type_implementing_for: engines
                                                     .help_out(type_id)
                                                     .to_string(),
-                                                span: decl_ref.name().span(),
+                                                type_implementing_for_unaliased: engines
+                                                    .help_out(unaliased_type_id)
+                                                    .to_string(),
+                                                existing_impl_span: existing_item
+                                                    .span(engines)
+                                                    .clone(),
+                                                second_impl_span: decl_ref.name().span(),
                                             },
                                         );
                                     }
@@ -442,7 +463,7 @@ impl TraitMap {
                 trait_name,
                 impl_span.clone(),
                 trait_decl_span,
-                type_id,
+                unaliased_type_id,
                 trait_items,
                 engines,
             );

sway-error/src/error.rs

@@ -625,6 +625,7 @@ pub enum CompileError {
     ConflictingImplsForTraitAndType {
         trait_name: String,
         type_implementing_for: String,
+        type_implementing_for_unaliased: String,
         existing_impl_span: Span,
         second_impl_span: Span,
     },
@@ -640,7 +641,9 @@ pub enum CompileError {
         decl_kind: String,
         decl_name: String,
         type_implementing_for: String,
-        span: Span,
+        type_implementing_for_unaliased: String,
+        existing_impl_span: Span,
+        second_impl_span: Span,
     },
     #[error("The function \"{fn_name}\" in {interface_name} is defined with {num_parameters} parameters, but the provided implementation has {provided_parameters} parameters.")]
     IncorrectNumberOfInterfaceSurfaceFunctionParameters {
@@ -1180,7 +1183,9 @@ impl Spanned for CompileError {
                 second_impl_span, ..
             } => second_impl_span.clone(),
             MarkerTraitExplicitlyImplemented { span, .. } => span.clone(),
-            DuplicateDeclDefinedForType { span, .. } => span.clone(),
+            DuplicateDeclDefinedForType {
+                second_impl_span, ..
+            } => second_impl_span.clone(),
             IncorrectNumberOfInterfaceSurfaceFunctionParameters { span, .. } => span.clone(),
             ArgumentParameterTypeMismatch { span, .. } => span.clone(),
             RecursiveCall { span, .. } => span.clone(),
@@ -2317,27 +2322,62 @@ impl ToDiagnostic for CompileError {
                     format!("{}- referencing a mutable copy of \"{decl_name}\", by returning it from a block: `&mut {{ {decl_name} }}`.", Indent::Single)
                 ],
             },
-            ConflictingImplsForTraitAndType { trait_name, type_implementing_for, existing_impl_span, second_impl_span } => Diagnostic {
+            ConflictingImplsForTraitAndType { trait_name, type_implementing_for, type_implementing_for_unaliased, existing_impl_span, second_impl_span } => Diagnostic {
                 reason: Some(Reason::new(code(1), "Trait is already implemented for type".to_string())),
                 issue: Issue::error(
                     source_engine,
                     second_impl_span.clone(),
-                    format!("Trait \"{trait_name}\" is already implemented for type \"{type_implementing_for}\".")
+                    if type_implementing_for == type_implementing_for_unaliased {
+                        format!("Trait \"{trait_name}\" is already implemented for type \"{type_implementing_for}\".")
+                    } else {
+                        format!("Trait \"{trait_name}\" is already implemented for type \"{type_implementing_for}\" (which is an alias for \"{type_implementing_for_unaliased}\").")
+                    }
                 ),
                 hints: vec![
                     Hint::info(
                         source_engine,
                         existing_impl_span.clone(),
-                        format!("This is the already existing implementation of \"{}\" for \"{type_implementing_for}\".",
-                            call_path_suffix_with_args(trait_name)
-                        )
+                        if type_implementing_for == type_implementing_for_unaliased {
+                            format!("This is the already existing implementation of \"{}\" for \"{type_implementing_for}\".",
+                                    call_path_suffix_with_args(trait_name)
+                            )
+                        } else {
+                            format!("This is the already existing implementation of \"{}\" for \"{type_implementing_for}\" (which is an alias for \"{type_implementing_for_unaliased}\").",
+                                    call_path_suffix_with_args(trait_name)
+                            )
+                        }
                     ),
                 ],
                 help: vec![
                     "In Sway, there can be at most one implementation of a trait for any given type.".to_string(),
                     "This property is called \"trait coherence\".".to_string(),
                 ],
             },
+            DuplicateDeclDefinedForType { decl_kind, decl_name, type_implementing_for, type_implementing_for_unaliased, existing_impl_span, second_impl_span } => {
+                let decl_kind_snake_case = sway_types::style::to_upper_camel_case(decl_kind);
+                Diagnostic {
+                    reason: Some(Reason::new(code(1), "Type contains duplicate declarations".to_string())),
+                    issue: Issue::error(
+                        source_engine,
+                        second_impl_span.clone(),
+                        if type_implementing_for == type_implementing_for_unaliased {
+                            format!("{decl_kind_snake_case} \"{decl_name}\" already declared in type \"{type_implementing_for}\".")
+                        } else {
+                            format!("{decl_kind_snake_case} \"{decl_name}\" already declared in type \"{type_implementing_for}\" (which is an alias for \"{type_implementing_for_unaliased}\").")
+                        }
+                    ),
+                    hints: vec![
+                        Hint::info(
+                            source_engine,
+                            existing_impl_span.clone(),
+                            format!("\"{decl_name}\" previously defined here.")
+                        )
+                    ],
+                    help: vec![
+                        "A type may not contain two or more declarations of the same name".to_string(),
+                    ],
+                }
+            },
             MarkerTraitExplicitlyImplemented { marker_trait_full_name, span} => Diagnostic {
                 reason: Some(Reason::new(code(1), "Marker traits cannot be explicitly implemented".to_string())),
                 issue: Issue::error(

test/src/e2e_vm_tests/test_programs/should_fail/impl_self_duplicated/test.toml

@@ -2,5 +2,9 @@ category = "fail"
 validate_abi = false
 expected_warnings = 3
 
-# check: fn foo(self) -> u64 {
-# nextln: $()Duplicate definitions for the method "foo" for type "S<T>".
+# check: $()Type contains duplicate declarations
+# check: $()fn foo(self) -> u64 {
+# check: $()"foo" previously defined here.
+# check: $()Method "foo" already declared in type "S<T>".
+
+# check: $()Aborting due to 1 error.

test/src/e2e_vm_tests/test_programs/should_fail/multiple_methods_with_the_same_name/test.toml

@@ -1,13 +1,27 @@
 category = "fail"
 
+# check: $()Type contains duplicate declarations
 # check: $()fn my_add(self, other: Self) -> Self {
-# check: $()Duplicate definitions for the method "my_add" for type "Data<T>".
+# check: $()"my_add" previously defined here.
+# check: $()fn my_add(self, other: Self) -> Self {
+# check: $()Method "my_add" already declared in type "Data<T>".
 
+# check: $()Type contains duplicate declarations
+# check: $()fn get_value(self) -> u64 {
+# check: $()"get_value" previously defined here.
 # check: $()fn get_value(self) -> u64 {
-# check: $()Duplicate definitions for the method "get_value" for type "Data<u64>".
+# check: $(Method "get_value" already declared in type "Data<u64>".
 
+# check: $()Type contains duplicate declarations
+# check: $()fn my_add(self, other: Self) -> Self {
+# check: $()"my_add" previously defined here.
 # check: $()fn my_add(self, other: Self) -> Self {
-# check: $()Duplicate definitions for the method "my_add" for type "Data<u64>".
+# check: $()Method "my_add" already declared in type "Data<u64>".
 
+# check: $()Type contains duplicate declarations
 # check: $()fn my_add(self, other: Self) -> Self {
-# check: $()Duplicate definitions for the method "my_add" for type "Data<u32>".
+# check: $()"my_add" previously defined here.
+# check: $()fn my_add(self, other: Self) -> Self {
+# check: $()Method "my_add" already declared in type "Data<u64>".
+
+# check: $()Aborting due to 6 errors

test/src/e2e_vm_tests/test_programs/should_fail/type_alias_shadowing_methods/test.toml

@@ -1,5 +1,7 @@
 category = "fail"
 
-#check: $()impl Alias2 {
-#nextln: $()fn foo() {}
-#nextln: $()Duplicate definitions for the method "foo" for type "MyStruct1".
+#check: $()Type contains duplicate declarations
+#check: $()fn foo() {}
+#nextln: $()"foo" previously defined here.
+#check: $()fn foo() {}
+#nextln: $()Method "foo" already declared in type "Alias2" (which is an alias for "MyStruct1").

test/src/e2e_vm_tests/test_programs/should_fail/type_alias_unification/Forc.lock

@@ -0,0 +1,13 @@
+[[package]]
+name = "core"
+source = "path+from-root-6E306998EEBF0DBD"
+
+[[package]]
+name = "std"
+source = "path+from-root-6E306998EEBF0DBD"
+dependencies = ["core"]
+
+[[package]]
+name = "type_alias_unification"
+source = "member"
+dependencies = ["std"]

test/src/e2e_vm_tests/test_programs/should_fail/type_alias_unification/Forc.toml

@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <contact@fuel.sh>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "type_alias_unification"
+
+[dependencies]
+std = { path = "../../../reduced_std_libs/sway-lib-std-assert" }

test/src/e2e_vm_tests/test_programs/should_fail/type_alias_unification/src/main.sw

@@ -0,0 +1,32 @@
+script;
+
+trait MyTrait {
+    fn extract_a(self) -> u64;
+}
+
+struct A {
+    a: u64,
+}
+
+impl MyTrait for A {
+    fn extract_a(self) -> u64 {
+        self.a
+    }
+}
+
+type B = A;
+
+// B is an alias for A, and A already has an implementation of MyTrait,
+// so this should cause a compilation error.
+impl MyTrait for B {
+    fn extract_a(self) -> u64 {
+        self.a + 1
+    }
+}
+
+fn main() {
+    let struct_a = A { a: 1 }; 
+    let struct_b = B { a: 42 };
+    assert(struct_a.extract_a() == 1);
+    assert(struct_b.extract_a() == 42);
+}

test/src/e2e_vm_tests/test_programs/should_fail/type_alias_unification/test.toml

@@ -0,0 +1,7 @@
+category = "fail"
+
+#check: $()Trait is already implemented for type
+#check: $()This is the already existing implementation of "MyTrait" for "B" (which is an alias for "A").
+#check: $()Trait "type_alias_unification::MyTrait" is already implemented for type "B" (which is an alias for "A").
+
+#check: $()Aborting due to 1 error.