fix(semantic): add diagnostic for duplicate var names in pattern

Author: Gilad Chase

crates/cairo-lang-semantic/src/diagnostic.rs

@@ -404,6 +404,9 @@ impl<'db> DiagnosticEntry<'db> for SemanticDiagnostic<'db> {
             SemanticDiagnosticKind::MissingVariableInPattern => {
                 "Missing variable in pattern.".into()
             }
+            SemanticDiagnosticKind::VariableDefinedMultipleTimesInPattern(name) => {
+                format!(r#"Redefinition of variable name "{}" in pattern."#, name.long(db))
+            }
             SemanticDiagnosticKind::StructMemberRedefinition { struct_id, member_name } => {
                 format!(
                     r#"Redefinition of member "{}" on struct "{}"."#,
@@ -1293,6 +1296,7 @@ pub enum SemanticDiagnosticKind<'db> {
     },
     VariableNotFound(SmolStrId<'db>),
     MissingVariableInPattern,
+    VariableDefinedMultipleTimesInPattern(SmolStrId<'db>),
     StructMemberRedefinition {
         struct_id: StructId<'db>,
         member_name: SmolStrId<'db>,

crates/cairo-lang-semantic/src/expr/compute.rs

@@ -1770,17 +1770,21 @@ fn compute_pattern_list_or_semantic<'db>(
     for (pattern_syntax, pattern) in patterns_syntax.elements(db).zip(patterns.iter()) {
         let variables = pattern.variables(&ctx.arenas.patterns);
 
-        if variables.len() != arm_patterns_variables.len() {
-            ctx.diagnostics.report(pattern_syntax.stable_ptr(db), MissingVariableInPattern);
-        }
-
+        let mut variable_names_in_pattern = UnorderedHashSet::<_>::default();
         for v in variables {
+            if !variable_names_in_pattern.insert(v.name) {
+                ctx.diagnostics.report(v.stable_ptr, VariableDefinedMultipleTimesInPattern(v.name));
+            }
             let var_def = Binding::LocalVar(v.var.clone());
             // TODO(spapini): Wrap this in a function to couple with semantic_defs
             // insertion.
             ctx.environment.variables.insert(v.name, var_def.clone());
             ctx.semantic_defs.insert(var_def.id(), var_def);
         }
+
+        if variable_names_in_pattern.len() != arm_patterns_variables.len() {
+            ctx.diagnostics.report(pattern_syntax.stable_ptr(db), MissingVariableInPattern);
+        }
     }
 
     patterns
@@ -2112,7 +2116,13 @@ fn compute_expr_for_semantic<'db>(
             &UnorderedHashMap::default(),
         );
         let variables = inner_pattern.variables(&new_ctx.arenas.patterns);
+        let mut variable_names_in_pattern = UnorderedHashSet::<_>::default();
         for v in variables {
+            if !variable_names_in_pattern.insert(v.name) {
+                new_ctx
+                    .diagnostics
+                    .report(v.stable_ptr, VariableDefinedMultipleTimesInPattern(v.name));
+            }
             let var_def = Binding::LocalVar(v.var.clone());
             new_ctx.environment.variables.insert(v.name, var_def.clone());
             new_ctx.semantic_defs.insert(var_def.id(), var_def);
@@ -4227,7 +4237,12 @@ pub fn compute_and_append_statement_semantic<'db>(
                 &UnorderedHashMap::default(),
             );
             let variables = pattern.variables(&ctx.arenas.patterns);
+            let mut variable_names_in_pattern = UnorderedHashSet::<_>::default();
             for v in variables {
+                if !variable_names_in_pattern.insert(v.name) {
+                    ctx.diagnostics
+                        .report(v.stable_ptr, VariableDefinedMultipleTimesInPattern(v.name));
+                }
                 let var_def = Binding::LocalVar(v.var.clone());
                 if let Some(old_var) = ctx.environment.variables.insert(v.name, var_def.clone()) {
                     if matches!(old_var, Binding::LocalItem(_)) {

crates/cairo-lang-semantic/src/expr/test_data/for

@@ -63,7 +63,7 @@ error[E0006]: Identifier not found.
 //! > Test for loop with duplicate variables in pattern.
 
 //! > test_runner_name
-test_function_diagnostics(expect_diagnostics: false)
+test_function_diagnostics(expect_diagnostics: true)
 
 //! > function
 fn foo() {
@@ -76,3 +76,7 @@ foo
 //! > module_code
 
 //! > expected_diagnostics
+error: Redefinition of variable name "_a" in pattern.
+ --> lib.cairo:2:14
+    for (_a, _a) in array![(1, 2)] {}
+             ^^

crates/cairo-lang-semantic/src/expr/test_data/pattern

@@ -276,7 +276,7 @@ error: Wrong number of tuple elements in pattern. Expected: 0. Got: 1.
 //! > Test duplicate variables in tuple pattern let.
 
 //! > test_runner_name
-test_function_diagnostics(expect_diagnostics: false)
+test_function_diagnostics(expect_diagnostics: true)
 
 //! > function
 fn foo() {
@@ -289,6 +289,10 @@ foo
 //! > module_code
 
 //! > expected_diagnostics
+error: Redefinition of variable name "_a" in pattern.
+ --> lib.cairo:2:14
+    let (_a, _a) = (1, 2);
+             ^^
 
 //! > ==========================================================================
 
@@ -314,12 +318,12 @@ foo
 //! > module_code
 
 //! > expected_diagnostics
-error: Missing variable in pattern.
- --> lib.cairo:3:9
+error: Redefinition of variable name "a" in pattern.
+ --> lib.cairo:3:13
         (a, a) => a,
-        ^^^^^^
+            ^
 
-error: Missing variable in pattern.
- --> lib.cairo:7:9
+error: Redefinition of variable name "b" in pattern.
+ --> lib.cairo:7:19
         ((b, _c), b) => b,
-        ^^^^^^^^^^^^
+                  ^