Improve error message if if is missing the else (#1252)

casey · Jun 30, 2022 · e1f729e · e1f729e
1 parent 0b750b6
commit e1f729e
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 16 deletions.
diff --git 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
@@ -42,7 +42,7 @@ pub(crate) enum CompileErrorKind<'src> {
   },
   ExpectedKeyword {
     expected: Vec<Keyword>,
-    found: &'src str,
+    found: Token<'src>,
   },
   ExtraLeadingWhitespace,
   FunctionArgumentCountMismatch {

diff --git a/src/parser.rs b/src/parser.rs
@@ -33,7 +33,7 @@ pub(crate) struct Parser<'tokens, 'src> {
   /// Current expected tokens
   expected: BTreeSet<TokenKind>,
   /// 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
@@ -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,
+}
diff --git 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
@@ -18,13 +18,13 @@ const RECURSION_LIMIT_REACHED: &str = "
 error: Parsing recursion depth exceeded
   |
 1 | foo: (x ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
-  |                                                                                                                                                                                                                                                                        ^
+  |                                                                                                                                                                                                                                                                         ^
 ";
 
 #[cfg(windows)]
 const RECURSION_LIMIT_REACHED: &str = "
 error: Parsing recursion depth exceeded
   |
 1 | foo: (x ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
-  |                                                                         ^
+  |                                                         ^
 ";