Skip to content

Commit

Permalink
Implement Val data model, a list of strings.
Browse files Browse the repository at this point in the history
In support of:

* #2458
* #1988

A `Val` is a list of strings, but at each point of usage we must decide whether
to view it as a singular joined string or as a list of its parts. Previously,
Just values were single strings, so in most places we have to invoke
`to_joined()` in order to maintain compatibility. In particular, recipes,
functions, and operators like `+` or `/` operate solely on strings. This
includes logical operators like `&&`, which continue to be defined on strings.
That means, the values `[]` and `['']` are currently completely equivalent.

So far, this is a purely internal change without externally visible effects.
Only the `Bindings`/`Scope`/`Evaluator` had API changes.

No new syntax is implemented. However, in expectation of expressions that build
lists, a new `evaluate_list_expression() -> Vec<String>` method is introduced
that could be used to implement splat or generator expressions. It is already
referenced wherever we have lists of arguments, e.g. variadic functions like
`shell()` and dependency arguments. But because singular expressions are
equivalent to a joined string, this is an unobservable detail for now.

For experimenting with lists of strings, variadic recipe parameters like `*args`
now produce a multi-part Val, and default to an empty list (not a list with an
empty string). Because all operators use `to_joined()`, this is an unobservable
implementation detail. However, if any operator becomes list-aware, then this
detail should be reverted, or moved behind an unstable setting.

For better understanding of the current behavior, I added a bunch of tests.
These will help detect regressions if functions or operators become list-aware.
No existing tests had to be touched.

Next steps: This change is just the foundation for other work, but some ideas
are mutually exclusive. Relevant aspects:

* list syntax in #2458
* list aware operators in #2458
* lossless forwarding of variadics: #1988
* invoking dependencies multiple times: #558

The preparatory work like `evaluate_list_expression()` is biased towards
implementing a splat operator that would enable #2458 list syntax and #1988 list
forwarding, but doesn't commit us to any particular design yet.
  • Loading branch information
latk committed Jan 11, 2025
1 parent 6f76dc3 commit 2db23b1
Show file tree
Hide file tree
Showing 11 changed files with 329 additions and 70 deletions.
2 changes: 1 addition & 1 deletion src/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::*;

/// A binding of `name` to `value`
#[derive(Debug, Clone, PartialEq, Serialize)]
pub(crate) struct Binding<'src, V = String> {
pub(crate) struct Binding<'src, V = Val> {
#[serde(skip)]
pub(crate) constant: bool,
pub(crate) export: bool,
Expand Down
2 changes: 1 addition & 1 deletion src/command_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl CommandExt for Command {

for binding in scope.bindings() {
if binding.export || (settings.export && !binding.constant) {
self.env(binding.name.lexeme(), &binding.value);
self.env(binding.name.lexeme(), binding.value.to_joined());
}
}
}
Expand Down
153 changes: 99 additions & 54 deletions src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
file_depth: 0,
name: assignment.name,
private: assignment.private,
value: value.clone(),
value: Val::from_str(value),
});
} else {
unknown_overrides.push(name.clone());
Expand All @@ -65,7 +65,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
Ok(evaluator.scope)
}

