Repository: huff-language/huff-rs (rust)

Issue:
Compiler doesn't fail on "macro clash"
### Issue

When two macros with the same name are defined in file (or imported file), the compiler does not throw anything. It doesn't look desirable, because it's not even really clear which of the two macros will be used.

### Reproduction

Compile a file with two macros with the same name.

Pull request description:
Throw error on macro name clash
Resolves #291.

Tried running `cargo +nightly clippy --all --all-features -- -D warnings`,fails with some errors the `main` branch as well. 


Patch:
diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs
index 48e88c0..1bc5ad4 100644
--- a/huff_parser/src/lib.rs
+++ b/huff_parser/src/lib.rs
@@ -104,6 +104,7 @@ impl Parser {
                     TokenKind::Macro | TokenKind::Fn | TokenKind::Test => {
                         let m = self.parse_macro()?;
                         tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name);
+                        self.check_duplicate_macro(&contract, m.clone())?;
                         contract.macros.push(m);
                     }
                     TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => {
@@ -132,7 +133,7 @@ impl Parser {
                         TokenKind::Include
                     )),
                     spans: AstSpan(self.spans.clone()),
-                })
+                });
             }
         }
 
@@ -157,7 +158,7 @@ impl Parser {
                     kind: ParserErrorKind::InvalidName(tok.clone()),
                     hint: Some(format!("Expected import string. Got: \"{tok}\"")),
                     spans: AstSpan(new_spans),
-                })
+                });
             }
         };
 
@@ -185,6 +186,24 @@ impl Parser {
         std::mem::discriminant(&self.current_token.kind) == std::mem::discriminant(&kind)
     }
 
+    /// Checks if there is a duplicate macro name
+    pub fn check_duplicate_macro(
+        &self,
+        contract: &Contract,
+        m: MacroDefinition,
+    ) -> Result<(), ParserError> {
+        if contract.macros.binary_search_by(|_macro| _macro.name.cmp(&m.name)).is_ok() {
+            tracing::error!(target: "parser", "DUPLICATE MACRO NAME FOUND: {}",  m.name);
+            Err(ParserError {
+                kind: ParserErrorKind::DuplicateMacro(m.name),
+                hint: Some("MACRO names should be unique".to_string()),
+                spans: AstSpan(vec![self.spans[2].clone()]),
+            })
+        } else {
+            Ok(())
+        }
+    }
+
     /// Consumes the next token.
     pub fn consume(&mut self) {
         self.spans.push(self.current_token.span.clone());
@@ -197,7 +216,7 @@ impl Parser {
         loop {
             let token = self.peek().unwrap();
             if !kinds.contains(&token.kind) {
-                break
+                break;
             }
             self.current_token = token;
             self.cursor += 1;
@@ -238,7 +257,7 @@ impl Parser {
                     kind: ParserErrorKind::InvalidName(tok.clone()),
                     hint: Some(format!("Expected function name, found: \"{tok}\"")),
                     spans: AstSpan(self.spans.clone()),
-                })
+                });
             }
         };
 
@@ -300,7 +319,7 @@ impl Parser {
                     kind: ParserErrorKind::InvalidName(tok.clone()),
                     hint: Some(format!("Expected event name, found: \"{tok}\"")),
                     spans: AstSpan(self.spans.clone()),
-                })
+                });
             }
         };
 
@@ -331,7 +350,7 @@ impl Parser {
                     kind: ParserErrorKind::UnexpectedType(tok),
                     hint: Some("Expected constant name.".to_string()),
                     spans: AstSpan(self.spans.clone()),
-                })
+                });
             }
         };
 
@@ -356,7 +375,7 @@ impl Parser {
                             .to_string(),
                     ),
                     spans: AstSpan(vec![self.current_token.span.clone()]),
-                })
+                });
             }
         };
 
@@ -384,7 +403,7 @@ impl Parser {
                     kind: ParserErrorKind::UnexpectedType(tok),
                     hint: Some("Expected error name.".to_string()),
                     spans: AstSpan(self.spans.clone()),
-                })
+                });
             }
         };
 
@@ -431,7 +450,7 @@ impl Parser {
                                 ),
                                 hint: Some(format!("Expected string for decorator flag: {s}")),
                                 spans: AstSpan(vec![self.current_token.span.clone()]),
-                            })
+                            });
                         }
                     }
                     // The value flag accepts a single literal as an argument
@@ -447,7 +466,7 @@ impl Parser {
                                 ),
                                 hint: Some(format!("Expected literal for decorator flag: {s}")),
                                 spans: AstSpan(vec![self.current_token.span.clone()]),
