fix(parser): do not use Ident::default()

compiler/noirc_frontend/src/ast/statement.rs

@@ -179,7 +179,7 @@ impl StatementKind {
     }
 }
 
-#[derive(Eq, Debug, Clone, Default)]
+#[derive(Eq, Debug, Clone)]
 pub struct Ident(Located<String>);
 
 impl PartialEq<Ident> for Ident {

compiler/noirc_frontend/src/parser/parser.rs

@@ -530,11 +530,14 @@ impl<'a> Parser<'a> {
     }
 
     fn location_at_previous_token_end(&self) -> Location {
-        Location::new(self.span_at_previous_token_end(), self.previous_token_location.file)
+        let span_at_previous_token_end = Span::from(
+            self.previous_token_location.span.end()..self.previous_token_location.span.end(),
+        );
+        Location::new(span_at_previous_token_end, self.previous_token_location.file)
     }
 
-    fn span_at_previous_token_end(&self) -> Span {
-        Span::from(self.previous_token_location.span.end()..self.previous_token_location.span.end())
+    fn unknown_ident_at_previous_token_end(&self) -> Ident {
+        Ident::new("(unknown)".to_string(), self.location_at_previous_token_end())
     }
 
     fn expected_identifier(&mut self) {

compiler/noirc_frontend/src/parser/parser/enums.rs

@@ -26,7 +26,7 @@ impl Parser<'_> {
         let Some(name) = self.eat_ident() else {
             self.expected_identifier();
             return self.empty_enum(
-                Ident::default(),
+                self.unknown_ident_at_previous_token_end(),
                 attributes,
                 visibility,
                 Vec::new(),
@@ -119,6 +119,8 @@ impl Parser<'_> {
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_snapshot;
+
     use crate::{
         ast::{IntegerBitSize, NoirEnumeration, UnresolvedGeneric, UnresolvedTypeData},
         parse_program_with_dummy_file,
@@ -258,6 +260,6 @@ mod tests {
 
         assert_eq!(errors.len(), 1);
         let error = &errors[0];
-        assert_eq!(error.to_string(), "Expected an identifier but found '42'");
+        assert_snapshot!(error.to_string(), @"Expected an identifier but found '42'");
     }
 }

compiler/noirc_frontend/src/parser/parser/expression.rs

@@ -704,7 +704,7 @@
     ///     = bool
     ///     | int
     ///     | str
     ///     | rawstr
     ///     | fmtstr
     ///     | QuoteExpression
     ///     | ArrayExpression
@@ -1015,6 +1015,7 @@
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_snapshot;
     use strum::IntoEnumIterator;
 
     use crate::{
@@ -1281,7 +1282,7 @@
         let mut parser = Parser::for_str_with_dummy_file(&src);
         parser.parse_expression();
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected an expression but found end of input");
+        assert_snapshot!(error.to_string(), @"Expected an expression but found end of input");
     }
 
     #[test]
@@ -1653,7 +1654,7 @@
         let expr = parser.parse_expression_or_error();
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a ':' but found '='");
+        assert_snapshot!(error.to_string(), @"Expected a ':' but found '='");
 
         let ExpressionKind::Constructor(mut constructor) = expr.kind else {
             panic!("Expected constructor");
@@ -1681,7 +1682,7 @@
         let expr = parser.parse_expression_or_error();
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a ':' but found '::'");
+        assert_snapshot!(error.to_string(), @"Expected a ':' but found '::'");
 
         let ExpressionKind::Constructor(mut constructor) = expr.kind else {
             panic!("Expected constructor");
@@ -1719,7 +1720,7 @@
         assert_eq!(expr.to_string(), "1");
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected an identifier but found '2'");
+        assert_snapshot!(error.to_string(), @"Expected an identifier but found '2'");
     }
 
     #[test]
@@ -1747,7 +1748,7 @@
         assert_eq!(expr.to_string(), "2");
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected an identifier but found '2'");
+        assert_snapshot!(error.to_string(), @"Expected an identifier but found '2'");
     }
 
     #[test]
@@ -1821,7 +1822,7 @@
         let mut parser = Parser::for_str_with_dummy_file(&src);
         parser.parse_expression();
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a type but found end of input");
+        assert_snapshot!(error.to_string(), @"Expected a type but found end of input");
     }
 
     #[test]

compiler/noirc_frontend/src/parser/parser/function.rs

@@ -87,9 +87,17 @@ impl Parser<'_> {
         allow_optional_body: bool,
         allow_self: bool,
     ) -> FunctionDefinitionWithOptionalBody {
-        let Some(name) = self.eat_ident() else {
+        let name = if let Some(name) = self.eat_ident() {
+            name
+        } else if self.at(Token::LeftParen) || self.at(Token::Less) {
+            // If it's `fn (...` or `fn <...` we assume the user missed the function name but a function
+            // definition follows. This can happen if the user is currently renaming a function by first
+            // erasing the name.
             self.expected_identifier();
-            return empty_function(self.previous_token_location);
+            self.unknown_ident_at_previous_token_end()
+        } else {
+            self.expected_identifier();
+            return empty_function(self.location_at_previous_token_end());
         };
 
         let generics = self.parse_generics_allowing_trait_bounds();
@@ -320,15 +328,14 @@ impl Parser<'_> {
 }
 
 fn empty_function(location: Location) -> FunctionDefinitionWithOptionalBody {
-    let span = Span::from(location.span.end()..location.span.end());
     FunctionDefinitionWithOptionalBody {
-        name: Ident::default(),
+        name: Ident::new(String::new(), location),
         generics: Vec::new(),
         parameters: Vec::new(),
         body: None,
-        location: Location::new(span, location.file),
+        location,
         where_clause: Vec::new(),
-        return_type: FunctionReturnType::Default(Location::new(span, location.file)),
+        return_type: FunctionReturnType::Default(location),
         return_visibility: Visibility::Private,
     }
 }
@@ -339,6 +346,8 @@ fn empty_body() -> BlockExpression {
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_snapshot;
+
     use crate::{
         ast::{
             ExpressionKind, IntegerBitSize, ItemVisibility, NoirFunction, StatementKind,
@@ -499,7 +508,7 @@ mod tests {
         assert_eq!(noir_function.parameters().len(), 1);
 
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a pattern but found '1'");
+        assert_snapshot!(error.to_string(), @"Expected a pattern but found '1'");
     }
 
     #[test]
@@ -535,7 +544,7 @@ mod tests {
         assert_eq!(noir_function.parameters().len(), 2);
 
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a type but found ','");
+        assert_snapshot!(error.to_string(), @"Expected a type but found ','");
     }
 
     #[test]
@@ -568,7 +577,7 @@ mod tests {
         let (src, span) = get_source_with_error_span(src);
         let (mut module, errors) = parse_program_with_dummy_file(&src);
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a type but found 'mut'");
+        assert_snapshot!(error.to_string(), @"Expected a type but found 'mut'");
 
         assert_eq!(module.items.len(), 1);
         let item = module.items.remove(0);
@@ -653,7 +662,7 @@ mod tests {
         let _ = parser.parse_program();
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Unexpected 'struct', expected one of 'where', '{', '->'");
+        assert_snapshot!(error.to_string(), @"Unexpected 'struct', expected one of 'where', '{', '->'");
     }
 
     #[test]
@@ -667,7 +676,7 @@ mod tests {
         let _ = parser.parse_program();
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Unexpected 'struct', expected one of 'where', '{'");
+        assert_snapshot!(error.to_string(), @"Unexpected 'struct', expected one of 'where', '{'");
     }
 
     #[test]
@@ -681,6 +690,20 @@ mod tests {
         let _ = parser.parse_program();
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a '{' but found 'struct'");
+        assert_snapshot!(error.to_string(), @"Expected a '{' but found 'struct'");
+    }
+
+    #[test]
+    fn errors_on_missing_function_name() {
+        let src = "
+          fn () {}
+             ^
+          ";
+        let (src, span) = get_source_with_error_span(src);
+        let mut parser = Parser::for_str_with_dummy_file(&src);
+        let _ = parser.parse_program();
+
+        let error = get_single_error(&parser.errors, span);
+        assert_snapshot!(error.to_string(), @"Expected an identifier but found '('");
     }
 }

compiler/noirc_frontend/src/parser/parser/global.rs

@@ -28,8 +28,9 @@ impl Parser<'_> {
         let Some(ident) = self.eat_ident() else {
             self.eat_semicolon();
             let location = self.location_at_previous_token_end();
+            let ident = self.unknown_ident_at_previous_token_end();
             return LetStatement {
-                pattern: ident_to_pattern(Ident::default(), mutable),
+                pattern: ident_to_pattern(ident, mutable),
                 r#type: UnresolvedType { typ: UnresolvedTypeData::Unspecified, location },
                 expression: Expression { kind: ExpressionKind::Error, location },
                 attributes,
@@ -68,6 +69,7 @@ fn ident_to_pattern(ident: Ident, mutable: bool) -> Pattern {
 #[cfg(test)]
 mod tests {
     use acvm::FieldElement;
+    use insta::assert_snapshot;
 
     use crate::{
         ast::{
@@ -168,7 +170,7 @@ mod tests {
         let (src, span) = get_source_with_error_span(src);
         let (_, errors) = parse_program_with_dummy_file(&src);
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a ';' but found end of input");
+        assert_snapshot!(error.to_string(), @"Expected a ';' but found end of input");
     }
 
     #[test]

compiler/noirc_frontend/src/parser/parser/impls.rs

@@ -2,7 +2,7 @@ use noirc_errors::Location;
 
 use crate::{
     ast::{
-        Documented, Expression, ExpressionKind, Ident, ItemVisibility, NoirFunction, NoirTraitImpl,
+        Documented, Expression, ExpressionKind, ItemVisibility, NoirFunction, NoirTraitImpl,
         TraitImplItem, TraitImplItemKind, TypeImpl, UnresolvedGeneric, UnresolvedType,
         UnresolvedTypeData,
     },
@@ -156,10 +156,9 @@ impl Parser<'_> {
             self.expected_identifier();
             self.eat_semicolons();
             let location = self.location_at_previous_token_end();
-            return Some(TraitImplItemKind::Type {
-                name: Ident::default(),
-                alias: UnresolvedType { typ: UnresolvedTypeData::Error, location },
-            });
+            let name = self.unknown_ident_at_previous_token_end();
+            let alias = UnresolvedType { typ: UnresolvedTypeData::Error, location };
+            return Some(TraitImplItemKind::Type { name, alias });
         };
 
         let alias = if self.eat_assign() {
@@ -184,7 +183,7 @@ impl Parser<'_> {
             Some(name) => name,
             None => {
                 self.expected_identifier();
-                Ident::default()
+                self.unknown_ident_at_previous_token_end()
             }
         };
 
@@ -235,6 +234,8 @@ impl Parser<'_> {
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_snapshot;
+
     use crate::{
         ast::{
             ItemVisibility, NoirTraitImpl, Pattern, TraitImplItemKind, TypeImpl, UnresolvedTypeData,
@@ -549,7 +550,7 @@ mod tests {
         assert_eq!(type_impl.methods.len(), 1);
 
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a function but found 'hello'");
+        assert_snapshot!(error.to_string(), @"Expected a function but found 'hello'");
     }
 
     #[test]
@@ -569,7 +570,7 @@ mod tests {
         assert_eq!(trait_imp.items.len(), 1);
 
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a trait impl item but found 'hello'");
+        assert_snapshot!(error.to_string(), @"Expected a trait impl item but found 'hello'");
     }
 
     #[test]

compiler/noirc_frontend/src/parser/parser/item.rs

@@ -153,7 +153,18 @@ impl<'a> Parser<'a> {
         if let Some(is_contract) = self.eat_mod_or_contract() {
             self.comptime_mutable_and_unconstrained_not_applicable(modifiers);
 
-            return vec![self.parse_mod_or_contract(attributes, is_contract, modifiers.visibility)];
+            if let Some(ident) = self.eat_ident() {
+                return vec![self.parse_mod_or_contract(
+                    ident,
+                    attributes,
+                    is_contract,
+                    modifiers.visibility,
+                )];
+            };
+
+            self.expected_identifier();
+            self.bump();
+            self.eat_semicolons();
         }
 
         if self.eat_keyword(Keyword::Struct) {
@@ -261,6 +272,8 @@ impl<'a> Parser<'a> {
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_snapshot;
+
     use crate::{
         parse_program_with_dummy_file,
         parser::{
@@ -279,7 +292,7 @@ mod tests {
         let (module, errors) = parse_program_with_dummy_file(&src);
         assert_eq!(module.items.len(), 2);
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected an item but found 'hello'");
+        assert_snapshot!(error.to_string(), @"Expected an item but found 'hello'");
     }
 
     #[test]
@@ -292,7 +305,7 @@ mod tests {
         let (module, errors) = parse_program_with_dummy_file(&src);
         assert_eq!(module.items.len(), 1);
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a '}' but found end of input");
+        assert_snapshot!(error.to_string(), @"Expected a '}' but found end of input");
     }
 
     #[test]
@@ -375,4 +388,17 @@ mod tests {
         let reason = get_single_error(&parser.errors, span);
         assert_eq!(reason.to_string(), "Expected a 'fn' but found 'foo'");
     }
+
+    #[test]
+    fn errors_on_missing_mod_identifier() {
+        let src = "
+        mod ; fn foo() {}
+            ^
+        ";
+        let (src, span) = get_source_with_error_span(src);
+        let (module, errors) = parse_program_with_dummy_file(&src);
+        assert_eq!(module.items.len(), 1);
+        let error = get_single_error(&errors, span);
+        assert_snapshot!(error.to_string(), @"Expected an identifier but found ';'");
+    }
 }