From 4a3f09b8457317dc0b5fdb71ad88820eab2ad980 Mon Sep 17 00:00:00 2001 From: xunilrj Date: Wed, 16 Aug 2023 12:43:01 +0100 Subject: [PATCH 1/7] improve parser recovery at the item level --- .../should_fail/recover_invalid_items/Forc.lock | 3 +++ .../should_fail/recover_invalid_items/Forc.toml | 6 ++++++ .../should_fail/recover_invalid_items/src/main.sw | 8 ++++++++ .../should_fail/recover_invalid_items/test.toml | 0 4 files changed, 17 insertions(+) create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.lock create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.toml create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw create mode 100644 test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.lock new file mode 100644 index 00000000000..226fc150475 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.lock @@ -0,0 +1,3 @@ +[[package]] +name = 'recover_invalid_items' +source = 'member' diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.toml new file mode 100644 index 00000000000..75b6b75d787 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/Forc.toml @@ -0,0 +1,6 @@ +[project] +name = "recover_invalid_items" +authors = ["Fuel Labs "] +entry = "main.sw" +license = "Apache-2.0" +implicit-std = false diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw new file mode 100644 index 00000000000..c352fbf574f --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw @@ -0,0 +1,8 @@ +script; + +random_words + +f9 + +fn main() { +} diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml new file mode 100644 index 00000000000..e69de29bb2d From bd11494dcb53d28a67ed35cd8d302adfd953bf0d Mon Sep 17 00:00:00 2001 From: xunilrj Date: Wed, 16 Aug 2023 12:43:05 +0100 Subject: [PATCH 2/7] improve parser recovery at the item level --- sway-ast/src/item/mod.rs | 5 ++ .../to_parsed_lang/convert_parse_tree.rs | 3 ++ sway-error/src/parser_error.rs | 2 + sway-parse/src/attribute.rs | 27 ++++++++++- sway-parse/src/item/mod.rs | 10 ++++ sway-parse/src/parse.rs | 25 +++++++++- sway-parse/src/parser.rs | 46 +++++++++++++++++-- 7 files changed, 110 insertions(+), 8 deletions(-) diff --git a/sway-ast/src/item/mod.rs b/sway-ast/src/item/mod.rs index f4c744ac564..e7059957df3 100644 --- a/sway-ast/src/item/mod.rs +++ b/sway-ast/src/item/mod.rs @@ -1,3 +1,5 @@ +use sway_error::handler::ErrorEmitted; + use crate::priv_prelude::*; pub mod item_abi; @@ -38,6 +40,8 @@ pub enum ItemKind { Storage(ItemStorage), Configurable(ItemConfigurable), TypeAlias(ItemTypeAlias), + // to handle parser recovery: Error represents an incomplete item + Error(Box<[Span]>, #[serde(skip_serializing)] ErrorEmitted), } impl Spanned for ItemKind { @@ -55,6 +59,7 @@ impl Spanned for ItemKind { ItemKind::Storage(item_storage) => item_storage.span(), ItemKind::Configurable(item_configurable) => item_configurable.span(), ItemKind::TypeAlias(item_type_alias) => item_type_alias.span(), + ItemKind::Error(spans, _) => Span::join_all(spans.iter().cloned()), } } } diff --git a/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs b/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs index 6af8612f1c1..e7a680005a1 100644 --- a/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs +++ b/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs @@ -222,6 +222,9 @@ fn item_to_ast_nodes( attributes, )?, )), + ItemKind::Error(spans, error) => { + vec![AstNodeContent::Error(spans, error)] + } }; Ok(contents diff --git a/sway-error/src/parser_error.rs b/sway-error/src/parser_error.rs index b402ea7de6b..2c34fe54a2d 100644 --- a/sway-error/src/parser_error.rs +++ b/sway-error/src/parser_error.rs @@ -24,6 +24,8 @@ pub enum ParseErrorKind { InvalidLiteralFieldName, #[error("Invalid statement.")] InvalidStatement, + #[error("Invalid item.")] + InvalidItem, #[error("Integer field names cannot have type suffixes.")] IntFieldWithTypeSuffix, #[error("Expected a field name.")] diff --git a/sway-parse/src/attribute.rs b/sway-parse/src/attribute.rs index be616b0884c..fb77dd63c81 100644 --- a/sway-parse/src/attribute.rs +++ b/sway-parse/src/attribute.rs @@ -55,6 +55,7 @@ impl Parse for Annotated { ), }); } + while let Some(attr) = parser.guarded_parse::()? { attribute_list.push(attr); } @@ -64,7 +65,18 @@ impl Parse for Annotated { Err(error) } else { // Parse the `T` value. - let value = parser.parse()?; + let value = match parser.parse_with_recovery() { + Ok(value) => value, + Err(r) => { + let (spans, error) = + r.recover_at_next_line_with_fallback_error(ParseErrorKind::InvalidItem); + if let Some(error) = T::error(spans, error) { + error + } else { + Err(error)? + } + } + }; Ok(Annotated { attribute_list, @@ -72,6 +84,19 @@ impl Parse for Annotated { }) } } + + fn error( + spans: Box<[sway_types::Span]>, + error: sway_error::handler::ErrorEmitted, + ) -> Option + where + Self: Sized, + { + T::error(spans, error).map(|value| Annotated { + attribute_list: vec![], + value, + }) + } } impl Parse for AttributeDecl { diff --git a/sway-parse/src/item/mod.rs b/sway-parse/src/item/mod.rs index b80c45b20c6..f887276eddd 100644 --- a/sway-parse/src/item/mod.rs +++ b/sway-parse/src/item/mod.rs @@ -76,6 +76,16 @@ impl Parse for ItemKind { Ok(kind) } + + fn error( + spans: Box<[sway_types::Span]>, + error: sway_error::handler::ErrorEmitted, + ) -> Option + where + Self: Sized, + { + Some(ItemKind::Error(spans, error)) + } } impl Parse for TypeField { diff --git a/sway-parse/src/parse.rs b/sway-parse/src/parse.rs index 3726502b567..fd6787eeb47 100644 --- a/sway-parse/src/parse.rs +++ b/sway-parse/src/parse.rs @@ -9,6 +9,16 @@ pub trait Parse { fn parse(parser: &mut Parser) -> ParseResult where Self: Sized; + + fn error( + _spans: Box<[sway_types::Span]>, + _error: sway_error::handler::ErrorEmitted, + ) -> Option + where + Self: Sized, + { + None + } } pub trait Peek { @@ -95,8 +105,19 @@ where if let Some(consumed) = parser.check_empty() { return Ok((ret, consumed)); } - let value = parser.parse()?; - ret.push(value); + + match parser.parse_with_recovery() { + Ok(value) => ret.push(value), + Err(r) => { + let (spans, error) = + r.recover_at_next_line_with_fallback_error(ParseErrorKind::InvalidItem); + if let Some(error) = T::error(spans, error) { + ret.push(error); + } else { + Err(error)? + } + } + } } } } diff --git a/sway-parse/src/parser.rs b/sway-parse/src/parser.rs index 2f529055822..be264c0e7e0 100644 --- a/sway-parse/src/parser.rs +++ b/sway-parse/src/parser.rs @@ -76,6 +76,39 @@ impl<'a, 'e> Parser<'a, 'e> { Peeker::with(self.token_trees).map(|(v, _)| v) } + pub fn parse_with_recovery<'original, T: Parse>( + &'original mut self, + ) -> Result> { + let handler = Handler::default(); + let mut fork = Parser { + token_trees: self.token_trees, + full_span: self.full_span.clone(), + handler: &handler, + }; + + match fork.parse() { + Ok(result) => { + self.token_trees = fork.token_trees; + self.handler.append(handler); + Ok(result) + } + Err(error) => { + let Parser { + token_trees, + full_span, + .. + } = fork; + Err(Recoverer { + original: RefCell::new(self), + handler, + fork_token_trees: token_trees, + fork_full_span: full_span, + error, + }) + } + } + } + /// This function do three things /// 1 - it peeks P; /// 2 - it forks the current parse, and try to parse @@ -376,7 +409,10 @@ impl<'original, 'a, 'e> Recoverer<'original, 'a, 'e> { &self, kind: ParseErrorKind, ) -> (Box<[Span]>, ErrorEmitted) { - let last_token_span = self.last_consumed_token().span(); + let last_token_span = self + .last_consumed_token() + .map(|x| x.span()) + .unwrap_or_else(|| self.fork_token_trees[0].span()); let last_token_span_line = last_token_span.start_pos().line_col().0; self.recover(|p| { p.consume_while_line_equals(last_token_span_line); @@ -413,15 +449,15 @@ impl<'original, 'a, 'e> Recoverer<'original, 'a, 'e> { /// This is the last consumed token of the forked parser. This the token /// immediately before the forked parser head. - pub fn last_consumed_token(&self) -> &GenericTokenTree { + pub fn last_consumed_token(&self) -> Option<&GenericTokenTree> { // find the last token consumed by the fork let original = self.original.borrow(); let fork_pos = original .token_trees .iter() - .position(|x| x.span() == self.fork_token_trees[0].span()) - .unwrap(); - &original.token_trees[fork_pos - 1] + .position(|x| x.span() == self.fork_token_trees[0].span())?; + let before_fork_pos = fork_pos.checked_sub(1)?; + original.token_trees.get(before_fork_pos) } /// This return a span encopassing all tokens that were consumed by the `p` since the start From 10feb3d47df9d716f47310f69b4cb906163a42c9 Mon Sep 17 00:00:00 2001 From: xunilrj Date: Wed, 16 Aug 2023 12:51:16 +0100 Subject: [PATCH 3/7] finishing lsp implementation --- sway-lsp/src/traverse/lexed_tree.rs | 1 + swayfmt/src/module/item.rs | 4 ++++ .../should_fail/recover_invalid_items/src/main.sw | 8 ++++++++ 3 files changed, 13 insertions(+) diff --git a/sway-lsp/src/traverse/lexed_tree.rs b/sway-lsp/src/traverse/lexed_tree.rs index 2620b4f221d..921c3d0d4bd 100644 --- a/sway-lsp/src/traverse/lexed_tree.rs +++ b/sway-lsp/src/traverse/lexed_tree.rs @@ -89,6 +89,7 @@ impl Parse for ItemKind { ItemKind::TypeAlias(item_type_alias) => { item_type_alias.parse(ctx); } + ItemKind::Error(_, _) => {} } } } diff --git a/swayfmt/src/module/item.rs b/swayfmt/src/module/item.rs index 00b9b550cf1..0c89cabefa4 100644 --- a/swayfmt/src/module/item.rs +++ b/swayfmt/src/module/item.rs @@ -23,6 +23,7 @@ impl Format for ItemKind { Storage(item_storage) => item_storage.format(formatted_code, formatter), Configurable(item_configurable) => item_configurable.format(formatted_code, formatter), TypeAlias(item_type_alias) => item_type_alias.format(formatted_code, formatter), + Error(_, _) => Ok(()), } } } @@ -42,6 +43,9 @@ impl LeafSpans for ItemKind { Use(item_use) => item_use.leaf_spans(), Configurable(item_configurable) => item_configurable.leaf_spans(), TypeAlias(item_type_alias) => item_type_alias.leaf_spans(), + Error(spans, _) => { + vec![sway_types::Span::join_all(spans.iter().cloned()).into()] + } } } } diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw index c352fbf574f..529ceb3b2b1 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/src/main.sw @@ -4,5 +4,13 @@ random_words f9 + +fn good1() { + +} + +fn a + fn main() { + good1(); } From c447777d1a57bd8094164c747e7bb927c83f5be3 Mon Sep 17 00:00:00 2001 From: xunilrj Date: Wed, 16 Aug 2023 14:19:55 +0100 Subject: [PATCH 4/7] fix test.toml --- .../should_fail/recover_invalid_items/test.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml index e69de29bb2d..01e43815c2f 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_invalid_items/test.toml @@ -0,0 +1,5 @@ +category = "fail" + +# check: $()Expected an item +# check: $()Expected an item +# check: $()Expected an opening parenthesis From 45762432e276303e7c4e26081860dd31fbe0fc6d Mon Sep 17 00:00:00 2001 From: xunilrj Date: Wed, 16 Aug 2023 15:22:04 +0100 Subject: [PATCH 5/7] avoid recover to panic --- sway-parse/src/parser.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/sway-parse/src/parser.rs b/sway-parse/src/parser.rs index be264c0e7e0..e420c983c83 100644 --- a/sway-parse/src/parser.rs +++ b/sway-parse/src/parser.rs @@ -409,13 +409,19 @@ impl<'original, 'a, 'e> Recoverer<'original, 'a, 'e> { &self, kind: ParseErrorKind, ) -> (Box<[Span]>, ErrorEmitted) { - let last_token_span = self - .last_consumed_token() - .map(|x| x.span()) - .unwrap_or_else(|| self.fork_token_trees[0].span()); - let last_token_span_line = last_token_span.start_pos().line_col().0; + let line = if self.fork_token_trees.is_empty() { + None + } else { + self.last_consumed_token() + .map(|x| x.span()) + .or_else(|| self.fork_token_trees.get(0).map(|x| x.span())) + .map(|x| x.start_pos().line_col().0) + }; + self.recover(|p| { - p.consume_while_line_equals(last_token_span_line); + if let Some(line) = line { + p.consume_while_line_equals(line); + } if !p.has_errors() { p.emit_error_with_span(kind, self.diff_span(p)); } @@ -450,12 +456,15 @@ impl<'original, 'a, 'e> Recoverer<'original, 'a, 'e> { /// This is the last consumed token of the forked parser. This the token /// immediately before the forked parser head. pub fn last_consumed_token(&self) -> Option<&GenericTokenTree> { + let fork_head_span = self.fork_token_trees.get(0)?.span(); + // find the last token consumed by the fork let original = self.original.borrow(); let fork_pos = original .token_trees .iter() - .position(|x| x.span() == self.fork_token_trees[0].span())?; + .position(|x| x.span() == fork_head_span)?; + let before_fork_pos = fork_pos.checked_sub(1)?; original.token_trees.get(before_fork_pos) } From 0261ddc063baff17348bc46e1978a73852bc0e92 Mon Sep 17 00:00:00 2001 From: xunilrj Date: Thu, 17 Aug 2023 17:46:54 +0100 Subject: [PATCH 6/7] test for unclosed comments --- sway-parse/src/parse.rs | 2 +- .../should_fail/recover_unclosed_multiline_comment/src/main.sw | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sway-parse/src/parse.rs b/sway-parse/src/parse.rs index fd6787eeb47..e0e72abb2c4 100644 --- a/sway-parse/src/parse.rs +++ b/sway-parse/src/parse.rs @@ -11,7 +11,7 @@ pub trait Parse { Self: Sized; fn error( - _spans: Box<[sway_types::Span]>, + #[allow(clippy::boxed_local)] _spans: Box<[sway_types::Span]>, _error: sway_error::handler::ErrorEmitted, ) -> Option where diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw index 486956ef293..456f402996d 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw @@ -1,5 +1,8 @@ contract; +/ +/ Comments + mod foo; mod bar; mod baz; From 0c9ce00da51fc2ccadab7f6e428b880f96767027 Mon Sep 17 00:00:00 2001 From: xunilrj Date: Thu, 17 Aug 2023 18:05:17 +0100 Subject: [PATCH 7/7] improvements to unfinished comments inside fns --- sway-parse/src/expr/mod.rs | 25 ++++++------ sway-parse/src/parser.rs | 38 +++++++++++++++++++ .../src/main.sw | 5 +++ 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/sway-parse/src/expr/mod.rs b/sway-parse/src/expr/mod.rs index 1869bb74625..16cb7cc951d 100644 --- a/sway-parse/src/expr/mod.rs +++ b/sway-parse/src/expr/mod.rs @@ -132,20 +132,29 @@ impl ParseToEnd for CodeBlockContents { mut parser: Parser<'a, '_>, ) -> ParseResult<(CodeBlockContents, ParserConsumed<'a>)> { let mut statements = Vec::new(); + let (final_expr_opt, consumed) = loop { if let Some(consumed) = parser.check_empty() { break (None, consumed); } - match parse_stmt(&mut parser)? { - StmtOrTail::Stmt(s) => statements.push(s), - StmtOrTail::Tail(e, c) => break (Some(e), c), + + match parser.parse_fn_with_recovery(parse_stmt) { + Ok(StmtOrTail::Stmt(s)) => statements.push(s), + Ok(StmtOrTail::Tail(e, c)) => break (Some(e), c), + Err(r) => { + let (spans, error) = r + .recover_at_next_line_with_fallback_error(ParseErrorKind::InvalidStatement); + statements.push(Statement::Error(spans, error)); + } } }; + let code_block_contents = CodeBlockContents { statements, final_expr_opt, span: parser.full_span().clone(), }; + Ok((code_block_contents, consumed)) } } @@ -187,14 +196,8 @@ fn parse_stmt<'a>(parser: &mut Parser<'a, '_>) -> ParseResult> { } // Try a `let` statement. - match parser.guarded_parse_with_recovery::() { - Ok(None) => {} - Ok(Some(item)) => return stmt(Statement::Let(item)), - Err(r) => { - let (spans, error) = - r.recover_at_next_line_with_fallback_error(ParseErrorKind::InvalidStatement); - return stmt(Statement::Error(spans, error)); - } + if let Some(item) = parser.guarded_parse::()? { + return stmt(Statement::Let(item)); } // Try an `expr;` statement. diff --git a/sway-parse/src/parser.rs b/sway-parse/src/parser.rs index e420c983c83..44a3b43f5e1 100644 --- a/sway-parse/src/parser.rs +++ b/sway-parse/src/parser.rs @@ -76,6 +76,44 @@ impl<'a, 'e> Parser<'a, 'e> { Peeker::with(self.token_trees).map(|(v, _)| v) } + pub fn parse_fn_with_recovery< + 'original, + T, + F: FnOnce(&mut Parser<'a, '_>) -> ParseResult, + >( + &'original mut self, + f: F, + ) -> Result> { + let handler = Handler::default(); + let mut fork = Parser { + token_trees: self.token_trees, + full_span: self.full_span.clone(), + handler: &handler, + }; + + match f(&mut fork) { + Ok(result) => { + self.token_trees = fork.token_trees; + self.handler.append(handler); + Ok(result) + } + Err(error) => { + let Parser { + token_trees, + full_span, + .. + } = fork; + Err(Recoverer { + original: RefCell::new(self), + handler, + fork_token_trees: token_trees, + fork_full_span: full_span, + error, + }) + } + } + } + pub fn parse_with_recovery<'original, T: Parse>( &'original mut self, ) -> Result> { diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw index 456f402996d..968b2586f1e 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_fail/recover_unclosed_multiline_comment/src/main.sw @@ -7,6 +7,11 @@ mod foo; mod bar; mod baz; +fn f() { + / + f2(); +} + fn a() -> bool { 0 } // recovery witness /*