-                            })
+                            });
                         }
                     }
                     Err(_) => {
@@ -456,7 +475,7 @@ impl Parser {
                             kind: ParserErrorKind::InvalidDecoratorFlag(s.clone()),
                             hint: Some(format!("Unknown decorator flag: {s}")),
                             spans: AstSpan(self.spans.clone()),
-                        })
+                        });
                     }
                 }
 
@@ -474,7 +493,7 @@ impl Parser {
                     )),
                     hint: Some(String::from("Unknown decorator flag")),
                     spans: AstSpan(self.spans.clone()),
-                })
+                });
             }
         }
 
@@ -683,7 +702,7 @@ impl Parser {
                         kind: ParserErrorKind::InvalidTokenInMacroBody(kind),
                         hint: None,
                         spans: AstSpan(vec![self.current_token.span.clone()]),
-                    })
+                    });
                 }
             };
         }
@@ -706,8 +725,8 @@ impl Parser {
     pub fn parse_label(&mut self) -> Result<Vec<Statement>, ParserError> {
         let mut statements: Vec<Statement> = Vec::new();
         self.match_kind(TokenKind::Colon)?;
-        while !self.check(TokenKind::Label("NEXT_LABEL".to_string())) &&
-            !self.check(TokenKind::CloseBrace)
+        while !self.check(TokenKind::Label("NEXT_LABEL".to_string()))
+            && !self.check(TokenKind::CloseBrace)
         {
             match self.current_token.kind.clone() {
                 TokenKind::Literal(val) => {
@@ -796,7 +815,7 @@ impl Parser {
                         kind: ParserErrorKind::InvalidTokenInLabelDefinition(kind),
                         hint: None,
                         spans: AstSpan(vec![self.current_token.span.clone()]),
-                    })
+                    });
                 }
             };
         }
@@ -848,7 +867,7 @@ impl Parser {
                         self.consume();
                         on_type = true;
                     }
-                    continue
+                    continue;
                 }
 
                 // Check for literals
@@ -868,7 +887,7 @@ impl Parser {
                         self.consume();
                         on_type = true;
                     }
-                    continue
+                    continue;
                 }
             }
 
@@ -909,9 +928,9 @@ impl Parser {
             }
 
             // name comes second (is optional)
-            if select_name &&
-                (self.check(TokenKind::Ident("x".to_string())) ||
-                    self.check(TokenKind::PrimitiveType(PrimitiveEVMType::Address)))
+            if select_name
+                && (self.check(TokenKind::Ident("x".to_string()))
+                    || self.check(TokenKind::PrimitiveType(PrimitiveEVMType::Address)))
             {
                 // We need to check if the name is a keyword - not the type
                 if !on_type {
@@ -927,7 +946,7 @@ impl Parser {
                                         "Argument names cannot be EVM types: {arg_str}"
                                     )),
                                     spans: AstSpan(vec![self.current_token.span.clone()]),
-                                })
+                                });
                             }
                         }
                         TokenKind::PrimitiveType(ty) => {
@@ -960,7 +979,7 @@ impl Parser {
                     kind: ParserErrorKind::InvalidArgs(self.current_token.kind.clone()),
                     hint: None,
                     spans: AstSpan(vec![self.current_token.span.clone()]),
-                })
+                });
             }
 
             arg.span = AstSpan(arg_spans);
@@ -1038,7 +1057,7 @@ impl Parser {
                                 .to_string(),
                         ),
                         spans: AstSpan(new_spans),
-                    })
+                    });
                 }
             }
             if self.check(TokenKind::Comma) {
@@ -1085,8 +1104,8 @@ impl Parser {
                             0_usize
                         }
                     })
-                    .sum::<usize>() /
-                    2
+                    .sum::<usize>()
+                    / 2
             }
         };
 
@@ -1126,7 +1145,7 @@ impl Parser {
                                     ),
                                     hint: Some("Expected valid hex bytecode.".to_string()),
                                     spans: AstSpan(new_spans),
-                                })
+                                });
                             }
                         } else {
                             StatementType::LabelCall(ident_str.to_string())
@@ -1141,7 +1160,7 @@ impl Parser {
                         kind: ParserErrorKind::InvalidTableBodyToken(kind.clone()),
                         hint: Some("Expected an identifier string.".to_string()),
                         spans: AstSpan(new_spans),
-                    })
+                    });
                 }
             };
         }
