From 4fbd03735a7aeb0a2cf5755f7f4bb96235126111 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sat, 8 Jun 2024 07:42:16 -0700 Subject: [PATCH] Refactor evaluator (#2138) --- src/evaluator.rs | 116 +++++++++--------- ...recipe_context.rs => execution_context.rs} | 3 +- src/function.rs | 73 ++++++++--- src/justfile.rs | 90 +++----------- src/lib.rs | 18 +-- src/recipe.rs | 16 +-- 6 files changed, 143 insertions(+), 173 deletions(-) rename src/{recipe_context.rs => execution_context.rs} (80%) diff --git a/src/evaluator.rs b/src/evaluator.rs index 1c6e6ae..ba33677 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -2,38 +2,56 @@ use super::*; pub(crate) struct Evaluator<'src: 'run, 'run> { pub(crate) assignments: Option<&'run Table<'src, Assignment<'src>>>, - pub(crate) config: &'run Config, - pub(crate) dotenv: &'run BTreeMap, - pub(crate) module_source: &'run Path, + pub(crate) context: ExecutionContext<'src, 'run>, pub(crate) scope: Scope<'src, 'run>, - pub(crate) search: &'run Search, - pub(crate) settings: &'run Settings<'run>, - unsets: &'run HashSet, } impl<'src, 'run> Evaluator<'src, 'run> { pub(crate) fn evaluate_assignments( - assignments: &'run Table<'src, Assignment<'src>>, config: &'run Config, dotenv: &'run BTreeMap, - module_source: &'run Path, - scope: Scope<'src, 'run>, + module: &'run Justfile<'src>, + overrides: &BTreeMap, + parent: &'run Scope<'src, 'run>, search: &'run Search, - settings: &'run Settings<'run>, - unsets: &'run HashSet, - ) -> RunResult<'src, Scope<'src, 'run>> { - let mut evaluator = Self { - assignments: Some(assignments), + ) -> RunResult<'src, Scope<'src, 'run>> + where + 'src: 'run, + { + let context = ExecutionContext { config, dotenv, - module_source, - scope, + module_source: &module.source, + scope: parent, search, - settings, - unsets, + settings: &module.settings, + unexports: &module.unexports, }; - for assignment in assignments.values() { + let mut scope = context.scope.child(); + let mut unknown_overrides = Vec::new(); + + for (name, value) in overrides { + if let Some(assignment) = module.assignments.get(name) { + scope.bind(assignment.export, assignment.name, value.clone()); + } else { + unknown_overrides.push(name.clone()); + } + } + + if !unknown_overrides.is_empty() { + return Err(Error::UnknownOverrides { + overrides: unknown_overrides, + }); + } + + let mut evaluator = Self { + context, + assignments: Some(&module.assignments), + scope, + }; + + for assignment in module.assignments.values() { evaluator.evaluate_assignment(assignment)?; } @@ -155,7 +173,7 @@ impl<'src, 'run> Evaluator<'src, 'run> { } Expression::StringLiteral { string_literal } => Ok(string_literal.cooked.clone()), Expression::Backtick { contents, token } => { - if self.config.dry_run { + if self.context.config.dry_run { Ok(format!("`{contents}`")) } else { Ok(self.run_backtick(contents, token)?) @@ -216,13 +234,18 @@ impl<'src, 'run> Evaluator<'src, 'run> { } pub(crate) fn run_command(&self, command: &str, args: &[&str]) -> Result { - let mut cmd = self.settings.shell_command(self.config); + let mut cmd = self.context.settings.shell_command(self.context.config); cmd.arg(command); cmd.args(args); - cmd.current_dir(&self.search.working_directory); - cmd.export(self.settings, self.dotenv, &self.scope, self.unsets); + cmd.current_dir(&self.context.search.working_directory); + cmd.export( + self.context.settings, + self.context.dotenv, + &self.scope, + self.context.unexports, + ); cmd.stdin(Stdio::inherit()); - cmd.stderr(if self.config.verbosity.quiet() { + cmd.stderr(if self.context.config.verbosity.quiet() { Stdio::null() } else { Stdio::inherit() @@ -256,28 +279,11 @@ impl<'src, 'run> Evaluator<'src, 'run> { } pub(crate) fn evaluate_parameters( + context: &ExecutionContext<'src, 'run>, arguments: &[String], - config: &'run Config, - dotenv: &'run BTreeMap, - module_source: &'run Path, parameters: &[Parameter<'src>], - scope: &'run Scope<'src, 'run>, - search: &'run Search, - settings: &'run Settings, - unsets: &'run HashSet, ) -> RunResult<'src, (Scope<'src, 'run>, Vec)> { - let mut evaluator = Self { - assignments: None, - config, - dotenv, - module_source, - scope: scope.child(), - search, - settings, - unsets, - }; - - let mut scope = scope.child(); + let mut evaluator = Self::new(context, context.scope); let mut positional = Vec::new(); @@ -308,30 +314,22 @@ impl<'src, 'run> Evaluator<'src, 'run> { rest = &rest[1..]; value }; - scope.bind(parameter.export, parameter.name, value); + evaluator + .scope + .bind(parameter.export, parameter.name, value); } - Ok((scope, positional)) + Ok((evaluator.scope, positional)) } - pub(crate) fn recipe_evaluator( - config: &'run Config, - dotenv: &'run BTreeMap, - module_source: &'run Path, + pub(crate) fn new( + context: &ExecutionContext<'src, 'run>, scope: &'run Scope<'src, 'run>, - search: &'run Search, - settings: &'run Settings, - unsets: &'run HashSet, ) -> Self { Self { + context: *context, assignments: None, - config, - dotenv, - module_source, - scope: Scope::child(scope), - search, - settings, - unsets, + scope: scope.child(), } } } diff --git a/src/recipe_context.rs b/src/execution_context.rs similarity index 80% rename from src/recipe_context.rs rename to src/execution_context.rs index a1e5434..2efd9e5 100644 --- a/src/recipe_context.rs +++ b/src/execution_context.rs @@ -1,6 +1,7 @@ use super::*; -pub(crate) struct RecipeContext<'src: 'run, 'run> { +#[derive(Copy, Clone)] +pub(crate) struct ExecutionContext<'src: 'run, 'run> { pub(crate) config: &'run Config, pub(crate) dotenv: &'run BTreeMap, pub(crate) module_source: &'run Path, diff --git a/src/function.rs b/src/function.rs index e6820c9..d44bbb5 100644 --- a/src/function.rs +++ b/src/function.rs @@ -121,6 +121,7 @@ impl Function { fn absolute_path(context: Context, path: &str) -> Result { let abs_path_unchecked = context .evaluator + .context .search .working_directory .join(path) @@ -129,7 +130,7 @@ fn absolute_path(context: Context, path: &str) -> Result { Some(absolute_path) => Ok(absolute_path.to_owned()), None => Err(format!( "Working directory is not valid unicode: {}", - context.evaluator.search.working_directory.display() + context.evaluator.context.search.working_directory.display() )), } } @@ -152,7 +153,12 @@ fn blake3(_context: Context, s: &str) -> Result { } fn blake3_file(context: Context, path: &str) -> Result { - let path = context.evaluator.search.working_directory.join(path); + let path = context + .evaluator + .context + .search + .working_directory + .join(path); let mut hasher = blake3::Hasher::new(); hasher .update_mmap_rayon(&path) @@ -245,7 +251,7 @@ fn encode_uri_component(_context: Context, s: &str) -> Result { fn env_var(context: Context, key: &str) -> Result { use std::env::VarError::*; - if let Some(value) = context.evaluator.dotenv.get(key) { + if let Some(value) = context.evaluator.context.dotenv.get(key) { return Ok(value.clone()); } @@ -261,7 +267,7 @@ fn env_var(context: Context, key: &str) -> Result { fn env_var_or_default(context: Context, key: &str, default: &str) -> Result { use std::env::VarError::*; - if let Some(value) = context.evaluator.dotenv.get(key) { + if let Some(value) = context.evaluator.context.dotenv.get(key) { return Ok(value.clone()); } @@ -308,8 +314,8 @@ fn file_stem(_context: Context, path: &str) -> Result { fn invocation_directory(context: Context) -> Result { Platform::convert_native_path( - &context.evaluator.search.working_directory, - &context.evaluator.config.invocation_directory, + &context.evaluator.context.search.working_directory, + &context.evaluator.context.config.invocation_directory, ) .map_err(|e| format!("Error getting shell path: {e}")) } @@ -317,6 +323,7 @@ fn invocation_directory(context: Context) -> Result { fn invocation_directory_native(context: Context) -> Result { context .evaluator + .context .config .invocation_directory .to_str() @@ -324,7 +331,12 @@ fn invocation_directory_native(context: Context) -> Result { .ok_or_else(|| { format!( "Invocation directory is not valid unicode: {}", - context.evaluator.config.invocation_directory.display() + context + .evaluator + .context + .config + .invocation_directory + .display() ) }) } @@ -365,6 +377,7 @@ fn just_pid(_context: Context) -> Result { fn justfile(context: Context) -> Result { context .evaluator + .context .search .justfile .to_str() @@ -372,18 +385,24 @@ fn justfile(context: Context) -> Result { .ok_or_else(|| { format!( "Justfile path is not valid unicode: {}", - context.evaluator.search.justfile.display() + context.evaluator.context.search.justfile.display() ) }) } fn justfile_directory(context: Context) -> Result { - let justfile_directory = context.evaluator.search.justfile.parent().ok_or_else(|| { - format!( - "Could not resolve justfile directory. Justfile `{}` had no parent.", - context.evaluator.search.justfile.display() - ) - })?; + let justfile_directory = context + .evaluator + .context + .search + .justfile + .parent() + .ok_or_else(|| { + format!( + "Could not resolve justfile directory. Justfile `{}` had no parent.", + context.evaluator.context.search.justfile.display() + ) + })?; justfile_directory .to_str() @@ -411,11 +430,12 @@ fn lowercase(_context: Context, s: &str) -> Result { fn module_directory(context: Context) -> Result { context .evaluator + .context .search .justfile .parent() .unwrap() - .join(context.evaluator.module_source) + .join(context.evaluator.context.module_source) .parent() .unwrap() .to_str() @@ -423,7 +443,13 @@ fn module_directory(context: Context) -> Result { .ok_or_else(|| { format!( "Module directory is not valid unicode: {}", - context.evaluator.module_source.parent().unwrap().display(), + context + .evaluator + .context + .module_source + .parent() + .unwrap() + .display(), ) }) } @@ -431,17 +457,18 @@ fn module_directory(context: Context) -> Result { fn module_file(context: Context) -> Result { context .evaluator + .context .search .justfile .parent() .unwrap() - .join(context.evaluator.module_source) + .join(context.evaluator.context.module_source) .to_str() .map(str::to_owned) .ok_or_else(|| { format!( "Module file path is not valid unicode: {}", - context.evaluator.module_source.display(), + context.evaluator.context.module_source.display(), ) }) } @@ -470,6 +497,7 @@ fn path_exists(context: Context, path: &str) -> Result { Ok( context .evaluator + .context .search .working_directory .join(path) @@ -510,7 +538,12 @@ fn sha256(_context: Context, s: &str) -> Result { fn sha256_file(context: Context, path: &str) -> Result { use sha2::{Digest, Sha256}; - let path = context.evaluator.search.working_directory.join(path); + let path = context + .evaluator + .context + .search + .working_directory + .join(path); let mut hasher = Sha256::new(); let mut file = fs::File::open(&path).map_err(|err| format!("Failed to open `{}`: {err}", path.display()))?; @@ -546,6 +579,7 @@ fn snakecase(_context: Context, s: &str) -> Result { fn source_directory(context: Context) -> Result { context .evaluator + .context .search .justfile .parent() @@ -566,6 +600,7 @@ fn source_directory(context: Context) -> Result { fn source_file(context: Context) -> Result { context .evaluator + .context .search .justfile .parent() diff --git a/src/justfile.rs b/src/justfile.rs index 150a405..be5d5f8 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -78,46 +78,6 @@ impl<'src> Justfile<'src> { .next() } - fn scope<'run>( - &'run self, - config: &'run Config, - dotenv: &'run BTreeMap, - search: &'run Search, - overrides: &BTreeMap, - parent: &'run Scope<'src, 'run>, - ) -> RunResult<'src, Scope<'src, 'run>> - where - 'src: 'run, - { - let mut scope = parent.child(); - let mut unknown_overrides = Vec::new(); - - for (name, value) in overrides { - if let Some(assignment) = self.assignments.get(name) { - scope.bind(assignment.export, assignment.name, value.clone()); - } else { - unknown_overrides.push(name.clone()); - } - } - - if !unknown_overrides.is_empty() { - return Err(Error::UnknownOverrides { - overrides: unknown_overrides, - }); - } - - Evaluator::evaluate_assignments( - &self.assignments, - config, - dotenv, - &self.source, - scope, - search, - &self.settings, - &self.unexports, - ) - } - pub(crate) fn run( &self, config: &Config, @@ -145,7 +105,7 @@ impl<'src> Justfile<'src> { let root = Scope::root(); - let scope = self.scope(config, &dotenv, search, overrides, &root)?; + let scope = Evaluator::evaluate_assignments(config, &dotenv, self, overrides, &root, search)?; match &config.subcommand { Subcommand::Command { @@ -196,11 +156,7 @@ impl<'src> Justfile<'src> { }); } } else { - let mut width = 0; - - for name in scope.names() { - width = cmp::max(name.len(), width); - } + let width = scope.names().fold(0, |max, name| name.len().max(max)); for binding in scope.bindings() { println!( @@ -281,7 +237,7 @@ impl<'src> Justfile<'src> { let mut ran = Ran::default(); for invocation in invocations { - let context = RecipeContext { + let context = ExecutionContext { config, dotenv: &dotenv, module_source: invocation.module_source, @@ -301,7 +257,6 @@ impl<'src> Justfile<'src> { &context, &mut ran, invocation.recipe, - search, )?; } @@ -339,7 +294,14 @@ impl<'src> Justfile<'src> { let scope = if let Some(scope) = scopes.get(path) { scope } else { - let scope = module.scope(config, dotenv, search, &BTreeMap::new(), parent)?; + let scope = Evaluator::evaluate_assignments( + config, + dotenv, + module, + &BTreeMap::new(), + parent, + search, + )?; let scope = arena.alloc(scope); scopes.insert(path.clone(), scope); scopes.get(path).unwrap() @@ -420,10 +382,9 @@ impl<'src> Justfile<'src> { fn run_recipe( arguments: &[String], - context: &RecipeContext<'src, '_>, + context: &ExecutionContext<'src, '_>, ran: &mut Ran<'src>, recipe: &Recipe<'src>, - search: &Search, ) -> RunResult<'src> { if ran.has_run(&recipe.namepath, arguments) { return Ok(()); @@ -435,29 +396,12 @@ impl<'src> Justfile<'src> { }); } - let (outer, positional) = Evaluator::evaluate_parameters( - arguments, - context.config, - context.dotenv, - context.module_source, - &recipe.parameters, - context.scope, - search, - context.settings, - context.unexports, - )?; + let (outer, positional) = + Evaluator::evaluate_parameters(context, arguments, &recipe.parameters)?; let scope = outer.child(); - let mut evaluator = Evaluator::recipe_evaluator( - context.config, - context.dotenv, - context.module_source, - &scope, - search, - context.settings, - context.unexports, - ); + let mut evaluator = Evaluator::new(context, &scope); if !context.config.no_dependencies { for Dependency { recipe, arguments } in recipe.dependencies.iter().take(recipe.priors) { @@ -466,7 +410,7 @@ impl<'src> Justfile<'src> { .map(|argument| evaluator.evaluate_expression(argument)) .collect::>>()?; - Self::run_recipe(&arguments, context, ran, recipe, search)?; + Self::run_recipe(&arguments, context, ran, recipe)?; } } @@ -482,7 +426,7 @@ impl<'src> Justfile<'src> { evaluated.push(evaluator.evaluate_expression(argument)?); } - Self::run_recipe(&evaluated, context, &mut ran, recipe, search)?; + Self::run_recipe(&evaluated, context, &mut ran, recipe)?; } } diff --git a/src/lib.rs b/src/lib.rs index 1d0817d..b0faaf0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,14 +22,14 @@ pub(crate) use { condition::Condition, conditional_operator::ConditionalOperator, config::Config, config_error::ConfigError, constants::constants, count::Count, delimiter::Delimiter, dependency::Dependency, dump_format::DumpFormat, enclosure::Enclosure, error::Error, - evaluator::Evaluator, expression::Expression, fragment::Fragment, function::Function, - interrupt_guard::InterruptGuard, interrupt_handler::InterruptHandler, item::Item, - justfile::Justfile, keyed::Keyed, keyword::Keyword, lexer::Lexer, line::Line, list::List, - load_dotenv::load_dotenv, loader::Loader, module_path::ModulePath, name::Name, - namepath::Namepath, ordinal::Ordinal, output::output, output_error::OutputError, - parameter::Parameter, parameter_kind::ParameterKind, parser::Parser, platform::Platform, - platform_interface::PlatformInterface, position::Position, positional::Positional, ran::Ran, - range_ext::RangeExt, recipe::Recipe, recipe_context::RecipeContext, + evaluator::Evaluator, execution_context::ExecutionContext, expression::Expression, + fragment::Fragment, function::Function, interrupt_guard::InterruptGuard, + interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, keyed::Keyed, + keyword::Keyword, lexer::Lexer, line::Line, list::List, load_dotenv::load_dotenv, + loader::Loader, module_path::ModulePath, name::Name, namepath::Namepath, ordinal::Ordinal, + output::output, output_error::OutputError, parameter::Parameter, parameter_kind::ParameterKind, + parser::Parser, platform::Platform, platform_interface::PlatformInterface, position::Position, + positional::Positional, ran::Ran, range_ext::RangeExt, recipe::Recipe, recipe_resolver::RecipeResolver, recipe_signature::RecipeSignature, scope::Scope, search::Search, search_config::SearchConfig, search_error::SearchError, set::Set, setting::Setting, settings::Settings, shebang::Shebang, shell::Shell, @@ -139,6 +139,7 @@ mod dump_format; mod enclosure; mod error; mod evaluator; +mod execution_context; mod expression; mod fragment; mod function; @@ -169,7 +170,6 @@ mod positional; mod ran; mod range_ext; mod recipe; -mod recipe_context; mod recipe_resolver; mod recipe_signature; mod run; diff --git a/src/recipe.rs b/src/recipe.rs index bad0463..ac06839 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -146,7 +146,7 @@ impl<'src, D> Recipe<'src, D> { pub(crate) fn run<'run>( &self, - context: &RecipeContext<'src, 'run>, + context: &ExecutionContext<'src, 'run>, scope: &Scope<'src, 'run>, positional: &[String], ) -> RunResult<'src, ()> { @@ -162,15 +162,7 @@ impl<'src, D> Recipe<'src, D> { ); } - let evaluator = Evaluator::recipe_evaluator( - context.config, - context.dotenv, - context.module_source, - scope, - context.search, - context.settings, - context.unexports, - ); + let evaluator = Evaluator::new(context, scope); if self.shebang { self.run_shebang(context, scope, positional, config, evaluator) @@ -181,7 +173,7 @@ impl<'src, D> Recipe<'src, D> { fn run_linewise<'run>( &self, - context: &RecipeContext<'src, 'run>, + context: &ExecutionContext<'src, 'run>, scope: &Scope<'src, 'run>, positional: &[String], config: &Config, @@ -313,7 +305,7 @@ impl<'src, D> Recipe<'src, D> { pub(crate) fn run_shebang<'run>( &self, - context: &RecipeContext<'src, 'run>, + context: &ExecutionContext<'src, 'run>, scope: &Scope<'src, 'run>, positional: &[String], config: &Config,