Revert "Span ID Refactor (Step 2): Use SpanId of expressions in some places (…"

This reverts commit e52d7bc585.

crates/nu-cmd-extra/src/extra/strings/format/command.rs

@@ -1,4 +1,5 @@
-use nu_engine::command_prelude::*;
+use nu_engine::{command_prelude::*, get_eval_expression};
+use nu_parser::parse_expression;
 use nu_protocol::{ast::PathMember, engine::StateWorkingSet, ListStream};
 
 #[derive(Clone)]
@@ -56,7 +57,14 @@ impl Command for FormatPattern {
                     string_span.start + 1,
                 )?;
 
-                format(input_val, &ops, engine_state, call.head)
+                format(
+                    input_val,
+                    &ops,
+                    engine_state,
+                    &mut working_set,
+                    stack,
+                    call.head,
+                )
             }
         }
     }
@@ -92,6 +100,8 @@ enum FormatOperation {
     FixedText(String),
     // raw input is something like {column1.column2}
     ValueFromColumn(String, Span),
+    // raw input is something like {$it.column1.column2} or {$var}.
+    ValueNeedEval(String, Span),
 }
 
 /// Given a pattern that is fed into the Format command, we can process it and subdivide it
@@ -100,6 +110,7 @@ enum FormatOperation {
 /// there without any further processing.
 /// FormatOperation::ValueFromColumn contains the name of a column whose values will be
 /// formatted according to the input pattern.
+/// FormatOperation::ValueNeedEval contains expression which need to eval, it has the following form:
 /// "$it.column1.column2" or "$variable"
 fn extract_formatting_operations(
     input: String,
@@ -150,17 +161,10 @@ fn extract_formatting_operations(
 
         if !column_name.is_empty() {
             if column_need_eval {
-                return Err(ShellError::GenericError {
+                output.push(FormatOperation::ValueNeedEval(
-                    error: "Removed functionality".into(),
+                    column_name.clone(),
-                    msg: "The ability to use variables ($it) in `format pattern` has been removed."
+                    Span::new(span_start + column_span_start, span_start + column_span_end),
-                        .into(),
+                ));
-                    span: Some(error_span),
-                    help: Some(
-                        "You can use other formatting options, such as string interpolation."
-                            .into(),
-                    ),
-                    inner: vec![],
-                });
             } else {
                 output.push(FormatOperation::ValueFromColumn(
                     column_name.clone(),
@@ -181,6 +185,8 @@ fn format(
     input_data: Value,
     format_operations: &[FormatOperation],
     engine_state: &EngineState,
+    working_set: &mut StateWorkingSet,
+    stack: &mut Stack,
     head_span: Span,
 ) -> Result<PipelineData, ShellError> {
     let data_as_value = input_data;
@@ -188,7 +194,13 @@ fn format(
     //  We can only handle a Record or a List of Records
     match data_as_value {
         Value::Record { .. } => {
-            match format_record(format_operations, &data_as_value, engine_state) {
+            match format_record(
+                format_operations,
+                &data_as_value,
+                engine_state,
+                working_set,
+                stack,
+            ) {
                 Ok(value) => Ok(PipelineData::Value(Value::string(value, head_span), None)),
                 Err(value) => Err(value),
             }
@@ -199,7 +211,13 @@ fn format(
             for val in vals.iter() {
                 match val {
                     Value::Record { .. } => {
-                        match format_record(format_operations, val, engine_state) {
+                        match format_record(
+                            format_operations,
+                            val,
+                            engine_state,
+                            working_set,
+                            stack,
+                        ) {
                             Ok(value) => {
                                 list.push(Value::string(value, head_span));
                             }
@@ -238,9 +256,12 @@ fn format_record(
     format_operations: &[FormatOperation],
     data_as_value: &Value,
     engine_state: &EngineState,
+    working_set: &mut StateWorkingSet,
+    stack: &mut Stack,
 ) -> Result<String, ShellError> {
     let config = engine_state.get_config();
     let mut output = String::new();
+    let eval_expression = get_eval_expression(engine_state);
 
     for op in format_operations {
         match op {
@@ -262,6 +283,23 @@ fn format_record(
                     Err(se) => return Err(se),
                 }
             }
+            FormatOperation::ValueNeedEval(_col_name, span) => {
+                let exp = parse_expression(working_set, &[*span]);
+                match working_set.parse_errors.first() {
+                    None => {
+                        let parsed_result = eval_expression(engine_state, stack, &exp);
+                        if let Ok(val) = parsed_result {
+                            output.push_str(&val.to_abbreviated_string(config))
+                        }
+                    }
+                    Some(err) => {
+                        return Err(ShellError::TypeMismatch {
+                            err_message: format!("expression is invalid, detail message: {err:?}"),
+                            span: *span,
+                        })
+                    }
+                }
+            }
         }
     }
     Ok(output)

crates/nu-command/tests/commands/format.rs

@@ -37,7 +37,7 @@ fn given_fields_can_be_column_paths() {
 }
 
 #[test]
-fn cant_use_variables() {
+fn can_use_variables() {
     let actual = nu!(
         cwd: "tests/fixtures/formats", pipeline(
         r#"
@@ -46,8 +46,7 @@ fn cant_use_variables() {
         "#
     ));
 
-    // TODO SPAN: This has been removed during SpanId refactor
+    assert_eq!(actual.out, "nu is a new type of shell");
-    assert!(actual.err.contains("Removed functionality"));
 }
 
 #[test]
@@ -56,7 +55,7 @@ fn error_unmatched_brace() {
         cwd: "tests/fixtures/formats", pipeline(
         r#"
         open cargo_sample.toml
-            | format pattern "{package.name"
+            | format pattern "{$it.package.name"
         "#
     ));
 

crates/nu-engine/src/eval.rs

@@ -208,13 +208,11 @@ fn eval_external(
 ) -> Result<PipelineData, ShellError> {
     let decl_id = engine_state
         .find_decl("run-external".as_bytes(), &[])
-        .ok_or(ShellError::ExternalNotSupported {
+        .ok_or(ShellError::ExternalNotSupported { span: head.span })?;
-            span: head.span(&engine_state),
-        })?;
 
     let command = engine_state.get_decl(decl_id);
 
-    let mut call = Call::new(head.span(&engine_state));
+    let mut call = Call::new(head.span);
 
     call.add_positional(head.clone());
 
@@ -714,7 +712,7 @@ impl Eval for EvalRuntime {
         args: &[ExternalArgument],
         _: Span,
     ) -> Result<Value, ShellError> {
-        let span = head.span(&engine_state);
+        let span = head.span;
         // FIXME: protect this collect with ctrl-c
         eval_external(engine_state, stack, head, args, PipelineData::empty())?.into_value(span)
     }
@@ -781,11 +779,9 @@ impl Eval for EvalRuntime {
                 let var_info = engine_state.get_var(*var_id);
                 if var_info.mutable {
                     stack.add_var(*var_id, rhs);
-                    Ok(Value::nothing(lhs.span(&engine_state)))
+                    Ok(Value::nothing(lhs.span))
                 } else {
-                    Err(ShellError::AssignmentRequiresMutableVar {
+                    Err(ShellError::AssignmentRequiresMutableVar { lhs_span: lhs.span })
-                        lhs_span: lhs.span(&engine_state),
-                    })
                 }
             }
             Expr::FullCellPath(cell_path) => {
@@ -801,7 +797,7 @@ impl Eval for EvalRuntime {
                                 // Reject attempts to assign to the entire $env
                                 if cell_path.tail.is_empty() {
                                     return Err(ShellError::CannotReplaceEnv {
-                                        span: cell_path.head.span(&engine_state),
+                                        span: cell_path.head.span,
                                     });
                                 }
 
@@ -841,21 +837,15 @@ impl Eval for EvalRuntime {
                                 lhs.upsert_data_at_cell_path(&cell_path.tail, rhs)?;
                                 stack.add_var(*var_id, lhs);
                             }
-                            Ok(Value::nothing(cell_path.head.span(&engine_state)))
+                            Ok(Value::nothing(cell_path.head.span))
                         } else {
-                            Err(ShellError::AssignmentRequiresMutableVar {
+                            Err(ShellError::AssignmentRequiresMutableVar { lhs_span: lhs.span })
-                                lhs_span: lhs.span(&engine_state),
-                            })
                         }
                     }
-                    _ => Err(ShellError::AssignmentRequiresVar {
+                    _ => Err(ShellError::AssignmentRequiresVar { lhs_span: lhs.span }),
-                        lhs_span: lhs.span(&engine_state),
-                    }),
                 }
             }
-            _ => Err(ShellError::AssignmentRequiresVar {
+            _ => Err(ShellError::AssignmentRequiresVar { lhs_span: lhs.span }),
-                lhs_span: lhs.span(&engine_state),
-            }),
         }
     }
 
@@ -892,8 +882,8 @@ impl Eval for EvalRuntime {
         Ok(Value::string(name, span))
     }
 
-    fn unreachable(engine_state: &EngineState, expr: &Expression) -> Result<Value, ShellError> {
+    fn unreachable(expr: &Expression) -> Result<Value, ShellError> {
-        Ok(Value::nothing(expr.span(&engine_state)))
+        Ok(Value::nothing(expr.span))
     }
 }
 

crates/nu-protocol/src/ast/expression.rs

@@ -1,7 +1,7 @@
 use crate::{
     ast::{Argument, Block, Expr, ExternalArgument, ImportPattern, MatchPattern, RecordItem},
-    engine::StateWorkingSet,
+    engine::{EngineState, StateWorkingSet},
-    BlockId, DeclId, GetSpan, Signature, Span, SpanId, Type, VarId, IN_VARIABLE_ID,
+    BlockId, DeclId, Signature, Span, SpanId, Type, VarId, IN_VARIABLE_ID,
 };
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
@@ -516,7 +516,7 @@ impl Expression {
         }
     }
 
-    pub fn span(&self, state: &impl GetSpan) -> Span {
+    pub fn span(&self, engine_state: &EngineState) -> Span {
-        state.get_span(self.span_id)
+        engine_state.get_span(self.span_id)
     }
 }

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

@@ -7,7 +7,7 @@ use crate::{
         Variable, Visibility, DEFAULT_OVERLAY_NAME,
     },
     eval_const::create_nu_constant,
-    BlockId, Category, Config, DeclId, FileId, GetSpan, HistoryConfig, Module, ModuleId, OverlayId,
+    BlockId, Category, Config, DeclId, FileId, HistoryConfig, Module, ModuleId, OverlayId,
     ShellError, Signature, Span, SpanId, Type, Value, VarId, VirtualPathId,
 };
 use fancy_regex::Regex;
@@ -1035,20 +1035,18 @@ impl EngineState {
         SpanId(self.num_spans() - 1)
     }
 
-    /// Find ID of a span (should be avoided if possible)
-    pub fn find_span_id(&self, span: Span) -> Option<SpanId> {
-        self.spans.iter().position(|sp| sp == &span).map(SpanId)
-    }
-}
-
-impl<'a> GetSpan for &'a EngineState {
     /// Get existing span
-    fn get_span(&self, span_id: SpanId) -> Span {
+    pub fn get_span(&self, span_id: SpanId) -> Span {
         *self
             .spans
             .get(span_id.0)
             .expect("internal error: missing span")
     }
+
+    /// Find ID of a span (should be avoided if possible)
+    pub fn find_span_id(&self, span: Span) -> Option<SpanId> {
+        self.spans.iter().position(|sp| sp == &span).map(SpanId)
+    }
 }
 
 impl Default for EngineState {

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

@@ -4,8 +4,8 @@ use crate::{
         usage::build_usage, CachedFile, Command, CommandType, EngineState, OverlayFrame,
         StateDelta, Variable, VirtualPath, Visibility,
     },
-    BlockId, Category, Config, DeclId, FileId, GetSpan, Module, ModuleId, ParseError, ParseWarning,
+    BlockId, Category, Config, DeclId, FileId, Module, ModuleId, ParseError, ParseWarning, Span,
-    Span, SpanId, Type, Value, VarId, VirtualPathId,
+    SpanId, Type, Value, VarId, VirtualPathId,
 };
 use core::panic;
 use std::{
@@ -1019,10 +1019,8 @@ impl<'a> StateWorkingSet<'a> {
         self.delta.spans.push(span);
         SpanId(num_permanent_spans + self.delta.spans.len() - 1)
     }
-}
 
-impl<'a> GetSpan for &'a StateWorkingSet<'a> {
+    pub fn get_span(&self, span_id: SpanId) -> Span {
-    fn get_span(&self, span_id: SpanId) -> Span {
         let num_permanent_spans = self.permanent_state.num_spans();
         if span_id.0 < num_permanent_spans {
             self.permanent_state.get_span(span_id)

crates/nu-protocol/src/eval_base.rs

@@ -4,7 +4,7 @@ use crate::{
         ExternalArgument, ListItem, Math, Operator, RecordItem,
     },
     debugger::DebugContext,
-    Config, GetSpan, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID,
+    Config, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID,
 };
 use std::{borrow::Cow, collections::HashMap};
 
@@ -12,7 +12,7 @@ use std::{borrow::Cow, collections::HashMap};
 pub trait Eval {
     /// State that doesn't need to be mutated.
     /// EngineState for regular eval and StateWorkingSet for const eval
-    type State<'a>: Copy + GetSpan;
+    type State<'a>: Copy;
 
     /// State that needs to be mutated.
     /// This is the stack for regular eval, and unused by const eval
@@ -23,19 +23,17 @@ pub trait Eval {
         mut_state: &mut Self::MutState,
         expr: &Expression,
     ) -> Result<Value, ShellError> {
-        let expr_span = expr.span(&state);
-
         match &expr.expr {
-            Expr::Bool(b) => Ok(Value::bool(*b, expr_span)),
+            Expr::Bool(b) => Ok(Value::bool(*b, expr.span)),
-            Expr::Int(i) => Ok(Value::int(*i, expr_span)),
+            Expr::Int(i) => Ok(Value::int(*i, expr.span)),
-            Expr::Float(f) => Ok(Value::float(*f, expr_span)),
+            Expr::Float(f) => Ok(Value::float(*f, expr.span)),
-            Expr::Binary(b) => Ok(Value::binary(b.clone(), expr_span)),
+            Expr::Binary(b) => Ok(Value::binary(b.clone(), expr.span)),
-            Expr::Filepath(path, quoted) => Self::eval_filepath(state, mut_state, path.clone(), *quoted, expr_span),
+            Expr::Filepath(path, quoted) => Self::eval_filepath(state, mut_state, path.clone(), *quoted, expr.span),
             Expr::Directory(path, quoted) => {
-                Self::eval_directory(state, mut_state, path.clone(), *quoted, expr_span)
+                Self::eval_directory(state, mut_state, path.clone(), *quoted, expr.span)
             }
-            Expr::Var(var_id) => Self::eval_var(state, mut_state, *var_id, expr_span),
+            Expr::Var(var_id) => Self::eval_var(state, mut_state, *var_id, expr.span),
-            Expr::CellPath(cell_path) => Ok(Value::cell_path(cell_path.clone(), expr_span)),
+            Expr::CellPath(cell_path) => Ok(Value::cell_path(cell_path.clone(), expr.span)),
             Expr::FullCellPath(cell_path) => {
                 let value = Self::eval::<D>(state, mut_state, &cell_path.head)?;
 
@@ -47,7 +45,7 @@ pub trait Eval {
                     value.follow_cell_path(&cell_path.tail, false)
                 }
             }
-            Expr::DateTime(dt) => Ok(Value::date(*dt, expr_span)),
+            Expr::DateTime(dt) => Ok(Value::date(*dt, expr.span)),
             Expr::List(list) => {
                 let mut output = vec![];
                 for item in list {
@@ -55,11 +53,11 @@ pub trait Eval {
                         ListItem::Item(expr) => output.push(Self::eval::<D>(state, mut_state, expr)?),
                         ListItem::Spread(_, expr) => match Self::eval::<D>(state, mut_state, expr)? {
                             Value::List { vals, .. } => output.extend(vals),
-                            _ => return Err(ShellError::CannotSpreadAsList { span: expr_span }),
+                            _ => return Err(ShellError::CannotSpreadAsList { span: expr.span }),
                         },
                     }
                 }
-                Ok(Value::list(output, expr_span))
+                Ok(Value::list(output, expr.span))
             }
             Expr::Record(items) => {
                 let mut record = Record::new();
@@ -69,38 +67,36 @@ pub trait Eval {
                         RecordItem::Pair(col, val) => {
                             // avoid duplicate cols
                             let col_name = Self::eval::<D>(state, mut_state, col)?.coerce_into_string()?;
-                            let col_span = col.span(&state);
                             if let Some(orig_span) = col_names.get(&col_name) {
                                 return Err(ShellError::ColumnDefinedTwice {
                                     col_name,
-                                    second_use: col_span,
+                                    second_use: col.span,
                                     first_use: *orig_span,
                                 });
                             } else {
-                                col_names.insert(col_name.clone(), col_span);
+                                col_names.insert(col_name.clone(), col.span);
                                 record.push(col_name, Self::eval::<D>(state, mut_state, val)?);
                             }
                         }
                         RecordItem::Spread(_, inner) => {
-                            let inner_span = inner.span(&state);
                             match Self::eval::<D>(state, mut_state, inner)? {
                                 Value::Record { val: inner_val, .. } => {
                                     for (col_name, val) in inner_val.into_owned() {
                                         if let Some(orig_span) = col_names.get(&col_name) {
                                             return Err(ShellError::ColumnDefinedTwice {
                                                 col_name,
-                                                second_use: inner_span,
+                                                second_use: inner.span,
                                                 first_use: *orig_span,
                                             });
                                         } else {
-                                            col_names.insert(col_name.clone(), inner_span);
+                                            col_names.insert(col_name.clone(), inner.span);
                                             record.push(col_name, val);
                                         }
                                     }
                                 }
                                 _ => {
                                     return Err(ShellError::CannotSpreadAsRecord {
-                                        span: inner_span,
+                                        span: inner.span,
                                     })
                                 }
                             }
@@ -108,7 +104,7 @@ pub trait Eval {
                     }
                 }
 
-                Ok(Value::record(record, expr_span))
+                Ok(Value::record(record, expr.span))
             }
             Expr::Table(table) => {
                 let mut output_headers = vec![];
@@ -118,11 +114,10 @@ pub trait Eval {
                         .iter()
                         .position(|existing| existing == &header)
                     {
-                        let first_use = table.columns[idx].span(&state);
                         return Err(ShellError::ColumnDefinedTwice {
                             col_name: header,
-                            second_use: expr_span,
+                            second_use: expr.span,
-                            first_use,
+                            first_use: table.columns[idx].span,
                         });
                     } else {
                         output_headers.push(header);
@@ -137,66 +132,66 @@ pub trait Eval {
 
                     output_rows.push(Value::record(
                         record,
-                        expr_span,
+                        expr.span,
                     ));
                 }
-                Ok(Value::list(output_rows, expr_span))
+                Ok(Value::list(output_rows, expr.span))
             }
             Expr::Keyword(kw) => Self::eval::<D>(state, mut_state, &kw.expr),
-            Expr::String(s) | Expr::RawString(s) => Ok(Value::string(s.clone(), expr_span)),
+            Expr::String(s) | Expr::RawString(s) => Ok(Value::string(s.clone(), expr.span)),
-            Expr::Nothing => Ok(Value::nothing(expr_span)),
+            Expr::Nothing => Ok(Value::nothing(expr.span)),
             Expr::ValueWithUnit(value) => match Self::eval::<D>(state, mut_state, &value.expr)? {
                 Value::Int { val, .. } => value.unit.item.build_value(val, value.unit.span),
                 x => Err(ShellError::CantConvert {
                     to_type: "unit value".into(),
                     from_type: x.get_type().to_string(),
-                    span: value.expr.span(&state),
+                    span: value.expr.span,
                     help: None,
                 }),
             },
-            Expr::Call(call) => Self::eval_call::<D>(state, mut_state, call, expr_span),
+            Expr::Call(call) => Self::eval_call::<D>(state, mut_state, call, expr.span),
             Expr::ExternalCall(head, args) => {
-                Self::eval_external_call(state, mut_state, head, args, expr_span)
+                Self::eval_external_call(state, mut_state, head, args, expr.span)
             }
             Expr::Subexpression(block_id) => {
-                Self::eval_subexpression::<D>(state, mut_state, *block_id, expr_span)
+                Self::eval_subexpression::<D>(state, mut_state, *block_id, expr.span)
             }
             Expr::Range(range) => {
                 let from = if let Some(f) = &range.from {
                     Self::eval::<D>(state, mut_state, f)?
                 } else {
-                    Value::nothing(expr_span)
+                    Value::nothing(expr.span)
                 };
 
                 let next = if let Some(s) = &range.next {
                     Self::eval::<D>(state, mut_state, s)?
                 } else {
-                    Value::nothing(expr_span)
+                    Value::nothing(expr.span)
                 };
 
                 let to = if let Some(t) = &range.to {
                     Self::eval::<D>(state, mut_state, t)?
                 } else {
-                    Value::nothing(expr_span)
+                    Value::nothing(expr.span)
                 };
 
                 Ok(Value::range(
-                    Range::new(from, next, to, range.operator.inclusion, expr_span)?,
+                    Range::new(from, next, to, range.operator.inclusion, expr.span)?,
-                    expr_span,
+                    expr.span,
                 ))
             }
             Expr::UnaryNot(expr) => {
                 let lhs = Self::eval::<D>(state, mut_state, expr)?;
                 match lhs {
-                    Value::Bool { val, .. } => Ok(Value::bool(!val, expr_span)),
+                    Value::Bool { val, .. } => Ok(Value::bool(!val, expr.span)),
                     other => Err(ShellError::TypeMismatch {
                         err_message: format!("expected bool, found {}", other.get_type()),
-                        span: expr_span,
+                        span: expr.span,
                     }),
                 }
             }
             Expr::BinaryOp(lhs, op, rhs) => {
-                let op_span = op.span(&state);
+                let op_span = op.span;
                 let op = eval_operator(op)?;
 
                 match op {
@@ -205,23 +200,23 @@ pub trait Eval {
                         match boolean {
                             Boolean::And => {
                                 if lhs.is_false() {
-                                    Ok(Value::bool(false, expr_span))
+                                    Ok(Value::bool(false, expr.span))
                                 } else {
                                     let rhs = Self::eval::<D>(state, mut_state, rhs)?;
-                                    lhs.and(op_span, &rhs, expr_span)
+                                    lhs.and(op_span, &rhs, expr.span)
                                 }
                             }
                             Boolean::Or => {
                                 if lhs.is_true() {
-                                    Ok(Value::bool(true, expr_span))
+                                    Ok(Value::bool(true, expr.span))
                                 } else {
                                     let rhs = Self::eval::<D>(state, mut_state, rhs)?;
-                                    lhs.or(op_span, &rhs, expr_span)
+                                    lhs.or(op_span, &rhs, expr.span)
                                 }
                             }
                             Boolean::Xor => {
                                 let rhs = Self::eval::<D>(state, mut_state, rhs)?;
-                                lhs.xor(op_span, &rhs, expr_span)
+                                lhs.xor(op_span, &rhs, expr.span)
                             }
                         }
                     }
@@ -230,35 +225,35 @@ pub trait Eval {
                         let rhs = Self::eval::<D>(state, mut_state, rhs)?;
 
                         match math {
-                            Math::Plus => lhs.add(op_span, &rhs, expr_span),
+                            Math::Plus => lhs.add(op_span, &rhs, expr.span),
-                            Math::Minus => lhs.sub(op_span, &rhs, expr_span),
+                            Math::Minus => lhs.sub(op_span, &rhs, expr.span),
-                            Math::Multiply => lhs.mul(op_span, &rhs, expr_span),
+                            Math::Multiply => lhs.mul(op_span, &rhs, expr.span),
-                            Math::Divide => lhs.div(op_span, &rhs, expr_span),
+                            Math::Divide => lhs.div(op_span, &rhs, expr.span),
-                            Math::Append => lhs.append(op_span, &rhs, expr_span),
+                            Math::Append => lhs.append(op_span, &rhs, expr.span),
-                            Math::Modulo => lhs.modulo(op_span, &rhs, expr_span),
+                            Math::Modulo => lhs.modulo(op_span, &rhs, expr.span),
-                            Math::FloorDivision => lhs.floor_div(op_span, &rhs, expr_span),
+                            Math::FloorDivision => lhs.floor_div(op_span, &rhs, expr.span),
-                            Math::Pow => lhs.pow(op_span, &rhs, expr_span),
+                            Math::Pow => lhs.pow(op_span, &rhs, expr.span),
                         }
                     }
                     Operator::Comparison(comparison) => {
                         let lhs = Self::eval::<D>(state, mut_state, lhs)?;
                         let rhs = Self::eval::<D>(state, mut_state, rhs)?;
                         match comparison {
-                            Comparison::LessThan => lhs.lt(op_span, &rhs, expr_span),
+                            Comparison::LessThan => lhs.lt(op_span, &rhs, expr.span),
-                            Comparison::LessThanOrEqual => lhs.lte(op_span, &rhs, expr_span),
+                            Comparison::LessThanOrEqual => lhs.lte(op_span, &rhs, expr.span),
-                            Comparison::GreaterThan => lhs.gt(op_span, &rhs, expr_span),
+                            Comparison::GreaterThan => lhs.gt(op_span, &rhs, expr.span),
-                            Comparison::GreaterThanOrEqual => lhs.gte(op_span, &rhs, expr_span),
+                            Comparison::GreaterThanOrEqual => lhs.gte(op_span, &rhs, expr.span),
-                            Comparison::Equal => lhs.eq(op_span, &rhs, expr_span),
+                            Comparison::Equal => lhs.eq(op_span, &rhs, expr.span),
-                            Comparison::NotEqual => lhs.ne(op_span, &rhs, expr_span),
+                            Comparison::NotEqual => lhs.ne(op_span, &rhs, expr.span),
-                            Comparison::In => lhs.r#in(op_span, &rhs, expr_span),
+                            Comparison::In => lhs.r#in(op_span, &rhs, expr.span),
-                            Comparison::NotIn => lhs.not_in(op_span, &rhs, expr_span),
+                            Comparison::NotIn => lhs.not_in(op_span, &rhs, expr.span),
-                            Comparison::StartsWith => lhs.starts_with(op_span, &rhs, expr_span),
+                            Comparison::StartsWith => lhs.starts_with(op_span, &rhs, expr.span),
-                            Comparison::EndsWith => lhs.ends_with(op_span, &rhs, expr_span),
+                            Comparison::EndsWith => lhs.ends_with(op_span, &rhs, expr.span),
                             Comparison::RegexMatch => {
-                                Self::regex_match(state, op_span, &lhs, &rhs, false, expr_span)
+                                Self::regex_match(state, op_span, &lhs, &rhs, false, expr.span)
                             }
                             Comparison::NotRegexMatch => {
-                                Self::regex_match(state, op_span, &lhs, &rhs, true, expr_span)
+                                Self::regex_match(state, op_span, &lhs, &rhs, true, expr.span)
                             }
                         }
                     }
@@ -266,20 +261,20 @@ pub trait Eval {
                         let lhs = Self::eval::<D>(state, mut_state, lhs)?;
                         let rhs = Self::eval::<D>(state, mut_state, rhs)?;
                         match bits {
-                            Bits::BitAnd => lhs.bit_and(op_span, &rhs, expr_span),
+                            Bits::BitAnd => lhs.bit_and(op_span, &rhs, expr.span),
-                            Bits::BitOr => lhs.bit_or(op_span, &rhs, expr_span),
+                            Bits::BitOr => lhs.bit_or(op_span, &rhs, expr.span),
-                            Bits::BitXor => lhs.bit_xor(op_span, &rhs, expr_span),
+                            Bits::BitXor => lhs.bit_xor(op_span, &rhs, expr.span),
-                            Bits::ShiftLeft => lhs.bit_shl(op_span, &rhs, expr_span),
+                            Bits::ShiftLeft => lhs.bit_shl(op_span, &rhs, expr.span),
-                            Bits::ShiftRight => lhs.bit_shr(op_span, &rhs, expr_span),
+                            Bits::ShiftRight => lhs.bit_shr(op_span, &rhs, expr.span),
                         }
                     }
                     Operator::Assignment(assignment) => Self::eval_assignment::<D>(
-                        state, mut_state, lhs, rhs, assignment, op_span, expr_span
+                        state, mut_state, lhs, rhs, assignment, op_span, expr.span
                     ),
                 }
             }
             Expr::RowCondition(block_id) | Expr::Closure(block_id) => {
-                Self::eval_row_condition_or_closure(state, mut_state, *block_id, expr_span)
+                Self::eval_row_condition_or_closure(state, mut_state, *block_id, expr.span)
             }
             Expr::StringInterpolation(exprs) => {
                 let config = Self::get_config(state, mut_state);
@@ -288,13 +283,13 @@ pub trait Eval {
                     .map(|expr| Self::eval::<D>(state, mut_state, expr).map(|v| v.to_expanded_string(", ", &config)))
                     .collect::<Result<String, _>>()?;
 
-                Ok(Value::string(str, expr_span))
+                Ok(Value::string(str, expr.span))
             }
-            Expr::Overlay(_) => Self::eval_overlay(state, expr_span),
+            Expr::Overlay(_) => Self::eval_overlay(state, expr.span),
             Expr::GlobPattern(pattern, quoted) => {
                 // GlobPattern is similar to Filepath
                 // But we don't want to expand path during eval time, it's required for `nu_engine::glob_from` to run correctly
-                Ok(Value::glob(pattern, *quoted, expr_span))
+                Ok(Value::glob(pattern, *quoted, expr.span))
             }
             Expr::MatchBlock(_) // match blocks are handled by `match`
             | Expr::Block(_) // blocks are handled directly by core commands
@@ -302,7 +297,7 @@ pub trait Eval {
             | Expr::ImportPattern(_)
             | Expr::Signature(_)
             | Expr::Operator(_)
-            | Expr::Garbage => Self::unreachable(state, expr),
+            | Expr::Garbage => Self::unreachable(expr),
         }
     }
 
@@ -383,5 +378,5 @@ pub trait Eval {
     fn eval_overlay(state: Self::State<'_>, span: Span) -> Result<Value, ShellError>;
 
     /// For expressions that should never actually be evaluated
-    fn unreachable(state: Self::State<'_>, expr: &Expression) -> Result<Value, ShellError>;
+    fn unreachable(expr: &Expression) -> Result<Value, ShellError>;
 }

crates/nu-protocol/src/eval_const.rs

@@ -251,7 +251,7 @@ pub fn eval_constant_with_input(
         Expr::Call(call) => eval_const_call(working_set, call, input),
         Expr::Subexpression(block_id) => {
             let block = working_set.get_block(*block_id);
-            eval_const_subexpression(working_set, block, input, expr.span(&working_set))
+            eval_const_subexpression(working_set, block, input, expr.span)
         }
         _ => eval_constant(working_set, expr).map(|v| PipelineData::Value(v, None)),
     }
@@ -379,9 +379,7 @@ impl Eval for EvalConst {
         Err(ShellError::NotAConstant { span })
     }
 
-    fn unreachable(working_set: &StateWorkingSet, expr: &Expression) -> Result<Value, ShellError> {
+    fn unreachable(expr: &Expression) -> Result<Value, ShellError> {
-        Err(ShellError::NotAConstant {
+        Err(ShellError::NotAConstant { span: expr.span })
-            span: expr.span(&working_set),
-        })
     }
 }

crates/nu-protocol/src/span.rs

@@ -1,12 +1,7 @@
-use crate::SpanId;
 use miette::SourceSpan;
 use serde::{Deserialize, Serialize};
 use std::ops::Deref;
 
-pub trait GetSpan {
-    fn get_span(&self, span_id: SpanId) -> Span;
-}
-
 /// A spanned area of interest, generic over what kind of thing is of interest
 #[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub struct Spanned<T> {