From 8229af75919dd4c300d95720616c26fa0bbdd43b Mon Sep 17 00:00:00 2001
From: Jonathan Turner <jonathandturner@users.noreply.github.com>
Date: Wed, 28 Oct 2020 07:47:11 +1300
Subject: [PATCH] Improve parameter inference for blocks (#2708)

---
 crates/nu-cli/src/commands/each/command.rs |  7 +++----
 crates/nu-parser/src/parse.rs              | 13 +++++++------
 crates/nu-protocol/src/hir.rs              | 22 +++++++++++-----------
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/crates/nu-cli/src/commands/each/command.rs b/crates/nu-cli/src/commands/each/command.rs
index 759a7355da..2f512838fe 100644
--- a/crates/nu-cli/src/commands/each/command.rs
+++ b/crates/nu-cli/src/commands/each/command.rs
@@ -82,17 +82,16 @@ pub async fn process_row(
     // 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() {
+    let input_stream = if !block.params.is_empty() {
         InputStream::empty()
     } else {
         once(async { Ok(input_clone) }).to_input_stream()
     };
 
-    let scope = if !params.is_empty() {
+    let scope = if !block.params.is_empty() {
         // FIXME: add check for more than parameter, once that's supported
-        Scope::append_var(scope, params[0].clone(), input)
+        Scope::append_var(scope, block.params[0].clone(), input)
     } else {
         scope
     };
diff --git a/crates/nu-parser/src/parse.rs b/crates/nu-parser/src/parse.rs
index 2cbab19940..967e11d5b6 100644
--- a/crates/nu-parser/src/parse.rs
+++ b/crates/nu-parser/src/parse.rs
@@ -603,7 +603,7 @@ fn parse_interpolated_string(
     }];
 
     let call = SpannedExpression {
-        expr: Expression::Invocation(Block::new(None, block, lite_arg.span)),
+        expr: Expression::Invocation(Block::new(vec![], block, lite_arg.span)),
         span: lite_arg.span,
     };
 
@@ -1372,8 +1372,8 @@ fn parse_positional_argument(
                     let span = arg.span;
                     let mut commands = hir::Commands::new(span);
                     commands.push(ClassifiedCommand::Expr(Box::new(arg)));
-                    let mut block = hir::Block::new(None, vec![], span);
-                    block.push(commands);
+
+                    let block = hir::Block::new(vec![], vec![commands], span);
 
                     let arg = SpannedExpression::new(Expression::Block(block), span);
 
@@ -1768,7 +1768,7 @@ fn expand_shorthand_forms(
 }
 
 pub fn classify_block(lite_block: &LiteBlock, registry: &dyn SignatureRegistry) -> ClassifiedBlock {
-    let mut block = Block::new(None, vec![], lite_block.span());
+    let mut command_list = vec![];
 
     let mut error = None;
     for lite_pipeline in &lite_block.block {
@@ -1781,7 +1781,7 @@ pub fn classify_block(lite_block: &LiteBlock, registry: &dyn SignatureRegistry)
 
         let pipeline = if let Some(vars) = vars {
             let span = pipeline.commands.span;
-            let block = hir::Block::new(None, vec![pipeline.commands.clone()], span);
+            let block = hir::Block::new(vec![], vec![pipeline.commands.clone()], span);
             let mut call = hir::Call::new(
                 Box::new(SpannedExpression {
                     expr: Expression::string("with-env".to_string()),
@@ -1823,11 +1823,12 @@ pub fn classify_block(lite_block: &LiteBlock, registry: &dyn SignatureRegistry)
             pipeline
         };
 
-        block.push(pipeline.commands);
+        command_list.push(pipeline.commands);
         if error.is_none() {
             error = err;
         }
     }
+    let block = Block::new(vec![], command_list, lite_block.span());
 
     ClassifiedBlock::new(block, error)
 }
diff --git a/crates/nu-protocol/src/hir.rs b/crates/nu-protocol/src/hir.rs
index 2ed67c5763..20fcd8b30e 100644
--- a/crates/nu-protocol/src/hir.rs
+++ b/crates/nu-protocol/src/hir.rs
@@ -113,22 +113,26 @@ impl Commands {
 
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
 pub struct Block {
-    params: Option<Vec<String>>,
+    pub params: Vec<String>,
     pub block: Vec<Commands>,
     pub span: Span,
 }
 
 impl Block {
-    pub fn new(params: Option<Vec<String>>, block: Vec<Commands>, span: Span) -> Block {
-        Block {
+    pub fn new(params: Vec<String>, block: Vec<Commands>, span: Span) -> Block {
+        let mut output = Block {
             params,
             block,
             span,
-        }
+        };
+
+        output.infer_params();
+        output
     }
 
     pub fn push(&mut self, commands: Commands) {
         self.block.push(commands);
+        self.infer_params();
     }
 
     pub fn set_redirect(&mut self, external_redirection: ExternalRedirection) {
@@ -145,13 +149,9 @@ impl Block {
         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![]
+    pub fn infer_params(&mut self) {
+        if self.params.is_empty() && self.has_it_usage() {
+            self.params = vec!["$it".into()];
         }
     }
 }