Resolve recipe dependencies (#547)

Make analysis resolve recipe dependencies from names (`Name`) to recipes
(`Rc<Recipe>`), to give type-level certainty that resolution was performed
correctly and remove the need to look up dependencies on run.

src/alias_resolver.rs

@@ -6,13 +6,13 @@ where
   'a: 'b,
 {
   aliases: &'b Table<'a, Alias<'a>>,
-  recipes: &'b Table<'a, Recipe<'a>>,
+  recipes: &'b Table<'a, Rc<Recipe<'a>>>,
 }
 
 impl<'a: 'b, 'b> AliasResolver<'a, 'b> {
   pub(crate) fn resolve_aliases(
     aliases: &Table<'a, Alias<'a>>,
-    recipes: &Table<'a, Recipe<'a>>,
+    recipes: &Table<'a, Rc<Recipe<'a>>>,
   ) -> CompilationResult<'a, ()> {
     let resolver = AliasResolver { aliases, recipes };
 

src/analyzer.rs

@@ -3,7 +3,7 @@ use crate::common::*;
 use CompilationErrorKind::*;
 
 pub(crate) struct Analyzer<'src> {
-  recipes: Table<'src, Recipe<'src>>,
+  recipes: Table<'src, Recipe<'src, Name<'src>>>,
   assignments: Table<'src, Assignment<'src>>,
   aliases: Table<'src, Alias<'src>>,
   sets: Table<'src, Set<'src>>,
@@ -50,13 +50,12 @@ impl<'src> Analyzer<'src> {
       }
     }
 
-    let recipes = self.recipes;
     let assignments = self.assignments;
     let aliases = self.aliases;
 
     AssignmentResolver::resolve_assignments(&assignments)?;
 
-    RecipeResolver::resolve_recipes(&recipes, &assignments)?;
+    let recipes = RecipeResolver::resolve_recipes(self.recipes, &assignments)?;
 
     for recipe in recipes.values() {
       for parameter in &recipe.parameters {
@@ -66,15 +65,6 @@ impl<'src> Analyzer<'src> {
           }));
         }
       }
-
-      for dependency in &recipe.dependencies {
-        if !recipes[dependency.lexeme()].parameters.is_empty() {
-          return Err(dependency.error(DependencyHasParameters {
-            recipe: recipe.name(),
-            dependency: dependency.lexeme(),
-          }));
-        }
-      }
     }
 
     AliasResolver::resolve_aliases(&aliases, &recipes)?;
@@ -99,7 +89,7 @@ impl<'src> Analyzer<'src> {
     })
   }
 
