Cleanup let varname and rhs (#3507)

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<OutputStream, ShellError> {
         .positional
         .expect("Internal error: type checker should require args");
 
-    let mut var_name: String = match &positional[0].expr {
+    let var_name = positional[0].var_name()?;
-        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 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));
 

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_protocol::{
-use nu_source::Tagged;
+    hir::{CapturedBlock, ClassifiedCommand},
+    Signature, SyntaxShape, UntaggedValue,
+};
 
 pub struct Let;
 
@@ -32,20 +34,41 @@ impl WholeStreamCommand for Let {
     }
 
     fn examples(&self) -> Vec<Example> {
-        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<ActionStream, ShellError> {
-    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 var_name = positional[0].var_name()?;
-    let name: Tagged<String> = args.req(0)?;
+    let rhs_raw = evaluate_baseline_expr(&positional[2], &ctx)?;
-    let rhs: CapturedBlock = args.req(2)?;
+    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<ActionStream, ShellError> {
     };
 
     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())
 }

crates/nu-protocol/src/hir.rs

@@ -681,6 +681,36 @@ impl SpannedExpression {
     pub fn get_free_variables(&self, known_variables: &mut Vec<String>) -> Vec<String> {
         self.expr.get_free_variables(known_variables)
     }
+
+    pub fn var_name(&self) -> Result<String, ShellError> {
+        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 {