@@ -1246,7 +1265,7 @@ impl Parser {
                         kind: ParserErrorKind::InvalidUint256(size),
                         hint: None,
                         spans: AstSpan(vec![self.current_token.span.clone()]),
-                    })
+                    });
                 }
                 Ok(self.match_kind(self.current_token.kind.clone())?)
             }
@@ -1256,7 +1275,7 @@ impl Parser {
                         kind: ParserErrorKind::InvalidBytes(size),
                         hint: None,
                         spans: AstSpan(vec![self.current_token.span.clone()]),
-                    })
+                    });
                 }
                 Ok(self.match_kind(self.current_token.kind.clone())?)
             }
@@ -1270,7 +1289,7 @@ impl Parser {
                         kind: ParserErrorKind::InvalidInt(size),
                         hint: None,
                         spans: AstSpan(vec![self.current_token.span.clone()]),
-                    })
+                    });
                 }
                 let curr_token_kind = self.current_token.kind.clone();
                 self.consume();
diff --git a/huff_utils/src/error.rs b/huff_utils/src/error.rs
index 9ecb263..b1d1af8 100644
--- a/huff_utils/src/error.rs
+++ b/huff_utils/src/error.rs
@@ -63,6 +63,8 @@ pub enum ParserErrorKind {
     InvalidDecoratorFlag(String),
     /// Invalid decorator flag argument
     InvalidDecoratorFlagArg(TokenKind),
+    /// Duplicate MACRO
+    DuplicateMacro(String),
 }
 
 /// A Lexing Error
@@ -488,6 +490,14 @@ impl fmt::Display for CompilerError {
                         pe.spans.error(pe.hint.as_ref())
                     )
                 }
+                ParserErrorKind::DuplicateMacro(mn) => {
+                    write!(
+                        f,
+                        "\nError: Duplicate MACRO name found: \"{}\" \n{}\n",
+                        mn,
+                        pe.spans.error(pe.hint.as_ref())
+                    )
+                }
             },
             CompilerError::PathBufRead(os_str) => {
                 write!(


Test patch:
diff --git a/huff_parser/tests/macro.rs b/huff_parser/tests/macro.rs
index bc34e90..a201799 100644
--- a/huff_parser/tests/macro.rs
+++ b/huff_parser/tests/macro.rs
@@ -1232,3 +1232,67 @@ fn empty_test_with_multi_flag_decorator() {
     assert_eq!(macro_definition, expected);
     assert_eq!(parser.current_token.kind, TokenKind::Eof);
 }
+
+#[test]
+fn test_duplicate_macro_error() {
+    let source = r#"
+    #define macro CONSTRUCTOR() = takes(0) returns (0) {}
+
+    #define macro MINT() = takes(0) returns (0) {
+        0x04 calldataload   // [to]
+        0x00                // [from (0x00), to]
+        0x24 calldataload   // [value, from, to]
+    }
+
+    #define macro MINT() = takes(0) returns (0) {
+        0x04 calldataload   // [to]
+        0x00                // [from (0x00), to]
+        0x24 calldataload   // [value, from, to]
+    }
+
+    #define macro MAIN() = takes(0) returns (0) {
+        0x00 calldataload 0xE0 shr
+        dup1 0x40c10f19 eq mints jumpi
+
+        mints:
+            MINT()
+    }
+    "#;
+
+    let mut start = 0;
+    let target = "MINT";
+    let target_len = target.len();
+    let mut occurrences = Vec::new();
+
+    while let Some(pos) = source[start..].find(target) {
+        let adjusted_start = start + pos;
+        let adjusted_end = adjusted_start + target_len - 1;
+
+        occurrences.push((adjusted_start, adjusted_end));
+        start = adjusted_end + 1;
+    }
+
+    let full_source = FullFileSource { source, file: None, spans: vec![] };
+    let lexer = Lexer::new(full_source.source);
+    let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::<Vec<Token>>();
+    let mut parser = Parser::new(tokens, Some("".to_string()));
+
+    // This should be caught before codegen invalid macro statement
+    match parser.parse() {
+        Ok(_) => panic!("moose"),
+        Err(e) => {
+            assert_eq!(
+                e,
+                ParserError {
+                    kind: ParserErrorKind::DuplicateMacro("MINT".to_string()),
+                    hint: Some("MACRO names should be unique".to_string()),
+                    spans: AstSpan(vec![Span {
+                        start: occurrences[1].0,
+                        end: occurrences[1].1,
+                        file: None
+                    }]),
+                }
+            )
+        }
+    }
+}