From 8232bfaf0a88dcba5a6949489b81d78c3413c5bc Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 10 Oct 2024 17:51:31 -0300 Subject: [PATCH] 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. --- compiler/noirc_frontend/src/parser/errors.rs | 9 +++- compiler/noirc_frontend/src/parser/labels.rs | 2 + .../src/parser/parser/expression.rs | 2 +- .../src/parser/parser/function.rs | 4 +- .../src/parser/parser/global.rs | 2 +- .../noirc_frontend/src/parser/parser/impls.rs | 51 +++++++++---------- .../noirc_frontend/src/parser/parser/item.rs | 4 +- .../src/parser/parser/item_visibility.rs | 6 +-- .../src/parser/parser/pattern.rs | 2 +- .../src/parser/parser/statement.rs | 2 +- .../src/parser/parser/structs.rs | 2 +- .../noirc_frontend/src/parser/parser/types.rs | 2 +- 12 files changed, 46 insertions(+), 42 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index f9cc539d7b7..3315b38a351 100644 --- 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 index 5c9ec236e07..604c87801f8 100644 --- 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 index 1a150d881de..93bb4980801 100644 --- 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 index acec7942e24..a60bc6e7c1d 100644 --- 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 index 2ea6457dc0b..d24064673b1 100644 --- 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 index 460c7bded15..9215aec2742 100644 --- 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, 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 index 0faa2ba80ee..4fbcd7abac5 100644 --- 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 index 1731284e354..5aea5f6a45f 100644 --- 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 index 72dc0f2ea07..c4dcab55d73 100644 --- 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 index d118be5d54a..ff6117c9348 100644 --- 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 index 5514a33ef0d..da8ac64e021 100644 --- 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 index 42fae40f669..be3d5287cab 100644 --- 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]