Allow depending on recipes in submodules (#2672)

README.md

@@ -690,6 +690,14 @@ cc main.c foo.c bar.c -o main
 testing… all tests passed!
 ```
 
+Recipes may depend on recipes in submodules:
+
+```justfile
+mod foo
+
+baz: foo::bar
+```
+
 Examples
 --------
 

src/analyzer.rs

@@ -145,7 +145,12 @@ impl<'run, 'src> Analyzer<'run, 'src> {
       }
     }
 
-    let recipes = RecipeResolver::resolve_recipes(&assignments, &settings, deduplicated_recipes)?;
+    let recipes = RecipeResolver::resolve_recipes(
+      &assignments,
+      &self.modules,
+      &settings,
+      deduplicated_recipes,
+    )?;
 
     let mut aliases = Table::new();
     while let Some(alias) = self.aliases.pop() {
@@ -295,7 +300,7 @@ impl<'run, 'src> Analyzer<'run, 'src> {
     recipes: &'a Table<'src, Rc<Recipe<'src>>>,
     alias: Alias<'src, Namepath<'src>>,
   ) -> CompileResult<'src, Alias<'src>> {
-    match Self::alias_target(&alias.target, modules, recipes) {
+    match Self::resolve_recipe(&alias.target, modules, recipes) {
       Some(target) => Ok(alias.resolve(target)),
       None => Err(alias.name.token.error(UnknownAliasTarget {
         alias: alias.name.lexeme(),
@@ -304,7 +309,7 @@ impl<'run, 'src> Analyzer<'run, 'src> {
     }
   }
 
-  fn alias_target<'a>(
+  pub(crate) fn resolve_recipe<'a>(
     path: &Namepath<'src>,
     mut modules: &'a Table<'src, Justfile<'src>>,
     mut recipes: &'a Table<'src, Rc<Recipe<'src>>>,

src/compile_error_kind.rs

@@ -18,7 +18,7 @@ pub(crate) enum CompileErrorKind<'src> {
     circle: Vec<&'src str>,
   },
   DependencyArgumentCountMismatch {
-    dependency: &'src str,
+    dependency: Namepath<'src>,
     found: usize,
     min: usize,
     max: usize,
@@ -145,7 +145,7 @@ pub(crate) enum CompileErrorKind<'src> {
   },
   UnknownDependency {
     recipe: &'src str,
-    unknown: &'src str,
+    unknown: Namepath<'src>,
   },
   UnknownFunction {
     function: &'src str,

src/namepath.rs

@@ -32,8 +32,7 @@ impl<'src> Namepath<'src> {
     self.0.iter()
   }
 
-  #[cfg(test)]
-  pub(crate) fn len(&self) -> usize {
+  pub(crate) fn components(&self) -> usize {
     self.0.len()
   }
 }

src/node.rs

@@ -65,7 +65,7 @@ impl<'src> Node<'src> for Item<'src> {
 
 impl<'src> Node<'src> for Namepath<'src> {
   fn tree(&self) -> Tree<'src> {
-    match self.len() {
+    match self.components() {
       1 => Tree::atom(self.last().lexeme()),
       _ => Tree::list(
         self
@@ -237,7 +237,7 @@ impl<'src> Node<'src> for UnresolvedRecipe<'src> {
       let mut subsequents = Tree::atom("sups");
 
       for (i, dependency) in self.dependencies.iter().enumerate() {
-        let mut d = Tree::atom(dependency.recipe.lexeme());
+        let mut d = dependency.recipe.tree();
 
         for argument in &dependency.arguments {
           d.push_mut(argument.tree());

src/parser.rs

@@ -246,10 +246,10 @@ impl<'run, 'src> Parser<'run, 'src> {
     }
   }
 
-  /// Accept a token of kind `Identifier` and parse into a `Name`
-  fn accept_name(&mut self) -> CompileResult<'src, Option<Name<'src>>> {
+  /// Accept a double-colon separated sequence of identifiers
+  fn accept_namepath(&mut self) -> CompileResult<'src, Option<Namepath<'src>>> {
     if self.next_is(Identifier) {
-      Ok(Some(self.parse_name()?))
+      Ok(Some(self.parse_namepath()?))
     } else {
       Ok(None)
     }
@@ -268,13 +268,13 @@ impl<'run, 'src> Parser<'run, 'src> {
 
   /// Accept a dependency
   fn accept_dependency(&mut self) -> CompileResult<'src, Option<UnresolvedDependency<'src>>> {
-    if let Some(recipe) = self.accept_name()? {
+    if let Some(recipe) = self.accept_namepath()? {
       Ok(Some(UnresolvedDependency {
         arguments: Vec::new(),
         recipe,
       }))
     } else if self.accepted(ParenL)? {
-      let recipe = self.parse_name()?;
+      let recipe = self.parse_namepath()?;
 
       let mut arguments = Vec::new();
 
@@ -1631,6 +1631,24 @@ mod tests {
     tree: (justfile (recipe foo (body ("bar")))),
   }
 
+  test! {
+    name: recipe_dependency_module,
+    text: "foo: bar::baz",
+    tree: (justfile (recipe foo (deps (bar baz)))),
+  }
+
+  test! {
+    name: recipe_dependency_parenthesis_module,
+    text: "foo: (bar::baz)",
+    tree: (justfile (recipe foo (deps (bar baz)))),
+  }
+
+  test! {
+    name: recipe_dependency_module_mixed,
+      text: "foo: bar::baz qux",
+    tree: (justfile (recipe foo (deps (bar baz) qux))),
+  }
+
   test! {
     name: recipe_line_multiple,
     text: "foo:\n bar\n baz\n {{\"bob\"}}biz",
@@ -2534,7 +2552,7 @@ mod tests {
     column:  9,
     width:   1,
     kind:    UnexpectedToken{
-      expected: vec![AmpersandAmpersand, Comment, Eof, Eol, Identifier, ParenL],
+      expected: vec![AmpersandAmpersand, ColonColon, Comment, Eof, Eol, Identifier, ParenL],
       found: Equals
     },
   }

src/recipe_resolver.rs

@@ -2,6 +2,7 @@ use {super::*, CompileErrorKind::*};
 
 pub(crate) struct RecipeResolver<'src: 'run, 'run> {
   assignments: &'run Table<'src, Assignment<'src>>,
+  modules: &'run Table<'src, Justfile<'src>>,
   resolved_recipes: Table<'src, Rc<Recipe<'src>>>,
   unresolved_recipes: Table<'src, UnresolvedRecipe<'src>>,
 }
@@ -9,6 +10,7 @@ pub(crate) struct RecipeResolver<'src: 'run, 'run> {
 impl<'src: 'run, 'run> RecipeResolver<'src, 'run> {
   pub(crate) fn resolve_recipes(
     assignments: &'run Table<'src, Assignment<'src>>,
+    modules: &'run Table<'src, Justfile<'src>>,
     settings: &Settings,
     unresolved_recipes: Table<'src, UnresolvedRecipe<'src>>,
   ) -> CompileResult<'src, Table<'src, Rc<Recipe<'src>>>> {
@@ -16,6 +18,7 @@ impl<'src: 'run, 'run> RecipeResolver<'src, 'run> {
       resolved_recipes: Table::new(),
       unresolved_recipes,
       assignments,
+      modules,
     };
 
     while let Some(unresolved) = resolver.unresolved_recipes.pop() {
@@ -86,37 +89,20 @@ impl<'src: 'run, 'run> RecipeResolver<'src, 'run> {
 
     stack.push(recipe.name());
 
-    let mut dependencies: Vec<Rc<Recipe>> = Vec::new();
-    for dependency in &recipe.dependencies {
-      let name = dependency.recipe.lexeme();
-
-      if let Some(resolved) = self.resolved_recipes.get(name) {
-        // dependency already resolved
-        dependencies.push(Rc::clone(resolved));
-      } else if stack.contains(&name) {
-        let first = stack[0];
-        stack.push(first);
-        return Err(
-          dependency.recipe.error(CircularRecipeDependency {
-            recipe: recipe.name(),
-            circle: stack
-              .iter()
-              .skip_while(|name| **name != dependency.recipe.lexeme())
-              .copied()
-              .collect(),
-          }),
-        );
-      } else if let Some(unresolved) = self.unresolved_recipes.remove(name) {
-        // resolve unresolved dependency
-        dependencies.push(self.resolve_recipe(stack, unresolved)?);
-      } else {
-        // dependency is unknown
-        return Err(dependency.recipe.error(UnknownDependency {
-          recipe: recipe.name(),
-          unknown: name,
-        }));
-      }
-    }
+    let dependencies = recipe
+      .dependencies
+      .iter()
+      .map(|dependency| {
+        self
+          .resolve_dependency(dependency, &recipe, stack)?
+          .ok_or_else(|| {
+            dependency.recipe.last().error(UnknownDependency {
+              recipe: recipe.name(),
+              unknown: dependency.recipe.clone(),
+            })
+          })
+      })
+      .collect::<CompileResult<Vec<Rc<Recipe>>>>()?;
 
     stack.pop();
 
@@ -124,6 +110,47 @@ impl<'src: 'run, 'run> RecipeResolver<'src, 'run> {
     self.resolved_recipes.insert(Rc::clone(&resolved));
     Ok(resolved)
   }
+
+  fn resolve_dependency(
+    &mut self,
+    dependency: &UnresolvedDependency<'src>,
+    recipe: &UnresolvedRecipe<'src>,
+    stack: &mut Vec<&'src str>,
+  ) -> CompileResult<'src, Option<Rc<Recipe<'src>>>> {
+    let name = dependency.recipe.last().lexeme();
+
+    if dependency.recipe.components() > 1 {
+      // recipe is in a submodule and is thus already resovled
+      Ok(Analyzer::resolve_recipe(
+        &dependency.recipe,
+        self.modules,
+        &self.resolved_recipes,
+      ))
+    } else if let Some(resolved) = self.resolved_recipes.get(name) {
+      // recipe is the current module and has already been resolved
+      Ok(Some(Rc::clone(resolved)))
+    } else if stack.contains(&name) {
+      // recipe depends on itself
+      let first = stack[0];
+      stack.push(first);
+      return Err(
+        dependency.recipe.last().error(CircularRecipeDependency {
+          recipe: recipe.name(),
+          circle: stack
+            .iter()
+            .skip_while(|name| **name != dependency.recipe.last().lexeme())
+            .copied()
+            .collect(),
+        }),
+      );
+    } else if let Some(unresolved) = self.unresolved_recipes.remove(name) {
+      // recipe is as of yet unresolved
+      Ok(Some(self.resolve_recipe(stack, unresolved)?))
+    } else {
+      // recipe is unknown
+      Ok(None)
+    }
+  }
 }
 
 #[cfg(test)]
@@ -157,7 +184,18 @@ mod tests {
     line:   0,
     column: 3,
     width:  1,
-    kind:   UnknownDependency{recipe: "a", unknown: "b"},
+    kind:   UnknownDependency{
+      recipe: "a",
+      unknown: Namepath::from(Name::from_identifier(
+        Token{
+          column: 3,
+          kind: TokenKind::Identifier,
+          length: 1,
+          line: 0,
+          offset: 3,
+          path: Path::new("justfile"),
+          src: "a: b" }))
+    },
   }
 
   analysis_error! {

src/unresolved_dependency.rs

@@ -3,7 +3,7 @@ use super::*;
 #[derive(PartialEq, Debug, Clone)]
 pub(crate) struct UnresolvedDependency<'src> {
   pub(crate) arguments: Vec<Expression<'src>>,
-  pub(crate) recipe: Name<'src>,
+  pub(crate) recipe: Namepath<'src>,
 }
 
 impl Display for UnresolvedDependency<'_> {

src/unresolved_recipe.rs

@@ -16,21 +16,19 @@ impl<'src> UnresolvedRecipe<'src> {
     );
 
     for (unresolved, resolved) in self.dependencies.iter().zip(&resolved) {
-      assert_eq!(unresolved.recipe.lexeme(), resolved.name.lexeme());
+      assert_eq!(unresolved.recipe.last().lexeme(), resolved.name.lexeme());
       if !resolved
         .argument_range()
         .contains(&unresolved.arguments.len())
       {
-        return Err(
-          unresolved
-            .recipe
-            .error(CompileErrorKind::DependencyArgumentCountMismatch {
-              dependency: unresolved.recipe.lexeme(),
-              found: unresolved.arguments.len(),
-              min: resolved.min_arguments(),
-              max: resolved.max_arguments(),
-            }),
-        );
+        return Err(unresolved.recipe.last().error(
+          CompileErrorKind::DependencyArgumentCountMismatch {
+            dependency: unresolved.recipe.clone(),
+            found: unresolved.arguments.len(),
+            min: resolved.min_arguments(),
+            max: resolved.max_arguments(),
+          },
+        ));
       }
     }
 

tests/dependencies.rs

@@ -0,0 +1,139 @@
+use super::*;
+
+#[test]
+fn recipe_doubly_nested_module_dependencies() {
+  Test::new()
+    .write("foo.just", "mod bar\nbaz: \n @echo FOO")
+    .write("bar.just", "baz:\n @echo BAZ")
+    .justfile(
+      "
+      mod foo
+
+      baz: foo::bar::baz
+      ",
+    )
+    .arg("baz")
+    .stdout("BAZ\n")
+    .run();
+}
+
+#[test]
+fn recipe_singly_nested_module_dependencies() {
+  Test::new()
+    .write("foo.just", "mod bar\nbaz: \n @echo BAR")
+    .write("bar.just", "baz:\n @echo BAZ")
+    .justfile(
+      "
+      mod foo
+      baz: foo::baz
+      ",
+    )
+    .arg("baz")
+    .stdout("BAR\n")
+    .run();
+}
+
+#[test]
+fn dependency_not_in_submodule() {
+  Test::new()
+    .write("foo.just", "qux: \n @echo QUX")
+    .justfile(
+      "
+      mod foo
+      baz: foo::baz
+      ",
+    )
+    .arg("baz")
+    .status(1)
+    .stderr(
+      "error: Recipe `baz` has unknown dependency `foo::baz`
+ ——▶ justfile:2:11
+  │
+2 │ baz: foo::baz
+  │           ^^^
+",
+    )
+    .run();
+}
+
+#[test]
+fn dependency_submodule_missing() {
+  Test::new()
+    .justfile(
+      "
+      foo:
+        @echo FOO
+      bar:
+        @echo BAR
+      baz: foo::bar
+      ",
+    )
+    .arg("baz")
+    .status(1)
+    .stderr(
+      "error: Recipe `baz` has unknown dependency `foo::bar`
+ ——▶ justfile:5:11
+  │
+5 │ baz: foo::bar
+  │           ^^^
+",
+    )
+    .run();
+}
+
+#[test]
+fn recipe_dependency_on_module_fails() {
+  Test::new()
+    .write("foo.just", "mod bar\nbaz: \n @echo BAR")
+    .write("bar.just", "baz:\n @echo BAZ")
+    .justfile(
+      "
+      mod foo
+      baz: foo::bar
+      ",
+    )
+    .arg("baz")
+    .status(1)
+    .stderr(
+      "error: Recipe `baz` has unknown dependency `foo::bar`
+ ——▶ justfile:2:11
+  │
+2 │ baz: foo::bar
+  │           ^^^
+",
+    )
+    .run();
+}
+
+#[test]
+fn recipe_module_dependency_subsequent_mix() {
+  Test::new()
+    .write("foo.just", "bar: \n @echo BAR")
+    .justfile(
+      "
+      mod foo
+      baz:
+        @echo BAZ
+      quux: foo::bar && baz
+        @echo QUUX
+      ",
+    )
+    .arg("quux")
+    .stdout("BAR\nQUUX\nBAZ\n")
+    .run();
+}
+
+#[test]
+fn recipe_module_dependency_only_runs_once() {
+  Test::new()
+    .write("foo.just", "bar: baz \n  \nbaz: \n @echo BAZ")
+    .justfile(
+      "
+      mod foo
+      qux: foo::bar foo::baz
+      ",
+    )
+    .arg("qux")
+    .stdout("BAZ\n")
+    .run();
+}

tests/lib.rs

@@ -63,6 +63,7 @@ mod confirm;
 mod constants;
 mod datetime;
 mod delimiters;
+mod dependencies;
 mod directories;
 mod dotenv;
 mod edit;