From 38bc394a1240ded91fe30bf1e34dab5d173792e6 Mon Sep 17 00:00:00 2001 From: JT Date: Tue, 12 Oct 2021 07:45:31 +1300 Subject: [PATCH 1/2] Expose errors early when possible --- crates/nu-command/src/core_commands/if_.rs | 2 +- crates/nu-command/src/filters/each.rs | 2 +- crates/nu-command/src/formats/from/json.rs | 2 +- crates/nu-command/src/strings/split/chars.rs | 40 ++--- crates/nu-command/src/strings/split/column.rs | 13 +- crates/nu-command/src/strings/split/row.rs | 47 +++--- crates/nu-engine/src/eval.rs | 2 +- crates/nu-engine/src/from_value.rs | 22 +-- crates/nu-protocol/src/value/mod.rs | 153 +++++++++--------- crates/nu-protocol/src/value/range.rs | 24 +-- 10 files changed, 167 insertions(+), 140 deletions(-) diff --git a/crates/nu-command/src/core_commands/if_.rs b/crates/nu-command/src/core_commands/if_.rs index a9b1489a66..fdb103dac0 100644 --- a/crates/nu-command/src/core_commands/if_.rs +++ b/crates/nu-command/src/core_commands/if_.rs @@ -61,7 +61,7 @@ impl Command for If { Ok(Value::Nothing { span }) } } - _ => Err(ShellError::CantConvert("bool".into(), result.span())), + _ => Err(ShellError::CantConvert("bool".into(), result.span()?)), } } } diff --git a/crates/nu-command/src/filters/each.rs b/crates/nu-command/src/filters/each.rs index ce847b9174..39303d005d 100644 --- a/crates/nu-command/src/filters/each.rs +++ b/crates/nu-command/src/filters/each.rs @@ -67,7 +67,7 @@ impl Command for Each { match input { Value::Range { val, .. } => Ok(Value::Stream { stream: val - .into_iter() + .into_range_iter()? .enumerate() .map(move |(idx, x)| { let engine_state = context.engine_state.borrow(); diff --git a/crates/nu-command/src/formats/from/json.rs b/crates/nu-command/src/formats/from/json.rs index f7fe0d5f94..c643f1ba07 100644 --- a/crates/nu-command/src/formats/from/json.rs +++ b/crates/nu-command/src/formats/from/json.rs @@ -71,7 +71,7 @@ impl Command for FromJson { call: &Call, input: Value, ) -> Result { - let span = input.span(); + let span = input.span()?; let mut string_input = input.collect_string(); string_input.push('\n'); diff --git a/crates/nu-command/src/strings/split/chars.rs b/crates/nu-command/src/strings/split/chars.rs index 382b50ee9d..b326c78642 100644 --- a/crates/nu-command/src/strings/split/chars.rs +++ b/crates/nu-command/src/strings/split/chars.rs @@ -68,24 +68,28 @@ fn split_chars(call: &Call, input: Value) -> Result Vec { - if let Ok(s) = v.as_string() { - let v_span = v.span(); - s.chars() - .collect::>() - .into_iter() - .map(move |x| Value::String { - val: x.to_string(), - span: v_span, - }) - .collect() - } else { - vec![Value::Error { - error: ShellError::PipelineMismatch { - expected: Type::String, - expected_span: name, - origin: v.span(), - }, - }] + match v.span() { + Ok(v_span) => { + if let Ok(s) = v.as_string() { + s.chars() + .collect::>() + .into_iter() + .map(move |x| Value::String { + val: x.to_string(), + span: v_span, + }) + .collect() + } else { + vec![Value::Error { + error: ShellError::PipelineMismatch { + expected: Type::String, + expected_span: name, + origin: v_span, + }, + }] + } + } + Err(error) => vec![Value::Error { error }], } } diff --git a/crates/nu-command/src/strings/split/column.rs b/crates/nu-command/src/strings/split/column.rs index c96b48a76b..c0a8e03bbd 100644 --- a/crates/nu-command/src/strings/split/column.rs +++ b/crates/nu-command/src/strings/split/column.rs @@ -109,12 +109,15 @@ fn split_column_helper( span: head, } } else { - Value::Error { - error: ShellError::PipelineMismatch { - expected: Type::String, - expected_span: head, - origin: v.span(), + match v.span() { + Ok(span) => Value::Error { + error: ShellError::PipelineMismatch { + expected: Type::String, + expected_span: head, + origin: span, + }, }, + Err(error) => Value::Error { error }, } } } diff --git a/crates/nu-command/src/strings/split/row.rs b/crates/nu-command/src/strings/split/row.rs index 4467598536..7f8c49a4b2 100644 --- a/crates/nu-command/src/strings/split/row.rs +++ b/crates/nu-command/src/strings/split/row.rs @@ -48,28 +48,33 @@ fn split_row( } fn split_row_helper(v: &Value, separator: &Spanned, name: Span) -> Vec { - if let Ok(s) = v.as_string() { - let splitter = separator.item.replace("\\n", "\n"); - s.split(&splitter) - .filter_map(|s| { - if s.trim() != "" { - Some(Value::String { - val: s.into(), - span: v.span(), + match v.span() { + Ok(v_span) => { + if let Ok(s) = v.as_string() { + let splitter = separator.item.replace("\\n", "\n"); + s.split(&splitter) + .filter_map(|s| { + if s.trim() != "" { + Some(Value::String { + val: s.into(), + span: v_span, + }) + } else { + None + } }) - } else { - None - } - }) - .collect() - } else { - vec![Value::Error { - error: ShellError::PipelineMismatch { - expected: Type::String, - expected_span: name, - origin: v.span(), - }, - }] + .collect() + } else { + vec![Value::Error { + error: ShellError::PipelineMismatch { + expected: Type::String, + expected_span: name, + origin: v_span, + }, + }] + } + } + Err(error) => vec![Value::Error { error }], } } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 16572d8ad8..5edf80cd15 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -45,7 +45,7 @@ fn eval_call(context: &EvaluationContext, call: &Call, input: Value) -> Result { span: *span, }), - v => Err(ShellError::CantConvert("integer".into(), v.span())), + v => Err(ShellError::CantConvert("integer".into(), v.span()?)), } } } @@ -51,7 +51,7 @@ impl FromValue for i64 { *val as i64, ), - v => Err(ShellError::CantConvert("integer".into(), v.span())), + v => Err(ShellError::CantConvert("integer".into(), v.span()?)), } } } @@ -69,7 +69,7 @@ impl FromValue for Spanned { span: *span, }), - v => Err(ShellError::CantConvert("float".into(), v.span())), + v => Err(ShellError::CantConvert("float".into(), v.span()?)), } } } @@ -79,7 +79,7 @@ impl FromValue for f64 { match v { Value::Float { val, .. } => Ok(*val), Value::Int { val, .. } => Ok(*val as f64), - v => Err(ShellError::CantConvert("float".into(), v.span())), + v => Err(ShellError::CantConvert("float".into(), v.span()?)), } } } @@ -95,7 +95,7 @@ impl FromValue for Spanned { fn from_value(v: &Value) -> Result { Ok(Spanned { item: v.clone().into_string(), - span: v.span(), + span: v.span()?, }) } } @@ -115,7 +115,7 @@ impl FromValue for ColumnPath { impl FromValue for CellPath { fn from_value(v: &Value) -> Result { - let span = v.span(); + let span = v.span()?; match v { Value::CellPath { val, .. } => Ok(val.clone()), Value::String { val, .. } => Ok(CellPath { @@ -130,7 +130,7 @@ impl FromValue for CellPath { span, }], }), - v => Err(ShellError::CantConvert("cell path".into(), v.span())), + _ => Err(ShellError::CantConvert("cell path".into(), span)), } } } @@ -139,7 +139,7 @@ impl FromValue for bool { fn from_value(v: &Value) -> Result { match v { Value::Bool { val, .. } => Ok(*val), - v => Err(ShellError::CantConvert("bool".into(), v.span())), + v => Err(ShellError::CantConvert("bool".into(), v.span()?)), } } } @@ -151,7 +151,7 @@ impl FromValue for Spanned { item: *val, span: *span, }), - v => Err(ShellError::CantConvert("bool".into(), v.span())), + v => Err(ShellError::CantConvert("bool".into(), v.span()?)), } } } @@ -182,7 +182,7 @@ impl FromValue for Range { fn from_value(v: &Value) -> Result { match v { Value::Range { val, .. } => Ok((**val).clone()), - v => Err(ShellError::CantConvert("range".into(), v.span())), + v => Err(ShellError::CantConvert("range".into(), v.span()?)), } } } @@ -194,7 +194,7 @@ impl FromValue for Spanned { item: (**val).clone(), span: *span, }), - v => Err(ShellError::CantConvert("range".into(), v.span())), + v => Err(ShellError::CantConvert("range".into(), v.span()?)), } } } diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 8ece8dc39b..c2e9b30803 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -90,29 +90,29 @@ impl Value { pub fn as_string(&self) -> Result { match self { Value::String { val, .. } => Ok(val.to_string()), - _ => Err(ShellError::CantConvert("string".into(), self.span())), + _ => Err(ShellError::CantConvert("string".into(), self.span()?)), } } /// Get the span for the current value - pub fn span(&self) -> Span { + pub fn span(&self) -> Result { match self { - Value::Error { .. } => Span::unknown(), - Value::Bool { span, .. } => *span, - Value::Int { span, .. } => *span, - Value::Float { span, .. } => *span, - Value::Filesize { span, .. } => *span, - Value::Duration { span, .. } => *span, - Value::Date { span, .. } => *span, - Value::Range { span, .. } => *span, - Value::String { span, .. } => *span, - Value::Record { span, .. } => *span, - Value::List { span, .. } => *span, - Value::Block { span, .. } => *span, - Value::Stream { span, .. } => *span, - Value::Nothing { span, .. } => *span, - Value::Binary { span, .. } => *span, - Value::CellPath { span, .. } => *span, + Value::Error { error } => Err(error.clone()), + Value::Bool { span, .. } => Ok(*span), + Value::Int { span, .. } => Ok(*span), + Value::Float { span, .. } => Ok(*span), + Value::Filesize { span, .. } => Ok(*span), + Value::Duration { span, .. } => Ok(*span), + Value::Date { span, .. } => Ok(*span), + Value::Range { span, .. } => Ok(*span), + Value::String { span, .. } => Ok(*span), + Value::Record { span, .. } => Ok(*span), + Value::List { span, .. } => Ok(*span), + Value::Block { span, .. } => Ok(*span), + Value::Stream { span, .. } => Ok(*span), + Value::Nothing { span, .. } => Ok(*span), + Value::Binary { span, .. } => Ok(*span), + Value::CellPath { span, .. } => Ok(*span), } } @@ -173,15 +173,17 @@ impl Value { Value::Filesize { val, .. } => format_filesize(val), Value::Duration { val, .. } => format_duration(val), Value::Date { val, .. } => HumanTime::from(val).to_string(), - Value::Range { val, .. } => { - format!( - "range: [{}]", - val.into_iter() - .map(|x| x.into_string()) - .collect::>() - .join(", ") - ) - } + Value::Range { val, .. } => match val.into_range_iter() { + Ok(iter) => { + format!( + "range: [{}]", + iter.map(|x| x.into_string()) + .collect::>() + .join(", ") + ) + } + Err(error) => format!("{:?}", error), + }, Value::String { val, .. } => val, Value::Stream { stream, .. } => stream.into_string(), Value::List { vals: val, .. } => format!( @@ -215,11 +217,15 @@ impl Value { Value::Filesize { val, .. } => format!("{} bytes", val), Value::Duration { val, .. } => format!("{} ns", val), Value::Date { val, .. } => format!("{:?}", val), - Value::Range { val, .. } => val - .into_iter() - .map(|x| x.into_string()) - .collect::>() - .join(", "), + Value::Range { val, .. } => match val.into_range_iter() { + Ok(iter) => iter + .map(|x| x.into_string()) + .collect::>() + .join(", "), + Err(error) => { + format!("{:?}", error) + } + }, Value::String { val, .. } => val, Value::Stream { stream, .. } => stream.collect_string(), Value::List { vals: val, .. } => val @@ -380,7 +386,7 @@ impl Value { span, }), Value::Range { val, .. } => Ok(Value::Stream { - stream: val.into_iter().map(f).into_value_stream(), + stream: val.into_range_iter()?.map(f).into_value_stream(), span, }), v => { @@ -409,9 +415,12 @@ impl Value { stream: stream.map(f).flatten().into_value_stream(), span, }, - Value::Range { val, .. } => Value::Stream { - stream: val.into_iter().map(f).flatten().into_value_stream(), - span, + Value::Range { val, .. } => match val.into_range_iter() { + Ok(iter) => Value::Stream { + stream: iter.map(f).flatten().into_value_stream(), + span, + }, + Err(error) => Value::Error { error }, }, v => Value::Stream { stream: f(v).into_iter().into_value_stream(), @@ -506,7 +515,7 @@ impl PartialEq for Value { impl Value { pub fn add(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match (self, rhs) { (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Int { @@ -545,14 +554,14 @@ impl Value { _ => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn sub(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match (self, rhs) { (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Int { @@ -587,14 +596,14 @@ impl Value { _ => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn mul(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match (self, rhs) { (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Int { @@ -617,14 +626,14 @@ impl Value { _ => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn div(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match (self, rhs) { (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => { @@ -678,14 +687,14 @@ impl Value { _ => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn lt(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { @@ -695,14 +704,14 @@ impl Value { None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn lte(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { @@ -712,14 +721,14 @@ impl Value { None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn gt(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { @@ -729,14 +738,14 @@ impl Value { None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn gte(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { @@ -746,14 +755,14 @@ impl Value { None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn eq(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { @@ -763,14 +772,14 @@ impl Value { None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn ne(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { @@ -780,15 +789,15 @@ impl Value { None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn r#in(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match (self, rhs) { (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { @@ -814,15 +823,15 @@ impl Value { _ => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } pub fn not_in(&self, op: Span, rhs: &Value) -> Result { - let span = span(&[self.span(), rhs.span()]); + let span = span(&[self.span()?, rhs.span()?]); match (self, rhs) { (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { @@ -848,9 +857,9 @@ impl Value { _ => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), - lhs_span: self.span(), + lhs_span: self.span()?, rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), + rhs_span: rhs.span()?, }), } } diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 8d2bbbb811..14bae6ea13 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -122,20 +122,26 @@ impl Range { (_, _) => false, } } -} -impl IntoIterator for Range { - type Item = Value; + pub fn into_range_iter(self) -> Result { + let span = self.from.span()?; - type IntoIter = RangeIterator; - - fn into_iter(self) -> Self::IntoIter { - let span = self.from.span(); - - RangeIterator::new(self, span) + Ok(RangeIterator::new(self, span)) } } +// impl IntoIterator for Range { +// type Item = Value; + +// type IntoIter = RangeIterator; + +// fn into_iter(self) -> Self::IntoIter { +// let span = self.from.span(); + +// RangeIterator::new(self, span) +// } +// } + pub struct RangeIterator { curr: Value, end: Value, From 576471cc3c52a7caead2e4385c3fec3cbb6000f9 Mon Sep 17 00:00:00 2001 From: JT Date: Tue, 12 Oct 2021 08:33:09 +1300 Subject: [PATCH 2/2] Fix test --- src/tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tests.rs b/src/tests.rs index 7ba1b952a6..f879214721 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -658,3 +658,11 @@ fn string_not_in_string() -> TestResult { fn float_not_in_inc_range() -> TestResult { run_test(r#"1.4 not-in 2..9.42"#, "true") } + +#[test] +fn earlier_errors() -> TestResult { + fail_test( + r#"[1, "bob"] | each { $it + 3 } | each { $it / $it } | table"#, + "int", + ) +}