-  fn analyze_recipe(&self, recipe: &Recipe<'src>) -> CompilationResult<'src, ()> {
+  fn analyze_recipe(&self, recipe: &Recipe<'src, Name<'src>>) -> CompilationResult<'src, ()> {
     if let Some(original) = self.recipes.get(recipe.name.lexeme()) {
       return Err(recipe.name.token().error(DuplicateRecipe {
         recipe: original.name(),

src/common.rs

@@ -11,6 +11,7 @@ pub(crate) use std::{
   ops::{Index, Range, RangeInclusive},
   path::{Path, PathBuf},
   process::{self, Command},
+  rc::Rc,
   str::{self, Chars},
   sync::{Mutex, MutexGuard},
   usize, vec,
@@ -50,12 +51,12 @@ pub(crate) use crate::{
   assignment_evaluator::AssignmentEvaluator, assignment_resolver::AssignmentResolver, color::Color,
   compilation_error::CompilationError, compilation_error_kind::CompilationErrorKind,
   compiler::Compiler, config::Config, config_error::ConfigError, count::Count,
-  enclosure::Enclosure, expression::Expression, fragment::Fragment, function::Function,
-  function_context::FunctionContext, functions::Functions, interrupt_guard::InterruptGuard,
-  interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, lexer::Lexer, line::Line,
-  list::List, load_error::LoadError, module::Module, name::Name, output_error::OutputError,
-  parameter::Parameter, parser::Parser, platform::Platform, position::Position,
-  positional::Positional, recipe::Recipe, recipe_context::RecipeContext,
+  dependency::Dependency, enclosure::Enclosure, expression::Expression, fragment::Fragment,
+  function::Function, function_context::FunctionContext, functions::Functions,
+  interrupt_guard::InterruptGuard, interrupt_handler::InterruptHandler, item::Item,
+  justfile::Justfile, lexer::Lexer, line::Line, list::List, load_error::LoadError, module::Module,
+  name::Name, output_error::OutputError, parameter::Parameter, parser::Parser, platform::Platform,
+  position::Position, positional::Positional, recipe::Recipe, recipe_context::RecipeContext,
   recipe_resolver::RecipeResolver, runtime_error::RuntimeError, search::Search,
   search_config::SearchConfig, search_error::SearchError, set::Set, setting::Setting,
   settings::Settings, shebang::Shebang, show_whitespace::ShowWhitespace, state::State,

src/dependency.rs

@@ -0,0 +1,4 @@
+use crate::common::*;
+
+#[derive(PartialEq, Debug)]
+pub(crate) struct Dependency<'a>(pub(crate) Rc<Recipe<'a>>);

src/expression.rs

@@ -26,9 +26,7 @@ pub(crate) enum Expression<'src> {
   /// `(contents)`
   Group { contents: Box<Expression<'src>> },
   /// `"string_literal"` or `'string_literal'`
-  StringLiteral {
-    string_literal: StringLiteral<'src>,
-  },
+  StringLiteral { string_literal: StringLiteral<'src> },
   /// `variable`
   Variable { name: Name<'src> },
 }

src/item.rs

@@ -5,6 +5,6 @@ use crate::common::*;
 pub(crate) enum Item<'src> {
   Alias(Alias<'src>),
   Assignment(Assignment<'src>),
-  Recipe(Recipe<'src>),
+  Recipe(Recipe<'src, Name<'src>>),
   Set(Set<'src>),
 }

src/justfile.rs

@@ -2,7 +2,7 @@ use crate::common::*;
 
 #[derive(Debug, PartialEq)]
 pub(crate) struct Justfile<'src> {
-  pub(crate) recipes: Table<'src, Recipe<'src>>,
+  pub(crate) recipes: Table<'src, Rc<Recipe<'src>>>,
   pub(crate) assignments: Table<'src, Assignment<'src>>,
   pub(crate) aliases: Table<'src, Alias<'src>>,
   pub(crate) settings: Settings<'src>,
@@ -11,7 +11,7 @@ pub(crate) struct Justfile<'src> {
 
 impl<'src> Justfile<'src> {
   pub(crate) fn first(&self) -> Option<&Recipe> {
-    let mut first: Option<&Recipe> = None;
+    let mut first: Option<&Recipe<Dependency>> = None;
     for recipe in self.recipes.values() {
       if let Some(first_recipe) = first {
         if recipe.line_number() < first_recipe.line_number() {
@@ -166,7 +166,7 @@ impl<'src> Justfile<'src> {
     if let Some(recipe) = self.recipes.get(name) {
       Some(recipe)
     } else if let Some(alias) = self.aliases.get(name) {
-      self.recipes.get(alias.target.lexeme())
+      self.recipes.get(alias.target.lexeme()).map(Rc::as_ref)
     } else {
       None
     }
@@ -181,10 +181,9 @@ impl<'src> Justfile<'src> {
     ran: &mut BTreeSet<&'src str>,
     overrides: &BTreeMap<String, String>,
   ) -> RunResult<()> {
-    for dependency_name in &recipe.dependencies {
-      let lexeme = dependency_name.lexeme();
-      if !ran.contains(lexeme) {
-        self.run_recipe(context, &self.recipes[lexeme], &[], dotenv, ran, overrides)?;
+    for Dependency(dependency) in &recipe.dependencies {
+      if !ran.contains(dependency.name()) {
+        self.run_recipe(context, dependency, &[], dotenv, ran, overrides)?;
       }
     }
     recipe.run(context, arguments, dotenv, overrides)?;

src/keyed.rs

@@ -1,3 +1,11 @@
+use crate::common::*;
+
 pub(crate) trait Keyed<'key> {
   fn key(&self) -> &'key str;
 }
+
+impl<'key, T: Keyed<'key>> Keyed<'key> for Rc<T> {
+  fn key(&self) -> &'key str {
+    self.as_ref().key()
+  }
+}

src/lib.rs

@@ -32,6 +32,7 @@ mod config;
 mod config_error;
 mod count;
 mod default;
+mod dependency;
 mod empty;
 mod enclosure;
 mod error;

src/node.rs

@@ -66,7 +66,7 @@ impl<'src> Node<'src> for Expression<'src> {
   }
 }
 
-impl<'src> Node<'src> for Recipe<'src> {
+impl<'src> Node<'src> for Recipe<'src, Name<'src>> {
   fn tree(&self) -> Tree<'src> {
     let mut t = Tree::atom("recipe");
 

src/parser.rs

@@ -471,7 +471,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     &mut self,
     doc: Option<&'src str>,
     quiet: bool,
-  ) -> CompilationResult<'src, Recipe<'src>> {
+  ) -> CompilationResult<'src, Recipe<'src, Name<'src>>> {
     let name = self.parse_name()?;
 
     let mut positional = Vec::new();

src/recipe.rs

@@ -24,8 +24,8 @@ fn error_from_signal(
 
 /// A recipe, e.g. `foo: bar baz`
 #[derive(PartialEq, Debug)]
-pub(crate) struct Recipe<'a> {
-  pub(crate) dependencies: Vec<Name<'a>>,
+pub(crate) struct Recipe<'a, D = Dependency<'a>> {
+  pub(crate) dependencies: Vec<D>,
   pub(crate) doc: Option<&'a str>,
   pub(crate) body: Vec<Line<'a>>,
   pub(crate) name: Name<'a>,
@@ -35,7 +35,7 @@ pub(crate) struct Recipe<'a> {
   pub(crate) shebang: bool,
 }
 
-impl<'a> Recipe<'a> {
+impl<'a, D> Recipe<'a, D> {
   pub(crate) fn argument_range(&self) -> RangeInclusive<usize> {
     self.min_arguments()..=self.max_arguments()
   }
@@ -319,7 +319,26 @@ impl<'a> Recipe<'a> {
   }
 }
 
-impl<'src> Keyed<'src> for Recipe<'src> {
+impl<'src> Recipe<'src, Name<'src>> {
+  pub(crate) fn resolve(self, resolved: Vec<Dependency<'src>>) -> Recipe<'src> {
+    assert_eq!(self.dependencies.len(), resolved.len());
+    for (name, resolved) in self.dependencies.iter().zip(&resolved) {
+      assert_eq!(name.lexeme(), resolved.0.name.lexeme());
+    }
+    Recipe {
+      dependencies: resolved,
+      doc: self.doc,
+      body: self.body,
+      name: self.name,
+      parameters: self.parameters,
+      private: self.private,
+      quiet: self.quiet,
+      shebang: self.shebang,
+    }
+  }
+}
+
+impl<'src, D> Keyed<'src> for Recipe<'src, D> {
   fn key(&self) -> &'src str {
     self.name.lexeme()
   }
@@ -342,7 +361,7 @@ impl<'a> Display for Recipe<'a> {
     }
     write!(f, ":")?;
     for dependency in &self.dependencies {
-      write!(f, " {}", dependency)?;
+      write!(f, " {}", dependency.0.name())?;
     }
 
     for (i, line) in self.body.iter().enumerate() {

src/recipe_resolver.rs

@@ -3,36 +3,31 @@ use crate::common::*;
 use CompilationErrorKind::*;
 
 pub(crate) struct RecipeResolver<'a: 'b, 'b> {
-  stack: Vec<&'a str>,
-  seen: BTreeSet<&'a str>,
-  resolved: BTreeSet<&'a str>,
-  recipes: &'b Table<'a, Recipe<'a>>,
+  unresolved_recipes: Table<'a, Recipe<'a, Name<'a>>>,
+  resolved_recipes: Table<'a, Rc<Recipe<'a>>>,
   assignments: &'b Table<'a, Assignment<'a>>,
 }
 
 impl<'a, 'b> RecipeResolver<'a, 'b> {
   pub(crate) fn resolve_recipes(
-    recipes: &Table<'a, Recipe<'a>>,
+    unresolved_recipes: Table<'a, Recipe<'a, Name<'a>>>,
     assignments: &Table<'a, Assignment<'a>>,
-  ) -> CompilationResult<'a, ()> {
+  ) -> CompilationResult<'a, Table<'a, Rc<Recipe<'a>>>> {
     let mut resolver = RecipeResolver {
-      seen: empty(),
-      stack: empty(),
-      resolved: empty(),
+      resolved_recipes: empty(),
+      unresolved_recipes,
       assignments,
-      recipes,
     };
 
-    for recipe in recipes.values() {
-      resolver.resolve_recipe(recipe)?;
-      resolver.seen = empty();
+    while let Some(unresolved) = resolver.unresolved_recipes.pop() {
+      resolver.resolve_recipe(&mut Vec::new(), unresolved)?;
     }
 
-    for recipe in recipes.values() {
+    for recipe in resolver.resolved_recipes.values() {
       for parameter in &recipe.parameters {
         if let Some(expression) = &parameter.default {
           for (function, argc) in expression.functions() {
-            resolver.resolve_function(function, argc)?;
+            Function::resolve(&function, argc)?;
           }
           for variable in expression.variables() {
             resolver.resolve_variable(&variable, &[])?;
@@ -44,7 +39,7 @@ impl<'a, 'b> RecipeResolver<'a, 'b> {
         for fragment in &line.fragments {
           if let Fragment::Interpolation { expression, .. } = fragment {
             for (function, argc) in expression.functions() {
-              resolver.resolve_function(function, argc)?;
+              Function::resolve(&function, argc)?;
             }
             for variable in expression.variables() {
               resolver.resolve_variable(&variable, &recipe.parameters)?;
@@ -54,11 +49,7 @@ impl<'a, 'b> RecipeResolver<'a, 'b> {
       }
     }
 
-    Ok(())
-  }
-
-  fn resolve_function(&self, function: Token<'a>, argc: usize) -> CompilationResult<'a, ()> {
-    Function::resolve(&function, argc)
+    Ok(resolver.resolved_recipes)
   }
 
   fn resolve_variable(
@@ -77,49 +68,67 @@ impl<'a, 'b> RecipeResolver<'a, 'b> {
     Ok(())
   }
 
-  fn resolve_recipe(&mut self, recipe: &Recipe<'a>) -> CompilationResult<'a, ()> {
-    if self.resolved.contains(recipe.name()) {
-      return Ok(());
+  fn resolve_recipe(
+    &mut self,
+    stack: &mut Vec<&'a str>,
+    recipe: Recipe<'a, Name<'a>>,
+  ) -> CompilationResult<'a, Rc<Recipe<'a>>> {
+    if let Some(resolved) = self.resolved_recipes.get(recipe.name()) {
+      return Ok(resolved.clone());
     }
-    self.stack.push(recipe.name());
-    self.seen.insert(recipe.name());
-    for dependency_token in recipe
-      .dependencies
-      .iter()
-      .map(|dependency| dependency.token())
-    {
-      match self.recipes.get(dependency_token.lexeme()) {
-        Some(dependency) => {
-          if !self.resolved.contains(dependency.name()) {
-            if self.seen.contains(dependency.name()) {
-              let first = self.stack[0];
-              self.stack.push(first);
-              return Err(
-                dependency_token.error(CircularRecipeDependency {
+
+    stack.push(recipe.name());
+
+    let mut dependencies: Vec<Dependency> = Vec::new();
+    for dependency in &recipe.dependencies {
+      let name = dependency.lexeme();
+
+      if let Some(resolved) = self.resolved_recipes.get(name) {
+        // dependency already resolved
+        if !resolved.parameters.is_empty() {
+          return Err(dependency.error(DependencyHasParameters {
             recipe: recipe.name(),
-                  circle: self
-                    .stack
+            dependency: name,
+          }));
+        }
+
+        dependencies.push(Dependency(resolved.clone()));
+      } else if stack.contains(&name) {
+        let first = stack[0];
+        stack.push(first);
+        return Err(
+          dependency.error(CircularRecipeDependency {
+            recipe: recipe.name(),
+            circle: stack
               .iter()
-                    .skip_while(|name| **name != dependency.name())
+              .skip_while(|name| **name != dependency.lexeme())
               .cloned()
               .collect(),
           }),
         );
-            }
-            self.resolve_recipe(dependency)?;
-          }
-        }
-        None => {
-          return Err(dependency_token.error(UnknownDependency {
+      } else if let Some(unresolved) = self.unresolved_recipes.remove(name) {
+        // resolve unresolved dependency
+        if !unresolved.parameters.is_empty() {
+          return Err(dependency.error(DependencyHasParameters {
             recipe: recipe.name(),
-            unknown: dependency_token.lexeme(),
+            dependency: name,
+          }));
+        }
+
+        dependencies.push(Dependency(self.resolve_recipe(stack, unresolved)?));
+      } else {
+        // dependency is unknown
+        return Err(dependency.error(UnknownDependency {
+          recipe: recipe.name(),
+          unknown: name,
         }));
       }
     }
-    }
-    self.resolved.insert(recipe.name());
-    self.stack.pop();
-    Ok(())
+
+    let resolved = Rc::new(recipe.resolve(dependencies));
+    self.resolved_recipes.insert(resolved.clone());
+    stack.pop();
+    Ok(resolved)
   }
 }
 

src/table.rs

@@ -35,6 +35,18 @@ impl<'key, V: Keyed<'key>> Table<'key, V> {
   pub(crate) fn iter(&self) -> btree_map::Iter<&'key str, V> {
     self.map.iter()
   }
+
+  pub(crate) fn pop(&mut self) -> Option<V> {
+    if let Some(key) = self.map.keys().next().map(|key| *key) {
+      self.map.remove(key)
+    } else {
+      None
+    }
+  }
+
+  pub(crate) fn remove(&mut self, key: &str) -> Option<V> {
+    self.map.remove(key)
+  }
 }
 
 impl<'key, V: Keyed<'key>> FromIterator<V> for Table<'key, V> {