From 0fea73455bfd329e9e144f057a01af8891a27569 Mon Sep 17 00:00:00 2001
From: Casey Rodarmor <casey@rodarmor.com>
Date: Mon, 19 Jul 2021 18:10:35 -0700
Subject: [PATCH] Fix circular variable dependency error message (#909)

---
 src/assignment_resolver.rs | 12 ++++++------
 src/color.rs               |  6 +++---
 src/common.rs              |  4 +---
 src/default.rs             |  3 ---
 src/empty.rs               |  5 -----
 src/lib.rs                 |  2 --
 src/recipe_resolver.rs     |  2 +-
 tests/misc.rs              | 19 +++++++++++++++++++
 8 files changed, 30 insertions(+), 23 deletions(-)
 delete mode 100644 src/default.rs
 delete mode 100644 src/empty.rs

diff --git a/src/assignment_resolver.rs b/src/assignment_resolver.rs
index 9192850..4e0b8b3 100644
--- a/src/assignment_resolver.rs
+++ b/src/assignment_resolver.rs
@@ -5,7 +5,6 @@ use CompilationErrorKind::*;
 pub(crate) struct AssignmentResolver<'src: 'run, 'run> {
   assignments: &'run Table<'src, Assignment<'src>>,
   stack:       Vec<&'src str>,
-  seen:        BTreeSet<&'src str>,
   evaluated:   BTreeSet<&'src str>,
 }
 
@@ -14,9 +13,8 @@ impl<'src: 'run, 'run> AssignmentResolver<'src, 'run> {
     assignments: &Table<'src, Assignment<'src>>,
   ) -> CompilationResult<'src, ()> {
     let mut resolver = AssignmentResolver {
-      stack: empty(),
-      seen: empty(),
-      evaluated: empty(),
+      stack: Vec::new(),
+      evaluated: BTreeSet::new(),
       assignments,
     };
 
@@ -32,7 +30,6 @@ impl<'src: 'run, 'run> AssignmentResolver<'src, 'run> {
       return Ok(());
     }
 
-    self.seen.insert(name);
     self.stack.push(name);
 
     if let Some(assignment) = self.assignments.get(name) {
@@ -53,6 +50,9 @@ impl<'src: 'run, 'run> AssignmentResolver<'src, 'run> {
         token,
       });
     }
+
+    self.stack.pop();
+
     Ok(())
   }
 
@@ -62,7 +62,7 @@ impl<'src: 'run, 'run> AssignmentResolver<'src, 'run> {
         let variable = name.lexeme();
         if self.evaluated.contains(variable) {
           Ok(())
-        } else if self.seen.contains(variable) {
+        } else if self.stack.contains(&variable) {
           let token = self.assignments[variable].name.token();
           self.stack.push(variable);
           Err(token.error(CircularVariableDependency {
diff --git a/src/color.rs b/src/color.rs
index e057fdc..0e21580 100644
--- a/src/color.rs
+++ b/src/color.rs
@@ -41,21 +41,21 @@ impl Color {
   pub(crate) fn auto() -> Self {
     Self {
       use_color: UseColor::Auto,
-      ..default()
+      ..Color::default()
     }
   }
 
   pub(crate) fn always() -> Self {
     Self {
       use_color: UseColor::Always,
-      ..default()
+      ..Color::default()
     }
   }
 
   pub(crate) fn never() -> Self {
     Self {
       use_color: UseColor::Never,
-      ..default()
+      ..Color::default()
     }
   }
 
diff --git a/src/common.rs b/src/common.rs
index c27d3f8..441024a 100644
--- a/src/common.rs
+++ b/src/common.rs
@@ -33,9 +33,7 @@ pub(crate) use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
 pub(crate) use crate::{config_error, setting};
 
 // functions
-pub(crate) use crate::{
-  default::default, empty::empty, load_dotenv::load_dotenv, output::output, unindent::unindent,
-};
+pub(crate) use crate::{load_dotenv::load_dotenv, output::output, unindent::unindent};
 
 // traits
 pub(crate) use crate::{
diff --git a/src/default.rs b/src/default.rs
deleted file mode 100644
index 8a5bc09..0000000
--- a/src/default.rs
+++ /dev/null
@@ -1,3 +0,0 @@
-pub(crate) fn default<T: Default>() -> T {
-  Default::default()
-}
diff --git a/src/empty.rs b/src/empty.rs
deleted file mode 100644
index 51e4695..0000000
--- a/src/empty.rs
+++ /dev/null
@@ -1,5 +0,0 @@
-use crate::common::*;
-
-pub(crate) fn empty<T, C: iter::FromIterator<T>>() -> C {
-  iter::empty().collect()
-}
diff --git a/src/lib.rs b/src/lib.rs
index 783c747..e9a1503 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -71,10 +71,8 @@ mod compiler;
 mod config;
 mod config_error;
 mod count;
-mod default;
 mod delimiter;
 mod dependency;
-mod empty;
 mod enclosure;
 mod error;
 mod error_result_ext;
diff --git a/src/recipe_resolver.rs b/src/recipe_resolver.rs
index e062bd2..27e134c 100644
--- a/src/recipe_resolver.rs
+++ b/src/recipe_resolver.rs
@@ -14,7 +14,7 @@ impl<'src: 'run, 'run> RecipeResolver<'src, 'run> {
     assignments: &Table<'src, Assignment<'src>>,
   ) -> CompilationResult<'src, Table<'src, Rc<Recipe<'src>>>> {
     let mut resolver = RecipeResolver {
-      resolved_recipes: empty(),
+      resolved_recipes: Table::new(),
       unresolved_recipes,
       assignments,
     };
diff --git a/tests/misc.rs b/tests/misc.rs
index 76f4726..9ced506 100644
--- a/tests/misc.rs
+++ b/tests/misc.rs
@@ -1410,6 +1410,25 @@ test! {
   status:   EXIT_FAILURE,
 }
 
+test! {
+  name:     variable_circular_dependency_with_additional_variable,
+  justfile: "
+    a := ''
+    x := y
+    y := x
+
+    a:
+  ",
+  args:     ("a"),
+  stdout:   "",
+  stderr:   "error: Variable `x` depends on its own value: `x -> y -> x`
+  |
+2 | x := y
+  | ^
+",
+  status:   EXIT_FAILURE,
+}
+
 test! {
   name:     plus_variadic_recipe,
   justfile: "