Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow the parser to add contextual help text to errors; Add contextual help text for parenthesized for loops #1828

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Allow the parser to add contextual help text to errors; Add contextual help text for parenthesized for loops #1828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad2a07b
wip
sezna Aug 9, 2024
27b3166
add help text to parser errors
sezna Aug 9, 2024
330fbc3
add tests; make help text trigger more specific
sezna Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions compiler/qsc_parse/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,22 @@ fn expr_base(s: &mut ParserContext) -> Result<Box<Expr>> {
} else if token(s, TokenKind::Keyword(Keyword::Fail)).is_ok() {
Ok(Box::new(ExprKind::Fail(expr(s)?)))
} else if token(s, TokenKind::Keyword(Keyword::For)).is_ok() {
let vars = pat(s)?;
let peeked = s.peek();
let vars = match pat(s) {
Ok(o) => o,
Err(e) => {
if let (
TokenKind::Open(Delim::Paren),
ErrorKind::Token(_, TokenKind::Keyword(Keyword::In), _),
) = (peeked.kind, &e.0)
{
return Err(
e.with_help("parenthesis are not permitted around for-loop iterations")
);
}
return Err(e);
}
};
token(s, TokenKind::Keyword(Keyword::In))?;
let iter = expr(s)?;
let body = stmt::parse_block(s)?;
Expand Down Expand Up @@ -209,7 +224,7 @@ fn expr_base(s: &mut ParserContext) -> Result<Box<Expr>> {
} else if let Some(p) = opt(s, single_ident_path)? {
Ok(Box::new(ExprKind::Path(p)))
} else {
Err(Error(ErrorKind::Rule(
Err(Error::new(ErrorKind::Rule(
"expression",
s.peek().kind,
s.peek().span,
Expand Down Expand Up @@ -294,7 +309,7 @@ fn expr_set(s: &mut ParserContext) -> Result<Box<ExprKind>> {
let rhs = expr(s)?;
Ok(Box::new(ExprKind::AssignOp(closed_bin_op(op), lhs, rhs)))
} else {
Err(Error(ErrorKind::Rule(
Err(Error::new(ErrorKind::Rule(
"assignment operator",
s.peek().kind,
s.peek().span,
Expand Down Expand Up @@ -355,7 +370,7 @@ fn expr_interpolate(s: &mut ParserContext) -> Result<Vec<StringComponent>> {
let TokenKind::String(StringToken::Interpolated(InterpolatedStart::DollarQuote, mut end)) =
token.kind
else {
return Err(Error(ErrorKind::Rule(
return Err(Error::new(ErrorKind::Rule(
"interpolated string",
token.kind,
token.span,
Expand All @@ -376,7 +391,7 @@ fn expr_interpolate(s: &mut ParserContext) -> Result<Vec<StringComponent>> {
let TokenKind::String(StringToken::Interpolated(InterpolatedStart::RBrace, next_end)) =
token.kind
else {
return Err(Error(ErrorKind::Rule(
return Err(Error::new(ErrorKind::Rule(
"interpolated string",
token.kind,
token.span,
Expand Down Expand Up @@ -419,20 +434,20 @@ fn lit_token(lexeme: &str, token: Token) -> Result<Option<Lit>> {
let offset = if radix == Radix::Decimal { 0 } else { 2 };
let lexeme = &lexeme[offset..lexeme.len() - 1]; // Slice off prefix and suffix.
let value = BigInt::from_str_radix(lexeme, radix.into())
.map_err(|_| Error(ErrorKind::Lit("big-integer", token.span)))?;
.map_err(|_| Error::new(ErrorKind::Lit("big-integer", token.span)))?;
Ok(Some(Lit::BigInt(Box::new(value))))
}
TokenKind::Float => {
let lexeme = lexeme.replace('_', "");
let value = lexeme
.parse()
.map_err(|_| Error(ErrorKind::Lit("floating-point", token.span)))?;
.map_err(|_| Error::new(ErrorKind::Lit("floating-point", token.span)))?;
Ok(Some(Lit::Double(value)))
}
TokenKind::Int(radix) => {
let offset = if radix == Radix::Decimal { 0 } else { 2 };
let value = lit_int(&lexeme[offset..], radix.into())
.ok_or(Error(ErrorKind::Lit("integer", token.span)))?;
.ok_or(Error::new(ErrorKind::Lit("integer", token.span)))?;
Ok(Some(Lit::Int(value)))
}
TokenKind::String(StringToken::Normal) => {
Expand All @@ -445,7 +460,7 @@ fn lit_token(lexeme: &str, token: Token) -> Result<Option<Lit>> {
let index: u32 = index.try_into().expect("index should fit into u32");
let lo = token.span.lo + index + 2;
let span = Span { lo, hi: lo + 1 };
Error(ErrorKind::Escape(ch, span))
Error::new(ErrorKind::Escape(ch, span))
})?;
Ok(Some(Lit::String(string.into())))
}
Expand Down Expand Up @@ -759,7 +774,7 @@ fn expr_as_pat(expr: Expr) -> Result<Box<Pat>> {
.collect::<Result<_>>()?;
Ok(PatKind::Tuple(pats))
}
_ => Err(Error(ErrorKind::Convert(
_ => Err(Error::new(ErrorKind::Convert(
"pattern",
"expression",
expr.span,
Expand Down
80 changes: 80 additions & 0 deletions compiler/qsc_parse/src/expr/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2521,3 +2521,83 @@ fn invalid_initial_commas_in_pattern() {
]"#]],
);
}

#[test]
fn friendly_error_on_parenthesized_for() {
check(
expr,
"for (x in xs) { () }",
&expect![[r#"
Error(
Token(
Close(
Paren,
),
Keyword(
In,
),
Span {
lo: 7,
hi: 9,
},
),
Some(
"parenthesis are not permitted around for-loop iterations",
),
)
"#]],
);
}

#[test]
fn no_help_text_on_non_parenthesized_for() {
check(
expr,
"for x in invalid syntax { () }",
&expect![[r#"
Error(
Token(
Open(
Brace,
),
Ident,
Span {
lo: 17,
hi: 23,
},
),
)
"#]],
);
}

#[test]
fn parenthesized_pat_in_for_is_valid() {
check(
expr,
"for (x) in xs { () }",
&expect![[r#"
Expr _id_ [0-20]: For:
Pat _id_ [4-7]: Paren:
Pat _id_ [5-6]: Bind:
Ident _id_ [5-6] "x"
Expr _id_ [11-13]: Path: Path _id_ [11-13] (Ident _id_ [11-13] "xs")
Block _id_ [14-20]:
Stmt _id_ [16-18]: Expr: Expr _id_ [16-18]: Unit"#]],
);
}

#[test]
fn parenthesized_expr_in_for_is_valid() {
check(
expr,
"for x in (xs) { () }",
&expect![[r#"
Expr _id_ [0-20]: For:
Pat _id_ [4-5]: Bind:
Ident _id_ [4-5] "x"
Expr _id_ [9-13]: Paren: Expr _id_ [10-12]: Path: Path _id_ [10-12] (Ident _id_ [10-12] "xs")
Block _id_ [14-20]:
Stmt _id_ [16-18]: Expr: Expr _id_ [16-18]: Unit"#]],
);
}
52 changes: 34 additions & 18 deletions compiler/qsc_parse/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ pub(super) fn parse(s: &mut ParserContext) -> Result<Box<Item>> {
Box::new(ItemKind::ImportOrExport(decl))
} else if visibility.is_some() {
let err_item = default(s.span(lo));
s.push_error(Error(ErrorKind::FloatingVisibility(err_item.span)));
s.push_error(Error::new(ErrorKind::FloatingVisibility(err_item.span)));
return Ok(err_item);
} else if !attrs.is_empty() {
let err_item = default(s.span(lo));
s.push_error(Error(ErrorKind::FloatingAttr(err_item.span)));
s.push_error(Error::new(ErrorKind::FloatingAttr(err_item.span)));
return Ok(err_item);
} else if doc.is_some() {
let err_item = default(s.span(lo));
s.push_error(Error(ErrorKind::FloatingDocComment(err_item.span)));
s.push_error(Error::new(ErrorKind::FloatingDocComment(err_item.span)));
return Ok(err_item);
} else {
let p = s.peek();
return Err(Error(ErrorKind::Rule("item", p.kind, p.span)));
return Err(Error::new(ErrorKind::Rule("item", p.kind, p.span)));
};

Ok(Box::new(Item {
Expand Down Expand Up @@ -133,7 +133,7 @@ fn parse_top_level_node(s: &mut ParserContext) -> Result<TopLevelNode> {
if let StmtKind::Item(item) = &mut *stmt.kind {
item.doc = doc.into();
} else if !doc.is_empty() {
return Err(Error(ErrorKind::Rule("item", kind, span)));
return Err(Error::new(ErrorKind::Rule("item", kind, span)));
}
Ok(TopLevelNode::Stmt(stmt))
}
Expand All @@ -153,7 +153,10 @@ pub fn parse_implicit_namespace(source_name: &str, s: &mut ParserContext) -> Res
let items = parse_namespace_block_contents(s)?;
recovering_token(s, TokenKind::Eof);
if items.is_empty() || s.peek().kind != TokenKind::Eof {
return Err(Error(ErrorKind::ExpectedItem(s.peek().kind, s.span(lo))));
return Err(Error::new(ErrorKind::ExpectedItem(
s.peek().kind,
s.span(lo),
)));
}
let span = s.span(lo);
let namespace_name = source_name_to_namespace_name(source_name, span)?;
Expand All @@ -177,7 +180,7 @@ fn source_name_to_namespace_name(raw: &str, span: Span) -> Result<Idents> {
match component {
std::path::Component::Normal(name) => {
// strip the extension off, if there is one
let mut name = name.to_str().ok_or(Error(ErrorKind::InvalidFileName(
let mut name = name.to_str().ok_or(Error::new(ErrorKind::InvalidFileName(
span,
name.to_string_lossy().to_string(),
)))?;
Expand All @@ -192,7 +195,12 @@ fn source_name_to_namespace_name(raw: &str, span: Span) -> Result<Idents> {

namespace.push(ident);
}
_ => return Err(Error(ErrorKind::InvalidFileName(span, raw.to_string()))),
_ => {
return Err(Error::new(ErrorKind::InvalidFileName(
span,
raw.to_string(),
)))
}
}
}

Expand All @@ -216,9 +224,9 @@ fn validate_namespace_name(error_span: Span, name: &str) -> Result<Ident> {
// we just directly use the ident parser here instead of trying to recreate
// validation rules
let ident = ident(&mut s)
.map_err(|_| Error(ErrorKind::InvalidFileName(error_span, name.to_string())))?;
.map_err(|_| Error::new(ErrorKind::InvalidFileName(error_span, name.to_string())))?;
if s.peek().kind != TokenKind::Eof {
return Err(Error(ErrorKind::InvalidFileName(
return Err(Error::new(ErrorKind::InvalidFileName(
error_span,
name.to_string(),
)));
Expand Down Expand Up @@ -311,7 +319,7 @@ fn parse_open(s: &mut ParserContext) -> Result<Box<ItemKind>> {
// Peek to see if the next token is a dot -- this means it is likely a dot ident alias, and
// we want to provide a more helpful error message
if s.peek().kind == TokenKind::Dot {
return Err(Error(ErrorKind::DotIdentAlias(s.peek().span)));
return Err(Error::new(ErrorKind::DotIdentAlias(s.peek().span)));
}

token(s, TokenKind::Semi)?;
Expand Down Expand Up @@ -410,7 +418,11 @@ fn parse_ty_def(s: &mut ParserContext) -> Result<Box<TyDef>> {

fn ty_as_ident(ty: Ty) -> Result<Box<Ident>> {
let TyKind::Path(path) = *ty.kind else {
return Err(Error(ErrorKind::Convert("identifier", "type", ty.span)));
return Err(Error::new(ErrorKind::Convert(
"identifier",
"type",
ty.span,
)));
};
if let Path {
segments: None,
Expand All @@ -420,7 +432,11 @@ fn ty_as_ident(ty: Ty) -> Result<Box<Ident>> {
{
Ok(name)
} else {
Err(Error(ErrorKind::Convert("identifier", "type", ty.span)))
Err(Error::new(ErrorKind::Convert(
"identifier",
"type",
ty.span,
)))
}
}

Expand All @@ -433,7 +449,7 @@ fn parse_callable_decl(s: &mut ParserContext) -> Result<Box<CallableDecl>> {
CallableKind::Operation
} else {
let token = s.peek();
return Err(Error(ErrorKind::Rule(
return Err(Error::new(ErrorKind::Rule(
"callable declaration",
token.kind,
token.span,
Expand Down Expand Up @@ -510,7 +526,7 @@ fn parse_spec_decl(s: &mut ParserContext) -> Result<Box<SpecDecl>> {
Spec::Ctl
}
} else {
return Err(Error(ErrorKind::Rule(
return Err(Error::new(ErrorKind::Rule(
"specialization",
s.peek().kind,
s.peek().span,
Expand Down Expand Up @@ -544,7 +560,7 @@ fn parse_spec_gen(s: &mut ParserContext) -> Result<SpecGen> {
} else if token(s, TokenKind::Keyword(Keyword::Slf)).is_ok() {
Ok(SpecGen::Slf)
} else {
Err(Error(ErrorKind::Rule(
Err(Error::new(ErrorKind::Rule(
"specialization generator",
s.peek().kind,
s.peek().span,
Expand All @@ -557,7 +573,7 @@ pub(super) fn check_input_parens(inputs: &Pat) -> Result<()> {
if matches!(*inputs.kind, PatKind::Paren(_) | PatKind::Tuple(_)) {
Ok(())
} else {
Err(Error(ErrorKind::MissingParens(inputs.span)))
Err(Error::new(ErrorKind::MissingParens(inputs.span)))
}
}

Expand All @@ -578,7 +594,7 @@ fn parse_import_or_export(s: &mut ParserContext) -> Result<ImportOrExportDecl> {
TokenKind::Keyword(Keyword::Export) => true,
TokenKind::Keyword(Keyword::Import) => false,
_ => {
return Err(Error(ErrorKind::Rule(
return Err(Error::new(ErrorKind::Rule(
"import or export",
s.peek().kind,
s.peek().span,
Expand Down
Loading
Loading