From 751de20f938ed200ae6128a30d06a5dd24a4fd33 Mon Sep 17 00:00:00 2001 From: JT Date: Fri, 21 May 2021 19:04:27 +1200 Subject: [PATCH] Do a bit more cleanup of block params (#3455) * Do a bit more cleanup of block params * Do a bit more cleanup of block params --- crates/nu-command/src/commands.rs | 1 + crates/nu-command/src/commands/any.rs | 4 +- .../nu-command/src/commands/each/command.rs | 13 +--- crates/nu-command/src/commands/empty.rs | 4 +- crates/nu-command/src/commands/insert.rs | 4 +- crates/nu-command/src/commands/keep/until.rs | 70 ++++++++----------- crates/nu-command/src/commands/keep/while_.rs | 67 ++++++++---------- crates/nu-command/src/commands/reduce.rs | 8 ++- crates/nu-command/src/commands/skip/until.rs | 67 ++++++++---------- crates/nu-command/src/commands/skip/while_.rs | 67 ++++++++---------- crates/nu-command/src/commands/update.rs | 5 +- crates/nu-command/src/commands/where_.rs | 5 +- crates/nu-command/src/examples.rs | 2 - crates/nu-parser/src/parse.rs | 4 +- tests/shell/pipeline/commands/internal.rs | 22 +++--- 15 files changed, 157 insertions(+), 186 deletions(-) diff --git a/crates/nu-command/src/commands.rs b/crates/nu-command/src/commands.rs index b6f821f42..aa148ce54 100644 --- a/crates/nu-command/src/commands.rs +++ b/crates/nu-command/src/commands.rs @@ -360,6 +360,7 @@ mod tests { #[test] fn examples_work_as_expected() -> Result<(), ShellError> { for cmd in only_examples() { + println!("cmd: {}", cmd.name()); test_examples(cmd)?; } diff --git a/crates/nu-command/src/commands/any.rs b/crates/nu-command/src/commands/any.rs index aacfcd214..d2ce5a99b 100644 --- a/crates/nu-command/src/commands/any.rs +++ b/crates/nu-command/src/commands/any.rs @@ -98,7 +98,9 @@ fn any(args: CommandArgs) -> Result { let result = args.input.fold(init, move |acc, row| { let condition = condition.clone(); let ctx = ctx.clone(); - ctx.scope.add_var("$it", row); + if let Some((arg, _)) = any_args.predicate.block.params.positional.first() { + ctx.scope.add_var(arg.name(), row); + } let condition = evaluate_baseline_expr(&condition, &ctx); diff --git a/crates/nu-command/src/commands/each/command.rs b/crates/nu-command/src/commands/each/command.rs index 1f56faf1f..cb2b5f4c0 100644 --- a/crates/nu-command/src/commands/each/command.rs +++ b/crates/nu-command/src/commands/each/command.rs @@ -88,17 +88,8 @@ pub fn process_row( context.scope.enter_scope(); context.scope.add_vars(&captured_block.captured.entries); - if !captured_block.block.params.positional.is_empty() { - if captured_block.block.params.positional.len() > 1 { - return Err(ShellError::labeled_error( - "Expected block with less than two parameters", - "too many parameters", - captured_block.block.span, - )); - } - context - .scope - .add_var(captured_block.block.params.positional[0].0.name(), input); + if let Some((arg, _)) = captured_block.block.params.positional.first() { + context.scope.add_var(arg.name(), input); } else { context.scope.add_var("$it", input); } diff --git a/crates/nu-command/src/commands/empty.rs b/crates/nu-command/src/commands/empty.rs index 2d13a8d8b..72e6d4704 100644 --- a/crates/nu-command/src/commands/empty.rs +++ b/crates/nu-command/src/commands/empty.rs @@ -140,7 +140,9 @@ fn process_row( context.scope.enter_scope(); context.scope.add_vars(&default_block.captured.entries); - context.scope.add_var("$it", input.clone()); + if let Some((arg, _)) = default_block.block.params.positional.first() { + context.scope.add_var(arg.name(), input.clone()); + } let stream = run_block( &default_block.block, diff --git a/crates/nu-command/src/commands/insert.rs b/crates/nu-command/src/commands/insert.rs index fdd39d11e..15fbc928a 100644 --- a/crates/nu-command/src/commands/insert.rs +++ b/crates/nu-command/src/commands/insert.rs @@ -80,7 +80,9 @@ fn process_row( context.scope.enter_scope(); context.scope.add_vars(&block.captured.entries); - context.scope.add_var("$it", input.clone()); + if let Some((arg, _)) = block.block.params.positional.first() { + context.scope.add_var(arg.name(), input.clone()); + } let result = run_block( &block.block, diff --git a/crates/nu-command/src/commands/keep/until.rs b/crates/nu-command/src/commands/keep/until.rs index a9b4caec0..811974f0f 100644 --- a/crates/nu-command/src/commands/keep/until.rs +++ b/crates/nu-command/src/commands/keep/until.rs @@ -1,10 +1,12 @@ use crate::prelude::*; -use log::trace; use nu_engine::evaluate_baseline_expr; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; use nu_parser::ParserScope; -use nu_protocol::{hir::ClassifiedCommand, Signature, SyntaxShape, UntaggedValue, Value}; +use nu_protocol::{ + hir::{CapturedBlock, ClassifiedCommand}, + Signature, SyntaxShape, +}; pub struct SubCommand; @@ -29,52 +31,38 @@ impl WholeStreamCommand for SubCommand { fn run(&self, args: CommandArgs) -> Result { let ctx = Arc::new(EvaluationContext::from_args(&args)); + let tag = args.call_info.name_tag.clone(); let call_info = args.evaluate_once()?; - let block = call_info.args.expect_nth(0)?.clone(); - - let (condition, captured) = match block { - Value { - value: UntaggedValue::Block(captured_block), - tag, - } => { - if captured_block.block.block.len() != 1 { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - match captured_block.block.block[0].pipelines.get(0) { - Some(item) => match item.list.get(0) { - Some(ClassifiedCommand::Expr(expr)) => { - (Arc::new(expr.clone()), captured_block.captured.clone()) - } - _ => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - }, - None => { + let block: CapturedBlock = call_info.req(0)?; + let condition = { + if block.block.block.len() != 1 { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); + } + match block.block.block[0].pipelines.get(0) { + Some(item) => match item.list.get(0) { + Some(ClassifiedCommand::Expr(expr)) => expr.clone(), + _ => { return Err(ShellError::labeled_error( "Expected a condition", "expected a condition", tag, )); } + }, + None => { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); } } - Value { tag, .. } => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } }; Ok(call_info @@ -83,13 +71,13 @@ impl WholeStreamCommand for SubCommand { let condition = condition.clone(); let ctx = ctx.clone(); ctx.scope.enter_scope(); - ctx.scope.add_vars(&captured.entries); - ctx.scope.add_var("$it", item.clone()); - trace!("ITEM = {:?}", item); + ctx.scope.add_vars(&block.captured.entries); + if let Some((arg, _)) = block.block.params.positional.first() { + ctx.scope.add_var(arg.name(), item.clone()); + } let result = evaluate_baseline_expr(&*condition, &*ctx); ctx.scope.exit_scope(); - trace!("RESULT = {:?}", result); !matches!(result, Ok(ref v) if v.is_true()) }) diff --git a/crates/nu-command/src/commands/keep/while_.rs b/crates/nu-command/src/commands/keep/while_.rs index 5c1150bcd..a60419ddf 100644 --- a/crates/nu-command/src/commands/keep/while_.rs +++ b/crates/nu-command/src/commands/keep/while_.rs @@ -3,7 +3,10 @@ use log::trace; use nu_engine::evaluate_baseline_expr; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; -use nu_protocol::{hir::ClassifiedCommand, Signature, SyntaxShape, UntaggedValue, Value}; +use nu_protocol::{ + hir::{CapturedBlock, ClassifiedCommand}, + Signature, SyntaxShape, +}; pub struct SubCommand; @@ -28,51 +31,37 @@ impl WholeStreamCommand for SubCommand { fn run(&self, args: CommandArgs) -> Result { let ctx = Arc::new(EvaluationContext::from_args(&args)); + let tag = args.call_info.name_tag.clone(); let call_info = args.evaluate_once()?; - let block = call_info.args.expect_nth(0)?.clone(); - - let (condition, captured) = match block { - Value { - value: UntaggedValue::Block(captured_block), - tag, - } => { - if captured_block.block.block.len() != 1 { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - match captured_block.block.block[0].pipelines.get(0) { - Some(item) => match item.list.get(0) { - Some(ClassifiedCommand::Expr(expr)) => { - (Arc::new(expr.clone()), captured_block.captured.clone()) - } - _ => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - }, - None => { + let block: CapturedBlock = call_info.req(0)?; + let condition = { + if block.block.block.len() != 1 { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); + } + match block.block.block[0].pipelines.get(0) { + Some(item) => match item.list.get(0) { + Some(ClassifiedCommand::Expr(expr)) => expr.clone(), + _ => { return Err(ShellError::labeled_error( "Expected a condition", "expected a condition", tag, )); } + }, + None => { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); } } - Value { tag, .. } => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } }; Ok(call_info @@ -82,8 +71,10 @@ impl WholeStreamCommand for SubCommand { let ctx = ctx.clone(); ctx.scope.enter_scope(); - ctx.scope.add_var("$it", item.clone()); - ctx.scope.add_vars(&captured.entries); + ctx.scope.add_vars(&block.captured.entries); + if let Some((arg, _)) = block.block.params.positional.first() { + ctx.scope.add_var(arg.name(), item.clone()); + } trace!("ITEM = {:?}", item); let result = evaluate_baseline_expr(&*condition, &*ctx); diff --git a/crates/nu-command/src/commands/reduce.rs b/crates/nu-command/src/commands/reduce.rs index f3a75fd6d..5eff6a30c 100644 --- a/crates/nu-command/src/commands/reduce.rs +++ b/crates/nu-command/src/commands/reduce.rs @@ -89,7 +89,13 @@ fn process_row( context.scope.enter_scope(); context.scope.add_vars(&block.captured.entries); - context.scope.add_var("$it", row); + + if let Some((arg, _)) = block.block.params.positional.first() { + context.scope.add_var(arg.name(), row); + } else { + context.scope.add_var("$it", row); + } + let result = run_block( &block.block, context, diff --git a/crates/nu-command/src/commands/skip/until.rs b/crates/nu-command/src/commands/skip/until.rs index b6a4d8e5b..9efdecc3c 100644 --- a/crates/nu-command/src/commands/skip/until.rs +++ b/crates/nu-command/src/commands/skip/until.rs @@ -3,7 +3,10 @@ use log::trace; use nu_engine::evaluate_baseline_expr; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; -use nu_protocol::{hir::ClassifiedCommand, Signature, SyntaxShape, UntaggedValue, Value}; +use nu_protocol::{ + hir::{CapturedBlock, ClassifiedCommand}, + Signature, SyntaxShape, +}; pub struct SubCommand; @@ -28,51 +31,37 @@ impl WholeStreamCommand for SubCommand { fn run_with_actions(&self, args: CommandArgs) -> Result { let ctx = Arc::new(EvaluationContext::from_args(&args)); + let tag = args.call_info.name_tag.clone(); let call_info = args.evaluate_once()?; - let block = call_info.args.expect_nth(0)?.clone(); - - let (condition, captured) = match block { - Value { - value: UntaggedValue::Block(captured_block), - tag, - } => { - if captured_block.block.block.len() != 1 { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - match captured_block.block.block[0].pipelines.get(0) { - Some(item) => match item.list.get(0) { - Some(ClassifiedCommand::Expr(expr)) => { - (Arc::new(expr.clone()), captured_block.captured.clone()) - } - _ => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - }, - None => { + let block: CapturedBlock = call_info.req(0)?; + let condition = { + if block.block.block.len() != 1 { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); + } + match block.block.block[0].pipelines.get(0) { + Some(item) => match item.list.get(0) { + Some(ClassifiedCommand::Expr(expr)) => expr.clone(), + _ => { return Err(ShellError::labeled_error( "Expected a condition", "expected a condition", tag, )); } + }, + None => { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); } } - Value { tag, .. } => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } }; Ok(call_info @@ -82,8 +71,10 @@ impl WholeStreamCommand for SubCommand { let ctx = ctx.clone(); ctx.scope.enter_scope(); - ctx.scope.add_var("$it", item.clone()); - ctx.scope.add_vars(&captured.entries); + ctx.scope.add_vars(&block.captured.entries); + if let Some((arg, _)) = block.block.params.positional.first() { + ctx.scope.add_var(arg.name(), item.clone()); + } trace!("ITEM = {:?}", item); let result = evaluate_baseline_expr(&*condition, &*ctx); diff --git a/crates/nu-command/src/commands/skip/while_.rs b/crates/nu-command/src/commands/skip/while_.rs index d53d0e007..22f41b28d 100644 --- a/crates/nu-command/src/commands/skip/while_.rs +++ b/crates/nu-command/src/commands/skip/while_.rs @@ -3,7 +3,10 @@ use log::trace; use nu_engine::evaluate_baseline_expr; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; -use nu_protocol::{hir::ClassifiedCommand, Signature, SyntaxShape, UntaggedValue, Value}; +use nu_protocol::{ + hir::{CapturedBlock, ClassifiedCommand}, + Signature, SyntaxShape, +}; pub struct SubCommand; @@ -28,51 +31,37 @@ impl WholeStreamCommand for SubCommand { fn run_with_actions(&self, args: CommandArgs) -> Result { let ctx = Arc::new(EvaluationContext::from_args(&args)); + let tag = args.call_info.name_tag.clone(); let call_info = args.evaluate_once()?; - let block = call_info.args.expect_nth(0)?.clone(); - - let (condition, captured) = match block { - Value { - value: UntaggedValue::Block(captured_block), - tag, - } => { - if captured_block.block.block.len() != 1 { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - match captured_block.block.block[0].pipelines.get(0) { - Some(item) => match item.list.get(0) { - Some(ClassifiedCommand::Expr(expr)) => { - (Arc::new(expr.clone()), captured_block.captured.clone()) - } - _ => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } - }, - None => { + let block: CapturedBlock = call_info.req(0)?; + let condition = { + if block.block.block.len() != 1 { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); + } + match block.block.block[0].pipelines.get(0) { + Some(item) => match item.list.get(0) { + Some(ClassifiedCommand::Expr(expr)) => expr.clone(), + _ => { return Err(ShellError::labeled_error( "Expected a condition", "expected a condition", tag, )); } + }, + None => { + return Err(ShellError::labeled_error( + "Expected a condition", + "expected a condition", + tag, + )); } } - Value { tag, .. } => { - return Err(ShellError::labeled_error( - "Expected a condition", - "expected a condition", - tag, - )); - } }; Ok(call_info @@ -83,8 +72,10 @@ impl WholeStreamCommand for SubCommand { let ctx = ctx.clone(); ctx.scope.enter_scope(); - ctx.scope.add_vars(&captured.entries); - ctx.scope.add_var("$it", item.clone()); + ctx.scope.add_vars(&block.captured.entries); + if let Some((arg, _)) = block.block.params.positional.first() { + ctx.scope.add_var(arg.name(), item.clone()); + } trace!("ITEM = {:?}", item); let result = evaluate_baseline_expr(&*condition, &*ctx); diff --git a/crates/nu-command/src/commands/update.rs b/crates/nu-command/src/commands/update.rs index bbf2a8a82..f08a1b752 100644 --- a/crates/nu-command/src/commands/update.rs +++ b/crates/nu-command/src/commands/update.rs @@ -84,7 +84,10 @@ fn process_row( let input_stream = vec![Ok(for_block)].into_iter().to_input_stream(); context.scope.enter_scope(); - context.scope.add_var("$it", input.clone()); + if let Some((arg, _)) = captured_block.block.params.positional.first() { + context.scope.add_var(arg.name(), input.clone()); + } + context.scope.add_vars(&captured_block.captured.entries); let result = run_block( diff --git a/crates/nu-command/src/commands/where_.rs b/crates/nu-command/src/commands/where_.rs index 698effd3c..4da1d12f8 100644 --- a/crates/nu-command/src/commands/where_.rs +++ b/crates/nu-command/src/commands/where_.rs @@ -130,7 +130,10 @@ impl Iterator for WhereIterator { while let Some(x) = self.input.next() { self.context.scope.enter_scope(); self.context.scope.add_vars(&self.block.captured.entries); - self.context.scope.add_var("$it", x.clone()); + + if let Some((arg, _)) = self.block.block.params.positional.first() { + self.context.scope.add_var(arg.name(), x.clone()); + } //FIXME: should we use the scope that's brought in as well? let condition = evaluate_baseline_expr(&self.condition, &self.context); diff --git a/crates/nu-command/src/examples.rs b/crates/nu-command/src/examples.rs index e59f5a815..ee7850460 100644 --- a/crates/nu-command/src/examples.rs +++ b/crates/nu-command/src/examples.rs @@ -51,8 +51,6 @@ pub fn test_examples(cmd: Command) -> Result<(), ShellError> { let block = parse_line(sample_pipeline.example, &ctx)?; - println!("{:#?}", block); - if let Some(expected) = &sample_pipeline.result { let result = evaluate_block(block, &mut ctx)?; diff --git a/crates/nu-parser/src/parse.rs b/crates/nu-parser/src/parse.rs index fad8ede65..b5c1c5779 100644 --- a/crates/nu-parser/src/parse.rs +++ b/crates/nu-parser/src/parse.rs @@ -1410,13 +1410,15 @@ fn parse_positional_argument( let mut commands = hir::Pipeline::new(span); commands.push(ClassifiedCommand::Expr(Box::new(arg))); - let block = hir::Block::new( + let mut block = hir::Block::new( Signature::new(""), vec![Group::new(vec![commands], lite_cmd.span())], IndexMap::new(), span, ); + block.infer_params(); + let arg = SpannedExpression::new(Expression::Block(Arc::new(block)), span); idx = new_idx - 1; diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index 744b27204..830a5c914 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/tests/shell/pipeline/commands/internal.rs @@ -379,17 +379,6 @@ fn proper_shadow_let_aliases() { assert_eq!(actual.out, "falsetruefalse"); } -#[test] -fn block_param_too_many() { - let actual = nu!( - cwd: ".", - r#" - [1, 2, 3] | each { |a, b| echo $a } - "# - ); - assert!(actual.err.contains("too many")); -} - #[test] fn block_params_override() { let actual = nu!( @@ -401,6 +390,17 @@ fn block_params_override() { assert!(actual.err.contains("unknown variable")); } +#[test] +fn block_params_override_correct() { + let actual = nu!( + cwd: ".", + r#" + [1, 2, 3] | each { |a| echo $a } | to json + "# + ); + assert_eq!(actual.out, "[1,2,3]"); +} + #[test] fn run_dynamic_blocks() { let actual = nu!(