Don't skip variables in variable iterator (#991)

src/variables.rs

@@ -14,33 +14,43 @@ impl<'expression, 'src> Iterator for Variables<'expression, 'src> {
   type Item = Token<'src>;
 
   fn next(&mut self) -> Option<Token<'src>> {
-    match self.stack.pop() {
+    loop {
-      None
+      match self.stack.pop()? {
-      | Some(Expression::StringLiteral { .. })
+        Expression::StringLiteral { .. } | Expression::Backtick { .. } => {}
-      | Some(Expression::Backtick { .. })
+        Expression::Call { thunk } => match thunk {
-      | Some(Expression::Call { .. }) => None,
+          Thunk::Nullary { .. } => {}
-      Some(Expression::Conditional {
+          Thunk::Unary { arg, .. } => self.stack.push(arg),
+          Thunk::Binary { args, .. } => {
+            for arg in args.iter().rev() {
+              self.stack.push(arg);
+            }
+          }
+          Thunk::Ternary { args, .. } => {
+            for arg in args.iter().rev() {
+              self.stack.push(arg);
+            }
+          }
+        },
+        Expression::Conditional {
           lhs,
           rhs,
           then,
           otherwise,
           ..
-      }) => {
+        } => {
-        self.stack.push(lhs);
-        self.stack.push(rhs);
-        self.stack.push(then);
           self.stack.push(otherwise);
-        self.next()
+          self.stack.push(then);
-      }
-      Some(Expression::Variable { name, .. }) => Some(name.token()),
-      Some(Expression::Concatination { lhs, rhs }) => {
-        self.stack.push(lhs);
           self.stack.push(rhs);
-        self.next()
+          self.stack.push(lhs);
         }
-      Some(Expression::Group { contents }) => {
+        Expression::Variable { name, .. } => return Some(name.token()),
+        Expression::Concatination { lhs, rhs } => {
+          self.stack.push(rhs);
+          self.stack.push(lhs);
+        }
+        Expression::Group { contents } => {
           self.stack.push(contents);
-        self.next()
+        }
       }
     }
   }

tests/lib.rs

@@ -35,4 +35,5 @@ mod string;
 mod sublime_syntax;
 mod subsequents;
 mod tempdir;
+mod undefined_variables;
 mod working_directory;

tests/undefined_variables.rs

@@ -0,0 +1,97 @@
+use crate::common::*;
+
+#[test]
+fn parameter_default_unknown_variable_in_expression() {
+  Test::new()
+    .justfile("foo a=(b+''):")
+    .stderr(
+      "
+      error: Variable `b` not defined
+        |
+      1 | foo a=(b+''):
+        |        ^
+    ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+#[test]
+fn unknown_variable_in_unary_call() {
+  Test::new()
+    .justfile(
+      "
+    foo x=env_var(a):
+  ",
+    )
+    .stderr(
+      "
+      error: Variable `a` not defined
+        |
+      1 | foo x=env_var(a):
+        |               ^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+#[test]
+fn unknown_first_variable_in_binary_call() {
+  Test::new()
+    .justfile(
+      "
+    foo x=env_var_or_default(a, b):
+  ",
+    )
+    .stderr(
+      "
+      error: Variable `a` not defined
+        |
+      1 | foo x=env_var_or_default(a, b):
+        |                          ^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+#[test]
+fn unknown_second_variable_in_binary_call() {
+  Test::new()
+    .justfile(
+      "
+    foo x=env_var_or_default('', b):
+  ",
+    )
+    .stderr(
+      "
+      error: Variable `b` not defined
+        |
+      1 | foo x=env_var_or_default('', b):
+        |                              ^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+#[test]
+fn unknown_variable_in_ternary_call() {
+  Test::new()
+    .justfile(
+      "
+    foo x=replace(a, b, c):
+  ",
+    )
+    .stderr(
+      "
+      error: Variable `a` not defined
+        |
+      1 | foo x=replace(a, b, c):
+        |               ^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}