From 58a196f434b7f6e74a55a3a95f2cd92b484c961a Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 14 Oct 2021 17:00:58 -0700 Subject: [PATCH] Make `join` accept two or more arguments (#1000) --- README.adoc | 2 +- src/assignment_resolver.rs | 11 +++++++++++ src/common.rs | 24 +++++++++++++----------- src/compile_error.rs | 2 +- src/compile_error_kind.rs | 2 +- src/evaluator.rs | 19 +++++++++++++++++++ src/function.rs | 27 +++++++++++++++++++-------- src/node.rs | 12 ++++++++++++ src/parser.rs | 34 +++++++++++++++++++++++++++++++--- src/range_ext.rs | 26 ++++++++++++++++++++++++++ src/summary.rs | 14 ++++++++++++++ src/thunk.rs | 27 +++++++++++++++++++++++++++ src/variables.rs | 9 +++++++++ tests/functions.rs | 35 +++++++++++++++++++++++++++++++++++ 14 files changed, 219 insertions(+), 25 deletions(-) diff --git a/README.adoc b/README.adoc index 92899d54..da4952de 100644 --- a/README.adoc +++ b/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 diff --git a/src/assignment_resolver.rs b/src/assignment_resolver.rs index e086e40c..e1f50c4c 100644 --- a/src/assignment_resolver.rs +++ b/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], .. } => { diff --git a/src/common.rs b/src/common.rs index cbfa0b20..f2659831 100644 --- a/src/common.rs +++ b/src/common.rs @@ -19,17 +19,19 @@ pub(crate) use std::{ }; // dependencies -pub(crate) use camino::Utf8Path; -pub(crate) use derivative::Derivative; -pub(crate) use edit_distance::edit_distance; -pub(crate) use lexiclean::Lexiclean; -pub(crate) use libc::EXIT_FAILURE; -pub(crate) use log::{info, warn}; -pub(crate) use regex::Regex; -pub(crate) use snafu::{ResultExt, Snafu}; -pub(crate) use strum::{Display, EnumString, IntoStaticStr}; -pub(crate) use typed_arena::Arena; -pub(crate) use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; +pub(crate) use ::{ + camino::Utf8Path, + derivative::Derivative, + edit_distance::edit_distance, + lexiclean::Lexiclean, + libc::EXIT_FAILURE, + log::{info, warn}, + regex::Regex, + snafu::{ResultExt, Snafu}, + strum::{Display, EnumString, IntoStaticStr}, + typed_arena::Arena, + unicode_width::{UnicodeWidthChar, UnicodeWidthStr}, +}; // modules pub(crate) use crate::{completions, config, config_error, setting}; diff --git a/src/compile_error.rs b/src/compile_error.rs index 2f49923a..5fe4d3e7 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -137,7 +137,7 @@ impl Display for CompileError<'_> { function, found, Count("argument", *found), - expected + expected.display(), )?; } InconsistentLeadingWhitespace { expected, found } => { diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs index dfe33c14..76ba82c6 100644 --- a/src/compile_error_kind.rs +++ b/src/compile_error_kind.rs @@ -48,7 +48,7 @@ pub(crate) enum CompileErrorKind<'src> { FunctionArgumentCountMismatch { function: &'src str, found: usize, - expected: usize, + expected: Range, }, InconsistentLeadingWhitespace { expected: &'src str, diff --git a/src/evaluator.rs b/src/evaluator.rs index 0ab52a58..8d654bff 100644 --- a/src/evaluator.rs +++ b/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, diff --git a/src/function.rs b/src/function.rs index 2e88eb51..db972205 100644 --- a/src/function.rs +++ b/src/function.rs @@ -5,6 +5,7 @@ pub(crate) enum Function { Nullary(fn(&FunctionContext) -> Result), Unary(fn(&FunctionContext, &str) -> Result), Binary(fn(&FunctionContext, &str, &str) -> Result), + BinaryPlus(fn(&FunctionContext, &str, &str, &[String]) -> Result), Ternary(fn(&FunctionContext, &str, &str, &str) -> Result), } @@ -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 { match *self { - Nullary(_) => 0, - Unary(_) => 1, - Binary(_) => 2, - Ternary(_) => 3, + Nullary(_) => 0..0, + Unary(_) => 1..1, + Binary(_) => 2..2, + BinaryPlus(_) => 2..usize::MAX, + Ternary(_) => 3..3, } } } @@ -127,8 +129,17 @@ fn invocation_directory(context: &FunctionContext) -> Result { .map_err(|e| format!("Error getting shell path: {}", e)) } -fn join(_context: &FunctionContext, base: &str, with: &str) -> Result { - Ok(Utf8Path::new(base).join(with).to_string()) +fn join( + _context: &FunctionContext, + base: &str, + with: &str, + and: &[String], +) -> Result { + 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 { diff --git a/src/node.rs b/src/node.rs index 727f39d0..15df6c79 100644 --- a/src/node.rs +++ b/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], diff --git a/src/parser.rs b/src/parser.rs index 43118c6a..32bf5c31 100644 --- a/src/parser.rs +++ b/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, }, } } diff --git a/src/range_ext.rs b/src/range_ext.rs index 8145fd22..325e5b91 100644 --- a/src/range_ext.rs +++ b/src/range_ext.rs @@ -2,6 +2,25 @@ use crate::common::*; pub(crate) trait RangeExt { fn range_contains(&self, i: &T) -> bool; + + fn display(&self) -> DisplayRange<&Self> { + DisplayRange(self) + } +} + +pub(crate) struct DisplayRange(T); + +impl Display for DisplayRange<&Range> { + 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 RangeExt for Range @@ -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"); + } } diff --git a/src/summary.rs b/src/summary.rs index 985f56e5..a53054b4 100644 --- a/src/summary.rs +++ b/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], diff --git a/src/thunk.rs b/src/thunk.rs index b711f996..44988ffe 100644 --- a/src/thunk.rs +++ b/src/thunk.rs @@ -20,6 +20,12 @@ pub(crate) enum Thunk<'src> { function: fn(&FunctionContext, &str, &str) -> Result, args: [Box>; 2], }, + BinaryPlus { + name: Name<'src>, + #[derivative(Debug = "ignore", PartialEq = "ignore")] + function: fn(&FunctionContext, &str, &str, &[String]) -> Result, + args: ([Box>; 2], Vec>), + }, 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], diff --git a/src/variables.rs b/src/variables.rs index af62aa6d..68310c5e 100644 --- a/src/variables.rs +++ b/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); diff --git a/tests/functions.rs b/tests/functions.rs index 8e7aff28..abdcdf36 100644 --- a/tests/functions.rs +++ b/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(); +}