Skip to content

Commit

Permalink
Feat: Improve robustness of separated list parsing for enhanced compl…
Browse files Browse the repository at this point in the history
…etions
  • Loading branch information
luscheller committed Aug 13, 2024
1 parent 5c79bc8 commit 950fe46
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 21 deletions.
1 change: 1 addition & 0 deletions vhdl_lang/src/syntax/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ pub fn parse_association_list_no_leftpar(
Comma,
parse_association_element,
Some(RightPar),
Some(Identifier),
)?;
let right_par = ctx.stream.expect_kind(RightPar)?;
Ok((list, right_par))
Expand Down
86 changes: 65 additions & 21 deletions vhdl_lang/src/syntax/separated_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use crate::ast::{Name, SeparatedList};
use crate::data::DiagnosticResult;
use crate::syntax::common::ParseResult;
use crate::syntax::names::parse_name;
use crate::syntax::Kind::Comma;
use crate::syntax::{kind_str, Kind, TokenAccess};
use crate::syntax::Kind::{Comma, Identifier};
use crate::syntax::{kind_str, kinds_error, Kind, TokenAccess};
use crate::Diagnostic;
use vhdl_lang::syntax::parser::ParsingContext;

Expand All @@ -30,29 +30,20 @@ fn skip_extraneous_tokens(ctx: &mut ParsingContext<'_>, separator: Kind) {
}
}

/// Parses a list of the form
/// `element { separator element }`
/// where `element` is an AST element and `separator` is a token of some `ast::Kind`.
/// The returned list retains information of the whereabouts of the separator tokens.
pub fn parse_list_with_separator<F, T>(
ctx: &mut ParsingContext<'_>,
separator: Kind,
parse_fn: F,
) -> DiagnosticResult<SeparatedList<T>>
where
F: Fn(&mut ParsingContext<'_>) -> ParseResult<T>,
{
parse_list_with_separator_or_recover(ctx, separator, parse_fn, None)
}

/// Same as `parse_list_with_separator`.
/// Parses a list that is separated by a single token, denoted by the `separator`
/// However, when supplied with a `recover_token` will skip until either the separator
/// or the recover token is found.
///
/// * recover_token: When supplied with a `recover_token`, the `parse_fn` fails and will forward
/// the stream until this token is seen
/// * start_token: The token-kind that `parse_fn` starts with.
/// If not `None`, this is used to detect missing separators and continue parsing.
pub fn parse_list_with_separator_or_recover<F, T>(
ctx: &mut ParsingContext<'_>,
separator: Kind,
parse_fn: F,
recover_token: Option<Kind>,
start_token: Option<Kind>,
) -> DiagnosticResult<SeparatedList<T>>
where
F: Fn(&mut ParsingContext<'_>) -> ParseResult<T>,
Expand All @@ -75,6 +66,15 @@ where
if let Some(separator_tok) = ctx.stream.pop_if_kind(separator) {
skip_extraneous_tokens(ctx, separator);
tokens.push(separator_tok);
} else if let Some(kind) = start_token {
if ctx.stream.next_kind_is(kind) {
ctx.diagnostics.push(kinds_error(
ctx.stream.pos_before(ctx.stream.peek().unwrap()),
&[separator],
));
} else {
break;
}
} else {
break;
}
Expand All @@ -83,17 +83,17 @@ where
}

pub fn parse_name_list(ctx: &mut ParsingContext<'_>) -> DiagnosticResult<Vec<WithTokenSpan<Name>>> {
Ok(parse_list_with_separator(ctx, Comma, parse_name)?.items)
Ok(parse_list_with_separator_or_recover(ctx, Comma, parse_name, None, Some(Identifier))?.items)
}

#[cfg(test)]
mod test {
use crate::ast::SeparatedList;
use crate::syntax::names::parse_association_element;
use crate::syntax::names::{parse_association_element, parse_name};
use crate::syntax::separated_list::{parse_list_with_separator_or_recover, parse_name_list};
use crate::syntax::test::Code;
use crate::syntax::Kind;
use crate::syntax::Kind::RightPar;
use crate::syntax::Kind::{Identifier, RightPar};
use crate::Diagnostic;

#[test]
Expand Down Expand Up @@ -176,6 +176,7 @@ mod test {
Kind::Comma,
parse_association_element,
Some(RightPar),
Some(Identifier),
);
ctx.stream.skip();
res
Expand Down Expand Up @@ -214,4 +215,47 @@ mod test {
)
);
}

#[test]
fn parse_list_with_missing_separator() {
let code = Code::new("a ,b, c d, e)");
let (res, diag) = code.with_stream_diagnostics(|ctx| {
let res = parse_list_with_separator_or_recover(
ctx,
Kind::Comma,
parse_name,
Some(RightPar),
Some(Identifier),
);
ctx.stream.skip();
res
});
assert_eq!(
res,
SeparatedList {
items: vec![
code.s1("a").name(),
code.s1("b").name(),
code.s1("c").name(),
code.s1("d").name(),
code.s1("e").name()
],
tokens: vec![
code.s(",", 1).token(),
code.s(",", 2).token(),
code.s(",", 3).token()
],
}
);

let mut c_pos = code.s1("c").pos().end_pos();
// single char position right after the 'c' character
c_pos.range.start.character += 1;
c_pos.range.end.character += 2;

assert_eq!(
diag,
vec![Diagnostic::syntax_error(c_pos, "Expected ','")]
);
}
}

0 comments on commit 950fe46

Please sign in to comment.