Avoid exponential blowup when traversing dependencies

Cargo.toml

@@ -71,6 +71,7 @@ unreachable_pub = "deny"
 all = { level = "deny", priority = -1 }
 arbitrary_source_item_ordering = "deny"
 enum_glob_use = "allow"
+format-push-string = "allow"
 ignore_without_reason = "allow"
 needless_pass_by_value = "allow"
 pedantic = { level = "deny", priority = -1 }

src/justfile.rs

@@ -200,15 +200,20 @@ impl<'src> Justfile<'src> {
         let mut variable_references = HashSet::new();
 
         let mut stack = Vec::new();
+        let mut visited = HashSet::new();
 
         for invocation in &invocations {
-          stack.push(invocation.recipe);
+          if visited.insert(invocation.recipe.number) {
+            stack.push(invocation.recipe);
+          }
         }
 
         while let Some(recipe) = stack.pop() {
           variable_references.extend(&recipe.variable_references);
           for dependency in &recipe.dependencies {
-            stack.push(&dependency.recipe);
+            if visited.insert(dependency.recipe.number) {
+              stack.push(&dependency.recipe);
+            }
           }
         }
 

src/numerator.rs

@@ -1,15 +1,27 @@
 use super::*;
 
-pub(crate) struct Numerator(u32);
+pub(crate) struct Numerator {
+  bindings: u32,
+  recipes: u32,
+}
 
 impl Numerator {
   pub(crate) fn new() -> Self {
-    Self(constants().len().try_into().unwrap())
+    Self {
+      bindings: constants().len().try_into().unwrap(),
+      recipes: 0,
+    }
+  }
+
+  pub(crate) fn next_binding(&mut self) -> Number {
+    let id = self.bindings;
+    self.bindings += 1;
+    Number(id)
   }
 
-  pub(crate) fn next(&mut self) -> Number {
-    let id = self.0;
-    self.0 += 1;
+  pub(crate) fn next_recipe(&mut self) -> Number {
+    let id = self.recipes;
+    self.recipes += 1;
     Number(id)
   }
 

src/parser.rs

@@ -640,7 +640,7 @@ impl<'run, 'src> Parser<'run, 'src> {
 
     let mut parameters = Vec::new();
     while !self.next_is(ParenR) {
-      parameters.push((self.parse_name()?, self.numerator.next()));
+      parameters.push((self.parse_name()?, self.numerator.next_binding()));
       if !self.accepted(Comma)? {
         break;
       }
@@ -681,7 +681,7 @@ impl<'run, 'src> Parser<'run, 'src> {
       export,
       file_depth: self.file_depth,
       name,
-      number: self.numerator.next(),
+      number: self.numerator.next_binding(),
       prelude: false,
       private: private || name.lexeme().starts_with('_'),
       value,
@@ -1422,6 +1422,7 @@ impl<'run, 'src> Parser<'run, 'src> {
       import_offsets: self.import_offsets.clone(),
       module_path: None,
       name,
+      number: self.numerator.next_recipe(),
       parameters: positional.into_iter().chain(variadic).collect(),
       priors,
       private,
@@ -1492,7 +1493,7 @@ impl<'run, 'src> Parser<'run, 'src> {
       min: min.map(|(_key, min)| min),
       multiple,
       name,
-      number: self.numerator.next(),
+      number: self.numerator.next_binding(),
       pattern,
       short,
       value,

src/recipe.rs

@@ -14,6 +14,8 @@ pub(crate) struct Recipe<'src, D = Dependency<'src>> {
   #[serde(skip)]
   pub(crate) module_path: Option<Modulepath>,
   pub(crate) name: Name<'src>,
+  #[serde(skip)]
+  pub(crate) number: Number,
   pub(crate) parameters: Vec<Parameter<'src>>,
   pub(crate) priors: usize,
   pub(crate) private: bool,

src/unresolved_recipe.rs

@@ -222,6 +222,7 @@ impl<'src> UnresolvedRecipe<'src> {
       import_offsets: self.import_offsets,
       module_path: Some(modulepath.clone()),
       name: self.name,
+      number: self.number,
       parameters: self.parameters,
       priors: self.priors,
       private: self.private,

tests/run.rs

@@ -56,3 +56,45 @@ fn time_reports_time_when_specified() {
     .stderr_regex(r"---> foo completed in \d+\.\d+s\n")
     .success();
 }
+
+#[test]
+fn dependency_traversal_is_not_exponential() {
+  let n = 40;
+
+  let mut justfile = String::new();
+
+  for i in 0..n {
+    justfile.push_str(&format!("r{i}: r{} r{}\n", i + 1, i + 1));
+  }
+
+  justfile.push_str(&format!("r{n}:\n"));
+
+  let tempdir = tempdir();
+
+  fs::write(tempdir.path().join("justfile"), justfile).unwrap();
+
+  let mut child = Command::new(JUST)
+    .args(["--dry-run", "r0"])
+    .current_dir(tempdir.path())
+    .stdin(Stdio::null())
+    .stdout(Stdio::null())
+    .stderr(Stdio::null())
+    .spawn()
+    .unwrap();
+
+  let start = Instant::now();
+
+  while start.elapsed() < Duration::from_secs(60) {
+    if let Some(status) = child.try_wait().unwrap() {
+      assert!(status.success());
+      return;
+    }
+
+    thread::sleep(Duration::from_millis(100));
+  }
+
+  child.kill().unwrap();
+  child.wait().unwrap();
+
+  panic!("dependency graph traversal of {n} recipes did not complete within 60 seconds");
+}