Remove usages of internal_span (#14700)

# Description
Remove usages of `internal_span` in matches and initializers. I think
this should be the last of the usages, meaning `internal_span` can
finally be refactored out of `Value`(!?)

crates/nu-cli/tests/completions/support/completions_helpers.rs

@@ -98,14 +98,14 @@ pub fn new_dotnu_engine() -> (AbsolutePathBuf, String, EngineState, Stack) {
 
     stack.add_env_var(
         "NU_LIB_DIRS".to_string(),
-        Value::List {
-            vals: vec![
+        Value::list(
+            vec![
                 Value::string(file(dir.join("lib-dir1")), dir_span),
                 Value::string(file(dir.join("lib-dir2")), dir_span),
                 Value::string(file(dir.join("lib-dir3")), dir_span),
             ],
-            internal_span: dir_span,
-        },
+            dir_span,
+        ),
     );
 
     // Merge environment into the permanent state

crates/nu-command/src/conversions/into/cell_path.rs

@@ -184,16 +184,11 @@ fn value_to_cell_path(value: &Value, span: Span) -> Result<Value, ShellError> {
 }
 
 fn value_to_path_member(val: &Value, span: Span) -> Result<PathMember, ShellError> {
+    let val_span = val.span();
     let member = match val {
-        Value::Int {
-            val,
-            internal_span: span,
-        } => int_to_path_member(*val, *span)?,
-        Value::String {
-            val,
-            internal_span: span,
-        } => PathMember::string(val.into(), false, *span),
-        Value::Record { val, internal_span } => record_to_path_member(val, *internal_span, span)?,
+        Value::Int { val, .. } => int_to_path_member(*val, val_span)?,
+        Value::String { val, .. } => PathMember::string(val.into(), false, val_span),
+        Value::Record { val, .. } => record_to_path_member(val, val_span, span)?,
         other => {
             return Err(ShellError::CantConvert {
                 to_type: "int or string".to_string(),

crates/nu-command/src/conversions/split_cell_path.rs

@@ -121,11 +121,8 @@ fn split_cell_path(val: CellPath, span: Span) -> Result<Value, ShellError> {
                 | PathMember::Int { optional, span, .. } => (optional, span),
             };
             let value = match pm {
-                PathMember::String { val, .. } => Value::String { val, internal_span },
-                PathMember::Int { val, .. } => Value::Int {
-                    val: val as i64,
-                    internal_span,
-                },
+                PathMember::String { val, .. } => Value::string(val, internal_span),
+                PathMember::Int { val, .. } => Value::int(val as i64, internal_span),
             };
             Self { value, optional }
         }
@@ -142,10 +139,7 @@ fn split_cell_path(val: CellPath, span: Span) -> Result<Value, ShellError> {
         })
         .collect();
 
-    Ok(Value::List {
-        vals: members,
-        internal_span: span,
-    })
+    Ok(Value::list(members, span))
 }
 
 #[cfg(test)]

crates/nu-command/src/database/commands/into_sqlite.rs

@@ -196,13 +196,13 @@ fn action(
         PipelineData::ListStream(stream, _) => {
             insert_in_transaction(stream.into_iter(), span, table, signals)
         }
-        PipelineData::Value(
-            Value::List {
-                vals,
-                internal_span,
-            },
-            _,
-        ) => insert_in_transaction(vals.into_iter(), internal_span, table, signals),
+        PipelineData::Value(value @ Value::List { .. }, _) => {
+            let span = value.span();
+            let vals = value
+                .into_list()
+                .expect("Value matched as list above, but is not a list");
+            insert_in_transaction(vals.into_iter(), span, table, signals)
+        }
         PipelineData::Value(val, _) => {
             insert_in_transaction(std::iter::once(val), span, table, signals)
         }

crates/nu-command/src/filters/select.rs

@@ -56,7 +56,7 @@ produce a table, a list will produce a list, and a record will produce a record.
         let columns: Vec<Value> = call.rest(engine_state, stack, 0)?;
         let mut new_columns: Vec<CellPath> = vec![];
         for col_val in columns {
-            let col_span = &col_val.span();
+            let col_span = col_val.span();
             match col_val {
                 Value::CellPath { val, .. } => {
                     new_columns.push(val);
@@ -65,25 +65,25 @@ produce a table, a list will produce a list, and a record will produce a record.
                     let cv = CellPath {
                         members: vec![PathMember::String {
                             val,
-                            span: *col_span,
+                            span: col_span,
                             optional: false,
                         }],
                     };
                     new_columns.push(cv);
                 }
-                Value::Int { val, internal_span } => {
+                Value::Int { val, .. } => {
                     if val < 0 {
                         return Err(ShellError::CantConvert {
                             to_type: "cell path".into(),
                             from_type: "negative number".into(),
-                            span: internal_span,
+                            span: col_span,
                             help: None,
                         });
                     }
                     let cv = CellPath {
                         members: vec![PathMember::Int {
                             val: val as usize,
-                            span: *col_span,
+                            span: col_span,
                             optional: false,
                         }],
                     };

crates/nu-command/src/network/url/query.rs

@@ -52,14 +52,17 @@ pub fn table_to_query_string(
 ) -> Result<String, ShellError> {
     let row_vec = table
         .iter()
-        .map(|val| match val {
-            Value::Record { val, internal_span } => key_value_from_record(val, *internal_span),
-            _ => Err(ShellError::UnsupportedInput {
-                msg: "expected a table".into(),
-                input: "not a table, contains non-record values".into(),
-                msg_span: head,
-                input_span: span,
-            }),
+        .map(|val| {
+            let val_span = val.span();
+            match val {
+                Value::Record { val, .. } => key_value_from_record(val, val_span),
+                _ => Err(ShellError::UnsupportedInput {
+                    msg: "expected a table".into(),
+                    input: "not a table, contains non-record values".into(),
+                    msg_span: head,
+                    input_span: span,
+                }),
+            }
         })
         .collect::<Result<Vec<_>, ShellError>>()?;
 

crates/nu-command/src/platform/ulimit.rs

@@ -431,12 +431,13 @@ fn parse_limit(
     hard_limit: rlim_t,
     call_span: Span,
 ) -> Result<rlim_t, ShellError> {
+    let val_span = limit_value.span();
     match limit_value {
-        Value::Int { val, internal_span } => {
+        Value::Int { val, .. } => {
             let value = rlim_t::try_from(*val).map_err(|e| ShellError::CantConvert {
                 to_type: "rlim_t".into(),
                 from_type: "i64".into(),
-                span: *internal_span,
+                span: val_span,
                 help: Some(e.to_string()),
             })?;
 
@@ -447,25 +448,25 @@ fn parse_limit(
                 Ok(limit)
             }
         }
-        Value::Filesize { val, internal_span } => {
+        Value::Filesize { val, .. } => {
             if res.multiplier != 1024 {
                 return Err(ShellError::TypeMismatch {
                     err_message: format!(
                         "filesize is not compatible with resource {:?}",
                         res.resource
                     ),
-                    span: *internal_span,
+                    span: val_span,
                 });
             }
 
             rlim_t::try_from(*val).map_err(|e| ShellError::CantConvert {
                 to_type: "rlim_t".into(),
                 from_type: "i64".into(),
-                span: *internal_span,
+                span: val_span,
                 help: Some(e.to_string()),
             })
         }
-        Value::String { val, internal_span } => {
+        Value::String { val, .. } => {
             if val == "unlimited" {
                 Ok(RLIM_INFINITY)
             } else if val == "soft" {
@@ -479,7 +480,7 @@ fn parse_limit(
             } else {
                 return Err(ShellError::IncorrectValue {
                     msg: "Only unlimited, soft and hard are supported for strings".into(),
-                    val_span: *internal_span,
+                    val_span,
                     call_span,
                 });
             }

crates/nu-command/src/viewers/table.rs

@@ -336,6 +336,7 @@ fn get_index_flag(
         Some(value) => value,
         None => return Ok(Some(0)),
     };
+    let span = value.span();
 
     match value {
         Value::Bool { val, .. } => {
@@ -345,13 +346,13 @@ fn get_index_flag(
                 Ok(None)
             }
         }
-        Value::Int { val, internal_span } => {
+        Value::Int { val, .. } => {
             if val < 0 {
                 Err(ShellError::UnsupportedInput {
                     msg: String::from("got a negative integer"),
                     input: val.to_string(),
                     msg_span: call.span(),
-                    input_span: internal_span,
+                    input_span: span,
                 })
             } else {
                 Ok(Some(val as usize))

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

@@ -1163,7 +1163,7 @@ mod test_cwd {
 
     use crate::{
         engine::{EngineState, Stack},
-        Span, Value,
+        Value,
     };
     use nu_path::{assert_path_eq, AbsolutePath, Path};
     use tempfile::{NamedTempFile, TempDir};
@@ -1226,14 +1226,7 @@ mod test_cwd {
     #[test]
     fn pwd_is_non_string_value() {
         let mut engine_state = EngineState::new();
-        engine_state.add_env_var(
-            "PWD".into(),
-            Value::Glob {
-                val: "*".into(),
-                no_expand: false,
-                internal_span: Span::unknown(),
-            },
-        );
+        engine_state.add_env_var("PWD".into(), Value::test_glob("*"));
         engine_state.cwd(None).unwrap_err();
     }
 

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

@@ -774,78 +774,76 @@ mod test {
 
     use super::Stack;
 
-    const ZERO_SPAN: Span = Span { start: 0, end: 0 };
-    fn string_value(s: &str) -> Value {
-        Value::String {
-            val: s.to_string(),
-            internal_span: ZERO_SPAN,
-        }
-    }
-
     #[test]
     fn test_children_see_inner_values() {
         let mut original = Stack::new();
-        original.add_var(VarId::new(0), string_value("hello"));
+        original.add_var(VarId::new(0), Value::test_string("hello"));
 
         let cloned = Stack::with_parent(Arc::new(original));
         assert_eq!(
-            cloned.get_var(VarId::new(0), ZERO_SPAN),
-            Ok(string_value("hello"))
+            cloned.get_var(VarId::new(0), Span::test_data()),
+            Ok(Value::test_string("hello"))
         );
     }
 
     #[test]
     fn test_children_dont_see_deleted_values() {
         let mut original = Stack::new();
-        original.add_var(VarId::new(0), string_value("hello"));
+        original.add_var(VarId::new(0), Value::test_string("hello"));
 
         let mut cloned = Stack::with_parent(Arc::new(original));
         cloned.remove_var(VarId::new(0));
 
         assert_eq!(
-            cloned.get_var(VarId::new(0), ZERO_SPAN),
-            Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN })
+            cloned.get_var(VarId::new(0), Span::test_data()),
+            Err(crate::ShellError::VariableNotFoundAtRuntime {
+                span: Span::test_data()
+            })
         );
     }
 
     #[test]
     fn test_children_changes_override_parent() {
         let mut original = Stack::new();
-        original.add_var(VarId::new(0), string_value("hello"));
+        original.add_var(VarId::new(0), Value::test_string("hello"));
 
         let mut cloned = Stack::with_parent(Arc::new(original));
-        cloned.add_var(VarId::new(0), string_value("there"));
+        cloned.add_var(VarId::new(0), Value::test_string("there"));
         assert_eq!(
-            cloned.get_var(VarId::new(0), ZERO_SPAN),
-            Ok(string_value("there"))
+            cloned.get_var(VarId::new(0), Span::test_data()),
+            Ok(Value::test_string("there"))
         );
 
         cloned.remove_var(VarId::new(0));
         // the underlying value shouldn't magically re-appear
         assert_eq!(
-            cloned.get_var(VarId::new(0), ZERO_SPAN),
-            Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN })
+            cloned.get_var(VarId::new(0), Span::test_data()),
+            Err(crate::ShellError::VariableNotFoundAtRuntime {
+                span: Span::test_data()
+            })
         );
     }
     #[test]
     fn test_children_changes_persist_in_offspring() {
         let mut original = Stack::new();
-        original.add_var(VarId::new(0), string_value("hello"));
+        original.add_var(VarId::new(0), Value::test_string("hello"));
 
         let mut cloned = Stack::with_parent(Arc::new(original));
-        cloned.add_var(VarId::new(1), string_value("there"));
+        cloned.add_var(VarId::new(1), Value::test_string("there"));
 
         cloned.remove_var(VarId::new(0));
         let cloned = Stack::with_parent(Arc::new(cloned));
 
         assert_eq!(
-            cloned.get_var(VarId::new(0), ZERO_SPAN),
-            Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN })
+            cloned.get_var(VarId::new(0), Span::test_data()),
+            Err(crate::ShellError::VariableNotFoundAtRuntime {
+                span: Span::test_data()
+            })
         );
 
         assert_eq!(
-            cloned.get_var(VarId::new(1), ZERO_SPAN),
-            Ok(string_value("there"))
+            cloned.get_var(VarId::new(1), Span::test_data()),
+            Ok(Value::test_string("there"))
         );
     }
 
@@ -853,33 +851,38 @@ mod test {
     fn test_merging_children_back_to_parent() {
         let mut original = Stack::new();
         let engine_state = EngineState::new();
-        original.add_var(VarId::new(0), string_value("hello"));
+        original.add_var(VarId::new(0), Value::test_string("hello"));
 
         let original_arc = Arc::new(original);
         let mut cloned = Stack::with_parent(original_arc.clone());
-        cloned.add_var(VarId::new(1), string_value("there"));
+        cloned.add_var(VarId::new(1), Value::test_string("there"));
 
         cloned.remove_var(VarId::new(0));
 
-        cloned.add_env_var("ADDED_IN_CHILD".to_string(), string_value("New Env Var"));
+        cloned.add_env_var(
+            "ADDED_IN_CHILD".to_string(),
+            Value::test_string("New Env Var"),
+        );
 
         let original = Stack::with_changes_from_child(original_arc, cloned);
 
         assert_eq!(
-            original.get_var(VarId::new(0), ZERO_SPAN),
-            Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN })
+            original.get_var(VarId::new(0), Span::test_data()),
+            Err(crate::ShellError::VariableNotFoundAtRuntime {
+                span: Span::test_data()
+            })
         );
 
         assert_eq!(
-            original.get_var(VarId::new(1), ZERO_SPAN),
-            Ok(string_value("there"))
+            original.get_var(VarId::new(1), Span::test_data()),
+            Ok(Value::test_string("there"))
         );
 
         assert_eq!(
             original
                 .get_env_var(&engine_state, "ADDED_IN_CHILD")
                 .cloned(),
-            Some(string_value("New Env Var")),
+            Some(Value::test_string("New Env Var")),
         );
     }
 }