From 9512eb403bbfde92b995ce46584d34a8123f1a75 Mon Sep 17 00:00:00 2001
From: Casey Rodarmor <casey@rodarmor.com>
Date: Fri, 8 Oct 2021 22:04:13 -0700
Subject: [PATCH] Don't skip variables in variable iterator (#991)

---
 src/variables.rs             | 64 ++++++++++++++----------
 tests/lib.rs                 |  1 +
 tests/undefined_variables.rs | 97 ++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+), 27 deletions(-)
 create mode 100644 tests/undefined_variables.rs

diff --git a/src/variables.rs b/src/variables.rs
index 87db132..af62aa6 100644
--- a/src/variables.rs
+++ b/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() {
-      None
-      | Some(Expression::StringLiteral { .. })
-      | Some(Expression::Backtick { .. })
-      | Some(Expression::Call { .. }) => None,
-      Some(Expression::Conditional {
-        lhs,
-        rhs,
-        then,
-        otherwise,
-        ..
-      }) => {
-        self.stack.push(lhs);
-        self.stack.push(rhs);
-        self.stack.push(then);
-        self.stack.push(otherwise);
-        self.next()
-      }
-      Some(Expression::Variable { name, .. }) => Some(name.token()),
-      Some(Expression::Concatination { lhs, rhs }) => {
-        self.stack.push(lhs);
-        self.stack.push(rhs);
-        self.next()
-      }
-      Some(Expression::Group { contents }) => {
-        self.stack.push(contents);
-        self.next()
+    loop {
+      match self.stack.pop()? {
+        Expression::StringLiteral { .. } | Expression::Backtick { .. } => {}
+        Expression::Call { thunk } => match thunk {
+          Thunk::Nullary { .. } => {}
+          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(otherwise);
+          self.stack.push(then);
+          self.stack.push(rhs);
+          self.stack.push(lhs);
+        }
+        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);
+        }
       }
     }
   }
diff --git a/tests/lib.rs b/tests/lib.rs
index 6f395d6..70bfd1e 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -35,4 +35,5 @@ mod string;
 mod sublime_syntax;
 mod subsequents;
 mod tempdir;
+mod undefined_variables;
 mod working_directory;
diff --git a/tests/undefined_variables.rs b/tests/undefined_variables.rs
new file mode 100644
index 0000000..0026253
--- /dev/null
+++ b/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();
+}