Parser recovery inside trait/abi (#4979)

## Description

Closes #4729.

The trick part of this PR is that `TraitItems` were being parsed as
`(TraitItem, SemicolonToken)`. The problem is that to recover from an
error we call the `error` method of the `Parser` trait. And it is very
hard to come up with a generic implementation for the tuple. Which type
of tuple do we call its `error` method, and how do we return a valid
tuple in all cases? Impossible.

So that is why I moved the `SemicolonToken` to inside the `TraitItem`.
Another benefit is that we can recover from missing semicolons with nice
error messages.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

sway-ast/src/item/item_abi.rs

@@ -5,7 +5,7 @@ pub struct ItemAbi {
     pub abi_token: AbiToken,
     pub name: Ident,
     pub super_traits: Option<(ColonToken, Traits)>,
-    pub abi_items: Braces<Vec<(Annotated<ItemTraitItem>, SemicolonToken)>>,
+    pub abi_items: Braces<Vec<Annotated<ItemTraitItem>>>,
     pub abi_defs_opt: Option<Braces<Vec<Annotated<ItemFn>>>>,
 }
 

sway-ast/src/item/item_trait.rs

@@ -1,9 +1,13 @@
+use sway_error::handler::ErrorEmitted;
+
 use crate::priv_prelude::*;
 
 #[derive(Clone, Debug, Serialize)]
 pub enum ItemTraitItem {
-    Fn(FnSignature),
-    Const(ItemConst),
+    Fn(FnSignature, Option<SemicolonToken>),
+    Const(ItemConst, Option<SemicolonToken>),
+    // to handle parser recovery: Error represents an incomplete trait item
+    Error(Box<[Span]>, #[serde(skip_serializing)] ErrorEmitted),
 }
 
 #[derive(Clone, Debug, Serialize)]
@@ -14,7 +18,7 @@ pub struct ItemTrait {
     pub generics: Option<GenericParams>,
     pub where_clause_opt: Option<WhereClause>,
     pub super_traits: Option<(ColonToken, Traits)>,
-    pub trait_items: Braces<Vec<(Annotated<ItemTraitItem>, SemicolonToken)>>,
+    pub trait_items: Braces<Vec<Annotated<ItemTraitItem>>>,
     pub trait_defs_opt: Option<Braces<Vec<Annotated<ItemFn>>>>,
 }
 
@@ -35,8 +39,17 @@ impl Spanned for ItemTrait {
 impl Spanned for ItemTraitItem {
     fn span(&self) -> Span {
         match self {
-            ItemTraitItem::Fn(fn_decl) => fn_decl.span(),
-            ItemTraitItem::Const(const_decl) => const_decl.span(),
+            ItemTraitItem::Fn(fn_decl, semicolon) => match semicolon.as_ref().map(|x| x.span()) {
+                Some(semicolon) => Span::join(fn_decl.span(), semicolon),
+                None => fn_decl.span(),
+            },
+            ItemTraitItem::Const(const_decl, semicolon) => {
+                match semicolon.as_ref().map(|x| x.span()) {
+                    Some(semicolon) => Span::join(const_decl.span(), semicolon),
+                    None => const_decl.span(),
+                }
+            }
+            ItemTraitItem::Error(spans, _) => Span::join_all(spans.iter().cloned()),
         }
     }
 }

sway-core/src/language/parsed/declaration/trait.rs

@@ -5,12 +5,15 @@ use super::{ConstantDeclaration, FunctionDeclaration, FunctionParameter};
 use crate::{
     decl_engine::DeclRefTrait, engine_threading::*, language::*, transform, type_system::*,
 };
+use sway_error::handler::ErrorEmitted;
 use sway_types::{ident::Ident, span::Span, Spanned};
 
 #[derive(Debug, Clone)]
 pub enum TraitItem {
     TraitFn(TraitFn),
     Constant(ConstantDeclaration),
+    // to handle parser recovery: Error represents an incomplete trait item
+    Error(Box<[Span]>, ErrorEmitted),
 }
 
 #[derive(Debug, Clone)]

sway-core/src/semantic_analysis/ast_node/declaration/abi.rs

@@ -133,6 +133,9 @@ impl ty::TyAbiDecl {
 
                     const_name
                 }
+                TraitItem::Error(_, _) => {
+                    continue;
+                }
             };
 
             if !ids.insert(decl_name.clone()) {

sway-core/src/semantic_analysis/ast_node/declaration/trait.rs

@@ -112,6 +112,9 @@ impl ty::TyTraitDecl {
 
                     const_name
                 }
+                TraitItem::Error(_, _) => {
+                    continue;
+                }
             };
 
             if !ids.insert(decl_name.clone()) {

sway-core/src/semantic_analysis/node_dependencies.rs

@@ -345,6 +345,7 @@ impl Dependencies {
                     TraitItem::Constant(const_decl) => {
                         deps.gather_from_constant_decl(engines, const_decl)
                     }
+                    TraitItem::Error(_, _) => deps,
                 })
                 .gather_from_iter(methods.iter(), |deps, fn_decl| {
                     deps.gather_from_fn_decl(engines, fn_decl)
@@ -395,6 +396,7 @@ impl Dependencies {
                     TraitItem::Constant(const_decl) => {
                         deps.gather_from_constant_decl(engines, const_decl)
                     }
+                    TraitItem::Error(_, _) => deps,
                 })
                 .gather_from_iter(methods.iter(), |deps, fn_decl| {
                     deps.gather_from_fn_decl(engines, fn_decl)

sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs

@@ -586,20 +586,21 @@ fn item_trait_to_trait_declaration(
         .trait_items
         .into_inner()
         .into_iter()
-        .map(|(annotated, _)| {
+        .map(|annotated| {
             let attributes = item_attrs_to_map(context, handler, &annotated.attribute_list)?;
             if !cfg_eval(context, handler, &attributes)? {
                 return Ok(None);
             }
             Ok(Some(match annotated.value {
-                ItemTraitItem::Fn(fn_sig) => {
+                ItemTraitItem::Fn(fn_sig, _) => {
                     fn_signature_to_trait_fn(context, handler, engines, fn_sig, attributes)
                         .map(TraitItem::TraitFn)
                 }
-                ItemTraitItem::Const(const_decl) => item_const_to_constant_declaration(
+                ItemTraitItem::Const(const_decl, _) => item_const_to_constant_declaration(
                     context, handler, engines, const_decl, attributes, false,
                 )
                 .map(TraitItem::Constant),
+                ItemTraitItem::Error(spans, error) => Ok(TraitItem::Error(spans, error)),
             }?))
         })
         .filter_map_ok(|item| item)
@@ -759,14 +760,14 @@ fn item_abi_to_abi_declaration(
                 .abi_items
                 .into_inner()
                 .into_iter()
-                .map(|(annotated, _)| {
+                .map(|annotated| {
                     let attributes =
                         item_attrs_to_map(context, handler, &annotated.attribute_list)?;
                     if !cfg_eval(context, handler, &attributes)? {
                         return Ok(None);
                     }
                     Ok(Some(match annotated.value {
-                        ItemTraitItem::Fn(fn_signature) => {
+                        ItemTraitItem::Fn(fn_signature, _) => {
                             let trait_fn = fn_signature_to_trait_fn(
                                 context,
                                 handler,
@@ -783,10 +784,11 @@ fn item_abi_to_abi_declaration(
                             )?;
                             Ok(TraitItem::TraitFn(trait_fn))
                         }
-                        ItemTraitItem::Const(const_decl) => item_const_to_constant_declaration(
+                        ItemTraitItem::Const(const_decl, _) => item_const_to_constant_declaration(
                             context, handler, engines, const_decl, attributes, false,
                         )
                         .map(TraitItem::Constant),
+                        ItemTraitItem::Error(spans, error) => Ok(TraitItem::Error(spans, error)),
                     }?))
                 })
                 .filter_map_ok(|item| item)

sway-error/src/error.rs

@@ -286,7 +286,6 @@ pub enum CompileError {
     },
     #[error("This opcode takes an immediate value but none was provided.")]
     MissingImmediate { span: Span },
-
     #[error("This immediate value is invalid.")]
     InvalidImmediateValue { span: Span },
     #[error("Variant \"{variant_name}\" does not exist on enum \"{enum_name}\"")]

sway-error/src/parser_error.rs

@@ -8,7 +8,7 @@ pub enum ParseErrorKind {
     ExpectedImportNameGroupOrGlob,
     #[error("Expected an item.")]
     ExpectedAnItem,
-    #[error("Expected item after doc comment.")]
+    #[error("Expected an item after doc comment.")]
     ExpectedAnItemAfterDocComment,
     #[error("Expected a comma or closing parenthesis in function arguments.")]
     ExpectedCommaOrCloseParenInFnArgs,

sway-lsp/src/traverse/lexed_tree.rs

@@ -300,9 +300,10 @@ impl Parse for ItemTrait {
         self.trait_items
             .get()
             .iter()
-            .for_each(|(annotated, _)| match &annotated.value {
-                sway_ast::ItemTraitItem::Fn(fn_sig) => fn_sig.parse(ctx),
-                sway_ast::ItemTraitItem::Const(item_const) => item_const.parse(ctx),
+            .for_each(|annotated| match &annotated.value {
+                sway_ast::ItemTraitItem::Fn(fn_sig, _) => fn_sig.parse(ctx),
+                sway_ast::ItemTraitItem::Const(item_const, _) => item_const.parse(ctx),
+                sway_ast::ItemTraitItem::Error(_, _) => {}
             });
 
         if let Some(trait_defs_opt) = &self.trait_defs_opt {
@@ -345,9 +346,10 @@ impl Parse for ItemAbi {
         self.abi_items
             .get()
             .iter()
-            .for_each(|(annotated, _)| match &annotated.value {
-                sway_ast::ItemTraitItem::Fn(fn_sig) => fn_sig.parse(ctx),
-                sway_ast::ItemTraitItem::Const(item_const) => item_const.parse(ctx),
+            .for_each(|annotated| match &annotated.value {
+                sway_ast::ItemTraitItem::Fn(fn_sig, _) => fn_sig.parse(ctx),
+                sway_ast::ItemTraitItem::Const(item_const, _) => item_const.parse(ctx),
+                sway_ast::ItemTraitItem::Error(_, _) => {}
             });
 
         if let Some(abi_defs_opt) = self.abi_defs_opt.as_ref() {

sway-lsp/src/traverse/parsed_tree.rs

@@ -732,6 +732,7 @@ impl Parse for TraitDeclaration {
         self.interface_surface.iter().for_each(|item| match item {
             TraitItem::TraitFn(trait_fn) => trait_fn.parse(ctx),
             TraitItem::Constant(const_decl) => const_decl.parse(ctx),
+            TraitItem::Error(_, _) => {}
         });
         self.methods.iter().for_each(|func_dec| {
             func_dec.parse(ctx);
@@ -848,6 +849,7 @@ impl Parse for AbiDeclaration {
         self.interface_surface.iter().for_each(|item| match item {
             TraitItem::TraitFn(trait_fn) => trait_fn.parse(ctx),
             TraitItem::Constant(const_decl) => const_decl.parse(ctx),
+            TraitItem::Error(_, _) => {}
         });
         self.supertraits.iter().for_each(|supertrait| {
             supertrait.parse(ctx);

sway-parse/src/expr/mod.rs

@@ -138,7 +138,7 @@ impl ParseToEnd for CodeBlockContents {
                 break (None, consumed);
             }
 
-            match parser.parse_fn_with_recovery(parse_stmt) {
+            match parser.call_parsing_function_with_recovery(parse_stmt) {
                 Ok(StmtOrTail::Stmt(s)) => statements.push(s),
                 Ok(StmtOrTail::Tail(e, c)) => break (Some(e), c),
                 Err(r) => {

sway-parse/src/item/item_abi.rs

@@ -14,13 +14,15 @@ impl Parse for ItemAbi {
             }
             None => None,
         };
-        let abi_items: Braces<Vec<(Annotated<ItemTraitItem>, _)>> = parser.parse()?;
-        for (annotated, _) in abi_items.get().iter() {
+
+        let abi_items: Braces<Vec<Annotated<ItemTraitItem>>> = parser.parse()?;
+        for annotated in abi_items.get().iter() {
             #[allow(irrefutable_let_patterns)]
-            if let ItemTraitItem::Fn(fn_signature) = &annotated.value {
+            if let ItemTraitItem::Fn(fn_signature, _) = &annotated.value {
                 parser.ban_visibility_qualifier(&fn_signature.visibility)?;
             }
         }
+
         let abi_defs_opt: Option<Braces<Vec<Annotated<ItemFn>>>> = Braces::try_parse(parser)?;
         if let Some(abi_defs) = &abi_defs_opt {
             for item_fn in abi_defs.get().iter() {

sway-parse/src/item/item_trait.rs

@@ -9,14 +9,26 @@ impl Parse for ItemTraitItem {
     fn parse(parser: &mut Parser) -> ParseResult<ItemTraitItem> {
         if parser.peek::<PubToken>().is_some() || parser.peek::<FnToken>().is_some() {
             let fn_decl = parser.parse()?;
-            Ok(ItemTraitItem::Fn(fn_decl))
+            let semicolon = parser.parse().ok();
+            Ok(ItemTraitItem::Fn(fn_decl, semicolon))
         } else if let Some(_const_keyword) = parser.peek::<ConstToken>() {
             let const_decl = parser.parse()?;
-            Ok(ItemTraitItem::Const(const_decl))
+            let semicolon = parser.parse().ok();
+            Ok(ItemTraitItem::Const(const_decl, semicolon))
         } else {
             Err(parser.emit_error(ParseErrorKind::ExpectedAnItem))
         }
     }
+
+    fn error(
+        spans: Box<[sway_types::Span]>,
+        error: sway_error::handler::ErrorEmitted,
+    ) -> Option<Self>
+    where
+        Self: Sized,
+    {
+        Some(ItemTraitItem::Error(spans, error))
+    }
 }
 
 impl Parse for ItemTrait {
@@ -34,9 +46,9 @@ impl Parse for ItemTrait {
         };
         let where_clause_opt = parser.guarded_parse::<WhereToken, _>()?;
 
-        let trait_items: Braces<Vec<(Annotated<ItemTraitItem>, _)>> = parser.parse()?;
-        for (annotated, _) in trait_items.get().iter() {
-            if let ItemTraitItem::Fn(fn_sig) = &annotated.value {
+        let trait_items: Braces<Vec<Annotated<ItemTraitItem>>> = parser.parse()?;
+        for item in trait_items.get().iter() {
+            if let ItemTraitItem::Fn(fn_sig, _) = &item.value {
                 parser.ban_visibility_qualifier(&fn_sig.visibility)?;
             }
         }

sway-parse/src/item/mod.rs

@@ -518,8 +518,8 @@ mod tests {
 
             let trait_item = decls.next();
             assert!(trait_item.is_some());
-            let (annotated, _) = trait_item.unwrap();
-            if let ItemTraitItem::Fn(_fn_sig) = &annotated.value {
+            let annotated = trait_item.unwrap();
+            if let ItemTraitItem::Fn(_fn_sig, _) = &annotated.value {
                 assert_eq!(
                     attributes(&annotated.attribute_list),
                     vec![[("foo", Some(vec!["one"]))], [("bar", None)]]
@@ -575,15 +575,15 @@ mod tests {
             assert!(f_sig.is_some());
 
             assert_eq!(
-                attributes(&f_sig.unwrap().0.attribute_list),
+                attributes(&f_sig.unwrap().attribute_list),
                 vec![[("bar", Some(vec!["one", "two", "three"]))],]
             );
 
             let g_sig = decls.next();
             assert!(g_sig.is_some());
 
             assert_eq!(
-                attributes(&g_sig.unwrap().0.attribute_list),
+                attributes(&g_sig.unwrap().attribute_list),
                 vec![[("foo", None)],]
             );
             assert!(decls.next().is_none());

sway-parse/src/parse.rs

@@ -6,6 +6,8 @@ use sway_error::parser_error::ParseErrorKind;
 use sway_types::{ast::Delimiter, Ident, Spanned};
 
 pub trait Parse {
+    const FALLBACK_ERROR: ParseErrorKind = ParseErrorKind::InvalidItem;
+
     fn parse(parser: &mut Parser) -> ParseResult<Self>
     where
         Self: Sized;
@@ -110,7 +112,7 @@ where
                 Ok(value) => ret.push(value),
                 Err(r) => {
                     let (spans, error) =
-                        r.recover_at_next_line_with_fallback_error(ParseErrorKind::InvalidItem);
+                        r.recover_at_next_line_with_fallback_error(T::FALLBACK_ERROR);
                     if let Some(error) = T::error(spans, error) {
                         ret.push(error);
                     } else {