From e8120a1f979b89564a10c47115ec7dd9f035ddc1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 1 Apr 2025 14:54:48 -0300 Subject: [PATCH] feat(parser): improve error recovery when `fn` is missing between modifiers and name --- .../noirc_frontend/src/parser/parser/item.rs | 56 ++++++++++++++++++- .../src/parser/parser/modifiers.rs | 9 +++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser/item.rs b/compiler/noirc_frontend/src/parser/parser/item.rs index 47bee93f6ed..4a53664b182 100644 --- a/compiler/noirc_frontend/src/parser/parser/item.rs +++ b/compiler/noirc_frontend/src/parser/parser/item.rs @@ -219,7 +219,21 @@ impl<'a> Parser<'a> { )]; } - if self.eat_keyword(Keyword::Fn) { + let is_function = if self.eat_keyword(Keyword::Fn) { + true + } else if !modifiers.is_empty() + && matches!(self.token.token(), Token::Ident(..)) + && self.next_is(Token::LeftParen) + { + // If it's something like `pub foo(` then it's likely the user forgot to put `fn` after `pub`, + // so we error but keep parsing what comes next as a function. + self.expected_token(Token::Keyword(Keyword::Fn)); + true + } else { + false + }; + + if is_function { self.mutable_not_applicable(modifiers); return vec![ItemKind::Function(self.parse_function( @@ -250,7 +264,7 @@ mod tests { use crate::{ parse_program_with_dummy_file, parser::{ - ItemKind, + ItemKind, Parser, parser::tests::{get_single_error, get_source_with_error_span}, }, }; @@ -323,4 +337,42 @@ mod tests { assert_eq!(attributes[0].to_string(), "#[one]"); assert_eq!(attributes[1].to_string(), "#[two]"); } + + #[test] + fn error_recovery_for_missing_fn_between_visibility_and_name() { + let src = " + pub foo() { } + ^^^ + "; + let (src, span) = get_source_with_error_span(src); + let mut parser = Parser::for_str_with_dummy_file(&src); + let module = parser.parse_program(); + assert_eq!(module.items.len(), 1); + let ItemKind::Function(noir_function) = &module.items[0].kind else { + panic!("Expected function"); + }; + assert_eq!(noir_function.name(), "foo"); + + let reason = get_single_error(&parser.errors, span); + assert_eq!(reason.to_string(), "Expected a 'fn' but found 'foo'"); + } + + #[test] + fn error_recovery_for_missing_fn_between_unconstrained_and_name() { + let src = " + unconstrained foo() { } + ^^^ + "; + let (src, span) = get_source_with_error_span(src); + let mut parser = Parser::for_str_with_dummy_file(&src); + let module = parser.parse_program(); + assert_eq!(module.items.len(), 1); + let ItemKind::Function(noir_function) = &module.items[0].kind else { + panic!("Expected function"); + }; + assert_eq!(noir_function.name(), "foo"); + + let reason = get_single_error(&parser.errors, span); + assert_eq!(reason.to_string(), "Expected a 'fn' but found 'foo'"); + } } diff --git a/compiler/noirc_frontend/src/parser/parser/modifiers.rs b/compiler/noirc_frontend/src/parser/parser/modifiers.rs index 896a27c0416..cb78a79ed71 100644 --- a/compiler/noirc_frontend/src/parser/parser/modifiers.rs +++ b/compiler/noirc_frontend/src/parser/parser/modifiers.rs @@ -13,6 +13,15 @@ pub(crate) struct Modifiers { pub(crate) mutable: Option, } +impl Modifiers { + pub(crate) fn is_empty(&self) -> bool { + self.visibility == ItemVisibility::Private + && self.unconstrained.is_none() + && self.comptime.is_none() + && self.mutable.is_none() + } +} + impl Parser<'_> { /// Modifiers = ItemVisibility 'unconstrained'? 'comptime'? 'mut'? ///