feat: slightly improve "unexpected token" error message (#6279)

# Description ## Problem Tokens appear like `]` or `)` in error messages where showing them as `']'` or `')'` might be clearer. ## Summary ## Additional Context ## 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.
noir-lang · Oct 10, 2024 · 8232bfa · 8232bfa
1 parent 8d8ea89
commit 8232bfa
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 42 deletions.
diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs
@@ -174,12 +174,17 @@ impl ParserError {
 
 impl std::fmt::Display for ParserError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let token_to_string = |token: &Token| match token {
+            Token::EOF => token.to_string(),
+            _ => format!("'{}'", token),
+        };
+
         let reason_str: String = if self.reason.is_none() {
             "".to_string()
         } else {
             format!("\nreason: {}", Diagnostic::from(self))
         };
-        let mut expected = vecmap(&self.expected_tokens, ToString::to_string);
+        let mut expected = vecmap(&self.expected_tokens, token_to_string);
         expected.append(&mut vecmap(&self.expected_labels, |label| format!("{label}")));
 
         if expected.is_empty() {
@@ -192,7 +197,7 @@ impl std::fmt::Display for ParserError {
                 "Expected a{} {} but found {}{}",
                 if vowel { "n" } else { "" },
                 first,
-                self.found,
+                token_to_string(&self.found),
                 reason_str
             )
         } else {

diff --git a/compiler/noirc_frontend/src/parser/labels.rs b/compiler/noirc_frontend/src/parser/labels.rs
@@ -11,6 +11,7 @@ pub enum ParsingRuleLabel {
     Cast,
     Expression,
     FieldAccess,
+    Function,
     GenericParameter,
     Global,
     Identifier,
@@ -41,6 +42,7 @@ impl fmt::Display for ParsingRuleLabel {
             ParsingRuleLabel::Cast => write!(f, "cast"),
             ParsingRuleLabel::Expression => write!(f, "expression"),
             ParsingRuleLabel::FieldAccess => write!(f, "field access"),
+            ParsingRuleLabel::Function => write!(f, "function"),
             ParsingRuleLabel::GenericParameter => write!(f, "generic parameter"),
             ParsingRuleLabel::Global => write!(f, "global"),
             ParsingRuleLabel::Identifier => write!(f, "identifier"),

diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs
@@ -1354,7 +1354,7 @@ mod tests {
         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_eq!(error.to_string(), "Expected a ':' but found '='");
 
         let ExpressionKind::Constructor(mut constructor) = expr.kind else {
             panic!("Expected constructor");

diff --git a/compiler/noirc_frontend/src/parser/parser/function.rs b/compiler/noirc_frontend/src/parser/parser/function.rs
@@ -442,7 +442,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_eq!(error.to_string(), "Expected a pattern but found '1'");
     }
 
     #[test]
@@ -478,7 +478,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_eq!(error.to_string(), "Expected a type but found ','");
     }
 
     #[test]

diff --git a/compiler/noirc_frontend/src/parser/parser/global.rs b/compiler/noirc_frontend/src/parser/parser/global.rs
@@ -163,6 +163,6 @@ mod tests {
         let (src, span) = get_source_with_error_span(src);
         let (_, errors) = parse_program(&src);
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a ; but found end of input");
+        assert_eq!(error.to_string(), "Expected a ';' but found end of input");
     }
 }
diff --git a/compiler/noirc_frontend/src/parser/parser/impls.rs b/compiler/noirc_frontend/src/parser/parser/impls.rs
@@ -7,7 +7,7 @@ use crate::{
         UnresolvedGeneric, UnresolvedType, UnresolvedTypeData,
     },
     parser::{labels::ParsingRuleLabel, ParserErrorReason},
-    token::{Keyword, Token, TokenKind},
+    token::{Keyword, Token},
 };
 
 use super::{parse_many::without_separator, Parser};
@@ -79,31 +79,28 @@ impl<'a> Parser<'a> {
     }
 
     fn parse_type_impl_method(&mut self) -> Option<(Documented<NoirFunction>, Span)> {
-        self.parse_item_in_list(
-            ParsingRuleLabel::TokenKind(TokenKind::Token(Token::Keyword(Keyword::Fn))),
-            |parser| {
-                let doc_comments = parser.parse_outer_doc_comments();
-                let start_span = parser.current_token_span;
-                let attributes = parser.parse_attributes();
-                let modifiers = parser.parse_modifiers(
-                    false, // allow mutable
-                );
+        self.parse_item_in_list(ParsingRuleLabel::Function, |parser| {
+            let doc_comments = parser.parse_outer_doc_comments();
+            let start_span = parser.current_token_span;
+            let attributes = parser.parse_attributes();
+            let modifiers = parser.parse_modifiers(
+                false, // allow mutable
+            );
 
-                if parser.eat_keyword(Keyword::Fn) {
-                    let method = parser.parse_function(
-                        attributes,
-                        modifiers.visibility,
-                        modifiers.comptime.is_some(),
-                        modifiers.unconstrained.is_some(),
-                        true, // allow_self
-                    );
-                    Some((Documented::new(method, doc_comments), parser.span_since(start_span)))
-                } else {
-                    parser.modifiers_not_followed_by_an_item(modifiers);
-                    None
-                }
-            },
-        )
+            if parser.eat_keyword(Keyword::Fn) {
+                let method = parser.parse_function(
+                    attributes,
+                    modifiers.visibility,
+                    modifiers.comptime.is_some(),
+                    modifiers.unconstrained.is_some(),
+                    true, // allow_self
+                );
+                Some((Documented::new(method, doc_comments), parser.span_since(start_span)))
+            } else {
+                parser.modifiers_not_followed_by_an_item(modifiers);
+                None
+            }
+        })
     }
 
     /// TraitImpl = 'impl' Generics Path GenericTypeArgs 'for' Type TraitImplBody
@@ -542,7 +539,7 @@ mod tests {
         assert_eq!(type_impl.methods.len(), 1);
 
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected a fn but found hello");
+        assert_eq!(error.to_string(), "Expected a function but found 'hello'");
     }
 
     #[test]
@@ -562,6 +559,6 @@ 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_eq!(error.to_string(), "Expected a trait impl item but found 'hello'");
     }
 }
diff --git a/compiler/noirc_frontend/src/parser/parser/item.rs b/compiler/noirc_frontend/src/parser/parser/item.rs
@@ -224,7 +224,7 @@ mod tests {
         let (module, errors) = parse_program(&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_eq!(error.to_string(), "Expected an item but found 'hello'");
     }
 
     #[test]
@@ -237,6 +237,6 @@ mod tests {
         let (module, errors) = parse_program(&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_eq!(error.to_string(), "Expected a '}' but found end of input");
     }
 }
diff --git a/compiler/noirc_frontend/src/parser/parser/item_visibility.rs b/compiler/noirc_frontend/src/parser/parser/item_visibility.rs
@@ -73,7 +73,7 @@ mod tests {
         let visibility = parser.parse_item_visibility();
         assert_eq!(visibility, ItemVisibility::Public);
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a crate but found end of input");
+        assert_eq!(error.to_string(), "Expected a 'crate' but found end of input");
     }
 
     #[test]
@@ -87,7 +87,7 @@ mod tests {
         let visibility = parser.parse_item_visibility();
         assert_eq!(visibility, ItemVisibility::Public);
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a crate but found hello");
+        assert_eq!(error.to_string(), "Expected a 'crate' but found 'hello'");
     }
     #[test]
     fn parses_public_visibility_missing_paren_after_pub_crate() {
@@ -100,7 +100,7 @@ mod tests {
         let visibility = parser.parse_item_visibility();
         assert_eq!(visibility, ItemVisibility::PublicCrate);
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a ) but found end of input");
+        assert_eq!(error.to_string(), "Expected a ')' but found end of input");
     }
 
     #[test]

diff --git a/compiler/noirc_frontend/src/parser/parser/pattern.rs b/compiler/noirc_frontend/src/parser/parser/pattern.rs
@@ -359,7 +359,7 @@ mod tests {
         let pattern = parser.parse_pattern_or_error();
 
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a : but found =");
+        assert_eq!(error.to_string(), "Expected a ':' but found '='");
 
         let Pattern::Struct(path, mut patterns, _) = pattern else {
             panic!("Expected a struct pattern")

diff --git a/compiler/noirc_frontend/src/parser/parser/statement.rs b/compiler/noirc_frontend/src/parser/parser/statement.rs
@@ -702,7 +702,7 @@ mod tests {
         let statement = parser.parse_statement_or_error();
         assert!(matches!(statement.kind, StatementKind::Let(..)));
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a statement but found ]");
+        assert_eq!(error.to_string(), "Expected a statement but found ']'");
     }
 
     #[test]

diff --git a/compiler/noirc_frontend/src/parser/parser/structs.rs b/compiler/noirc_frontend/src/parser/parser/structs.rs
@@ -272,6 +272,6 @@ mod tests {
         assert_eq!(noir_struct.fields.len(), 1);
 
         let error = get_single_error(&errors, span);
-        assert_eq!(error.to_string(), "Expected an identifier but found 42");
+        assert_eq!(error.to_string(), "Expected an identifier but found '42'");
     }
 }
diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs
@@ -588,7 +588,7 @@ mod tests {
         let mut parser = Parser::for_str(&src);
         parser.parse_type();
         let error = get_single_error(&parser.errors, span);
-        assert_eq!(error.to_string(), "Expected a ] but found end of input");
+        assert_eq!(error.to_string(), "Expected a ']' but found end of input");
     }
 
     #[test]