From 3aed671d2fdca661fdef160b3e2468ce10eda028 Mon Sep 17 00:00:00 2001
From: Ary Borenszweig <asterite@gmail.com>
Date: Tue, 23 Jul 2024 15:24:53 -0300
Subject: [PATCH] fix: error on duplicate struct field (#5585)

# Description

## Problem

Resolves #5030

## Summary

We weren't checking for duplicate struct fields anywhere.

## Additional Context

None.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
---
 .../src/hir/def_collector/dc_mod.rs           | 28 ++++++++++++++++++-
 .../src/hir/def_collector/errors.rs           | 19 +++++++++++++
 compiler/noirc_frontend/src/tests.rs          | 28 +++++++++++++++++++
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index e5893dc43d5..1dca6ded786 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -262,7 +262,8 @@ impl<'a> ModCollector<'a> {
     }
 
     /// Collect any struct definitions declared within the ast.
-    /// Returns a vector of errors if any structs were already defined.
+    /// Returns a vector of errors if any structs were already defined,
+    /// or if a struct has duplicate fields in it.
     fn collect_structs(
         &mut self,
         context: &mut Context,
@@ -271,6 +272,8 @@ impl<'a> ModCollector<'a> {
     ) -> Vec<(CompilationError, FileId)> {
         let mut definition_errors = vec![];
         for struct_definition in types {
+            self.check_duplicate_field_names(&struct_definition, &mut definition_errors);
+
             let name = struct_definition.name.clone();
 
             let unresolved = UnresolvedStruct {
@@ -330,6 +333,29 @@ impl<'a> ModCollector<'a> {
         definition_errors
     }
 
+    fn check_duplicate_field_names(
+        &self,
+        struct_definition: &NoirStruct,
+        definition_errors: &mut Vec<(CompilationError, FileId)>,
+    ) {
+        let mut seen_field_names = std::collections::HashSet::new();
+        for (field_name, _) in &struct_definition.fields {
+            if seen_field_names.insert(field_name) {
+                continue;
+            }
+
+            let previous_field_name = *seen_field_names.get(field_name).unwrap();
+            definition_errors.push((
+                DefCollectorErrorKind::DuplicateField {
+                    first_def: previous_field_name.clone(),
+                    second_def: field_name.clone(),
+                }
+                .into(),
+                self.file_id,
+            ));
+        }
+    }
+
     /// Collect any type aliases definitions declared within the ast.
     /// Returns a vector of errors if any type aliases were already defined.
     fn collect_type_aliases(
diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs
index 1ccf8dd4792..9e9471c0cb3 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs
@@ -26,6 +26,8 @@ pub enum DuplicateType {
 pub enum DefCollectorErrorKind {
     #[error("duplicate {typ} found in namespace")]
     Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident },
+    #[error("duplicate struct field {first_def}")]
+    DuplicateField { first_def: Ident, second_def: Ident },
     #[error("unresolved import")]
     UnresolvedModuleDecl { mod_name: Ident, expected_path: String, alternative_path: String },
     #[error("overlapping imports")]
@@ -132,6 +134,23 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
                     diag
                 }
             }
+            DefCollectorErrorKind::DuplicateField { first_def, second_def } => {
+                let primary_message = format!(
+                    "Duplicate definitions of struct field with name {} found",
+                    &first_def.0.contents
+                );
+                {
+                    let first_span = first_def.0.span();
+                    let second_span = second_def.0.span();
+                    let mut diag = Diagnostic::simple_error(
+                        primary_message,
+                    "First definition found here".to_string(),
+                        first_span,
+                    );
+                    diag.add_secondary("Second definition found here".to_string(), second_span);
+                    diag
+                }
+            }
             DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path, alternative_path } => {
                 let span = mod_name.0.span();
                 let mod_name = &mod_name.0.contents;
diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs
index cbc15da20ff..c29d9b03c7a 100644
--- a/compiler/noirc_frontend/src/tests.rs
+++ b/compiler/noirc_frontend/src/tests.rs
@@ -2494,3 +2494,31 @@ fn bit_not_on_untyped_integer() {
     "#;
     assert_no_errors(src);
 }
+
+#[test]
+fn duplicate_struct_field() {
+    let src = r#"
+    struct Foo {
+        x: i32,
+        x: i32,
+    }
+
+    fn main() {}
+    "#;
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::DefinitionError(DefCollectorErrorKind::DuplicateField {
+        first_def,
+        second_def,
+    }) = &errors[0].0
+    else {
+        panic!("Expected a duplicate field error, got {:?}", errors[0].0);
+    };
+
+    assert_eq!(first_def.to_string(), "x");
+    assert_eq!(second_def.to_string(), "x");
+
+    assert_eq!(first_def.span().start(), 26);
+    assert_eq!(second_def.span().start(), 42);
+}