From e1f729efbc85bf5bfe60253391838cc0e2ead3cb Mon Sep 17 00:00:00 2001 From: Nick Kocharhook Date: Thu, 30 Jun 2022 11:34:11 +0100 Subject: [PATCH] Improve error message if `if` is missing the `else` (#1252) --- src/compile_error.rs | 23 +++++++++++++++++------ src/compile_error_kind.rs | 2 +- src/parser.rs | 14 +++++++------- tests/assignment.rs | 31 +++++++++++++++++++++++++++++++ tests/conditional.rs | 30 ++++++++++++++++++++++++++++++ tests/recursion_limit.rs | 4 ++-- 6 files changed, 88 insertions(+), 16 deletions(-) create mode 100644 tests/assignment.rs diff --git a/src/compile_error.rs b/src/compile_error.rs index b4def32..f36a393 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -117,12 +117,23 @@ impl Display for CompileError<'_> { DuplicateVariable { variable } => { write!(f, "Variable `{}` has multiple definitions", variable)?; } - ExpectedKeyword { expected, found } => write!( - f, - "Expected keyword {} but found identifier `{}`", - List::or_ticked(expected), - found - )?, + ExpectedKeyword { expected, found } => { + if found.kind == TokenKind::Identifier { + write!( + f, + "Expected keyword {} but found identifier `{}`", + List::or_ticked(expected), + found.lexeme() + )?; + } else { + write!( + f, + "Expected keyword {} but found `{}`", + List::or_ticked(expected), + found.kind + )?; + } + } ExtraLeadingWhitespace => { write!(f, "Recipe line has extra leading whitespace")?; } diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs index a425e24..29f508e 100644 --- a/src/compile_error_kind.rs +++ b/src/compile_error_kind.rs @@ -42,7 +42,7 @@ pub(crate) enum CompileErrorKind<'src> { }, ExpectedKeyword { expected: Vec, - found: &'src str, + found: Token<'src>, }, ExtraLeadingWhitespace, FunctionArgumentCountMismatch { diff --git a/src/parser.rs b/src/parser.rs index 99297fe..00ec7e7 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -33,7 +33,7 @@ pub(crate) struct Parser<'tokens, 'src> { /// Current expected tokens expected: BTreeSet, /// Current recursion depth - depth: u8, + depth: usize, } impl<'tokens, 'src> Parser<'tokens, 'src> { @@ -157,13 +157,12 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { } fn expect_keyword(&mut self, expected: Keyword) -> CompileResult<'src, ()> { - let identifier = self.expect(Identifier)?; - let found = identifier.lexeme(); + let found = self.advance()?; - if expected == found { + if found.kind == Identifier && expected == found.lexeme() { Ok(()) } else { - Err(identifier.error(CompileErrorKind::ExpectedKeyword { + Err(found.error(CompileErrorKind::ExpectedKeyword { expected: vec![expected], found, })) @@ -394,7 +393,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { /// Parse an expression, e.g. `1 + 2` fn parse_expression(&mut self) -> CompileResult<'src, Expression<'src>> { - if self.depth == if cfg!(windows) { 64 } else { 255 } { + if self.depth == if cfg!(windows) { 48 } else { 256 } { return Err(CompileError { token: self.next()?, kind: CompileErrorKind::ParsingRecursionDepthExceeded, @@ -422,6 +421,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { }; self.depth -= 1; + Ok(expression) } @@ -735,7 +735,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { } else { return Err(identifier.error(CompileErrorKind::ExpectedKeyword { expected: vec![Keyword::True, Keyword::False], - found: identifier.lexeme(), + found: identifier, })); }; diff --git a/tests/assignment.rs b/tests/assignment.rs new file mode 100644 index 0000000..ae6ea6e --- /dev/null +++ b/tests/assignment.rs @@ -0,0 +1,31 @@ +use super::*; + +test! { + name: set_export_parse_error, + justfile: " + set export := fals + ", + stdout: "", + stderr: " + error: Expected keyword `true` or `false` but found identifier `fals` + | + 1 | set export := fals + | ^^^^ + ", + status: EXIT_FAILURE, +} + +test! { + name: set_export_parse_error_EOL, + justfile: " + set export := fals + ", + stdout: "", + stderr: " + error: Expected keyword `true` or `false` but found `end of line` + | + 1 | set export := + | ^ + ", + status: EXIT_FAILURE, +} \ No newline at end of file diff --git a/tests/conditional.rs b/tests/conditional.rs index a0896c3..db8e389 100644 --- a/tests/conditional.rs +++ b/tests/conditional.rs @@ -168,3 +168,33 @@ test! { stdout: "b\n", stderr: "echo b\n", } + +test! { + name: missing_else, + justfile: " + TEST := if path_exists('/bin/bash') == 'true' {'yes'} + ", + stdout: "", + stderr: " + error: Expected keyword `else` but found `end of line` + | + 1 | TEST := if path_exists('/bin/bash') == 'true' {'yes'} + | ^ + ", + status: EXIT_FAILURE, +} + +test! { + name: incorrect_else_identifier, + justfile: " + TEST := if path_exists('/bin/bash') == 'true' {'yes'} els {'no'} + ", + stdout: "", + stderr: " + error: Expected keyword `else` but found identifier `els` + | + 1 | TEST := if path_exists('/bin/bash') == 'true' {'yes'} els {'no'} + | ^^^ + ", + status: EXIT_FAILURE, +} diff --git a/tests/recursion_limit.rs b/tests/recursion_limit.rs index 40782c3..7941c24 100644 --- a/tests/recursion_limit.rs +++ b/tests/recursion_limit.rs @@ -18,7 +18,7 @@ const RECURSION_LIMIT_REACHED: &str = " error: Parsing recursion depth exceeded | 1 | foo: (x (((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((( - | ^ + | ^ "; #[cfg(windows)] @@ -26,5 +26,5 @@ const RECURSION_LIMIT_REACHED: &str = " error: Parsing recursion depth exceeded | 1 | foo: (x (((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((( - | ^ + | ^ ";