fn evaluate_assignment(&mut self, assignment: &Assignment<'src>) -> RunResult<'src, &str> {
fn evaluate_assignment(&mut self, assignment: &Assignment<'src>) -> RunResult<'src, &Val> {
let name = assignment.name.lexeme();

if !self.scope.bound(name) {
Expand All @@ -83,50 +83,63 @@ impl<'src, 'run> Evaluator<'src, 'run> {
Ok(self.scope.value(name).unwrap())
}

/// A place for adding list operators in the future.
///
/// List expressions return zero or more strings.
pub(crate) fn evaluate_list_expression(
&mut self,
expression: &Expression<'src>,
) -> RunResult<'src, Vec<String>> {
// currently, all expression produce a single item
Ok(vec![self.evaluate_expression(expression)?.to_joined()])
}

pub(crate) fn evaluate_expression(
&mut self,
expression: &Expression<'src>,
) -> RunResult<'src, String> {
) -> RunResult<'src, Val> {
match expression {
Expression::And { lhs, rhs } => {
let lhs = self.evaluate_expression(lhs)?;
if lhs.is_empty() {
return Ok(String::new());
if lhs.to_joined().is_empty() {
return Ok(Val::new());
}
self.evaluate_expression(rhs)
}
Expression::Assert { condition, error } => {
if self.evaluate_condition(condition)? {
Ok(String::new())
Ok(Val::new())
} else {
Err(Error::Assert {
message: self.evaluate_expression(error)?,
message: self.evaluate_expression(error)?.to_joined(),
})
}
}
Expression::Backtick { contents, token } => {
if self.context.config.dry_run {
Ok(format!("`{contents}`"))
Ok(format!("`{contents}`").into())
} else {
Ok(self.run_backtick(contents, token)?)
Ok(self.run_backtick(contents, token)?.into())
}
}
Expression::Call { thunk } => {
use Thunk::*;
let result = match thunk {
// All functions are currently of type (...String) -> Result<String>.
// They do not take or return a `Val`.
let result: Result<String, String> = match thunk {
Nullary { function, .. } => function(function::Context::new(self, thunk.name())),
Unary { function, arg, .. } => {
let arg = self.evaluate_expression(arg)?;
let arg = self.evaluate_expression(arg)?.to_joined();
function(function::Context::new(self, thunk.name()), &arg)
}
UnaryOpt {
function,
args: (a, b),
..
} => {
let a = self.evaluate_expression(a)?;
let a = self.evaluate_expression(a)?.to_joined();
let b = match b.as_ref() {
Some(b) => Some(self.evaluate_expression(b)?),
Some(b) => Some(self.evaluate_expression(b)?.to_joined()),
None => None,
};
function(function::Context::new(self, thunk.name()), &a, b.as_deref())
Expand All @@ -136,10 +149,10 @@ impl<'src, 'run> Evaluator<'src, 'run> {
args: (a, rest),
..
} => {
let a = self.evaluate_expression(a)?;
let a = self.evaluate_expression(a)?.to_joined();
let mut rest_evaluated = Vec::new();
for arg in rest {
rest_evaluated.push(self.evaluate_expression(arg)?);
rest_evaluated.extend(self.evaluate_list_expression(arg)?);
}
function(
function::Context::new(self, thunk.name()),
Expand All @@ -152,20 +165,20 @@ impl<'src, 'run> Evaluator<'src, 'run> {
args: [a, b],
..
} => {
let a = self.evaluate_expression(a)?;
let b = self.evaluate_expression(b)?;
let a = self.evaluate_expression(a)?.to_joined();
let b = self.evaluate_expression(b)?.to_joined();
function(function::Context::new(self, thunk.name()), &a, &b)
}
BinaryPlus {
function,
args: ([a, b], rest),
..
} => {
let a = self.evaluate_expression(a)?;
let b = self.evaluate_expression(b)?;
let a = self.evaluate_expression(a)?.to_joined();
let b = self.evaluate_expression(b)?.to_joined();
let mut rest_evaluated = Vec::new();
for arg in rest {
rest_evaluated.push(self.evaluate_expression(arg)?);
rest_evaluated.extend(self.evaluate_list_expression(arg)?);
}
function(
function::Context::new(self, thunk.name()),
Expand All @@ -179,19 +192,23 @@ impl<'src, 'run> Evaluator<'src, 'run> {
args: [a, b, c],
..
} => {
let a = self.evaluate_expression(a)?;
let b = self.evaluate_expression(b)?;
let c = self.evaluate_expression(c)?;
let a = self.evaluate_expression(a)?.to_joined();
let b = self.evaluate_expression(b)?.to_joined();
let c = self.evaluate_expression(c)?.to_joined();
function(function::Context::new(self, thunk.name()), &a, &b, &c)
}
};
result.map_err(|message| Error::FunctionCall {
function: thunk.name(),
message,
})
result
.map(Val::from_str)
.map_err(|message| Error::FunctionCall {
function: thunk.name(),
message,
})
}
Expression::Concatenation { lhs, rhs } => {
Ok(self.evaluate_expression(lhs)? + &self.evaluate_expression(rhs)?)
let a = self.evaluate_expression(lhs)?.to_joined();
let b = self.evaluate_expression(rhs)?.to_joined();
Ok(Val::from_str(a + &b))
}
Expression::Conditional {
condition,
Expand All @@ -205,19 +222,26 @@ impl<'src, 'run> Evaluator<'src, 'run> {
}
}
Expression::Group { contents } => self.evaluate_expression(contents),
Expression::Join { lhs: None, rhs } => Ok("/".to_string() + &self.evaluate_expression(rhs)?),
Expression::Join { lhs: None, rhs } => {
let rhs = self.evaluate_expression(rhs)?.to_joined();
Ok(Val::from_str("/".to_string() + &rhs))
}
Expression::Join {
lhs: Some(lhs),
rhs,
} => Ok(self.evaluate_expression(lhs)? + "/" + &self.evaluate_expression(rhs)?),
} => {
let lhs = self.evaluate_expression(lhs)?.to_joined();
let rhs = self.evaluate_expression(rhs)?.to_joined();
Ok(Val::from_str(lhs + "/" + &rhs))
}
Expression::Or { lhs, rhs } => {
let lhs = self.evaluate_expression(lhs)?;
if !lhs.is_empty() {
if !lhs.to_joined().is_empty() {
return Ok(lhs);
}
self.evaluate_expression(rhs)
}
Expression::StringLiteral { string_literal } => Ok(string_literal.cooked.clone()),
Expression::StringLiteral { string_literal } => Ok(Val::from_str(&string_literal.cooked)),
Expression::Variable { name, .. } => {
let variable = name.lexeme();
if let Some(value) = self.scope.value(variable) {
Expand All @@ -240,14 +264,14 @@ impl<'src, 'run> Evaluator<'src, 'run> {
let lhs_value = self.evaluate_expression(&condition.lhs)?;
let rhs_value = self.evaluate_expression(&condition.rhs)?;
let condition = match condition.operator {
ConditionalOperator::Equality => lhs_value == rhs_value,
ConditionalOperator::Inequality => lhs_value != rhs_value,
ConditionalOperator::RegexMatch => Regex::new(&rhs_value)
ConditionalOperator::Equality => lhs_value.to_joined() == rhs_value.to_joined(),
ConditionalOperator::Inequality => lhs_value.to_joined() != rhs_value.to_joined(),
ConditionalOperator::RegexMatch => Regex::new(&rhs_value.to_joined())
.map_err(|source| Error::RegexCompile { source })?
.is_match(&lhs_value),
ConditionalOperator::RegexMismatch => !Regex::new(&rhs_value)
.is_match(&lhs_value.to_joined()),
ConditionalOperator::RegexMismatch => !Regex::new(&rhs_value.to_joined())
.map_err(|source| Error::RegexCompile { source })?
.is_match(&lhs_value),
.is_match(&lhs_value.to_joined()),
};
Ok(condition)
}
Expand Down Expand Up @@ -303,14 +327,20 @@ impl<'src, 'run> Evaluator<'src, 'run> {
}
}
Fragment::Interpolation { expression } => {
evaluated += &self.evaluate_expression(expression)?;
evaluated += &self.evaluate_expression(expression)?.to_joined();
}
}
}
Ok(evaluated)
}

pub(crate) fn evaluate_parameters(
/// Bind recipe arguments to their parameters.
///
/// Returns a `(scope, positional_arguments)` tuple if successful.
///
/// May evaluate defaults, which can append strings to the positional-arguments.
/// Defaults are evaluated left-to-right, and may reference preceding params.
pub(crate) fn evaluate_recipe_parameters(
context: &ExecutionContext<'src, 'run>,
is_dependency: bool,
arguments: &[String],
Expand All @@ -322,30 +352,45 @@ impl<'src, 'run> Evaluator<'src, 'run> {

let mut rest = arguments;
for parameter in parameters {
// Each recipe argument must be a singular string, as if it was provided as a CLI argument.
// This prevents lists from leaking into dependencies unexpectedly.
// The one exception is an explicitly variadic parameter.
let value = if rest.is_empty() {
if let Some(ref default) = parameter.default {
let value = evaluator.evaluate_expression(default)?;
positional.push(value.clone());
value
} else if parameter.kind == ParameterKind::Star {
String::new()
} else {
return Err(Error::Internal {
message: "missing parameter without default".to_owned(),
});
match (&parameter.default, parameter.kind) {
(Some(default), ParameterKind::Star | ParameterKind::Plus) => {
let value = evaluator.evaluate_expression(default)?;
// auto-splat variadic defaults, in case we want to support expressions like
// `recipe *args=['a', 'b']: ...`
for part in value.to_parts() {
positional.push(part.to_string());
}
value
}
(Some(default), ParameterKind::Singular) => {
let value = evaluator.evaluate_expression(default)?;
let value = Val::from_str(value.to_joined()); // singularize
positional.push(value.to_string());
value
}
(None, ParameterKind::Star) => Val::new(),
(None, ParameterKind::Plus | ParameterKind::Singular) => {
return Err(Error::Internal {
message: "missing parameter without default".to_owned(),
});
}
}
} else if parameter.kind.is_variadic() {
for value in rest {
positional.push(value.clone());
}
let value = rest.to_vec().join(" ");
let value = Val::from_parts(rest);
rest = &[];
value
} else {
let value = rest[0].clone();
positional.push(value.clone());
let value = rest[0].as_str();
positional.push(value.to_string());
rest = &rest[1..];
value
Val::from_str(value)
};
evaluator.scope.bind(Binding {
constant: false,
Expand Down
19 changes: 9 additions & 10 deletions src/justfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,20 +323,20 @@ impl<'src> Justfile<'src> {
}

let (outer, positional) =
Evaluator::evaluate_parameters(context, is_dependency, arguments, &recipe.parameters)?;
Evaluator::evaluate_recipe_parameters(context, is_dependency, arguments, &recipe.parameters)?;

let scope = outer.child();

let mut evaluator = Evaluator::new(context, true, &scope);

if !context.config.no_dependencies {
for Dependency { recipe, arguments } in recipe.dependencies.iter().take(recipe.priors) {
let arguments = arguments
.iter()
.map(|argument| evaluator.evaluate_expression(argument))
.collect::<RunResult<Vec<String>>>()?;
let mut evaluated_args = Vec::new();
for argument in arguments {
evaluated_args.extend(evaluator.evaluate_list_expression(argument)?);
}

Self::run_recipe(&arguments, context, ran, recipe, true)?;
Self::run_recipe(&evaluated_args, context, ran, recipe, true)?;
}
}

Expand All @@ -346,13 +346,12 @@ impl<'src> Justfile<'src> {
let mut ran = Ran::default();

for Dependency { recipe, arguments } in recipe.subsequents() {
let mut evaluated = Vec::new();

let mut evaluated_args = Vec::new();
for argument in arguments {
evaluated.push(evaluator.evaluate_expression(argument)?);
evaluated_args.extend(evaluator.evaluate_list_expression(argument)?);
}

Self::run_recipe(&evaluated, context, &mut ran, recipe, true)?;
Self::run_recipe(&evaluated_args, context, &mut ran, recipe, true)?;
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub(crate) use {
unresolved_recipe::UnresolvedRecipe,
unstable_feature::UnstableFeature,
use_color::UseColor,
val::Val,
variables::Variables,
verbosity::Verbosity,
warning::Warning,
Expand Down Expand Up @@ -270,6 +271,7 @@ mod unresolved_dependency;
mod unresolved_recipe;
mod unstable_feature;
mod use_color;
mod val;
mod variables;
mod verbosity;
mod warning;
Loading

0 comments on commit 2db23b1

Please sign in to comment.