Allow const expressions in all settings (#3037)

Author: casey

README.md

@@ -1054,11 +1054,12 @@ Which is equivalent to:
 set NAME := true
 ```
 
-The `working-directory` setting can be set to a string, or an
-expression<sup>master</sup>. Because the `working-directory` setting affects
-backtick execution and the behavior of many functions, the expression may not
-contain backticks, function calls, or references to variable assignments which
-themselves, directly or transitively, contain backticks or function calls.
+Non-boolean settings can be set to both strings and
+expressions.<sup>master</sup>
+
+However, because settings affect the behavior of backticks and many functions,
+those expressions may not contain backticks or function calls, directly or
+transitively via reference.
 
 #### Allow Duplicate Recipes
 

src/analyzer.rs

@@ -143,7 +143,7 @@ impl<'run, 'src> Analyzer<'run, 'src> {
     AssignmentResolver::resolve_assignments(&assignments)?;
 
     for set in self.sets.values() {
-      if let Some(expression) = set.value.expression() {
+      for expression in set.value.expressions() {
         for variable in expression.variables() {
           let name = variable.lexeme();
           if !assignments.contains_key(name) && !constants().contains_key(name) {

src/attribute.rs

@@ -43,7 +43,7 @@ pub(crate) enum Attribute<'src> {
   Parallel,
   PositionalArguments,
   Private,
-  Script(Option<Interpreter>),
+  Script(Option<Interpreter<StringLiteral>>),
   Unix,
   Windows,
   WorkingDirectory(StringLiteral),

src/evaluator.rs

@@ -74,72 +74,86 @@ impl<'src, 'run> Evaluator<'src, 'run> {
 
     for (_name, set) in sets {
       match set.value {
-        Setting::AllowDuplicateRecipes(allow_duplicate_recipes) => {
-          settings.allow_duplicate_recipes = allow_duplicate_recipes;
+        Setting::AllowDuplicateRecipes(value) => {
+          settings.allow_duplicate_recipes = value;
         }
-        Setting::AllowDuplicateVariables(allow_duplicate_variables) => {
-          settings.allow_duplicate_variables = allow_duplicate_variables;
+        Setting::AllowDuplicateVariables(value) => {
+          settings.allow_duplicate_variables = value;
         }
-        Setting::DotenvFilename(filename) => {
-          settings.dotenv_filename = Some(filename.cooked);
+        Setting::DotenvFilename(value) => {
+          settings.dotenv_filename = Some(self.evaluate_expression(&value)?);
         }
-        Setting::DotenvLoad(dotenv_load) => {
-          settings.dotenv_load = dotenv_load;
+        Setting::DotenvLoad(value) => {
+          settings.dotenv_load = value;
         }
-        Setting::DotenvPath(path) => {
-          settings.dotenv_path = Some(path.cooked.into());
+        Setting::DotenvPath(value) => {
+          settings.dotenv_path = Some(self.evaluate_expression(&value)?.into());
         }
-        Setting::DotenvOverride(dotenv_overrride) => {
-          settings.dotenv_override = dotenv_overrride;
+        Setting::DotenvOverride(value) => {
+          settings.dotenv_override = value;
         }
-        Setting::DotenvRequired(dotenv_required) => {
-          settings.dotenv_required = dotenv_required;
+        Setting::DotenvRequired(value) => {
+          settings.dotenv_required = value;
         }
-        Setting::Export(export) => {
-          settings.export = export;
+        Setting::Export(value) => {
+          settings.export = value;
         }
-        Setting::Fallback(fallback) => {
-          settings.fallback = fallback;
+        Setting::Fallback(value) => {
+          settings.fallback = value;
         }
-        Setting::IgnoreComments(ignore_comments) => {
-          settings.ignore_comments = ignore_comments;
+        Setting::IgnoreComments(value) => {
+          settings.ignore_comments = value;
         }
-        Setting::NoExitMessage(no_exit_message) => {
-          settings.no_exit_message = no_exit_message;
+        Setting::NoExitMessage(value) => {
+          settings.no_exit_message = value;
         }
-        Setting::PositionalArguments(positional_arguments) => {
-          settings.positional_arguments = positional_arguments;
+        Setting::PositionalArguments(value) => {
+          settings.positional_arguments = value;
         }
-        Setting::Quiet(quiet) => {
-          settings.quiet = quiet;
+        Setting::Quiet(value) => {
+          settings.quiet = value;
         }
-        Setting::ScriptInterpreter(script_interpreter) => {
-          settings.script_interpreter = Some(script_interpreter);
+        Setting::ScriptInterpreter(value) => {
+          settings.script_interpreter = Some(self.evaluate_intepreter(&value)?);
         }
-        Setting::Shell(shell) => {
-          settings.shell = Some(shell);
+        Setting::Shell(value) => {
+          settings.shell = Some(self.evaluate_intepreter(&value)?);
         }
-        Setting::Unstable(unstable) => {
-          settings.unstable = unstable;
+        Setting::Unstable(value) => {
+          settings.unstable = value;
         }
-        Setting::WindowsPowerShell(windows_powershell) => {
-          settings.windows_powershell = windows_powershell;
+        Setting::WindowsPowerShell(value) => {
+          settings.windows_powershell = value;
         }
-        Setting::WindowsShell(windows_shell) => {
-          settings.windows_shell = Some(windows_shell);
+        Setting::WindowsShell(value) => {
+          settings.windows_shell = Some(self.evaluate_intepreter(&value)?);
         }
-        Setting::Tempdir(tempdir) => {
-          settings.tempdir = Some(tempdir.cooked);
+        Setting::Tempdir(value) => {
+          settings.tempdir = Some(self.evaluate_expression(&value)?);
         }
-        Setting::WorkingDirectory(working_directory) => {
-          settings.working_directory = Some(self.evaluate_expression(&working_directory)?.into());
+        Setting::WorkingDirectory(value) => {
+          settings.working_directory = Some(self.evaluate_expression(&value)?.into());
         }
       }
     }
 
     Ok(settings)
   }
 
+  pub(crate) fn evaluate_intepreter(
+    &mut self,
+    interpreter: &Interpreter<Expression<'src>>,
+  ) -> RunResult<'src, Interpreter<String>> {
+    Ok(Interpreter {
+      command: self.evaluate_expression(&interpreter.command)?,
+      arguments: interpreter
+        .arguments
+        .iter()
+        .map(|argument| self.evaluate_expression(argument))
+        .collect::<RunResult<Vec<String>>>()?,
+    })
+  }
+
   pub(crate) fn evaluate_assignments(
     config: &'run Config,
     dotenv: &'run BTreeMap<String, String>,

src/executor.rs

@@ -1,7 +1,7 @@
 use super::*;
 
 pub(crate) enum Executor<'a> {
-  Command(&'a Interpreter),
+  Command(Interpreter<String>),
   Shebang(Shebang<'a>),
 }
 
@@ -15,14 +15,14 @@ impl Executor<'_> {
   ) -> RunResult<'src, Command> {
     match self {
       Self::Command(interpreter) => {
-        let mut command = Command::new(&interpreter.command.cooked);
+        let mut command = Command::new(&interpreter.command);
 
         if let Some(working_directory) = working_directory {
           command.current_dir(working_directory);
         }
 
         for arg in &interpreter.arguments {
-          command.arg(&arg.cooked);
+          command.arg(arg);
         }
 
         command.arg(path);
@@ -50,7 +50,7 @@ impl Executor<'_> {
   pub(crate) fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String {
     let extension = extension.unwrap_or_else(|| {
       let interpreter = match self {
-        Self::Command(interpreter) => &interpreter.command.cooked,
+        Self::Command(interpreter) => &interpreter.command,
         Self::Shebang(shebang) => shebang.interpreter_filename(),
       };
 
@@ -67,11 +67,11 @@ impl Executor<'_> {
   pub(crate) fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> {
     match self {
       Self::Command(Interpreter { command, arguments }) => {
-        let mut command = command.cooked.clone();
+        let mut command = command.clone();
 
         for arg in arguments {
           command.push(' ');
-          command.push_str(&arg.cooked);
+          command.push_str(arg);
         }
 
         Error::Script {
@@ -139,8 +139,8 @@ mod tests {
         expected
       );
       assert_eq!(
-        Executor::Command(&Interpreter {
-          command: StringLiteral::from_raw(interpreter),
+        Executor::Command(Interpreter {
+          command: interpreter.into(),
           arguments: Vec::new()
         })
         .script_filename(recipe, extension),

src/interpreter.rs

@@ -1,22 +1,22 @@
 use super::*;
 
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize)]
-pub(crate) struct Interpreter {
-  pub(crate) arguments: Vec<StringLiteral>,
-  pub(crate) command: StringLiteral,
+pub(crate) struct Interpreter<T> {
+  pub(crate) arguments: Vec<T>,
+  pub(crate) command: T,
 }
 
-impl Interpreter {
-  pub(crate) fn default_script_interpreter() -> &'static Interpreter {
-    static INSTANCE: LazyLock<Interpreter> = LazyLock::new(|| Interpreter {
-      arguments: vec![StringLiteral::from_raw("-eu")],
-      command: StringLiteral::from_raw("sh"),
+impl Interpreter<String> {
+  pub(crate) fn default_script_interpreter() -> &'static Self {
+    static INSTANCE: LazyLock<Interpreter<String>> = LazyLock::new(|| Interpreter::<String> {
+      arguments: vec!["-eu".into()],
+      command: "sh".into(),
     });
     &INSTANCE
   }
 }
 
-impl Display for Interpreter {
+impl<T: Display> Display for Interpreter<T> {
   fn fmt(&self, f: &mut Formatter) -> fmt::Result {
     write!(f, "{}", self.command)?;
 

src/node.rs

@@ -325,20 +325,20 @@ impl<'src> Node<'src> for Set<'src> {
       | Setting::IgnoreComments(value) => {
         set.push_mut(value.to_string());
       }
+      Setting::DotenvFilename(value)
+      | Setting::DotenvPath(value)
+      | Setting::Tempdir(value)
+      | Setting::WorkingDirectory(value) => {
+        set.push_mut(value.tree());
+      }
       Setting::ScriptInterpreter(Interpreter { command, arguments })
       | Setting::Shell(Interpreter { command, arguments })
       | Setting::WindowsShell(Interpreter { command, arguments }) => {
-        set.push_mut(Tree::string(&command.cooked));
+        set.push_mut(command.tree());
         for argument in arguments {
-          set.push_mut(Tree::string(&argument.cooked));
+          set.push_mut(argument.tree());
         }
       }
-      Setting::DotenvFilename(value) | Setting::DotenvPath(value) | Setting::Tempdir(value) => {
-        set.push_mut(Tree::string(&value.cooked));
-      }
-      Setting::WorkingDirectory(value) => {
-        set.push_mut(value.tree());
-      }
     }
 
     set

src/parser.rs

@@ -1351,11 +1351,11 @@ impl<'run, 'src> Parser<'run, 'src> {
     self.expect(ColonEquals)?;
 
     let set_value = match keyword {
-      Keyword::DotenvFilename => Some(Setting::DotenvFilename(self.parse_string_literal()?)),
-      Keyword::DotenvPath => Some(Setting::DotenvPath(self.parse_string_literal()?)),
+      Keyword::DotenvFilename => Some(Setting::DotenvFilename(self.parse_expression()?)),
+      Keyword::DotenvPath => Some(Setting::DotenvPath(self.parse_expression()?)),
       Keyword::ScriptInterpreter => Some(Setting::ScriptInterpreter(self.parse_interpreter()?)),
       Keyword::Shell => Some(Setting::Shell(self.parse_interpreter()?)),
-      Keyword::Tempdir => Some(Setting::Tempdir(self.parse_string_literal()?)),
+      Keyword::Tempdir => Some(Setting::Tempdir(self.parse_expression()?)),
       Keyword::WindowsShell => Some(Setting::WindowsShell(self.parse_interpreter()?)),
       Keyword::WorkingDirectory => Some(Setting::WorkingDirectory(self.parse_expression()?)),
       _ => None,
@@ -1371,16 +1371,16 @@ impl<'run, 'src> Parser<'run, 'src> {
   }
 
   /// Parse interpreter setting value, i.e., `['sh', '-eu']`
-  fn parse_interpreter(&mut self) -> CompileResult<'src, Interpreter> {
+  fn parse_interpreter(&mut self) -> CompileResult<'src, Interpreter<Expression<'src>>> {
     self.expect(BracketL)?;
 
-    let command = self.parse_string_literal()?;
+    let command = self.parse_expression()?;
 
     let mut arguments = Vec::new();
 
     if self.accepted(Comma)? {
       while !self.next_is(BracketR) {
-        arguments.push(self.parse_string_literal()?);
+        arguments.push(self.parse_expression()?);
 
         if !self.accepted(Comma)? {
           break;
@@ -2921,39 +2921,16 @@ mod tests {
     width:  1,
     kind:   UnexpectedToken {
       expected: vec![
+        Backtick,
         Identifier,
+        ParenL,
+        Slash,
         StringToken,
       ],
       found: BracketR,
     },
   }
 
-  error! {
-    name:   set_shell_non_literal_first,
-    input:  "set shell := ['bar' + 'baz']",
-    offset: 20,
-    line:   0,
-    column: 20,
-    width:  1,
-    kind:   UnexpectedToken {
-      expected: vec![BracketR, Comma],
-      found: Plus,
-    },
-  }
-
-  error! {
-    name:   set_shell_non_literal_second,
-    input:  "set shell := ['biz', 'bar' + 'baz']",
-    offset: 27,
-    line:   0,
-    column: 27,
-    width:  1,
-    kind:   UnexpectedToken {
-      expected: vec![BracketR, Comma],
-      found: Plus,
-    },
-  }
-
   error! {
     name:   set_shell_bad_comma,
     input:  "set shell := ['bash',",
@@ -2963,8 +2940,11 @@ mod tests {
     width:  0,
     kind:   UnexpectedToken {
       expected: vec![
+        Backtick,
         BracketR,
         Identifier,
+        ParenL,
+        Slash,
         StringToken,
       ],
       found: Eof,
@@ -2979,7 +2959,7 @@ mod tests {
     column: 20,
     width:  0,
     kind:   UnexpectedToken {
-      expected: vec![BracketR, Comma],
+      expected: vec![AmpersandAmpersand, BarBar, BracketR, Comma, Plus, Slash],
       found: Eof,
     },
   }