Variables should error on use rather than value span (#881)

crates/nu-engine/src/eval.rs

@@ -301,10 +301,7 @@ pub fn eval_expression(
             let block = engine_state.get_block(*block_id);
 
             for var_id in &block.captures {
-                captures.insert(
+                captures.insert(*var_id, stack.get_var(*var_id, expr.span)?);
-                    *var_id,
-                    stack.get_var(*var_id)?, //.map_err(|_| ShellError::VariableNotFoundAtRuntime(expr.span))?,
-                );
             }
             Ok(Value::Block {
                 val: *block_id,
@@ -550,7 +547,8 @@ pub fn eval_variable(
     var_id: VarId,
     span: Span,
 ) -> Result<Value, ShellError> {
-    if var_id == nu_protocol::NU_VARIABLE_ID {
+    match var_id {
+        nu_protocol::NU_VARIABLE_ID => {
             // $nu
             let mut output_cols = vec![];
             let mut output_vals = vec![];
@@ -628,7 +626,8 @@ pub fn eval_variable(
                 vals: output_vals,
                 span,
             })
-    } else if var_id == nu_protocol::SCOPE_VARIABLE_ID {
+        }
+        nu_protocol::SCOPE_VARIABLE_ID => {
             let mut output_cols = vec![];
             let mut output_vals = vec![];
 
@@ -644,7 +643,7 @@ pub fn eval_variable(
 
                     let var_type = Value::string(engine_state.get_var(*var.1).to_string(), span);
 
-                let var_value = if let Ok(val) = stack.get_var(*var.1) {
+                    let var_value = if let Ok(val) = stack.get_var(*var.1, span) {
                         val
                     } else {
                         Value::nothing(span)
@@ -887,9 +886,10 @@ pub fn eval_variable(
                     // The names of the commands should be a value string
                     match (rec_a.get(0), rec_b.get(0)) {
                         (Some(val_a), Some(val_b)) => match (val_a, val_b) {
-                        (Value::String { val: str_a, .. }, Value::String { val: str_b, .. }) => {
+                            (
-                            str_a.cmp(str_b)
+                                Value::String { val: str_a, .. },
-                        }
+                                Value::String { val: str_b, .. },
+                            ) => str_a.cmp(str_b),
                             _ => Ordering::Equal,
                         },
                         _ => Ordering::Equal,
@@ -929,7 +929,8 @@ pub fn eval_variable(
                 vals: output_vals,
                 span,
             })
-    } else if var_id == ENV_VARIABLE_ID {
+        }
+        ENV_VARIABLE_ID => {
             let env_vars = stack.get_env_vars(engine_state);
             let env_columns = env_vars.keys();
             let env_values = env_vars.values();
@@ -948,10 +949,8 @@ pub fn eval_variable(
                 vals: env_values,
                 span,
             })
-    } else {
+        }
-        stack
+        var_id => stack.get_var(var_id, span),
-            .get_var(var_id)
-            .map_err(move |_| ShellError::VariableNotFoundAtRuntime(span))
     }
 }
 

crates/nu-protocol/src/engine/stack.rs

@@ -1,7 +1,7 @@
 use std::collections::{HashMap, HashSet};
 
 use crate::engine::EngineState;
-use crate::{Config, ShellError, Value, VarId, CONFIG_VARIABLE_ID};
+use crate::{Config, ShellError, Span, Value, VarId, CONFIG_VARIABLE_ID};
 
 /// A runtime value stack used during evaluation
 ///
@@ -57,15 +57,12 @@ impl Stack {
         }
     }
 
-    pub fn get_var(&self, var_id: VarId) -> Result<Value, ShellError> {
+    pub fn get_var(&self, var_id: VarId, span: Span) -> Result<Value, ShellError> {
         if let Some(v) = self.vars.get(&var_id) {
-            return Ok(v.clone());
+            return Ok(v.clone().with_span(span));
         }
 
-        Err(ShellError::NushellFailed(format!(
+        Err(ShellError::VariableNotFoundAtRuntime(span))
-            "variable (var_id: {}) not found",
-            var_id
-        )))
     }
 
     pub fn add_var(&mut self, var_id: VarId, value: Value) {
@@ -93,7 +90,7 @@ impl Stack {
         output.env_vars.push(HashMap::new());
 
         let config = self
-            .get_var(CONFIG_VARIABLE_ID)
+            .get_var(CONFIG_VARIABLE_ID, Span::new(0, 0))
             .expect("internal error: config is missing");
         output.vars.insert(CONFIG_VARIABLE_ID, config);
 
@@ -103,10 +100,12 @@ impl Stack {
     pub fn gather_captures(&self, captures: &[VarId]) -> Stack {
         let mut output = Stack::new();
 
+        let fake_span = Span::new(0, 0);
+
         for capture in captures {
             // Note: this assumes we have calculated captures correctly and that commands
             // that take in a var decl will manually set this into scope when running the blocks
-            if let Ok(value) = self.get_var(*capture) {
+            if let Ok(value) = self.get_var(*capture, fake_span) {
                 output.vars.insert(*capture, value);
             }
         }
@@ -116,7 +115,7 @@ impl Stack {
         output.env_vars.push(HashMap::new());
 
         let config = self
-            .get_var(CONFIG_VARIABLE_ID)
+            .get_var(CONFIG_VARIABLE_ID, fake_span)
             .expect("internal error: config is missing");
         output.vars.insert(CONFIG_VARIABLE_ID, config);
 
@@ -175,7 +174,7 @@ impl Stack {
     }
 
     pub fn get_config(&self) -> Result<Config, ShellError> {
-        let config = self.get_var(CONFIG_VARIABLE_ID);
+        let config = self.get_var(CONFIG_VARIABLE_ID, Span::new(0, 0));
 
         match config {
             Ok(config) => config.into_config(),