Make arg eval lazy, remove old arg evaluation code (#3603)

* Remove old argument eval

* Merge main

* fmt

* clippy

* clippy

* clippy

crates/nu-engine/src/command_args.rs

@@ -1,18 +1,15 @@
-use crate::env::host::Host;
 use crate::evaluate::scope::Scope;
 use crate::evaluation_context::EvaluationContext;
 use crate::shell::shell_manager::ShellManager;
 use crate::FromValue;
 use crate::{call_info::UnevaluatedCallInfo, config_holder::ConfigHolder};
-use derive_new::new;
+use crate::{env::host::Host, evaluate_baseline_expr};
 use getset::Getters;
 use nu_errors::ShellError;
-use nu_protocol::EvaluatedArgs;
-use nu_protocol::{CallInfo, Value};
+use nu_protocol::hir::SpannedExpression;
 use nu_source::Tag;
 use nu_stream::InputStream;
 use parking_lot::Mutex;
-use std::ops::Deref;
 use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 
@@ -48,131 +45,19 @@ impl CommandArgs {
     pub fn shell_manager(&self) -> ShellManager {
         self.context.shell_manager.clone()
     }
-}
 
-pub type RunnableContext = CommandArgs;
-
-impl std::fmt::Debug for CommandArgs {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.call_info.fmt(f)
-    }
-}
-
-impl CommandArgs {
-    pub fn evaluate_once(self) -> Result<EvaluatedCommandArgs, ShellError> {
-        let ctx = self.context.clone();
-
-        let input = self.input;
-        let call_info = self.call_info.evaluate(&ctx)?;
-
-        Ok(EvaluatedCommandArgs::new(ctx, call_info, input))
-    }
-
-    pub fn extract<T>(
-        self,
-        f: impl FnOnce(&EvaluatedCommandArgsWithoutInput) -> Result<T, ShellError>,
-    ) -> Result<(T, InputStream), ShellError> {
-        let evaluated_args = self.evaluate_once()?;
-
-        Ok((f(&evaluated_args.args)?, evaluated_args.input))
-    }
-}
-
-pub struct EvaluatedCommandArgs {
-    pub args: EvaluatedCommandArgsWithoutInput,
-    pub input: InputStream,
-}
-
-impl Deref for EvaluatedCommandArgs {
-    type Target = EvaluatedCommandArgsWithoutInput;
-    fn deref(&self) -> &Self::Target {
-        &self.args
-    }
-}
-
-impl EvaluatedCommandArgs {
-    pub fn new(
-        context: EvaluationContext,
-        call_info: CallInfo,
-        input: impl Into<InputStream>,
-    ) -> EvaluatedCommandArgs {
-        EvaluatedCommandArgs {
-            args: EvaluatedCommandArgsWithoutInput { context, call_info },
-            input: input.into(),
-        }
-    }
-
-    pub fn name_tag(&self) -> Tag {
-        self.args.call_info.name_tag.clone()
-    }
-
-    pub fn parts(self) -> (InputStream, EvaluatedArgs) {
-        let EvaluatedCommandArgs { args, input } = self;
-
-        (input, args.call_info.args)
-    }
-
-    pub fn split(self) -> (InputStream, EvaluatedCommandArgsWithoutInput) {
-        let EvaluatedCommandArgs { args, input } = self;
-
-        (input, args)
-    }
-}
-
-#[derive(Getters, new)]
-#[get = "pub(crate)"]
-pub struct EvaluatedCommandArgsWithoutInput {
-    pub context: EvaluationContext,
-    pub call_info: CallInfo,
-}
-
-impl EvaluatedCommandArgsWithoutInput {
-    pub fn nth(&self, pos: usize) -> Option<&Value> {
-        self.call_info.args.nth(pos)
-    }
-
-    pub fn scope(&self) -> Scope {
-        self.context.scope.clone()
-    }
-
-    pub fn configs(&self) -> Arc<Mutex<ConfigHolder>> {
-        self.context.configs.clone()
-    }
-
-    pub fn host(&self) -> Arc<parking_lot::Mutex<Box<dyn Host>>> {
-        self.context.host.clone()
-    }
-
-    /// Get the nth positional argument, error if not possible
-    pub fn expect_nth(&self, pos: usize) -> Result<&Value, ShellError> {
-        self.call_info
-            .args
-            .nth(pos)
-            .ok_or_else(|| ShellError::unimplemented("Better error: expect_nth"))
-    }
-
-    pub fn get_flag<T: FromValue>(&self, name: &str) -> Result<Option<T>, ShellError> {
-        if let Some(arg) = self.call_info.args.get(name) {
-            FromValue::from_value(arg).map(Some)
+    pub fn nth(&self, pos: usize) -> Option<&SpannedExpression> {
+        if let Some(positional) = &self.call_info.args.positional {
+            positional.get(pos)
         } else {
-            Ok(None)
+            None
         }
     }
 
-    pub fn req_named<T: FromValue>(&self, name: &str) -> Result<T, ShellError> {
-        self.call_info
-            .args
-            .expect_get(name)
-            .and_then(|x| FromValue::from_value(x))
-    }
-
-    pub fn has_flag(&self, name: &str) -> bool {
-        self.call_info.args.has(name)
-    }
-
     pub fn req<T: FromValue>(&self, pos: usize) -> Result<T, ShellError> {
-        if let Some(v) = self.nth(pos) {
-            FromValue::from_value(v)
+        if let Some(expr) = self.nth(pos) {
+            let result = evaluate_baseline_expr(expr, &self.context)?;
+            FromValue::from_value(&result)
         } else {
             Err(ShellError::labeled_error(
                 "Position beyond end of command arguments",
@@ -182,9 +67,34 @@ impl EvaluatedCommandArgsWithoutInput {
         }
     }
 
+    pub fn req_named<T: FromValue>(&self, name: &str) -> Result<T, ShellError> {
+        match self.get_flag(name)? {
+            Some(v) => Ok(v),
+            None => Err(ShellError::labeled_error(
+                "Missing flag",
+                format!("expected {} flag", name),
+                &self.call_info.name_tag,
+            )),
+        }
+    }
+
+    pub fn has_flag(&self, name: &str) -> bool {
+        self.call_info.args.switch_preset(name)
+    }
+
+    pub fn get_flag<T: FromValue>(&self, name: &str) -> Result<Option<T>, ShellError> {
+        if let Some(expr) = self.call_info.args.get_flag(name) {
+            let result = evaluate_baseline_expr(expr, &self.context)?;
+            FromValue::from_value(&result).map(Some)
+        } else {
+            Ok(None)
+        }
+    }
+
     pub fn opt<T: FromValue>(&self, pos: usize) -> Result<Option<T>, ShellError> {
-        if let Some(v) = self.nth(pos) {
-            FromValue::from_value(v).map(Some)
+        if let Some(expr) = self.nth(pos) {
+            let result = evaluate_baseline_expr(expr, &self.context)?;
+            FromValue::from_value(&result).map(Some)
         } else {
             Ok(None)
         }
@@ -193,8 +103,11 @@ impl EvaluatedCommandArgsWithoutInput {
     pub fn rest<T: FromValue>(&self, starting_pos: usize) -> Result<Vec<T>, ShellError> {
         let mut output = vec![];
 
-        for val in self.call_info.args.positional_iter().skip(starting_pos) {
-            output.push(FromValue::from_value(val)?);
+        if let Some(positional) = &self.call_info.args.positional {
+            for expr in positional.iter().skip(starting_pos) {
+                let result = evaluate_baseline_expr(expr, &self.context)?;
+                output.push(FromValue::from_value(&result)?);
+            }
         }
 
         Ok(output)
@@ -213,4 +126,16 @@ impl EvaluatedCommandArgsWithoutInput {
 
         Ok(output)
     }
+
+    pub fn name_tag(&self) -> Tag {
+        self.call_info.name_tag.clone()
+    }
+}
+
+pub type RunnableContext = CommandArgs;
+
+impl std::fmt::Debug for CommandArgs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.call_info.fmt(f)
+    }
 }

crates/nu-engine/src/filesystem/filesystem_shell.rs

@@ -1,10 +1,13 @@
-use crate::filesystem::dir_info::{DirBuilder, DirInfo};
 use crate::filesystem::path::canonicalize;
 use crate::filesystem::utils::FileStructure;
 use crate::maybe_text_codec::{MaybeTextCodec, StringOrBinary};
 use crate::shell::shell_args::{CdArgs, CopyArgs, LsArgs, MkdirArgs, MvArgs, RemoveArgs};
 use crate::shell::Shell;
-use crate::{command_args::EvaluatedCommandArgs, BufCodecReader};
+use crate::BufCodecReader;
+use crate::{
+    filesystem::dir_info::{DirBuilder, DirInfo},
+    CommandArgs,
+};
 use encoding_rs::Encoding;
 use nu_data::config::LocalConfigDiff;
 use nu_protocol::{CommandAction, ConfigPath, TaggedDictBuilder, Value};
@@ -776,7 +779,7 @@ impl Shell for FilesystemShell {
         self.path.clone()
     }
 
-    fn pwd(&self, args: EvaluatedCommandArgs) -> Result<ActionStream, ShellError> {
+    fn pwd(&self, args: CommandArgs) -> Result<ActionStream, ShellError> {
         let path = PathBuf::from(self.path());
         let p = match dunce::canonicalize(path.as_path()) {
             Ok(p) => p,

crates/nu-engine/src/lib.rs

@@ -16,9 +16,7 @@ pub mod shell;
 mod whole_stream_command;
 
 pub use crate::call_info::UnevaluatedCallInfo;
-pub use crate::command_args::{
-    CommandArgs, EvaluatedCommandArgs, EvaluatedCommandArgsWithoutInput, RunnableContext,
-};
+pub use crate::command_args::{CommandArgs, RunnableContext};
 pub use crate::config_holder::ConfigHolder;
 pub use crate::documentation::{generate_docs, get_brief_help, get_documentation, get_full_help};
 pub use crate::env::host::FakeHost;

crates/nu-engine/src/plugin/run_plugin.rs

@@ -1,12 +1,17 @@
-use crate::command_args::CommandArgs;
-use crate::whole_stream_command::{whole_stream_command, WholeStreamCommand};
+use crate::{command_args::CommandArgs, evaluate_baseline_expr, UnevaluatedCallInfo};
+use crate::{
+    whole_stream_command::{whole_stream_command, WholeStreamCommand},
+    EvaluationContext,
+};
 use derive_new::new;
 
+use indexmap::IndexMap;
 use log::trace;
 use nu_errors::ShellError;
 use nu_plugin::jsonrpc::JsonRpc;
-use nu_protocol::{Primitive, ReturnValue, Signature, UntaggedValue, Value};
-use nu_stream::{ActionStream, ToActionStream};
+use nu_protocol::{hir, Primitive, ReturnValue, Signature, UntaggedValue, Value};
+use nu_source::Tag;
+use nu_stream::{ActionStream, InputStream, ToActionStream};
 use serde::{self, Deserialize, Serialize};
 use std::collections::VecDeque;
 use std::io::prelude::*;
@@ -119,7 +124,7 @@ fn run_filter(path: String, args: CommandArgs) -> Result<ActionStream, ShellErro
     let eos =
         vec![UntaggedValue::Primitive(Primitive::EndOfStream).into_untagged_value()].into_iter();
 
-    let args = args.evaluate_once()?;
+    let (call_info, input) = evaluate_once(args)?;
 
     let real_path = Path::new(&path);
     let ext = real_path.extension();
@@ -150,12 +155,10 @@ fn run_filter(path: String, args: CommandArgs) -> Result<ActionStream, ShellErro
             .expect("Failed to spawn child process")
     };
 
-    let call_info = args.call_info.clone();
-
     trace!("filtering :: {:?}", call_info);
 
     Ok(bos
-        .chain(args.input)
+        .chain(input)
         .chain(eos)
         .map(move |item| {
             match item {
@@ -386,10 +389,9 @@ impl WholeStreamCommand for PluginSink {
 }
 
 fn run_sink(path: String, args: CommandArgs) -> Result<ActionStream, ShellError> {
-    let args = args.evaluate_once()?;
-    let call_info = args.call_info.clone();
+    let (call_info, input) = evaluate_once(args)?;
 
-    let input: Vec<Value> = args.input.into_vec();
+    let input: Vec<Value> = input.into_vec();
 
     let request = JsonRpc::new("sink", (call_info, input));
     let request_raw = serde_json::to_string(&request);
@@ -446,3 +448,80 @@ fn run_sink(path: String, args: CommandArgs) -> Result<ActionStream, ShellError>
         ))
     }
 }
+
+/// Associated information for the call of a command, including the args passed to the command and a tag that spans the name of the command being called
+#[derive(Deserialize, Serialize, Debug, Clone)]
+pub struct CallInfo {
+    /// The arguments associated with this call
+    pub args: EvaluatedArgs,
+    /// The tag (underline-able position) of the name of the call itself
+    pub name_tag: Tag,
+}
+
+/// The set of positional and named arguments, after their values have been evaluated.
+///
+/// * Positional arguments are those who are given as values, without any associated flag. For example, in `foo arg1 arg2`, both `arg1` and `arg2` are positional arguments.
+/// * Named arguments are those associated with a flag. For example, `foo --given bar` the named argument would be name `given` and the value `bar`.
+#[derive(Debug, Default, new, Serialize, Deserialize, Clone)]
+pub struct EvaluatedArgs {
+    pub positional: Option<Vec<Value>>,
+    pub named: Option<IndexMap<String, Value>>,
+}
+
+fn evaluate_once(args: CommandArgs) -> Result<(CallInfo, InputStream), ShellError> {
+    let input = args.input;
+    let call_info = evaluate_command(args.call_info, args.context)?;
+
+    Ok((call_info, input))
+}
+
+fn evaluate_command(
+    args: UnevaluatedCallInfo,
+    ctx: EvaluationContext,
+) -> Result<CallInfo, ShellError> {
+    let name_tag = args.name_tag.clone();
+    let args = evaluate_args(&args.args, &ctx)?;
+
+    Ok(CallInfo { args, name_tag })
+}
+
+fn evaluate_args(call: &hir::Call, ctx: &EvaluationContext) -> Result<EvaluatedArgs, ShellError> {
+    let mut positional_args: Vec<Value> = vec![];
+
+    if let Some(positional) = &call.positional {
+        for pos in positional {
+            let result = evaluate_baseline_expr(pos, ctx)?;
+            positional_args.push(result);
+        }
+    }
+
+    let positional = if !positional_args.is_empty() {
+        Some(positional_args)
+    } else {
+        None
+    };
+
+    let mut named_args = IndexMap::new();
+
+    if let Some(named) = &call.named {
+        for (name, value) in named.iter() {
+            match value {
+                hir::NamedValue::PresentSwitch(tag) => {
+                    named_args.insert(name.clone(), UntaggedValue::boolean(true).into_value(tag));
+                }
+                hir::NamedValue::Value(_, expr) => {
+                    named_args.insert(name.clone(), evaluate_baseline_expr(expr, ctx)?);
+                }
+                _ => {}
+            };
+        }
+    }
+
+    let named = if !named_args.is_empty() {
+        Some(named_args)
+    } else {
+        None
+    };
+
+    Ok(EvaluatedArgs::new(positional, named))
+}

crates/nu-engine/src/shell/mod.rs

@@ -1,8 +1,8 @@
 use nu_stream::{ActionStream, OutputStream};
 
-use crate::command_args::EvaluatedCommandArgs;
 use crate::maybe_text_codec::StringOrBinary;
 pub use crate::shell::shell_args::{CdArgs, CopyArgs, LsArgs, MkdirArgs, MvArgs, RemoveArgs};
+use crate::CommandArgs;
 use encoding_rs::Encoding;
 use nu_errors::ShellError;
 use nu_source::{Span, Tag};
@@ -33,7 +33,7 @@ pub trait Shell: std::fmt::Debug {
     fn mv(&self, args: MvArgs, name: Tag, path: &str) -> Result<ActionStream, ShellError>;
     fn rm(&self, args: RemoveArgs, name: Tag, path: &str) -> Result<ActionStream, ShellError>;
     fn path(&self) -> String;
-    fn pwd(&self, args: EvaluatedCommandArgs) -> Result<ActionStream, ShellError>;
+    fn pwd(&self, args: CommandArgs) -> Result<ActionStream, ShellError>;
     fn set_path(&mut self, path: String);
     fn open(
         &self,

crates/nu-engine/src/shell/shell_manager.rs

@@ -1,6 +1,6 @@
 use crate::shell::Shell;
-use crate::{command_args::EvaluatedCommandArgs, FilesystemShell};
 use crate::{filesystem::filesystem_shell::FilesystemShellMode, maybe_text_codec::StringOrBinary};
+use crate::{CommandArgs, FilesystemShell};
 use nu_stream::{ActionStream, OutputStream};
 
 use crate::shell::shell_args::{CdArgs, CopyArgs, LsArgs, MkdirArgs, MvArgs, RemoveArgs};
@@ -78,7 +78,7 @@ impl ShellManager {
         self.shells.lock()[self.current_shell()].path()
     }
 
-    pub fn pwd(&self, args: EvaluatedCommandArgs) -> Result<ActionStream, ShellError> {
+    pub fn pwd(&self, args: CommandArgs) -> Result<ActionStream, ShellError> {
         let env = self.shells.lock();
 
         env[self.current_shell()].pwd(args)

crates/nu-engine/src/shell/value_shell.rs

@@ -1,7 +1,7 @@
-use crate::command_args::EvaluatedCommandArgs;
 use crate::maybe_text_codec::StringOrBinary;
 use crate::shell::shell_args::{CdArgs, CopyArgs, LsArgs, MkdirArgs, MvArgs, RemoveArgs};
 use crate::shell::Shell;
+use crate::CommandArgs;
 use encoding_rs::Encoding;
 use nu_errors::ShellError;
 use nu_protocol::ValueStructure;
@@ -217,7 +217,7 @@ impl Shell for ValueShell {
         self.path.clone()
     }
 
-    fn pwd(&self, args: EvaluatedCommandArgs) -> Result<ActionStream, ShellError> {
+    fn pwd(&self, args: CommandArgs) -> Result<ActionStream, ShellError> {
         Ok(ActionStream::one(
             UntaggedValue::string(self.path()).into_value(&args.call_info.name_tag),
         ))