Refactor evaluator (#2138)

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) context: ExecutionContext<'src, 'run>,
-  pub(crate) dotenv: &'run BTreeMap<String, String>,
-  pub(crate) module_source: &'run Path,
   pub(crate) scope: Scope<'src, 'run>,
-  pub(crate) search: &'run Search,
-  pub(crate) settings: &'run Settings<'run>,
-  unsets: &'run HashSet<String>,
 }
 
 impl<'src, 'run> Evaluator<'src, 'run> {
   pub(crate) fn evaluate_assignments(
-    assignments: &'run Table<'src, Assignment<'src>>,
     config: &'run Config,
     dotenv: &'run BTreeMap<String, String>,
-    module_source: &'run Path,
+    module: &'run Justfile<'src>,
-    scope: Scope<'src, 'run>,
+    overrides: &BTreeMap<String, String>,
+    parent: &'run Scope<'src, 'run>,
     search: &'run Search,
-    settings: &'run Settings<'run>,
+  ) -> RunResult<'src, Scope<'src, 'run>>
-    unsets: &'run HashSet<String>,
+  where
-  ) -> RunResult<'src, Scope<'src, 'run>> {
+    'src: 'run,
-    let mut evaluator = Self {
+  {
-      assignments: Some(assignments),
+    let context = ExecutionContext {
       config,
       dotenv,
-      module_source,
+      module_source: &module.source,
-      scope,
+      scope: parent,
       search,
-      settings,
+      settings: &module.settings,
-      unsets,
+      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<String, OutputError> {
-    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.current_dir(&self.context.search.working_directory);
-    cmd.export(self.settings, self.dotenv, &self.scope, self.unsets);
+    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<String, String>,
-    module_source: &'run Path,
     parameters: &[Parameter<'src>],
-    scope: &'run Scope<'src, 'run>,
-    search: &'run Search,
-    settings: &'run Settings,
-    unsets: &'run HashSet<String>,
   ) -> RunResult<'src, (Scope<'src, 'run>, Vec<String>)> {
-    let mut evaluator = Self {
+    let mut evaluator = Self::new(context, context.scope);
-      assignments: None,
-      config,
-      dotenv,
-      module_source,
-      scope: scope.child(),
-      search,
-      settings,
-      unsets,
-    };
-
-    let mut scope = scope.child();
 
     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(
+  pub(crate) fn new(
-    config: &'run Config,
+    context: &ExecutionContext<'src, 'run>,
-    dotenv: &'run BTreeMap<String, String>,
-    module_source: &'run Path,
     scope: &'run Scope<'src, 'run>,
-    search: &'run Search,
-    settings: &'run Settings,
-    unsets: &'run HashSet<String>,
   ) -> Self {
     Self {
+      context: *context,
       assignments: None,
-      config,
+      scope: scope.child(),
-      dotenv,
-      module_source,
-      scope: Scope::child(scope),
-      search,
-      settings,
-      unsets,
     }
   }
 }

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<String, String>,
   pub(crate) module_source: &'run Path,

src/function.rs

@@ -121,6 +121,7 @@ impl Function {
 fn absolute_path(context: Context, path: &str) -> Result<String, String> {
   let abs_path_unchecked = context
     .evaluator
+    .context
     .search
     .working_directory
     .join(path)
@@ -129,7 +130,7 @@ fn absolute_path(context: Context, path: &str) -> Result<String, String> {
     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<String, String> {
 }
 
 fn blake3_file(context: Context, path: &str) -> Result<String, String> {
-  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<String, String> {
 fn env_var(context: Context, key: &str) -> Result<String, String> {
   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<String, String> {
 fn env_var_or_default(context: Context, key: &str, default: &str) -> Result<String, String> {
   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<String, String> {
 
 fn invocation_directory(context: Context) -> Result<String, String> {
   Platform::convert_native_path(
-    &context.evaluator.search.working_directory,
+    &context.evaluator.context.search.working_directory,
-    &context.evaluator.config.invocation_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<String, String> {
 fn invocation_directory_native(context: Context) -> Result<String, String> {
   context
     .evaluator
+    .context
     .config
     .invocation_directory
     .to_str()
@@ -324,7 +331,12 @@ fn invocation_directory_native(context: Context) -> Result<String, String> {
     .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<String, String> {
 fn justfile(context: Context) -> Result<String, String> {
   context
     .evaluator
+    .context
     .search
     .justfile
     .to_str()
@@ -372,18 +385,24 @@ fn justfile(context: Context) -> Result<String, String> {
     .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<String, String> {
-  let justfile_directory = context.evaluator.search.justfile.parent().ok_or_else(|| {
+  let justfile_directory = context
-    format!(
+    .evaluator
-      "Could not resolve justfile directory. Justfile `{}` had no parent.",
+    .context
-      context.evaluator.search.justfile.display()
+    .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<String, String> {
 fn module_directory(context: Context) -> Result<String, String> {
   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<String, String> {
     .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<String, String> {
 fn module_file(context: Context) -> Result<String, String> {
   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<String, String> {
   Ok(
     context
       .evaluator
+      .context
       .search
       .working_directory
       .join(path)
@@ -510,7 +538,12 @@ fn sha256(_context: Context, s: &str) -> Result<String, String> {
 
 fn sha256_file(context: Context, path: &str) -> Result<String, String> {
   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<String, String> {
 fn source_directory(context: Context) -> Result<String, String> {
   context
     .evaluator
+    .context
     .search
     .justfile
     .parent()
@@ -566,6 +600,7 @@ fn source_directory(context: Context) -> Result<String, String> {
 fn source_file(context: Context) -> Result<String, String> {
   context
     .evaluator
+    .context
     .search
     .justfile
     .parent()

src/justfile.rs

@@ -78,46 +78,6 @@ impl<'src> Justfile<'src> {
       .next()
   }
 
-  fn scope<'run>(
-    &'run self,
-    config: &'run Config,
-    dotenv: &'run BTreeMap<String, String>,
-    search: &'run Search,
-    overrides: &BTreeMap<String, String>,
-    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;
+          let width = scope.names().fold(0, |max, name| name.len().max(max));
-
-          for name in scope.names() {
-            width = cmp::max(name.len(), width);
-          }
 
           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(
+    let (outer, positional) =
-      arguments,
+      Evaluator::evaluate_parameters(context, arguments, &recipe.parameters)?;
-      context.config,
-      context.dotenv,
-      context.module_source,
-      &recipe.parameters,
-      context.scope,
-      search,
-      context.settings,
-      context.unexports,
-    )?;
 
     let scope = outer.child();
 
-    let mut evaluator = Evaluator::recipe_evaluator(
+    let mut evaluator = Evaluator::new(context, &scope);
-      context.config,
-      context.dotenv,
-      context.module_source,
-      &scope,
-      search,
-      context.settings,
-      context.unexports,
-    );
 
     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::<RunResult<Vec<String>>>()?;
 
-        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)?;
       }
     }
 

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,
+    evaluator::Evaluator, execution_context::ExecutionContext, expression::Expression,
-    interrupt_guard::InterruptGuard, interrupt_handler::InterruptHandler, item::Item,
+    fragment::Fragment, function::Function, interrupt_guard::InterruptGuard,
-    justfile::Justfile, keyed::Keyed, keyword::Keyword, lexer::Lexer, line::Line, list::List,
+    interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, keyed::Keyed,
-    load_dotenv::load_dotenv, loader::Loader, module_path::ModulePath, name::Name,
+    keyword::Keyword, lexer::Lexer, line::Line, list::List, load_dotenv::load_dotenv,
-    namepath::Namepath, ordinal::Ordinal, output::output, output_error::OutputError,
+    loader::Loader, module_path::ModulePath, name::Name, namepath::Namepath, ordinal::Ordinal,
-    parameter::Parameter, parameter_kind::ParameterKind, parser::Parser, platform::Platform,
+    output::output, output_error::OutputError, parameter::Parameter, parameter_kind::ParameterKind,
-    platform_interface::PlatformInterface, position::Position, positional::Positional, ran::Ran,
+    parser::Parser, platform::Platform, platform_interface::PlatformInterface, position::Position,
-    range_ext::RangeExt, recipe::Recipe, recipe_context::RecipeContext,
+    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;

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(
+    let evaluator = Evaluator::new(context, scope);
-      context.config,
-      context.dotenv,
-      context.module_source,
-      scope,
-      context.search,
-      context.settings,
-      context.unexports,
-    );
 
     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,