Add in parameter inference for blocks (#2706)

crates/nu-cli/src/commands/classified/external.rs

@@ -237,6 +237,7 @@ fn spawn(
                             }
                         }
                         unsupported => {
+                            println!("Unsupported: {:?}", unsupported);
                             let _ = stdin_write_tx.send(Ok(Value {
                                 value: UntaggedValue::Error(ShellError::labeled_error(
                                     format!(

crates/nu-cli/src/commands/each/command.rs

@@ -6,8 +6,7 @@ use crate::prelude::*;
 use futures::stream::once;
 use nu_errors::ShellError;
 use nu_protocol::{
-    hir::Block, hir::Expression, hir::SpannedExpression, hir::Synthetic, Scope, Signature,
+    hir::Block, Scope, Signature, SyntaxShape, TaggedDictBuilder, UntaggedValue, Value,
-    SyntaxShape, TaggedDictBuilder, UntaggedValue, Value,
 };
 use nu_source::Tagged;
 
@@ -73,34 +72,36 @@ impl WholeStreamCommand for Each {
     }
 }
 
-fn is_expanded_it_usage(head: &SpannedExpression) -> bool {
-    matches!(&*head, SpannedExpression {
-        expr: Expression::Synthetic(Synthetic::String(s)),
-        ..
-    } if s == "expanded-each")
-}
-
 pub async fn process_row(
     block: Arc<Block>,
     scope: Arc<Scope>,
-    head: Arc<Box<SpannedExpression>>,
     mut context: Arc<EvaluationContext>,
     input: Value,
 ) -> Result<OutputStream, ShellError> {
     let input_clone = input.clone();
-    let input_stream = if is_expanded_it_usage(&head) {
+    // When we process a row, we need to know whether the block wants to have the contents of the row as
+    // a parameter to the block (so it gets assigned to a variable that can be used inside the block) or
+    // if it wants the contents as as an input stream
+    let params = block.params();
+
+    let input_stream = if !params.is_empty() {
         InputStream::empty()
     } else {
         once(async { Ok(input_clone) }).to_input_stream()
     };
-    Ok(run_block(
+
-        &block,
+    let scope = if !params.is_empty() {
-        Arc::make_mut(&mut context),
+        // FIXME: add check for more than parameter, once that's supported
-        input_stream,
+        Scope::append_var(scope, params[0].clone(), input)
-        Scope::append_var(scope, "$it", input),
+    } else {
-    )
+        scope
+    };
+
+    Ok(
+        run_block(&block, Arc::make_mut(&mut context), input_stream, scope)
             .await?
-    .to_output_stream())
+            .to_output_stream(),
+    )
 }
 
 pub(crate) fn make_indexed_item(index: usize, item: Value) -> Value {
@@ -116,7 +117,6 @@ async fn each(
     registry: &CommandRegistry,
 ) -> Result<OutputStream, ShellError> {
     let registry = registry.clone();
-    let head = Arc::new(raw_args.call_info.args.head.clone());
     let scope = raw_args.call_info.scope.clone();
     let context = Arc::new(EvaluationContext::from_raw(&raw_args, &registry));
     let (each_args, input): (EachArgs, _) = raw_args.process(&registry).await?;
@@ -128,12 +128,11 @@ async fn each(
             .then(move |input| {
                 let block = block.clone();
                 let scope = scope.clone();
-                let head = head.clone();
                 let context = context.clone();
                 let row = make_indexed_item(input.0, input.1);
 
                 async {
-                    match process_row(block, scope, head, context, row).await {
+                    match process_row(block, scope, context, row).await {
                         Ok(s) => s,
                         Err(e) => OutputStream::one(Err(e)),
                     }
@@ -146,11 +145,10 @@ async fn each(
             .then(move |input| {
                 let block = block.clone();
                 let scope = scope.clone();
-                let head = head.clone();
                 let context = context.clone();
 
                 async {
-                    match process_row(block, scope, head, context, input).await {
+                    match process_row(block, scope, context, input).await {
                         Ok(s) => s,
                         Err(e) => OutputStream::one(Err(e)),
                     }

crates/nu-cli/src/commands/each/group.rs

@@ -2,10 +2,7 @@ use crate::commands::each::process_row;
 use crate::commands::WholeStreamCommand;
 use crate::prelude::*;
 use nu_errors::ShellError;
-use nu_protocol::{
+use nu_protocol::{hir::Block, ReturnSuccess, Scope, Signature, SyntaxShape, UntaggedValue, Value};
-    hir::Block, hir::SpannedExpression, ReturnSuccess, Scope, Signature, SyntaxShape,
-    UntaggedValue, Value,
-};
 use nu_source::Tagged;
 use serde::Deserialize;
 
@@ -52,7 +49,6 @@ impl WholeStreamCommand for EachGroup {
         registry: &CommandRegistry,
     ) -> Result<OutputStream, ShellError> {
         let registry = registry.clone();
-        let head = Arc::new(raw_args.call_info.args.head.clone());
         let scope = raw_args.call_info.scope.clone();
         let context = Arc::new(EvaluationContext::from_raw(&raw_args, &registry));
         let (each_args, input): (EachGroupArgs, _) = raw_args.process(&registry).await?;
@@ -61,13 +57,7 @@ impl WholeStreamCommand for EachGroup {
         Ok(input
             .chunks(each_args.group_size.item)
             .then(move |input| {
-                run_block_on_vec(
+                run_block_on_vec(input, block.clone(), scope.clone(), context.clone())
-                    input,
-                    block.clone(),
-                    scope.clone(),
-                    head.clone(),
-                    context.clone(),
-                )
             })
             .flatten()
             .to_output_stream())
@@ -78,7 +68,6 @@ pub(crate) fn run_block_on_vec(
     input: Vec<Value>,
     block: Arc<Block>,
     scope: Arc<Scope>,
-    head: Arc<Box<SpannedExpression>>,
     context: Arc<EvaluationContext>,
 ) -> impl Future<Output = OutputStream> {
     let value = Value {
@@ -87,7 +76,7 @@ pub(crate) fn run_block_on_vec(
     };
 
     async {
-        match process_row(block, scope, head, context, value).await {
+        match process_row(block, scope, context, value).await {
             Ok(s) => {
                 // We need to handle this differently depending on whether process_row
                 // returned just 1 value or if it returned multiple as a stream.

crates/nu-cli/src/commands/each/window.rs

@@ -56,7 +56,6 @@ impl WholeStreamCommand for EachWindow {
         registry: &CommandRegistry,
     ) -> Result<OutputStream, ShellError> {
         let registry = registry.clone();
-        let head = Arc::new(raw_args.call_info.args.head.clone());
         let scope = raw_args.call_info.scope.clone();
         let context = Arc::new(EvaluationContext::from_raw(&raw_args, &registry));
         let (each_args, mut input): (EachWindowArgs, _) = raw_args.process(&registry).await?;
@@ -82,13 +81,12 @@ impl WholeStreamCommand for EachWindow {
 
                 let block = block.clone();
                 let scope = scope.clone();
-                let head = head.clone();
                 let context = context.clone();
                 let local_window = window.clone();
 
                 async move {
                     if i % stride == 0 {
-                        Some(run_block_on_vec(local_window, block, scope, head, context).await)
+                        Some(run_block_on_vec(local_window, block, scope, context).await)
                     } else {
                         None
                     }

crates/nu-cli/src/commands/group_by.rs

@@ -139,7 +139,6 @@ pub async fn group_by(
 ) -> Result<OutputStream, ShellError> {
     let name = args.call_info.name_tag.clone();
     let registry = registry.clone();
-    let head = Arc::new(args.call_info.args.head.clone());
     let scope = args.call_info.scope.clone();
     let context = Arc::new(EvaluationContext::from_raw(&args, &registry));
     let (Arguments { grouper }, input) = args.process(&registry).await?;
@@ -159,12 +158,9 @@ pub async fn group_by(
             for value in values.iter() {
                 let run = block.clone();
                 let scope = scope.clone();
-                let head = head.clone();
                 let context = context.clone();
 
-                match crate::commands::each::process_row(run, scope, head, context, value.clone())
+                match crate::commands::each::process_row(run, scope, context, value.clone()).await {
-                    .await
-                {
                     Ok(mut s) => {
                         let collection: Vec<Result<ReturnSuccess, ShellError>> =
                             s.drain_vec().await;

crates/nu-cli/tests/commands/each.rs

@@ -47,3 +47,27 @@ fn each_window_stride() {
 
     assert_eq!(actual.out, "[[1,2,3],[3,4,5]]");
 }
+
+#[test]
+fn each_no_args_in_block() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+        echo [[foo bar]; [a b] [c d] [e f]] | each { to json } | nth 1 | str collect
+        "#
+    ));
+
+    assert_eq!(actual.out, r#"{"foo":"c","bar":"d"}"#);
+}
+
+#[test]
+fn each_implicit_it_in_block() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+        echo [[foo bar]; [a b] [c d] [e f]] | each { nu --testbin cococo $it.foo }
+        "#
+    ));
+
+    assert_eq!(actual.out, "ace");
+}

crates/nu-data/src/base.rs

@@ -11,7 +11,6 @@ use nu_source::{Span, Tag};
 use nu_value_ext::ValueExt;
 use num_bigint::BigInt;
 use num_traits::Zero;
-use query_interface::{interfaces, vtable_for, ObjectHash};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, new, Serialize)]
@@ -21,14 +20,6 @@ pub struct Operation {
     pub(crate) right: Value,
 }
 
-#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash, Serialize, Deserialize, new)]
-pub struct Block {
-    pub(crate) commands: hir::Commands,
-    pub(crate) tag: Tag,
-}
-
-interfaces!(Block: dyn ObjectHash);
-
 #[derive(Serialize, Deserialize)]
 pub enum Switch {
     Present,

crates/nu-protocol/src/hir.rs

@@ -40,6 +40,10 @@ impl InternalCommand {
             ),
         }
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        self.args.has_it_usage()
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
@@ -76,6 +80,17 @@ pub enum ClassifiedCommand {
     Error(ParseError),
 }
 
+impl ClassifiedCommand {
+    fn has_it_usage(&self) -> bool {
+        match self {
+            ClassifiedCommand::Expr(expr) => expr.has_it_usage(),
+            ClassifiedCommand::Dynamic(call) => call.has_it_usage(),
+            ClassifiedCommand::Internal(internal) => internal.has_it_usage(),
+            ClassifiedCommand::Error(_) => false,
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
 pub struct Commands {
     pub list: Vec<ClassifiedCommand>,
@@ -90,28 +105,25 @@ impl Commands {
     pub fn push(&mut self, command: ClassifiedCommand) {
         self.list.push(command);
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        self.list.iter().any(|cc| cc.has_it_usage())
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
 pub struct Block {
-    pub params: Vec<String>,
+    params: Option<Vec<String>>,
     pub block: Vec<Commands>,
     pub span: Span,
 }
 
 impl Block {
     pub fn new(params: Option<Vec<String>>, block: Vec<Commands>, span: Span) -> Block {
-        match params {
+        Block {
-            Some(params) => Block {
             params,
             block,
             span,
-            },
-            None => Block {
-                params: vec!["$it".into()],
-                block,
-                span,
-            },
         }
     }
 
@@ -128,6 +140,20 @@ impl Block {
             }
         }
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        self.block.iter().any(|x| x.has_it_usage())
+    }
+
+    pub fn params(&self) -> Vec<String> {
+        if let Some(params) = &self.params {
+            params.clone()
+        } else if self.has_it_usage() {
+            vec!["$it".into()]
+        } else {
+            vec![]
+        }
+    }
 }
 
 #[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash, Deserialize, Serialize)]
@@ -165,7 +191,7 @@ pub struct ExternalCommand {
 }
 
 impl ExternalCommand {
-    pub fn has_it_argument(&self) -> bool {
+    pub fn has_it_usage(&self) -> bool {
         self.args.iter().any(|arg| match arg {
             SpannedExpression {
                 expr: Expression::Path(path),
@@ -516,6 +542,10 @@ impl SpannedExpression {
             _ => 0,
         }
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        self.expr.has_it_usage()
+    }
 }
 
 impl std::ops::Deref for SpannedExpression {
@@ -956,6 +986,32 @@ impl Expression {
     pub fn boolean(b: bool) -> Expression {
         Expression::Boolean(b)
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        match self {
+            Expression::Variable(name, _) if name == "$it" => true,
+            Expression::Table(headers, values) => {
+                headers.iter().any(|se| se.has_it_usage())
+                    || values.iter().any(|v| v.iter().any(|se| se.has_it_usage()))
+            }
+            Expression::List(list) => list.iter().any(|se| se.has_it_usage()),
+            Expression::Invocation(block) => block.has_it_usage(),
+            Expression::Binary(binary) => binary.left.has_it_usage() || binary.right.has_it_usage(),
+            Expression::Path(path) => path.head.has_it_usage(),
+            Expression::Range(range) => {
+                (if let Some(left) = &range.left {
+                    left.has_it_usage()
+                } else {
+                    false
+                }) || (if let Some(right) = &range.right {
+                    right.has_it_usage()
+                } else {
+                    false
+                })
+            }
+            _ => false,
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
@@ -966,6 +1022,16 @@ pub enum NamedValue {
     Value(Span, SpannedExpression),
 }
 
+impl NamedValue {
+    fn has_it_usage(&self) -> bool {
+        if let NamedValue::Value(_, se) = self {
+            se.has_it_usage()
+        } else {
+            false
+        }
+    }
+}
+
 impl PrettyDebugWithSource for NamedValue {
     fn pretty_debug(&self, source: &str) -> DebugDocBuilder {
         match self {
@@ -1028,6 +1094,20 @@ impl Call {
             }
         }
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        self.head.has_it_usage()
+            || (if let Some(pos) = &self.positional {
+                pos.iter().any(|x| x.has_it_usage())
+            } else {
+                false
+            })
+            || (if let Some(named) = &self.named {
+                named.has_it_usage()
+            } else {
+                false
+            })
+    }
 }
 
 impl PrettyDebugWithSource for Call {
@@ -1188,6 +1268,10 @@ impl NamedArguments {
     pub fn is_empty(&self) -> bool {
         self.named.is_empty()
     }
+
+    pub fn has_it_usage(&self) -> bool {
+        self.iter().any(|x| x.1.has_it_usage())
+    }
 }
 
 impl NamedArguments {

crates/nu-protocol/src/value.rs

@@ -991,14 +991,14 @@ mod tests {
     #[test]
     fn test_string_to_string_value_create_tag_extension() {
         let end = "a_string".to_string().len();
-        let the_tag = Tag {
+        let tag = Tag {
             anchor: None,
             span: Span::new(0, end),
         };
 
         let expected = Value {
             value: UntaggedValue::Primitive(Primitive::String("a_string".to_string())),
-            tag: the_tag.clone(),
+            tag,
         };
 
         assert_eq!(