diff --git a/vhdl_lang/src/ast.rs b/vhdl_lang/src/ast.rs index cf2aa63a..8a975fd9 100644 --- a/vhdl_lang/src/ast.rs +++ b/vhdl_lang/src/ast.rs @@ -1144,6 +1144,15 @@ pub struct SeparatedList { pub tokens: Vec, } +impl Default for SeparatedList { + fn default() -> Self { + SeparatedList { + items: Vec::default(), + tokens: Vec::default(), + } + } +} + impl SeparatedList { /// Returns an iterator over the formal elements of this list pub fn formals(&self) -> impl Iterator { diff --git a/vhdl_lang/src/syntax/concurrent_statement.rs b/vhdl_lang/src/syntax/concurrent_statement.rs index dba79810..6bfe5622 100644 --- a/vhdl_lang/src/syntax/concurrent_statement.rs +++ b/vhdl_lang/src/syntax/concurrent_statement.rs @@ -94,7 +94,7 @@ fn parse_block_header( "Duplicate generic map", )); } - let (list, closing_paren) = parse_association_list(stream)?; + let (list, closing_paren) = parse_association_list(stream, diagnostics)?; stream.expect_kind(SemiColon)?; if generic_map.is_none() { generic_map = Some(MapAspect { @@ -133,7 +133,7 @@ fn parse_block_header( "Duplicate port map", )); } - let (list, closing_paren) = parse_association_list(stream)?; + let (list, closing_paren) = parse_association_list(stream, diagnostics)?; stream.expect_kind(SemiColon)?; if port_map.is_none() { port_map = Some(MapAspect { @@ -332,10 +332,14 @@ pub fn parse_concurrent_assert_statement( }) } -pub fn parse_map_aspect(stream: &TokenStream, aspect_kind: Kind) -> ParseResult> { +pub fn parse_map_aspect( + stream: &TokenStream, + aspect_kind: Kind, + diagnostics: &mut dyn DiagnosticHandler, +) -> ParseResult> { if let Some(aspect) = stream.pop_if_kind(aspect_kind) { stream.expect_kind(Map)?; - let (list, closing_paren) = parse_association_list(stream)?; + let (list, closing_paren) = parse_association_list(stream, diagnostics)?; Ok(Some(MapAspect { start: aspect, list, @@ -349,9 +353,10 @@ pub fn parse_map_aspect(stream: &TokenStream, aspect_kind: Kind) -> ParseResult< #[allow(clippy::type_complexity)] pub fn parse_generic_and_port_map( stream: &TokenStream, + diagnostics: &mut dyn DiagnosticHandler, ) -> ParseResult<(Option, Option)> { - let generic_map = parse_map_aspect(stream, Generic)?; - let port_map = parse_map_aspect(stream, Port)?; + let generic_map = parse_map_aspect(stream, Generic, diagnostics)?; + let port_map = parse_map_aspect(stream, Port, diagnostics)?; Ok((generic_map, port_map)) } @@ -359,8 +364,9 @@ pub fn parse_generic_and_port_map( pub fn parse_instantiation_statement( stream: &TokenStream, unit: InstantiatedUnit, + diagnostics: &mut dyn DiagnosticHandler, ) -> ParseResult { - let (generic_map, port_map) = parse_generic_and_port_map(stream)?; + let (generic_map, port_map) = parse_generic_and_port_map(stream, diagnostics)?; let semi = stream.expect_kind(SemiColon)?; @@ -586,12 +592,12 @@ pub fn parse_concurrent_statement( Component => { stream.skip(); let unit = InstantiatedUnit::Component(parse_selected_name(stream)?); - ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit)?) + ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit, diagnostics)?) }, Configuration => { stream.skip(); let unit = InstantiatedUnit::Configuration(parse_selected_name(stream)?); - ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit)?) + ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit, diagnostics)?) }, Entity => { stream.skip(); @@ -606,7 +612,7 @@ pub fn parse_concurrent_statement( } }; let unit = InstantiatedUnit::Entity(name, arch.map(WithRef::new)); - ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit)?) + ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit, diagnostics)?) }, For => ConcurrentStatement::ForGenerate(parse_for_generate_statement(stream, label, diagnostics)?), If => ConcurrentStatement::IfGenerate(parse_if_generate_statement(stream, label, diagnostics)?), @@ -633,7 +639,7 @@ pub fn parse_concurrent_statement( match token.kind { Generic|Port => { let unit = InstantiatedUnit::Component(into_selected_name(name)?); - ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit)?) + ConcurrentStatement::Instance(parse_instantiation_statement(stream, unit, diagnostics)?) } _ => { parse_assignment_or_procedure_call(stream, name.map_into(Target::Name))? diff --git a/vhdl_lang/src/syntax/configuration.rs b/vhdl_lang/src/syntax/configuration.rs index a794c04a..70270f85 100644 --- a/vhdl_lang/src/syntax/configuration.rs +++ b/vhdl_lang/src/syntax/configuration.rs @@ -40,8 +40,9 @@ fn parse_entity_aspect(stream: &TokenStream) -> ParseResult { fn parse_binding_indication_known_entity_aspect( entity_aspect: Option, stream: &TokenStream, + diagnostics: &mut dyn DiagnosticHandler, ) -> ParseResult { - let (generic_map, port_map) = parse_generic_and_port_map(stream)?; + let (generic_map, port_map) = parse_generic_and_port_map(stream, diagnostics)?; stream.expect_kind(SemiColon)?; Ok(BindingIndication { @@ -52,13 +53,16 @@ fn parse_binding_indication_known_entity_aspect( } /// LRM 7.3.2 -fn parse_binding_indication(stream: &TokenStream) -> ParseResult { +fn parse_binding_indication( + stream: &TokenStream, + diagnostics: &mut dyn DiagnosticHandler, +) -> ParseResult { let entity_aspect = if stream.skip_if_kind(Use) { Some(parse_entity_aspect(stream)?) } else { None }; - parse_binding_indication_known_entity_aspect(entity_aspect, stream) + parse_binding_indication_known_entity_aspect(entity_aspect, stream, diagnostics) } fn parse_component_configuration_known_spec( @@ -78,7 +82,7 @@ fn parse_component_configuration_known_spec( (None, vunit_bind_inds) } else { let aspect = parse_entity_aspect(stream)?; - let bind_ind = parse_binding_indication_known_entity_aspect(Some(aspect), stream)?; + let bind_ind = parse_binding_indication_known_entity_aspect(Some(aspect), stream, diagnostics)?; if stream.skip_if_kind(Use) { (Some(bind_ind), parse_vunit_binding_indication_list_known_keyword(stream)?) @@ -284,7 +288,10 @@ pub fn parse_configuration_declaration( break parse_vunit_binding_indication_list_known_keyword(stream)?; } - decl.push(ConfigurationDeclarativeItem::Use(parse_use_clause(stream)?)); + decl.push(ConfigurationDeclarativeItem::Use(parse_use_clause( + stream, + diagnostics, + )?)); } _ => break Vec::new(), } @@ -312,11 +319,12 @@ pub fn parse_configuration_declaration( /// LRM 7.3 Configuration Specification pub fn parse_configuration_specification( stream: &TokenStream, + diagnsotics: &mut dyn DiagnosticHandler, ) -> ParseResult { stream.expect_kind(For)?; match parse_component_specification_or_name(stream)? { ComponentSpecificationOrName::ComponentSpec(spec) => { - let bind_ind = parse_binding_indication(stream)?; + let bind_ind = parse_binding_indication(stream, diagnsotics)?; if stream.skip_if_kind(Use) { let vunit_bind_inds = parse_vunit_binding_indication_list_known_keyword(stream)?; stream.expect_kind(End)?; @@ -814,7 +822,7 @@ end configuration cfg; let code = Code::new("for all : lib.pkg.comp use entity work.foo(rtl);"); assert_eq!( - code.with_stream(parse_configuration_specification), + code.with_stream_no_diagnostics(parse_configuration_specification), ConfigurationSpecification { spec: ComponentSpecification { instantiation_list: InstantiationList::All, @@ -838,7 +846,7 @@ end configuration cfg; let code = Code::new("for all : lib.pkg.comp use entity work.foo(rtl); end for;"); assert_eq!( - code.with_stream(parse_configuration_specification), + code.with_stream_no_diagnostics(parse_configuration_specification), ConfigurationSpecification { spec: ComponentSpecification { instantiation_list: InstantiationList::All, @@ -864,7 +872,7 @@ end configuration cfg; ); assert_eq!( - code.with_stream(parse_configuration_specification), + code.with_stream_no_diagnostics(parse_configuration_specification), ConfigurationSpecification { spec: ComponentSpecification { instantiation_list: InstantiationList::All, diff --git a/vhdl_lang/src/syntax/context.rs b/vhdl_lang/src/syntax/context.rs index 0382694c..9ffc1b29 100644 --- a/vhdl_lang/src/syntax/context.rs +++ b/vhdl_lang/src/syntax/context.rs @@ -13,9 +13,12 @@ use crate::data::*; use crate::syntax::separated_list::{parse_ident_list, parse_name_list}; /// LRM 13. Design units and their analysis -pub fn parse_library_clause(stream: &TokenStream) -> ParseResult { +pub fn parse_library_clause( + stream: &TokenStream, + diagnsotics: &mut dyn DiagnosticHandler, +) -> ParseResult { let library_token = stream.expect_kind(Library)?; - let name_list = parse_ident_list(stream)?; + let name_list = parse_ident_list(stream, diagnsotics)?; let semi_token = stream.expect_kind(SemiColon)?; Ok(LibraryClause { library_token, @@ -25,10 +28,13 @@ pub fn parse_library_clause(stream: &TokenStream) -> ParseResult } /// LRM 12.4. Use clauses -pub fn parse_use_clause(stream: &TokenStream) -> ParseResult { +pub fn parse_use_clause( + stream: &TokenStream, + diagnsotics: &mut dyn DiagnosticHandler, +) -> ParseResult { let use_token = stream.expect_kind(Use)?; - let name_list = parse_name_list(stream)?; + let name_list = parse_name_list(stream, diagnsotics)?; let semi_token = stream.expect_kind(SemiColon)?; Ok(UseClause { use_token, @@ -43,10 +49,13 @@ pub enum DeclarationOrReference { Reference(ContextReference), } -pub fn parse_context_reference(stream: &TokenStream) -> ParseResult { +pub fn parse_context_reference( + stream: &TokenStream, + diagnostics: &mut dyn DiagnosticHandler, +) -> ParseResult { let context_token = stream.expect_kind(Context)?; - let name_list = parse_name_list(stream)?; + let name_list = parse_name_list(stream, diagnostics)?; let semi_token = stream.expect_kind(SemiColon)?; Ok(ContextReference { context_token, @@ -69,9 +78,9 @@ pub fn parse_context( let token = stream.peek_expect()?; try_init_token_kind!( token, - Library => items.push(ContextItem::Library(parse_library_clause(stream)?)), - Use => items.push(ContextItem::Use(parse_use_clause(stream)?)), - Context => items.push(ContextItem::Context(parse_context_reference(stream)?)), + Library => items.push(ContextItem::Library(parse_library_clause(stream, diagnostics)?)), + Use => items.push(ContextItem::Use(parse_use_clause(stream, diagnostics)?)), + Context => items.push(ContextItem::Context(parse_context_reference(stream, diagnostics)?)), End => { stream.skip(); stream.pop_if_kind(Context); @@ -117,7 +126,7 @@ mod tests { fn test_library_clause_single_name() { let code = Code::new("library foo;"); assert_eq!( - code.with_stream(parse_library_clause), + code.with_stream_no_diagnostics(parse_library_clause), LibraryClause { library_token: code.s1("library").token(), name_list: code.s1("foo").ident_list(), @@ -130,7 +139,7 @@ mod tests { fn test_library_clause_multiple_names() { let code = Code::new("library foo, bar;"); assert_eq!( - code.with_stream(parse_library_clause), + code.with_stream_no_diagnostics(parse_library_clause), LibraryClause { library_token: code.s1("library").token(), name_list: code.s1("foo, bar").ident_list(), @@ -143,7 +152,7 @@ mod tests { fn test_use_clause_single_name() { let code = Code::new("use lib.foo;"); assert_eq!( - code.with_stream(parse_use_clause), + code.with_stream_no_diagnostics(parse_use_clause), UseClause { use_token: code.s1("use").token(), name_list: code.s1("lib.foo").name_list(), @@ -156,7 +165,7 @@ mod tests { fn test_use_clause_multiple_names() { let code = Code::new("use foo.'a', lib.bar.all;"); assert_eq!( - code.with_stream(parse_use_clause), + code.with_stream_no_diagnostics(parse_use_clause), UseClause { use_token: code.s1("use").token(), name_list: code.s1("foo.'a', lib.bar.all").name_list(), diff --git a/vhdl_lang/src/syntax/declarative_part.rs b/vhdl_lang/src/syntax/declarative_part.rs index fd06c7b8..eebe75c6 100644 --- a/vhdl_lang/src/syntax/declarative_part.rs +++ b/vhdl_lang/src/syntax/declarative_part.rs @@ -19,13 +19,16 @@ use crate::ast::{ContextClause, Declaration, PackageInstantiation}; use crate::data::DiagnosticHandler; use crate::syntax::concurrent_statement::parse_map_aspect; -pub fn parse_package_instantiation(stream: &TokenStream) -> ParseResult { +pub fn parse_package_instantiation( + stream: &TokenStream, + diagnsotics: &mut dyn DiagnosticHandler, +) -> ParseResult { stream.expect_kind(Package)?; let ident = stream.expect_ident()?; stream.expect_kind(Is)?; stream.expect_kind(New)?; let package_name = parse_selected_name(stream)?; - let generic_map = parse_map_aspect(stream, Generic)?; + let generic_map = parse_map_aspect(stream, Generic, diagnsotics)?; stream.expect_kind(SemiColon)?; Ok(PackageInstantiation { @@ -98,10 +101,10 @@ pub fn parse_declarative_part( Component => parse_component_declaration(stream, diagnostics) .map(Declaration::Component)?, Impure | Pure | Function | Procedure => parse_subprogram(stream, diagnostics)?, - Package => parse_package_instantiation(stream).map(Declaration::Package)?, - For => { - parse_configuration_specification(stream).map(Declaration::Configuration)? - } + Package => parse_package_instantiation(stream, diagnostics) + .map(Declaration::Package)?, + For => parse_configuration_specification(stream, diagnostics) + .map(Declaration::Configuration)?, _ => unreachable!(), }; declarations.push(decl); @@ -128,7 +131,7 @@ pub fn parse_declarative_part( Use | Alias => { let decl: ParseResult = match token.kind { - Use => parse_use_clause(stream).map(Declaration::Use), + Use => parse_use_clause(stream, diagnostics).map(Declaration::Use), Alias => parse_alias_declaration(stream).map(Declaration::Alias), _ => unreachable!(), }; @@ -170,7 +173,7 @@ package ident is new lib.foo.bar; ", ); assert_eq!( - code.with_stream(parse_package_instantiation), + code.with_stream_no_diagnostics(parse_package_instantiation), PackageInstantiation { context_clause: ContextClause::default(), ident: code.s1("ident").decl_ident(), @@ -191,7 +194,7 @@ package ident is new lib.foo.bar ", ); assert_eq!( - code.with_stream(parse_package_instantiation), + code.with_stream_no_diagnostics(parse_package_instantiation), PackageInstantiation { context_clause: ContextClause::default(), ident: code.s1("ident").decl_ident(), diff --git a/vhdl_lang/src/syntax/design_unit.rs b/vhdl_lang/src/syntax/design_unit.rs index 8944f4d1..73ac7ec7 100644 --- a/vhdl_lang/src/syntax/design_unit.rs +++ b/vhdl_lang/src/syntax/design_unit.rs @@ -169,7 +169,7 @@ pub fn parse_design_file( try_init_token_kind!( token, Library => { - match parse_library_clause(stream) { + match parse_library_clause(stream, diagnostics) { Ok(library) => { context_clause.push(ContextItem::Library(library)); }, @@ -177,7 +177,7 @@ pub fn parse_design_file( } }, Use => { - match parse_use_clause(stream) { + match parse_use_clause(stream, diagnostics) { Ok(use_clause) => { context_clause.push(ContextItem::Use(use_clause)); }, @@ -243,7 +243,7 @@ pub fn parse_design_file( Err(diagnostic) => diagnostics.push(diagnostic), }; } else if stream.next_kinds_are(&[Package, Identifier, Is, New]) { - match parse_package_instantiation(stream) { + match parse_package_instantiation(stream, diagnostics) { Ok(mut inst) => { let tokens = stream.slice_tokens(); inst.context_clause = take_context_clause(&mut context_clause); diff --git a/vhdl_lang/src/syntax/interface_declaration.rs b/vhdl_lang/src/syntax/interface_declaration.rs index f961fa0a..891ad0cc 100644 --- a/vhdl_lang/src/syntax/interface_declaration.rs +++ b/vhdl_lang/src/syntax/interface_declaration.rs @@ -177,7 +177,10 @@ fn parse_subprogram_default(stream: &TokenStream) -> ParseResult ParseResult { +fn parse_interface_package( + stream: &TokenStream, + diagnsotics: &mut dyn DiagnosticHandler, +) -> ParseResult { stream.expect_kind(Package)?; let ident = stream.expect_ident()?; stream.expect_kind(Is)?; @@ -187,7 +190,7 @@ fn parse_interface_package(stream: &TokenStream) -> ParseResult { @@ -201,7 +204,7 @@ fn parse_interface_package(stream: &TokenStream) -> ParseResult { - let (list, _) = parse_association_list_no_leftpar(stream)?; + let (list, _) = parse_association_list_no_leftpar(stream, left_par, diagnsotics)?; InterfacePackageGenericMapAspect::Map(list) } } @@ -237,7 +240,7 @@ fn parse_interface_declaration( Ok(vec![InterfaceDeclaration::Subprogram(decl, default)]) }, Package => { - Ok(vec![InterfaceDeclaration::Package (parse_interface_package(stream)?)]) + Ok(vec![InterfaceDeclaration::Package (parse_interface_package(stream, diagnostics)?)]) } ) } diff --git a/vhdl_lang/src/syntax/names.rs b/vhdl_lang/src/syntax/names.rs index 538897d7..e7086076 100644 --- a/vhdl_lang/src/syntax/names.rs +++ b/vhdl_lang/src/syntax/names.rs @@ -12,8 +12,8 @@ use super::subtype_indication::parse_subtype_indication; use super::tokens::{Kind::*, TokenAccess, TokenStream}; use crate::ast; use crate::ast::*; -use crate::data::{Diagnostic, WithPos}; -use crate::syntax::separated_list::parse_list_with_separator; +use crate::data::{Diagnostic, DiagnosticHandler, WithPos}; +use crate::syntax::separated_list::parse_list_with_separator_or_recover; use crate::syntax::TokenId; pub fn parse_designator(stream: &TokenStream) -> ParseResult> { @@ -165,7 +165,7 @@ fn parse_actual_part(stream: &TokenStream) -> ParseResult> { } } -fn parse_association_element(stream: &TokenStream) -> ParseResult { +pub fn parse_association_element(stream: &TokenStream) -> ParseResult { let actual = parse_actual_part(stream)?; if stream.skip_if_kind(RightArrow) { Ok(AssociationElement { @@ -182,23 +182,33 @@ fn parse_association_element(stream: &TokenStream) -> ParseResult ParseResult<(SeparatedList, TokenId)> { - stream.expect_kind(LeftPar)?; - parse_association_list_no_leftpar(stream) + let left_par = stream.expect_kind(LeftPar)?; + parse_association_list_no_leftpar(stream, left_par, diagnsotics) } pub fn parse_association_list_no_leftpar( stream: &TokenStream, + left_par: TokenId, + diagnostics: &mut dyn DiagnosticHandler, ) -> ParseResult<(SeparatedList, TokenId)> { if let Some(right_par) = stream.pop_if_kind(RightPar) { - return Err(Diagnostic::error( - stream.get_pos(right_par), + diagnostics.error( + stream.get_span(left_par, right_par), "Association list cannot be empty", - )); + ); + return Ok((SeparatedList::default(), right_par)); } - let list = parse_list_with_separator(stream, Comma, parse_association_element)?; - let comma = stream.expect_kind(RightPar)?; - Ok((list, comma)) + let list = parse_list_with_separator_or_recover( + stream, + Comma, + diagnostics, + parse_association_element, + Some(RightPar), + )?; + let right_par = stream.expect_kind(RightPar)?; + Ok((list, right_par)) } fn parse_function_call( @@ -1022,7 +1032,7 @@ mod tests { actual: WithPos::new(ActualPart::Open, code.s("open", 2)), }; assert_eq!( - code.with_stream(parse_association_list), + code.with_stream_no_diagnostics(parse_association_list), ( SeparatedList { items: vec![elem1, elem2], @@ -1165,4 +1175,33 @@ mod tests { )) ); } + + #[test] + fn empty_association_list_diagnostic() { + let code = Code::new("()"); + let (list, diag) = code.with_stream_diagnostics(parse_association_list); + assert_eq!( + diag, + vec![Diagnostic::error( + code.pos(), + "Association list cannot be empty" + )] + ); + assert!(list.0.tokens.is_empty()); + assert!(list.0.items.is_empty()); + } + + #[test] + fn trailing_comma_diagnostic() { + let code = Code::new("(a => b,)"); + let (list, diag) = code.with_stream_diagnostics(parse_association_list); + assert_eq!( + diag, + vec![Diagnostic::error( + code.s1(")").pos(), + "Expected {expression}" + )] + ); + assert_eq!(list.0.items, vec![code.s1("a => b").association_element()]); + } } diff --git a/vhdl_lang/src/syntax/separated_list.rs b/vhdl_lang/src/syntax/separated_list.rs index fda669b3..4e91924c 100644 --- a/vhdl_lang/src/syntax/separated_list.rs +++ b/vhdl_lang/src/syntax/separated_list.rs @@ -5,11 +5,31 @@ // Copyright (c) 2023, Olof Kraigher olof.kraigher@gmail.com use crate::ast::{IdentList, NameList, SeparatedList, WithRef}; -use crate::data::DiagnosticResult; +use crate::data::{DiagnosticHandler, DiagnosticResult}; use crate::syntax::common::ParseResult; use crate::syntax::names::parse_name; use crate::syntax::Kind::Comma; -use crate::syntax::{Kind, TokenStream}; +use crate::syntax::{kind_str, Kind, TokenAccess, TokenStream}; + +/// Skip extraneous tokens of kind `separator`. +/// When there are any extra tokens of that kind, mark all the positions of these tokens as erroneous +fn skip_extraneous_tokens( + stream: &TokenStream, + separator: Kind, + diagnostics: &mut dyn DiagnosticHandler, +) { + if let Some(separator_tok) = stream.pop_if_kind(separator) { + let start_pos = stream.get_pos(separator_tok); + let mut end_pos = start_pos; + while let Some(separator_tok) = stream.pop_if_kind(separator) { + end_pos = stream.get_pos(separator_tok) + } + diagnostics.error( + start_pos.combine(end_pos), + format!("Extraneous '{}'", kind_str(separator)), + ); + } +} /// Parses a list of the form /// `element { separator element }` @@ -18,48 +38,96 @@ use crate::syntax::{Kind, TokenStream}; pub fn parse_list_with_separator( stream: &TokenStream, separator: Kind, + diagnostics: &mut dyn DiagnosticHandler, parse_fn: F, ) -> DiagnosticResult> where F: Fn(&TokenStream) -> ParseResult, { - let mut items = vec![parse_fn(stream)?]; - let mut tokens = Vec::new(); - while let Some(separator) = stream.pop_if_kind(separator) { - items.push(parse_fn(stream)?); - tokens.push(separator); + parse_list_with_separator_or_recover(stream, separator, diagnostics, parse_fn, None) +} + +/// Same as `parse_list_with_separator`. +/// However, when supplied with a `recover_token` will skip until either the separator +/// or the recover token is found. +pub fn parse_list_with_separator_or_recover( + stream: &TokenStream, + separator: Kind, + diagnostics: &mut dyn DiagnosticHandler, + parse_fn: F, + recover_token: Option, +) -> DiagnosticResult> +where + F: Fn(&TokenStream) -> ParseResult, +{ + let mut items = vec![]; + let mut tokens = vec![]; + loop { + match parse_fn(stream) { + Ok(item) => items.push(item), + Err(err) => { + if let Some(tok) = recover_token { + stream.skip_until(|kind| kind == separator || kind == tok)?; + diagnostics.push(err); + } else { + return Err(err); + } + } + } + if let Some(separator_tok) = stream.pop_if_kind(separator) { + skip_extraneous_tokens(stream, separator, diagnostics); + tokens.push(separator_tok); + } else { + break; + } } Ok(SeparatedList { items, tokens }) } -pub fn parse_name_list(stream: &TokenStream) -> DiagnosticResult { - parse_list_with_separator(stream, Comma, parse_name) +pub fn parse_name_list( + stream: &TokenStream, + diagnostics: &mut dyn DiagnosticHandler, +) -> DiagnosticResult { + parse_list_with_separator(stream, Comma, diagnostics, parse_name) } -pub fn parse_ident_list(stream: &TokenStream) -> DiagnosticResult { - parse_list_with_separator(stream, Comma, |stream| { +pub fn parse_ident_list( + stream: &TokenStream, + diagnostics: &mut dyn DiagnosticHandler, +) -> DiagnosticResult { + parse_list_with_separator(stream, Comma, diagnostics, |stream| { stream.expect_ident().map(WithRef::new) }) } #[cfg(test)] mod test { - use crate::ast::{IdentList, NameList}; - use crate::syntax::separated_list::{parse_ident_list, parse_name_list}; + use crate::ast::{IdentList, NameList, SeparatedList}; + use crate::syntax::names::parse_association_element; + use crate::syntax::separated_list::{ + parse_ident_list, parse_list_with_separator_or_recover, parse_name_list, + }; use crate::syntax::test::Code; - use assert_matches::assert_matches; + use crate::syntax::Kind; + use crate::syntax::Kind::RightPar; + use crate::Diagnostic; #[test] pub fn test_error_on_empty_list() { let code = Code::new(""); - assert_matches!(code.parse(parse_ident_list), Err(_)) + let (res, diagnostics) = code.with_partial_stream_diagnostics(parse_ident_list); + assert_eq!( + res, + Err(Diagnostic::error(code.eof_pos(), "Unexpected EOF")) + ); + assert!(diagnostics.is_empty()); } #[test] pub fn parse_single_element_list() { let code = Code::new("abc"); assert_eq!( - code.parse_ok(parse_ident_list), + code.parse_ok_no_diagnostics(parse_ident_list), IdentList::single(code.s1("abc").ident().into_ref()) ) } @@ -68,7 +136,7 @@ mod test { pub fn parse_list_with_multiple_elements() { let code = Code::new("abc, def, ghi"); assert_eq!( - code.parse_ok(parse_ident_list), + code.parse_ok_no_diagnostics(parse_ident_list), IdentList { items: vec![ code.s1("abc").ident().into_ref(), @@ -84,11 +152,96 @@ mod test { fn parse_list_with_many_names() { let code = Code::new("work.foo, lib.bar.all"); assert_eq!( - code.parse_ok(parse_name_list), + code.parse_ok_no_diagnostics(parse_name_list), NameList { items: vec![code.s1("work.foo").name(), code.s1("lib.bar.all").name()], tokens: vec![code.s1(",").token()], } ) } + + #[test] + fn parse_extraneous_single_separators() { + let code = Code::new("a,,b,c"); + let (res, diag) = code.with_stream_diagnostics(parse_ident_list); + assert_eq!( + res, + IdentList { + items: vec![ + code.s1("a").ident().into_ref(), + code.s1("b").ident().into_ref(), + code.s1("c").ident().into_ref() + ], + tokens: vec![code.s(",", 1).token(), code.s(",", 3).token()] + } + ); + assert_eq!( + diag, + vec![Diagnostic::error(code.s(",", 2).pos(), "Extraneous ','")] + ) + } + + #[test] + fn parse_extraneous_multiple_separators() { + let code = Code::new("a,,,,b,c"); + let (res, diag) = code.with_stream_diagnostics(parse_ident_list); + assert_eq!( + res, + IdentList { + items: vec![ + code.s1("a").ident().into_ref(), + code.s1("b").ident().into_ref(), + code.s1("c").ident().into_ref() + ], + tokens: vec![code.s(",", 1).token(), code.s(",", 5).token()] + } + ); + assert_eq!( + diag, + vec![Diagnostic::error(code.s(",,,", 2).pos(), "Extraneous ','")] + ) + } + + #[test] + fn parse_recoverable_list() { + let code = Code::new("a => b,c => d, e =>)"); + let (res, diag) = code.with_stream_diagnostics(|stream, diag| { + let res = parse_list_with_separator_or_recover( + stream, + Kind::Comma, + diag, + parse_association_element, + Some(RightPar), + ); + stream.skip(); + res + }); + assert_eq!( + res, + SeparatedList { + items: vec![ + code.s1("a => b").association_element(), + code.s1("c => d").association_element() + ], + tokens: vec![code.s(",", 1).token(), code.s(",", 2).token(),], + } + ); + assert_eq!( + diag, + vec![Diagnostic::error(code.s1(")"), "Expected {expression}")] + ); + } + + #[test] + fn parse_list_with_erroneous_elements() { + let code = Code::new("1,c,d"); + let mut diag: Vec = vec![]; + let diag = code + .parse(|stream| parse_ident_list(stream, &mut diag)) + .expect_err("Should not parse OK"); + assert_eq!( + diag, + Diagnostic::error(code.s1("1"), "Expected '{identifier}'") + ); + } } diff --git a/vhdl_lang/src/syntax/test.rs b/vhdl_lang/src/syntax/test.rs index 32a0945e..6a466d4f 100644 --- a/vhdl_lang/src/syntax/test.rs +++ b/vhdl_lang/src/syntax/test.rs @@ -31,6 +31,7 @@ use crate::data::Range; use crate::data::*; use crate::syntax::concurrent_statement::parse_map_aspect; use crate::syntax::context::{parse_context, DeclarationOrReference}; +use crate::syntax::names::parse_association_element; use crate::syntax::{TokenAccess, TokenId}; use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::Entry; @@ -414,11 +415,11 @@ impl Code { } pub fn name_list(&self) -> SeparatedList> { - self.parse_ok(parse_name_list) + self.parse_ok_no_diagnostics(parse_name_list) } pub fn ident_list(&self) -> SeparatedList> { - self.parse_ok(parse_ident_list) + self.parse_ok_no_diagnostics(parse_ident_list) } pub fn selected_name(&self) -> WithPos { @@ -514,17 +515,21 @@ impl Code { } pub fn association_list(&self) -> SeparatedList { - self.parse_ok(parse_association_list).0 + self.parse_ok_no_diagnostics(parse_association_list).0 } pub fn port_map_aspect(&self) -> MapAspect { - self.parse_ok(|stream| parse_map_aspect(stream, Kind::Port)) - .expect("Expecting port map aspect") + self.parse_ok_no_diagnostics(|stream, diagnsotics| { + parse_map_aspect(stream, Kind::Port, diagnsotics) + }) + .expect("Expecting port map aspect") } pub fn generic_map_aspect(&self) -> MapAspect { - self.parse_ok(|stream| parse_map_aspect(stream, Kind::Generic)) - .expect("Expecting generic map aspect") + self.parse_ok_no_diagnostics(|stream, diagnostics| { + parse_map_aspect(stream, Kind::Generic, diagnostics) + }) + .expect("Expecting generic map aspect") } pub fn waveform(&self) -> Waveform { @@ -548,11 +553,11 @@ impl Code { } pub fn use_clause(&self) -> UseClause { - self.parse_ok(parse_use_clause) + self.parse_ok_no_diagnostics(parse_use_clause) } pub fn library_clause(&self) -> LibraryClause { - self.parse_ok(parse_library_clause) + self.parse_ok_no_diagnostics(parse_library_clause) } pub fn context_declaration(&self) -> ContextDeclaration { @@ -576,6 +581,10 @@ impl Code { name => panic!("Expected attribute got {name:?}"), } } + + pub fn association_element(&self) -> AssociationElement { + self.parse_ok(parse_association_element) + } } fn substr_range(source: &Source, range: Range, substr: &str, occurence: usize) -> Range { diff --git a/vhdl_lang/src/syntax/tokens/tokenstream.rs b/vhdl_lang/src/syntax/tokens/tokenstream.rs index 8014b019..c3e58f94 100644 --- a/vhdl_lang/src/syntax/tokens/tokenstream.rs +++ b/vhdl_lang/src/syntax/tokens/tokenstream.rs @@ -216,7 +216,10 @@ impl<'a> TokenStream<'a> { self.pop_if_kind(kind).is_some() } - pub fn skip_until(&self, cond: fn(Kind) -> bool) -> DiagnosticResult<()> { + pub fn skip_until(&self, cond: F) -> DiagnosticResult<()> + where + F: Fn(Kind) -> bool, + { loop { let token = self.peek_expect()?; if cond(token.kind) { @@ -287,33 +290,39 @@ impl<'a> TokenAccess for TokenStream<'a> { } pub trait Recover { - fn or_recover_until( + fn or_recover_until( self, stream: &TokenStream, msgs: &mut dyn DiagnosticHandler, - cond: fn(Kind) -> bool, - ) -> DiagnosticResult; + cond: F, + ) -> DiagnosticResult + where + F: Fn(Kind) -> bool; fn log(self, msgs: &mut dyn DiagnosticHandler); } -impl Recover for DiagnosticResult { - fn or_recover_until( +impl Recover for DiagnosticResult { + fn or_recover_until( self, stream: &TokenStream, msgs: &mut dyn DiagnosticHandler, - cond: fn(Kind) -> bool, - ) -> DiagnosticResult { - if self.is_ok() { - return self; - } - - let res = stream.skip_until(cond); - match res { - Ok(_) => self, - Err(err) => { - msgs.push(self.unwrap_err()); - Err(err) + cond: F, + ) -> DiagnosticResult + where + F: Fn(Kind) -> bool, + { + match self { + Ok(res) => Ok(res), + Err(ref original_err) => { + let res = stream.skip_until(cond); + match res { + Ok(_) => self, + Err(err) => { + msgs.push(original_err.clone()); + Err(err) + } + } } } }