Make join accept two or more arguments (#1000)

README.adoc

@@ -907,7 +907,7 @@ These functions can fail, for example if a path does not have an extension, whic
 
 ===== Infallible
 
-- `join(a, b)` - Join path `a` with path `b`. `join("foo/bar", "baz")` is `foo/bar/baz`.
+- `join(a, b…)` - Join path `a` with path `b`. `join("foo/bar", "baz")` is `foo/bar/baz`. Accepts two or more arguments.
 - `clean(path)` - Simplify `path` by removing extra path separators, intermediate `.` components, and `..` where possible. `clean("foo//bar")` is `foo/bar`, `clean("foo/..")` is `.`, `clean("foo/./bar")` is `foo/bar`.
 
 === Command Evaluation Using Backticks

src/assignment_resolver.rs

@@ -82,6 +82,17 @@ impl<'src: 'run, 'run> AssignmentResolver<'src, 'run> {
           self.resolve_expression(a)?;
           self.resolve_expression(b)
         }
+        Thunk::BinaryPlus {
+          args: ([a, b], rest),
+          ..
+        } => {
+          self.resolve_expression(a)?;
+          self.resolve_expression(b)?;
+          for arg in rest {
+            self.resolve_expression(arg)?;
+          }
+          Ok(())
+        }
         Thunk::Ternary {
           args: [a, b, c], ..
         } => {

src/common.rs

@@ -19,17 +19,19 @@ pub(crate) use std::{
 };
 
 // dependencies
-pub(crate) use camino::Utf8Path;
+pub(crate) use ::{
-pub(crate) use derivative::Derivative;
+  camino::Utf8Path,
-pub(crate) use edit_distance::edit_distance;
+  derivative::Derivative,
-pub(crate) use lexiclean::Lexiclean;
+  edit_distance::edit_distance,
-pub(crate) use libc::EXIT_FAILURE;
+  lexiclean::Lexiclean,
-pub(crate) use log::{info, warn};
+  libc::EXIT_FAILURE,
-pub(crate) use regex::Regex;
+  log::{info, warn},
-pub(crate) use snafu::{ResultExt, Snafu};
+  regex::Regex,
-pub(crate) use strum::{Display, EnumString, IntoStaticStr};
+  snafu::{ResultExt, Snafu},
-pub(crate) use typed_arena::Arena;
+  strum::{Display, EnumString, IntoStaticStr},
-pub(crate) use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
+  typed_arena::Arena,
+  unicode_width::{UnicodeWidthChar, UnicodeWidthStr},
+};
 
 // modules
 pub(crate) use crate::{completions, config, config_error, setting};

src/compile_error.rs

@@ -137,7 +137,7 @@ impl Display for CompileError<'_> {
           function,
           found,
           Count("argument", *found),
-          expected
+          expected.display(),
         )?;
       }
       InconsistentLeadingWhitespace { expected, found } => {

src/compile_error_kind.rs

@@ -48,7 +48,7 @@ pub(crate) enum CompileErrorKind<'src> {
   FunctionArgumentCountMismatch {
     function: &'src str,
     found: usize,
-    expected: usize,
+    expected: Range<usize>,
   },
   InconsistentLeadingWhitespace {
     expected: &'src str,

src/evaluator.rs

@@ -106,6 +106,25 @@ impl<'src, 'run> Evaluator<'src, 'run> {
             function: *name,
             message,
           }),
+          BinaryPlus {
+            name,
+            function,
+            args: ([a, b], rest),
+            ..
+          } => {
+            let a = self.evaluate_expression(a)?;
+            let b = self.evaluate_expression(b)?;
+
+            let mut rest_evaluated = Vec::new();
+            for arg in rest {
+              rest_evaluated.push(self.evaluate_expression(arg)?);
+            }
+
+            function(&context, &a, &b, &rest_evaluated).map_err(|message| Error::FunctionCall {
+              function: *name,
+              message,
+            })
+          }
           Ternary {
             name,
             function,

src/function.rs

@@ -5,6 +5,7 @@ pub(crate) enum Function {
   Nullary(fn(&FunctionContext) -> Result<String, String>),
   Unary(fn(&FunctionContext, &str) -> Result<String, String>),
   Binary(fn(&FunctionContext, &str, &str) -> Result<String, String>),
+  BinaryPlus(fn(&FunctionContext, &str, &str, &[String]) -> Result<String, String>),
   Ternary(fn(&FunctionContext, &str, &str, &str) -> Result<String, String>),
 }
 
@@ -18,7 +19,7 @@ lazy_static! {
     ("file_name", Unary(file_name)),
     ("file_stem", Unary(file_stem)),
     ("invocation_directory", Nullary(invocation_directory)),
-    ("join", Binary(join)),
+    ("join", BinaryPlus(join)),
     ("just_executable", Nullary(just_executable)),
     ("justfile", Nullary(justfile)),
     ("justfile_directory", Nullary(justfile_directory)),
@@ -42,12 +43,13 @@ lazy_static! {
 }
 
 impl Function {
-  pub(crate) fn argc(&self) -> usize {
+  pub(crate) fn argc(&self) -> Range<usize> {
     match *self {
-      Nullary(_) => 0,
+      Nullary(_) => 0..0,
-      Unary(_) => 1,
+      Unary(_) => 1..1,
-      Binary(_) => 2,
+      Binary(_) => 2..2,
-      Ternary(_) => 3,
+      BinaryPlus(_) => 2..usize::MAX,
+      Ternary(_) => 3..3,
     }
   }
 }
@@ -127,8 +129,17 @@ fn invocation_directory(context: &FunctionContext) -> Result<String, String> {
   .map_err(|e| format!("Error getting shell path: {}", e))
 }
 
-fn join(_context: &FunctionContext, base: &str, with: &str) -> Result<String, String> {
+fn join(
-  Ok(Utf8Path::new(base).join(with).to_string())
+  _context: &FunctionContext,
+  base: &str,
+  with: &str,
+  and: &[String],
+) -> Result<String, String> {
+  let mut result = Utf8Path::new(base).join(with);
+  for arg in and {
+    result.push(arg);
+  }
+  Ok(result.to_string())
 }
 
 fn just_executable(_context: &FunctionContext) -> Result<String, String> {

src/node.rs

@@ -86,6 +86,18 @@ impl<'src> Node<'src> for Expression<'src> {
             tree.push_mut(a.tree());
             tree.push_mut(b.tree());
           }
+          BinaryPlus {
+            name,
+            args: ([a, b], rest),
+            ..
+          } => {
+            tree.push_mut(name.lexeme());
+            tree.push_mut(a.tree());
+            tree.push_mut(b.tree());
+            for arg in rest {
+              tree.push_mut(arg.tree());
+            }
+          }
           Ternary {
             name,
             args: [a, b, c],

src/parser.rs

@@ -2139,7 +2139,7 @@ mod tests {
     kind: FunctionArgumentCountMismatch {
       function: "arch",
       found: 1,
-      expected: 0,
+      expected: 0..0,
     },
   }
 
@@ -2153,7 +2153,7 @@ mod tests {
     kind: FunctionArgumentCountMismatch {
       function: "env_var",
       found: 0,
-      expected: 1,
+      expected: 1..1,
     },
   }
 
@@ -2167,7 +2167,35 @@ mod tests {
     kind: FunctionArgumentCountMismatch {
       function: "env_var_or_default",
       found: 1,
-      expected: 2,
+      expected: 2..2,
+    },
+  }
+
+  error! {
+    name: function_argument_count_binary_plus,
+    input: "x := join('foo')",
+    offset: 5,
+    line: 0,
+    column: 5,
+    width: 4,
+    kind: FunctionArgumentCountMismatch {
+      function: "join",
+      found: 1,
+      expected: 2..usize::MAX,
+    },
+  }
+
+  error! {
+    name: function_argument_count_ternary,
+    input: "x := replace('foo')",
+    offset: 5,
+    line: 0,
+    column: 5,
+    width: 7,
+    kind: FunctionArgumentCountMismatch {
+      function: "replace",
+      found: 1,
+      expected: 3..3,
     },
   }
 }

src/range_ext.rs

@@ -2,6 +2,25 @@ use crate::common::*;
 
 pub(crate) trait RangeExt<T> {
   fn range_contains(&self, i: &T) -> bool;
+
+  fn display(&self) -> DisplayRange<&Self> {
+    DisplayRange(self)
+  }
+}
+
+pub(crate) struct DisplayRange<T>(T);
+
+impl Display for DisplayRange<&Range<usize>> {
+  fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+    if self.0.start == self.0.end {
+      write!(f, "{}", self.0.start)?;
+    } else if self.0.end == usize::MAX {
+      write!(f, "{} or more", self.0.start)?;
+    } else {
+      write!(f, "{} to {}", self.0.start, self.0.end)?;
+    }
+    Ok(())
+  }
 }
 
 impl<T> RangeExt<T> for Range<T>
@@ -53,4 +72,11 @@ mod tests {
     assert!((1..=10).range_contains(&10));
     assert!((10..=20).range_contains(&15));
   }
+
+  #[test]
+  fn display() {
+    assert_eq!((1..1).display().to_string(), "1");
+    assert_eq!((1..2).display().to_string(), "1 to 2");
+    assert_eq!((1..usize::MAX).display().to_string(), "1 or more");
+  }
 }

src/summary.rs

@@ -230,6 +230,20 @@ impl Expression {
           name: name.lexeme().to_owned(),
           arguments: vec![Expression::new(a), Expression::new(b)],
         },
+        full::Thunk::BinaryPlus {
+          name,
+          args: ([a, b], rest),
+          ..
+        } => {
+          let mut arguments = vec![Expression::new(a), Expression::new(b)];
+          for arg in rest {
+            arguments.push(Expression::new(arg));
+          }
+          Expression::Call {
+            name: name.lexeme().to_owned(),
+            arguments,
+          }
+        }
         full::Thunk::Ternary {
           name,
           args: [a, b, c],

src/thunk.rs

@@ -20,6 +20,12 @@ pub(crate) enum Thunk<'src> {
     function: fn(&FunctionContext, &str, &str) -> Result<String, String>,
     args: [Box<Expression<'src>>; 2],
   },
+  BinaryPlus {
+    name: Name<'src>,
+    #[derivative(Debug = "ignore", PartialEq = "ignore")]
+    function: fn(&FunctionContext, &str, &str, &[String]) -> Result<String, String>,
+    args: ([Box<Expression<'src>>; 2], Vec<Expression<'src>>),
+  },
   Ternary {
     name: Name<'src>,
     #[derivative(Debug = "ignore", PartialEq = "ignore")]
@@ -56,6 +62,16 @@ impl<'src> Thunk<'src> {
             name,
           })
         }
+        (Function::BinaryPlus(function), 2..=usize::MAX) => {
+          let rest = arguments.drain(2..).collect();
+          let b = Box::new(arguments.pop().unwrap());
+          let a = Box::new(arguments.pop().unwrap());
+          Ok(Thunk::BinaryPlus {
+            function: *function,
+            args: ([a, b], rest),
+            name,
+          })
+        }
         (Function::Ternary(function), 3) => {
           let c = Box::new(arguments.pop().unwrap());
           let b = Box::new(arguments.pop().unwrap());
@@ -85,6 +101,17 @@ impl Display for Thunk<'_> {
       Binary {
         name, args: [a, b], ..
       } => write!(f, "{}({}, {})", name.lexeme(), a, b),
+      BinaryPlus {
+        name,
+        args: ([a, b], rest),
+        ..
+      } => {
+        write!(f, "{}({}, {}", name.lexeme(), a, b)?;
+        for arg in rest {
+          write!(f, ", {}", arg)?;
+        }
+        write!(f, ")")
+      }
       Ternary {
         name,
         args: [a, b, c],

src/variables.rs

@@ -25,6 +25,15 @@ impl<'expression, 'src> Iterator for Variables<'expression, 'src> {
               self.stack.push(arg);
             }
           }
+          Thunk::BinaryPlus {
+            args: ([a, b], rest),
+            ..
+          } => {
+            let first: &[&Expression] = &[a, b];
+            for arg in first.iter().copied().chain(rest).rev() {
+              self.stack.push(arg);
+            }
+          }
           Thunk::Ternary { args, .. } => {
             for arg in args.iter().rev() {
               self.stack.push(arg);

tests/functions.rs

@@ -347,3 +347,38 @@ fn trim_start() {
 fn trim_end() {
   assert_eval_eq("trim_end('  f  ')", "  f");
 }
+
+#[test]
+#[cfg(not(windows))]
+fn join() {
+  assert_eval_eq("join('a', 'b', 'c', 'd')", "a/b/c/d");
+  assert_eval_eq("join('a', '/b', 'c', 'd')", "/b/c/d");
+  assert_eval_eq("join('a', '/b', '/c', 'd')", "/c/d");
+  assert_eval_eq("join('a', '/b', '/c', '/d')", "/d");
+}
+
+#[test]
+#[cfg(windows)]
+fn join() {
+  assert_eval_eq("join('a', 'b', 'c', 'd')", "a\\b\\c\\d");
+  assert_eval_eq("join('a', '\\b', 'c', 'd')", "\\b\\c\\d");
+  assert_eval_eq("join('a', '\\b', '\\c', 'd')", "\\c\\d");
+  assert_eval_eq("join('a', '\\b', '\\c', '\\d')", "\\d");
+}
+
+#[test]
+fn join_argument_count_error() {
+  Test::new()
+    .justfile("x := join('a')")
+    .args(&["--evaluate"])
+    .stderr(
+      "
+      error: Function `join` called with 1 argument but takes 2 or more
+        |
+      1 | x := join(\'a\')
+        |      ^^^^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}