Move Value to helpers, separate span call (#10121)

# Description

As part of the refactor to split spans off of Value, this moves to using
helper functions to create values, and using `.span()` instead of
matching span out of Value directly.

Hoping to get a few more helping hands to finish this, as there are a
lot of commands to update :)

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
Co-authored-by: WindSoilder <windsoilder@outlook.com>
This commit is contained in:
JT
2023-09-04 02:27:29 +12:00
committed by GitHub
parent af79eb2943
commit 6cdfee3573
372 changed files with 5811 additions and 7448 deletions

View File

@ -267,29 +267,37 @@ pub(super) fn compute_series_single_value(
rhs_span: right.span(),
}),
},
Operator::Math(Math::Divide) => match &right {
Value::Int { val, span } => {
if *val == 0 {
Err(ShellError::DivisionByZero { span: *span })
} else {
compute_series_i64(&lhs, *val, <ChunkedArray<Int64Type>>::div, lhs_span)
Operator::Math(Math::Divide) => {
let span = right.span();
match &right {
Value::Int { val, .. } => {
if *val == 0 {
Err(ShellError::DivisionByZero { span })
} else {
compute_series_i64(&lhs, *val, <ChunkedArray<Int64Type>>::div, lhs_span)
}
}
}
Value::Float { val, span } => {
if val.is_zero() {
Err(ShellError::DivisionByZero { span: *span })
} else {
compute_series_decimal(&lhs, *val, <ChunkedArray<Float64Type>>::div, lhs_span)
Value::Float { val, .. } => {
if val.is_zero() {
Err(ShellError::DivisionByZero { span })
} else {
compute_series_decimal(
&lhs,
*val,
<ChunkedArray<Float64Type>>::div,
lhs_span,
)
}
}
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span(),
}),
}
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span(),
}),
},
}
Operator::Comparison(Comparison::Equal) => match &right {
Value::Int { val, .. } => compare_series_i64(&lhs, *val, ChunkedArray::equal, lhs_span),
Value::Float { val, .. } => {

View File

@ -452,7 +452,7 @@ fn series_to_values(
) -> Result<Vec<Value>, ShellError> {
match series.dtype() {
DataType::Null => {
let it = std::iter::repeat(Value::Nothing { span });
let it = std::iter::repeat(Value::nothing(span));
let values = if let Some(size) = maybe_size {
Either::Left(it.take(size))
} else {
@ -480,11 +480,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -508,11 +505,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -536,11 +530,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -564,11 +555,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -592,11 +580,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -620,11 +605,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -648,11 +630,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int {
val: a as i64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::int(a as i64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -676,8 +655,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Int { val: a, span },
None => Value::Nothing { span },
Some(a) => Value::int(a, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -701,11 +680,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Float {
val: a as f64,
span,
},
None => Value::Nothing { span },
Some(a) => Value::float(a as f64, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -729,8 +705,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Float { val: a, span },
None => Value::Nothing { span },
Some(a) => Value::float(a, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -754,8 +730,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::Bool { val: a, span },
None => Value::Nothing { span },
Some(a) => Value::bool(a, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -779,11 +755,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(a) => Value::String {
val: a.into(),
span,
},
None => Value::Nothing { span },
Some(a) => Value::string(a.to_string(), span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -812,7 +785,7 @@ fn series_to_values(
}
.map(|v| match v {
Some(a) => a.get_value(),
None => Value::Nothing { span },
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -850,10 +823,7 @@ fn series_to_values(
}
})
.unwrap_or(vec![]);
Value::List {
vals: sublist,
span,
}
Value::list(sublist, span)
})
.collect::<Vec<Value>>();
Ok(values)
@ -884,43 +854,40 @@ fn series_to_values(
let naive_datetime = match NaiveDateTime::from_timestamp_opt(seconds, 0) {
Some(val) => val,
None => {
return Value::Error {
error: Box::new(ShellError::UnsupportedInput(
return Value::error(
ShellError::UnsupportedInput(
"The given local datetime representation is invalid."
.to_string(),
format!("timestamp is {a:?}"),
span,
Span::unknown(),
)),
),
span,
}
)
}
};
// Zero length offset
let offset = match FixedOffset::east_opt(0) {
Some(val) => val,
None => {
return Value::Error {
error: Box::new(ShellError::UnsupportedInput(
return Value::error(
ShellError::UnsupportedInput(
"The given local datetime representation is invalid."
.to_string(),
format!("timestamp is {a:?}"),
span,
Span::unknown(),
)),
),
span,
}
)
}
};
let datetime =
DateTime::<FixedOffset>::from_naive_utc_and_offset(naive_datetime, offset);
Value::Date {
val: datetime,
span,
}
Value::date(datetime, span)
}
None => Value::Nothing { span },
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -955,43 +922,40 @@ fn series_to_values(
let naive_datetime = match NaiveDateTime::from_timestamp_opt(seconds, 0) {
Some(val) => val,
None => {
return Value::Error {
error: Box::new(ShellError::UnsupportedInput(
return Value::error(
ShellError::UnsupportedInput(
"The given local datetime representation is invalid."
.to_string(),
format!("timestamp is {a:?}"),
span,
Span::unknown(),
)),
),
span,
}
)
}
};
// Zero length offset
let offset = match FixedOffset::east_opt(0) {
Some(val) => val,
None => {
return Value::Error {
error: Box::new(ShellError::UnsupportedInput(
return Value::error(
ShellError::UnsupportedInput(
"The given local datetime representation is invalid."
.to_string(),
format!("timestamp is {a:?}"),
span,
Span::unknown(),
)),
),
span,
}
)
}
};
let datetime =
DateTime::<FixedOffset>::from_naive_utc_and_offset(naive_datetime, offset);
Value::Date {
val: datetime,
span,
}
Value::date(datetime, span)
}
None => Value::Nothing { span },
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -1015,11 +979,8 @@ fn series_to_values(
Either::Right(it)
}
.map(|v| match v {
Some(nanoseconds) => Value::Duration {
val: nanoseconds,
span,
},
None => Value::Nothing { span },
Some(nanoseconds) => Value::duration(nanoseconds, span),
None => Value::nothing(span),
})
.collect::<Vec<Value>>();
@ -1043,20 +1004,14 @@ mod tests {
#[test]
fn test_parsed_column_string_list() -> Result<(), Box<dyn std::error::Error>> {
let values = vec![
Value::List {
vals: vec![Value::String {
val: "bar".to_string(),
span: Span::test_data(),
}],
span: Span::test_data(),
},
Value::List {
vals: vec![Value::String {
val: "baz".to_string(),
span: Span::test_data(),
}],
span: Span::test_data(),
},
Value::list(
vec![Value::string("bar".to_string(), Span::test_data())],
Span::test_data(),
),
Value::list(
vec![Value::string("baz".to_string(), Span::test_data())],
Span::test_data(),
),
];
let column = Column {
name: "foo".to_string(),

View File

@ -17,10 +17,7 @@ impl CustomValue for NuDataFrame {
from_lazy: false,
};
Value::CustomValue {
val: Box::new(cloned),
span,
}
Value::custom_value(Box::new(cloned), span)
}
fn value_string(&self) -> String {
@ -30,7 +27,7 @@ impl CustomValue for NuDataFrame {
fn to_base_value(&self, span: Span) -> Result<Value, ShellError> {
let vals = self.print(span)?;
Ok(Value::List { vals, span })
Ok(Value::list(vals, span))
}
fn as_any(&self) -> &dyn std::any::Any {

View File

@ -38,9 +38,7 @@ impl Display for DataFrameValue {
impl Default for DataFrameValue {
fn default() -> Self {
Self(Value::Nothing {
span: Span::unknown(),
})
Self(Value::nothing(Span::unknown()))
}
}
@ -111,24 +109,15 @@ impl NuDataFrame {
}
pub fn dataframe_into_value(dataframe: DataFrame, span: Span) -> Value {
Value::CustomValue {
val: Box::new(Self::new(false, dataframe)),
span,
}
Value::custom_value(Box::new(Self::new(false, dataframe)), span)
}
pub fn into_value(self, span: Span) -> Value {
if self.from_lazy {
let lazy = NuLazyFrame::from_dataframe(self);
Value::CustomValue {
val: Box::new(lazy),
span,
}
Value::custom_value(Box::new(lazy), span)
} else {
Value::CustomValue {
val: Box::new(self),
span,
}
Value::custom_value(Box::new(self), span)
}
}
@ -207,16 +196,19 @@ impl NuDataFrame {
pub fn fill_list_nan(list: Vec<Value>, list_span: Span, fill: Value) -> Value {
let newlist = list
.into_iter()
.map(|value| match value {
Value::Float { val, .. } => {
if val.is_nan() {
fill.clone()
} else {
value
.map(|value| {
let span = value.span();
match value {
Value::Float { val, .. } => {
if val.is_nan() {
fill.clone()
} else {
value
}
}
Value::List { vals, .. } => Self::fill_list_nan(vals, span, fill.clone()),
_ => value,
}
Value::List { vals, span } => Self::fill_list_nan(vals, span, fill.clone()),
_ => value,
})
.collect::<Vec<Value>>();
Value::list(newlist, list_span)
@ -250,8 +242,9 @@ impl NuDataFrame {
}
pub fn get_df(value: Value) -> Result<Self, ShellError> {
let span = value.span();
match value {
Value::CustomValue { val, span } => match val.as_any().downcast_ref::<Self>() {
Value::CustomValue { val, .. } => match val.as_any().downcast_ref::<Self>() {
Some(df) => Ok(NuDataFrame {
df: df.df.clone(),
from_lazy: false,

View File

@ -20,15 +20,13 @@ impl NuDataFrame {
op_span: Span,
right: &Value,
) -> Result<Value, ShellError> {
let rhs_span = right.span();
match right {
Value::CustomValue {
val: rhs,
span: rhs_span,
} => {
Value::CustomValue { val: rhs, .. } => {
let rhs = rhs.as_any().downcast_ref::<NuDataFrame>().ok_or_else(|| {
ShellError::DowncastNotPossible(
"Unable to create dataframe".to_string(),
*rhs_span,
rhs_span,
)
})?;
@ -38,7 +36,7 @@ impl NuDataFrame {
.as_series(lhs_span)
.expect("Already checked that is a series");
let rhs = &rhs
.as_series(*rhs_span)
.as_series(rhs_span)
.expect("Already checked that is a series");
if lhs.dtype() != rhs.dtype() {
@ -46,7 +44,7 @@ impl NuDataFrame {
left_message: format!("datatype {}", lhs.dtype()),
left_span: lhs_span,
right_message: format!("datatype {}", lhs.dtype()),
right_span: *rhs_span,
right_span: rhs_span,
});
}
@ -55,7 +53,7 @@ impl NuDataFrame {
left_message: format!("len {}", lhs.len()),
left_span: lhs_span,
right_message: format!("len {}", rhs.len()),
right_span: *rhs_span,
right_span: rhs_span,
});
}
@ -78,7 +76,7 @@ impl NuDataFrame {
left_message: format!("rows {}", self.df.height()),
left_span: lhs_span,
right_message: format!("rows {}", rhs.df.height()),
right_span: *rhs_span,
right_span: rhs_span,
});
}

View File

@ -20,10 +20,7 @@ impl CustomValue for NuExpression {
fn clone_value(&self, span: nu_protocol::Span) -> Value {
let cloned = NuExpression(self.0.clone());
Value::CustomValue {
val: Box::new(cloned),
span,
}
Value::custom_value(Box::new(cloned), span)
}
fn value_string(&self) -> String {
@ -56,16 +53,11 @@ fn compute_with_value(
op: Span,
right: &Value,
) -> Result<Value, ShellError> {
let rhs_span = right.span();
match right {
Value::CustomValue {
val: rhs,
span: rhs_span,
} => {
Value::CustomValue { val: rhs, .. } => {
let rhs = rhs.as_any().downcast_ref::<NuExpression>().ok_or_else(|| {
ShellError::DowncastNotPossible(
"Unable to create expression".to_string(),
*rhs_span,
)
ShellError::DowncastNotPossible("Unable to create expression".to_string(), rhs_span)
})?;
match rhs.as_ref() {

View File

@ -55,15 +55,13 @@ impl From<Expr> for NuExpression {
impl NuExpression {
pub fn into_value(self, span: Span) -> Value {
Value::CustomValue {
val: Box::new(self),
span,
}
Value::custom_value(Box::new(self), span)
}
pub fn try_from_value(value: Value) -> Result<Self, ShellError> {
let span = value.span();
match value {
Value::CustomValue { val, span } => match val.as_any().downcast_ref::<Self>() {
Value::CustomValue { val, .. } => match val.as_any().downcast_ref::<Self>() {
Some(expr) => Ok(NuExpression(expr.0.clone())),
None => Err(ShellError::CantConvert {
to_type: "lazy expression".into(),
@ -277,13 +275,10 @@ pub fn expr_to_value(expr: &Expr, span: Span) -> Result<Value, ShellError> {
Expr::DtypeColumn(dtypes) => {
let vals = dtypes
.iter()
.map(|d| Value::String {
val: format!("{d}"),
span,
})
.map(|d| Value::string(format!("{d}"), span))
.collect();
Ok(Value::List { vals, span })
Ok(Value::list(vals, span))
}
Expr::Sort { expr, options } => Ok(Value::record(
record! {
@ -318,10 +313,7 @@ pub fn expr_to_value(expr: &Expr, span: Span) -> Result<Value, ShellError> {
} => {
let by: Result<Vec<Value>, ShellError> =
by.iter().map(|b| expr_to_value(b, span)).collect();
let descending: Vec<Value> = descending
.iter()
.map(|r| Value::Bool { val: *r, span })
.collect();
let descending: Vec<Value> = descending.iter().map(|r| Value::bool(*r, span)).collect();
Ok(Value::record(
record! {
@ -354,10 +346,7 @@ pub fn expr_to_value(expr: &Expr, span: Span) -> Result<Value, ShellError> {
Expr::Exclude(expr, excluded) => {
let excluded = excluded
.iter()
.map(|e| Value::String {
val: format!("{e:?}"),
span,
})
.map(|e| Value::string(format!("{e:?}"), span))
.collect();
Ok(Value::record(

View File

@ -18,10 +18,7 @@ impl CustomValue for NuLazyFrame {
schema: self.schema.clone(),
};
Value::CustomValue {
val: Box::new(cloned),
span,
}
Value::custom_value(Box::new(cloned), span)
}
fn value_string(&self) -> String {

View File

@ -90,15 +90,9 @@ impl NuLazyFrame {
pub fn into_value(self, span: Span) -> Result<Value, ShellError> {
if self.from_eager {
let df = self.collect(span)?;
Ok(Value::CustomValue {
val: Box::new(df),
span,
})
Ok(Value::custom_value(Box::new(df), span))
} else {
Ok(Value::CustomValue {
val: Box::new(self),
span,
})
Ok(Value::custom_value(Box::new(self), span))
}
}
@ -147,8 +141,9 @@ impl NuLazyFrame {
}
pub fn get_lazy_df(value: Value) -> Result<Self, ShellError> {
let span = value.span();
match value {
Value::CustomValue { val, span } => match val.as_any().downcast_ref::<Self>() {
Value::CustomValue { val, .. } => match val.as_any().downcast_ref::<Self>() {
Some(expr) => Ok(Self {
lazy: expr.lazy.clone(),
from_eager: false,

View File

@ -18,10 +18,7 @@ impl CustomValue for NuLazyGroupBy {
from_eager: self.from_eager,
};
Value::CustomValue {
val: Box::new(cloned),
span,
}
Value::custom_value(Box::new(cloned), span)
}
fn value_string(&self) -> String {

View File

@ -74,10 +74,7 @@ impl From<LazyGroupBy> for NuLazyGroupBy {
impl NuLazyGroupBy {
pub fn into_value(self, span: Span) -> Value {
Value::CustomValue {
val: Box::new(self),
span,
}
Value::custom_value(Box::new(self), span)
}
pub fn into_polars(self) -> LazyGroupBy {
@ -85,22 +82,21 @@ impl NuLazyGroupBy {
}
pub fn try_from_value(value: Value) -> Result<Self, ShellError> {
let span = value.span();
match value {
Value::CustomValue { val, span } => {
match val.as_any().downcast_ref::<NuLazyGroupBy>() {
Some(group) => Ok(Self {
group_by: group.group_by.clone(),
schema: group.schema.clone(),
from_eager: group.from_eager,
}),
None => Err(ShellError::CantConvert {
to_type: "lazy groupby".into(),
from_type: "custom value".into(),
span,
help: None,
}),
}
}
Value::CustomValue { val, .. } => match val.as_any().downcast_ref::<NuLazyGroupBy>() {
Some(group) => Ok(Self {
group_by: group.group_by.clone(),
schema: group.schema.clone(),
from_eager: group.from_eager,
}),
None => Err(ShellError::CantConvert {
to_type: "lazy groupby".into(),
from_type: "custom value".into(),
span,
help: None,
}),
},
x => Err(ShellError::CantConvert {
to_type: "lazy groupby".into(),
from_type: x.get_type().to_string(),

View File

@ -14,10 +14,7 @@ impl CustomValue for NuWhen {
fn clone_value(&self, span: nu_protocol::Span) -> Value {
let cloned = self.clone();
Value::CustomValue {
val: Box::new(cloned),
span,
}
Value::custom_value(Box::new(cloned), span)
}
fn value_string(&self) -> String {
@ -25,12 +22,12 @@ impl CustomValue for NuWhen {
}
fn to_base_value(&self, span: Span) -> Result<Value, ShellError> {
let val = match self {
let val: String = match self {
NuWhen::Then(_) => "whenthen".into(),
NuWhen::ChainedThen(_) => "whenthenthen".into(),
};
let value = Value::String { val, span };
let value = Value::string(val, span);
Ok(value)
}

View File

@ -51,15 +51,13 @@ impl From<ChainedThen> for NuWhen {
impl NuWhen {
pub fn into_value(self, span: Span) -> Value {
Value::CustomValue {
val: Box::new(self),
span,
}
Value::custom_value(Box::new(self), span)
}
pub fn try_from_value(value: Value) -> Result<Self, ShellError> {
let span = value.span();
match value {
Value::CustomValue { val, span } => match val.as_any().downcast_ref::<Self>() {
Value::CustomValue { val, .. } => match val.as_any().downcast_ref::<Self>() {
Some(expr) => Ok(expr.clone()),
None => Err(ShellError::CantConvert {
to_type: "when expression".into(),

View File

@ -25,18 +25,21 @@ pub(crate) fn convert_columns(
let res = columns
.into_iter()
.map(|value| match value {
Value::String { val, span } => {
col_span = span_join(&[col_span, span]);
Ok(Spanned { item: val, span })
.map(|value| {
let span = value.span();
match value {
Value::String { val, .. } => {
col_span = span_join(&[col_span, span]);
Ok(Spanned { item: val, span })
}
_ => Err(ShellError::GenericError(
"Incorrect column format".into(),
"Only string as column name".into(),
Some(span),
None,
Vec::new(),
)),
}
_ => Err(ShellError::GenericError(
"Incorrect column format".into(),
"Only string as column name".into(),
Some(span),
None,
Vec::new(),
)),
})
.collect::<Result<Vec<Spanned<String>>, _>>()?;
@ -65,18 +68,21 @@ pub(crate) fn convert_columns_string(
let res = columns
.into_iter()
.map(|value| match value {
Value::String { val, span } => {
col_span = span_join(&[col_span, span]);
Ok(val)
.map(|value| {
let span = value.span();
match value {
Value::String { val, .. } => {
col_span = span_join(&[col_span, span]);
Ok(val)
}
_ => Err(ShellError::GenericError(
"Incorrect column format".into(),
"Only string as column name".into(),
Some(span),
None,
Vec::new(),
)),
}
_ => Err(ShellError::GenericError(
"Incorrect column format".into(),
"Only string as column name".into(),
Some(span),
None,
Vec::new(),
)),
})
.collect::<Result<Vec<String>, _>>()?;