From ad11e25fc53f07abd337551f84fd005574324058 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Wed, 12 Jul 2023 06:36:34 +1200 Subject: [PATCH] allow mut to take pipelines (#9658) # Description This extends the syntax fix for `let` (#9589) to `mut` as well. Example: `mut x = "hello world" | str length; print $x` closes #9634 # User-Facing Changes `mut` now joins `let` in being able to be assigned from a pipeline # Tests + Formatting # After Submitting --- crates/nu-cmd-lang/src/core_commands/mut_.rs | 18 +- crates/nu-command/tests/commands/let_.rs | 24 +++ crates/nu-parser/src/parse_keywords.rs | 178 ++++++++++--------- crates/nu-parser/src/parser.rs | 152 ++++++++-------- 4 files changed, 200 insertions(+), 172 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/mut_.rs b/crates/nu-cmd-lang/src/core_commands/mut_.rs index 45add079cbba..3a93d85bb737 100644 --- a/crates/nu-cmd-lang/src/core_commands/mut_.rs +++ b/crates/nu-cmd-lang/src/core_commands/mut_.rs @@ -1,4 +1,4 @@ -use nu_engine::eval_expression_with_input; +use nu_engine::eval_block; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type}; @@ -54,25 +54,25 @@ impl Command for Mut { .as_var() .expect("internal error: missing variable"); - let keyword_expr = call + let block_id = call .positional_nth(1) .expect("checked through parser") - .as_keyword() - .expect("internal error: missing keyword"); + .as_block() + .expect("internal error: missing right hand side"); - let rhs = eval_expression_with_input( + let block = engine_state.get_block(block_id); + let pipeline_data = eval_block( engine_state, stack, - keyword_expr, + block, input, call.redirect_stdout, call.redirect_stderr, - )? - .0; + )?; //println!("Adding: {:?} to {}", rhs, var_id); - stack.add_var(var_id, rhs.into_value(call.head)); + stack.add_var(var_id, pipeline_data.into_value(call.head)); Ok(PipelineData::empty()) } diff --git a/crates/nu-command/tests/commands/let_.rs b/crates/nu-command/tests/commands/let_.rs index 9671dd171aca..363f1bfdf472 100644 --- a/crates/nu-command/tests/commands/let_.rs +++ b/crates/nu-command/tests/commands/let_.rs @@ -50,6 +50,30 @@ fn let_pipeline_allows_in() { assert_eq!(actual.out, "21"); } +#[test] +fn mut_takes_pipeline() { + let actual = nu!( + cwd: ".", pipeline( + r#" + mut x = "hello world" | str length; print $x + "# + )); + + assert_eq!(actual.out, "11"); +} + +#[test] +fn mut_pipeline_allows_in() { + let actual = nu!( + cwd: ".", pipeline( + r#" + def foo [] { mut x = $in | str length; print ($x + 10) }; "hello world" | foo + "# + )); + + assert_eq!(actual.out, "21"); +} + #[ignore] #[test] fn let_with_external_failed() { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 8b3728ad4e6e..afa720f0ee9d 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3034,109 +3034,115 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin } pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline { - let name = working_set.get_span_contents(spans[0]); + trace!("parsing: mut"); - if name == b"mut" { - // if let Some(span) = check_name(working_set, spans) { - // return Pipeline::from_vec(vec![garbage(*span)]); - // } - - if let Some(decl_id) = working_set.find_decl(b"mut", &Type::Nothing) { - let cmd = working_set.get_decl(decl_id); - let call_signature = cmd.signature().call_signature(); - - if spans.len() >= 4 { - // This is a bit of by-hand parsing to get around the issue where we want to parse in the reverse order - // so that the var-id created by the variable isn't visible in the expression that init it - for span in spans.iter().enumerate() { - let item = working_set.get_span_contents(*span.1); - if item == b"=" && spans.len() > (span.0 + 1) { - let mut idx = span.0; - let rvalue = parse_multispan_value( - working_set, - spans, - &mut idx, - &SyntaxShape::Keyword( - b"=".to_vec(), - Box::new(SyntaxShape::MathExpression), - ), - ); + // JT: Disabling check_name because it doesn't work with optional types in the declaration + // if let Some(span) = check_name(working_set, spans) { + // return Pipeline::from_vec(vec![garbage(*span)]); + // } - if idx < (spans.len() - 1) { - working_set - .error(ParseError::ExtraPositional(call_signature, spans[idx + 1])); - } + if let Some(decl_id) = working_set.find_decl(b"mut", &Type::Nothing) { + if spans.len() >= 4 { + // This is a bit of by-hand parsing to get around the issue where we want to parse in the reverse order + // so that the var-id created by the variable isn't visible in the expression that init it + for span in spans.iter().enumerate() { + let item = working_set.get_span_contents(*span.1); + if item == b"=" && spans.len() > (span.0 + 1) { + let (tokens, parse_error) = lex( + working_set.get_span_contents(nu_protocol::span(&spans[(span.0 + 1)..])), + spans[span.0 + 1].start, + &[], + &[], + true, + ); - let mut idx = 0; - let (lvalue, explicit_type) = parse_var_with_opt_type( - working_set, - &spans[1..(span.0)], - &mut idx, - true, - ); + if let Some(parse_error) = parse_error { + working_set.parse_errors.push(parse_error) + } - let var_name = - String::from_utf8_lossy(working_set.get_span_contents(lvalue.span)) - .trim_start_matches('$') - .to_string(); + let rvalue_span = nu_protocol::span(&spans[(span.0 + 1)..]); + let rvalue_block = parse_block(working_set, &tokens, rvalue_span, false, true); - if ["in", "nu", "env", "nothing"].contains(&var_name.as_str()) { - working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span)) - } + let output_type = rvalue_block.output_type(); - let var_id = lvalue.as_var(); - let rhs_type = rvalue.ty.clone(); + let block_id = working_set.add_block(rvalue_block); - if let Some(explicit_type) = &explicit_type { - if &rhs_type != explicit_type && explicit_type != &Type::Any { - working_set.error(ParseError::TypeMismatch( - explicit_type.clone(), - rhs_type.clone(), - rvalue.span, - )); - } + let rvalue = Expression { + expr: Expr::Block(block_id), + span: rvalue_span, + ty: output_type, + custom_completion: None, + }; + + let mut idx = 0; + + let (lvalue, explicit_type) = + parse_var_with_opt_type(working_set, &spans[1..(span.0)], &mut idx, true); + + let var_name = + String::from_utf8_lossy(working_set.get_span_contents(lvalue.span)) + .trim_start_matches('$') + .to_string(); + + if ["in", "nu", "env", "nothing"].contains(&var_name.as_str()) { + working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span)) + } + + let var_id = lvalue.as_var(); + let rhs_type = rvalue.ty.clone(); + + if let Some(explicit_type) = &explicit_type { + if !type_compatible(explicit_type, &rhs_type) { + working_set.error(ParseError::TypeMismatch( + explicit_type.clone(), + rhs_type.clone(), + nu_protocol::span(&spans[(span.0 + 1)..]), + )); } + } - if let Some(var_id) = var_id { - if explicit_type.is_none() { - working_set.set_variable_type(var_id, rhs_type); - } + if let Some(var_id) = var_id { + if explicit_type.is_none() { + working_set.set_variable_type(var_id, rhs_type); } + } - let call = Box::new(Call { - decl_id, - head: spans[0], - arguments: vec![ - Argument::Positional(lvalue), - Argument::Positional(rvalue), - ], - redirect_stdout: true, - redirect_stderr: false, - parser_info: HashMap::new(), - }); + let call = Box::new(Call { + decl_id, + head: spans[0], + arguments: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + redirect_stdout: true, + redirect_stderr: false, + parser_info: HashMap::new(), + }); - return Pipeline::from_vec(vec![Expression { - expr: Expr::Call(call), - span: nu_protocol::span(spans), - ty: Type::Any, - custom_completion: None, - }]); - } + return Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: nu_protocol::span(spans), + ty: Type::Any, + custom_completion: None, + }]); } } - let ParsedInternalCall { call, output } = - parse_internal_call(working_set, spans[0], &spans[1..], decl_id); - - return Pipeline::from_vec(vec![Expression { - expr: Expr::Call(call), - span: nu_protocol::span(spans), - ty: output, - custom_completion: None, - }]); } + let ParsedInternalCall { call, output } = + parse_internal_call(working_set, spans[0], &spans[1..], decl_id); + + return Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: nu_protocol::span(spans), + ty: output, + custom_completion: None, + }]); + } else { + working_set.error(ParseError::UnknownState( + "internal error: let or const statements not found in core language".into(), + span(spans), + )) } + working_set.error(ParseError::UnknownState( - "internal error: mut statement unparsable".into(), + "internal error: let or const statement unparsable".into(), span(spans), )); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 0c6e31989f5f..faa2dd69179f 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5308,10 +5308,12 @@ pub fn parse_pipeline( pipeline_index: usize, ) -> Pipeline { if pipeline.commands.len() > 1 { - // Special case: allow `let` to consume the whole pipeline, eg) `let abc = "foo" | str length` + // Special case: allow `let` and `mut` to consume the whole pipeline, eg) `let abc = "foo" | str length` match &pipeline.commands[0] { LiteElement::Command(_, command) if !command.parts.is_empty() => { - if working_set.get_span_contents(command.parts[0]) == b"let" { + if working_set.get_span_contents(command.parts[0]) == b"let" + || working_set.get_span_contents(command.parts[0]) == b"mut" + { let mut new_command = LiteCommand { comments: vec![], parts: command.parts.clone(), @@ -5340,59 +5342,54 @@ pub fn parse_pipeline( parse_builtin_commands(working_set, &new_command, is_subexpression); if pipeline_index == 0 { - if let Some(let_decl_id) = working_set.find_decl(b"let", &Type::Nothing) - { - for element in pipeline.elements.iter_mut() { - if let PipelineElement::Expression( - _, - Expression { - expr: Expr::Call(call), - .. - }, - ) = element + let let_decl_id = working_set.find_decl(b"let", &Type::Nothing); + let mut_decl_id = working_set.find_decl(b"mut", &Type::Nothing); + for element in pipeline.elements.iter_mut() { + if let PipelineElement::Expression( + _, + Expression { + expr: Expr::Call(call), + .. + }, + ) = element + { + if Some(call.decl_id) == let_decl_id + || Some(call.decl_id) == mut_decl_id { - if call.decl_id == let_decl_id { - // Do an expansion - if let Some(Expression { - expr: Expr::Block(block_id), - .. - }) = call.positional_iter_mut().nth(1) - { - let block = working_set.get_block(*block_id); - - let element = - block.pipelines[0].elements[0].clone(); - - if let PipelineElement::Expression(prepend, expr) = - element - { - if expr.has_in_variable(working_set) { - let new_expr = PipelineElement::Expression( - prepend, - wrap_expr_with_collect( - working_set, - &expr, - ), - ); + // Do an expansion + if let Some(Expression { + expr: Expr::Block(block_id), + .. + }) = call.positional_iter_mut().nth(1) + { + let block = working_set.get_block(*block_id); - let block = - working_set.get_block_mut(*block_id); - block.pipelines[0].elements[0] = new_expr; - } + let element = block.pipelines[0].elements[0].clone(); + + if let PipelineElement::Expression(prepend, expr) = + element + { + if expr.has_in_variable(working_set) { + let new_expr = PipelineElement::Expression( + prepend, + wrap_expr_with_collect(working_set, &expr), + ); + + let block = + working_set.get_block_mut(*block_id); + block.pipelines[0].elements[0] = new_expr; } } - continue; - } else if element.has_in_variable(working_set) - && !is_subexpression - { - *element = - wrap_element_with_collect(working_set, element); } + continue; } else if element.has_in_variable(working_set) && !is_subexpression { *element = wrap_element_with_collect(working_set, element); } + } else if element.has_in_variable(working_set) && !is_subexpression + { + *element = wrap_element_with_collect(working_set, element); } } } @@ -5482,49 +5479,50 @@ pub fn parse_pipeline( } => { let mut pipeline = parse_builtin_commands(working_set, command, is_subexpression); + let let_decl_id = working_set.find_decl(b"let", &Type::Nothing); + let mut_decl_id = working_set.find_decl(b"mut", &Type::Nothing); + if pipeline_index == 0 { - if let Some(let_decl_id) = working_set.find_decl(b"let", &Type::Nothing) { - for element in pipeline.elements.iter_mut() { - if let PipelineElement::Expression( - _, - Expression { - expr: Expr::Call(call), - .. - }, - ) = element + for element in pipeline.elements.iter_mut() { + if let PipelineElement::Expression( + _, + Expression { + expr: Expr::Call(call), + .. + }, + ) = element + { + if Some(call.decl_id) == let_decl_id + || Some(call.decl_id) == mut_decl_id { - if call.decl_id == let_decl_id { - // Do an expansion - if let Some(Expression { - expr: Expr::Block(block_id), - .. - }) = call.positional_iter_mut().nth(1) - { - let block = working_set.get_block(*block_id); + // Do an expansion + if let Some(Expression { + expr: Expr::Block(block_id), + .. + }) = call.positional_iter_mut().nth(1) + { + let block = working_set.get_block(*block_id); - let element = block.pipelines[0].elements[0].clone(); + let element = block.pipelines[0].elements[0].clone(); - if let PipelineElement::Expression(prepend, expr) = element - { - if expr.has_in_variable(working_set) { - let new_expr = PipelineElement::Expression( - prepend, - wrap_expr_with_collect(working_set, &expr), - ); + if let PipelineElement::Expression(prepend, expr) = element { + if expr.has_in_variable(working_set) { + let new_expr = PipelineElement::Expression( + prepend, + wrap_expr_with_collect(working_set, &expr), + ); - let block = working_set.get_block_mut(*block_id); - block.pipelines[0].elements[0] = new_expr; - } + let block = working_set.get_block_mut(*block_id); + block.pipelines[0].elements[0] = new_expr; } } - continue; - } else if element.has_in_variable(working_set) && !is_subexpression - { - *element = wrap_element_with_collect(working_set, element); } + continue; } else if element.has_in_variable(working_set) && !is_subexpression { *element = wrap_element_with_collect(working_set, element); } + } else if element.has_in_variable(working_set) && !is_subexpression { + *element = wrap_element_with_collect(working_set, element); } } }