From 55baee9a9a7b24207551c74bb9998927584f53a6 Mon Sep 17 00:00:00 2001 From: JT Date: Fri, 28 May 2021 19:48:54 +1200 Subject: [PATCH] Cleanup let varname and rhs (#3507) --- crates/nu-command/src/commands/for_in.rs | 30 ++----------- crates/nu-command/src/commands/let_.rs | 54 +++++++++++++++--------- crates/nu-protocol/src/hir.rs | 30 +++++++++++++ 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/crates/nu-command/src/commands/for_in.rs b/crates/nu-command/src/commands/for_in.rs index d302e0037f..eb6f1cdcf2 100644 --- a/crates/nu-command/src/commands/for_in.rs +++ b/crates/nu-command/src/commands/for_in.rs @@ -4,7 +4,7 @@ use nu_engine::{FromValue, WholeStreamCommand}; use nu_errors::ShellError; use nu_protocol::{ - hir::{CapturedBlock, ExternalRedirection, Literal}, + hir::{CapturedBlock, ExternalRedirection}, Signature, SyntaxShape, TaggedDictBuilder, UntaggedValue, Value, }; @@ -120,36 +120,12 @@ fn for_in(raw_args: CommandArgs) -> Result { .positional .expect("Internal error: type checker should require args"); - let mut var_name: String = match &positional[0].expr { - nu_protocol::hir::Expression::FullColumnPath(path) => match &path.head.expr { - nu_protocol::hir::Expression::Variable(v, _) => v, - x => { - return Err(ShellError::labeled_error( - format!("Expected a variable (got {:?})", x), - "expected a variable", - positional[0].span, - )) - } - }, - nu_protocol::hir::Expression::Literal(Literal::String(x)) => x, - x => { - return Err(ShellError::labeled_error( - format!("Expected a variable (got {:?})", x), - "expected a variable", - positional[0].span, - )) - } - } - .to_string(); - + let var_name = positional[0].var_name()?; let rhs = evaluate_baseline_expr(&positional[2], &context)?; + let block: CapturedBlock = FromValue::from_value(&evaluate_baseline_expr(&positional[3], &context)?)?; - if !var_name.starts_with('$') { - var_name = format!("${}", var_name); - } - let input = crate::commands::echo::expand_value_to_stream(rhs); let block = Arc::new(Box::new(block)); diff --git a/crates/nu-command/src/commands/let_.rs b/crates/nu-command/src/commands/let_.rs index 2ad6372505..059dbf951a 100644 --- a/crates/nu-command/src/commands/let_.rs +++ b/crates/nu-command/src/commands/let_.rs @@ -1,9 +1,11 @@ use crate::prelude::*; -use nu_engine::{evaluate_baseline_expr, WholeStreamCommand}; +use nu_engine::{evaluate_baseline_expr, FromValue, WholeStreamCommand}; use nu_errors::ShellError; -use nu_protocol::{hir::CapturedBlock, hir::ClassifiedCommand, Signature, SyntaxShape}; -use nu_source::Tagged; +use nu_protocol::{ + hir::{CapturedBlock, ClassifiedCommand}, + Signature, SyntaxShape, UntaggedValue, +}; pub struct Let; @@ -32,20 +34,41 @@ impl WholeStreamCommand for Let { } fn examples(&self) -> Vec { - vec![] + vec![ + Example { + description: "Assign a simple value to a variable", + example: "let x = 3", + result: Some(vec![]), + }, + Example { + description: "Assign the result of an expression to a variable", + example: "let result = (3 + 7); echo $result", + result: Some(vec![UntaggedValue::int(1).into()]), + }, + Example { + description: "Create a variable using the full name", + example: "let $three = 3", + result: Some(vec![]), + }, + ] } } pub fn letcmd(args: CommandArgs) -> Result { - let tag = args.call_info.name_tag.clone(); let ctx = EvaluationContext::from_args(&args); - let args = args.evaluate_once()?; + let positional = args + .call_info + .args + .positional + .expect("Internal error: type checker should require args"); - //let (LetArgs { name, rhs, .. }, _) = args.process()?; - let name: Tagged = args.req(0)?; - let rhs: CapturedBlock = args.req(2)?; + let var_name = positional[0].var_name()?; + let rhs_raw = evaluate_baseline_expr(&positional[2], &ctx)?; + let tag: Tag = positional[2].span.into(); - let (expr, captured) = { + let rhs: CapturedBlock = FromValue::from_value(&rhs_raw)?; + + let (expr, _) = { if rhs.block.block.len() != 1 { return Err(ShellError::labeled_error( "Expected a value", @@ -75,24 +98,15 @@ pub fn letcmd(args: CommandArgs) -> Result { }; ctx.scope.enter_scope(); - ctx.scope.add_vars(&captured.entries); - let value = evaluate_baseline_expr(&expr, &ctx); - ctx.scope.exit_scope(); let value = value?; - let name = if name.item.starts_with('$') { - name.item - } else { - format!("${}", name.item) - }; - // Note: this is a special case for setting the context from a command // In this case, if we don't set it now, we'll lose the scope that this // variable should be set into. - ctx.scope.add_var(name, value); + ctx.scope.add_var(var_name, value); Ok(ActionStream::empty()) } diff --git a/crates/nu-protocol/src/hir.rs b/crates/nu-protocol/src/hir.rs index e8f7a525d8..cea520765e 100644 --- a/crates/nu-protocol/src/hir.rs +++ b/crates/nu-protocol/src/hir.rs @@ -681,6 +681,36 @@ impl SpannedExpression { pub fn get_free_variables(&self, known_variables: &mut Vec) -> Vec { self.expr.get_free_variables(known_variables) } + + pub fn var_name(&self) -> Result { + let var_name = match &self.expr { + Expression::FullColumnPath(path) => match &path.head.expr { + Expression::Variable(v, _) => v, + x => { + return Err(ShellError::labeled_error( + format!("Expected a variable (got {:?})", x), + "expected a variable", + self.span, + )) + } + }, + Expression::Literal(Literal::String(x)) => x, + x => { + return Err(ShellError::labeled_error( + format!("Expected a variable (got {:?})", x), + "expected a variable", + self.span, + )) + } + } + .to_string(); + + if !var_name.starts_with('$') { + Ok(format!("${}", var_name)) + } else { + Ok(var_name) + } + } } impl std::ops::Deref for SpannedExpression {