From eae83d85d27e5518e1b141f6e8dfd8f556392c34 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Sun, 16 Jun 2019 11:03:49 +1200 Subject: [PATCH] Add more error checking --- src/cli.rs | 4 ++- src/commands/clip.rs | 11 +++++++- src/commands/enter.rs | 52 ++++++++++++++++++++++++++++++++---- src/commands/from_json.rs | 28 +++++++++++++------ src/commands/from_toml.rs | 41 ++++++++++++++++++++-------- src/commands/from_xml.rs | 27 ++++++++++--------- src/commands/from_yaml.rs | 28 +++++++++++++------ src/commands/open.rs | 52 ++++++++++++++++++++++++++++++++---- src/commands/split_column.rs | 7 ++++- src/commands/split_row.rs | 10 ++++++- src/commands/to_json.rs | 10 ++++++- src/commands/to_toml.rs | 10 ++++++- src/commands/trim.rs | 7 ++++- 13 files changed, 231 insertions(+), 56 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 162a0a9c78..df448d33e5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -292,7 +292,9 @@ async fn process_line(readline: Result, ctx: &mut Context (Some(ClassifiedCommand::Sink(left)), None) => { let input_vec: Vec = input.objects.collect().await; - left.run(ctx, input_vec)?; + if let Err(err) = left.run(ctx, input_vec) { + return LineResult::Error(line.clone(), err); + } break; } diff --git a/src/commands/clip.rs b/src/commands/clip.rs index 42e2af5027..a55e7119b1 100644 --- a/src/commands/clip.rs +++ b/src/commands/clip.rs @@ -13,7 +13,16 @@ pub fn clip(args: SinkCommandArgs) -> Result<(), ShellError> { } else { first = false; } - new_copy_data.push_str(&i.as_string().unwrap()); + match i.as_string() { + Ok(s) => new_copy_data.push_str(&s), + Err(_) => { + return Err(ShellError::maybe_labeled_error( + "Given non-string data", + "expected strings from pipeline", + args.name_span, + )) + } + } } } clip_context.set_contents(new_copy_data).unwrap(); diff --git a/src/commands/enter.rs b/src/commands/enter.rs index 4506c57073..c36c83807b 100644 --- a/src/commands/enter.rs +++ b/src/commands/enter.rs @@ -14,6 +14,8 @@ pub fn enter(args: CommandArgs) -> Result { )); } + let span = args.name_span; + let cwd = args .env .lock() @@ -118,27 +120,67 @@ pub fn enter(args: CommandArgs) -> Result { match file_extension { Some(x) if x == "toml" => { stream.push_back(ReturnValue::Action(CommandAction::Enter( - crate::commands::from_toml::from_toml_string_to_value(contents), + crate::commands::from_toml::from_toml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not load as TOML", + "could not load as TOML", + span, + ) + }, + )?, ))); } Some(x) if x == "json" => { stream.push_back(ReturnValue::Action(CommandAction::Enter( - crate::commands::from_json::from_json_string_to_value(contents), + crate::commands::from_json::from_json_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not load as JSON", + "could not load as JSON", + span, + ) + }, + )?, ))); } Some(x) if x == "xml" => { stream.push_back(ReturnValue::Action(CommandAction::Enter( - crate::commands::from_xml::from_xml_string_to_value(contents), + crate::commands::from_xml::from_xml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not load as XML", + "could not load as XML", + span, + ) + }, + )?, ))); } Some(x) if x == "yml" => { stream.push_back(ReturnValue::Action(CommandAction::Enter( - crate::commands::from_yaml::from_yaml_string_to_value(contents), + crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not load as YAML", + "could not load as YAML", + span, + ) + }, + )?, ))); } Some(x) if x == "yaml" => { stream.push_back(ReturnValue::Action(CommandAction::Enter( - crate::commands::from_yaml::from_yaml_string_to_value(contents), + crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not load as YAML", + "could not load as YAML", + span, + ) + }, + )?, ))); } _ => { diff --git a/src/commands/from_json.rs b/src/commands/from_json.rs index 1624d9c7e1..b8c796f0d3 100644 --- a/src/commands/from_json.rs +++ b/src/commands/from_json.rs @@ -28,19 +28,31 @@ fn convert_json_value_to_nu_value(v: &serde_hjson::Value) -> Value { } } -pub fn from_json_string_to_value(s: String) -> Value { - let v: serde_hjson::Value = serde_hjson::from_str(&s).unwrap(); - convert_json_value_to_nu_value(&v) +pub fn from_json_string_to_value(s: String) -> serde_hjson::Result { + let v: serde_hjson::Value = serde_hjson::from_str(&s)?; + Ok(convert_json_value_to_nu_value(&v)) } pub fn from_json(args: CommandArgs) -> Result { let out = args.input; + let span = args.name_span; Ok(out - .map(|a| match a { - Value::Primitive(Primitive::String(s)) => { - ReturnValue::Value(from_json_string_to_value(s)) - } - _ => ReturnValue::Value(Value::Primitive(Primitive::String("".to_string()))), + .map(move |a| match a { + Value::Primitive(Primitive::String(s)) => match from_json_string_to_value(s) { + Ok(x) => ReturnValue::Value(x), + Err(_) => { + ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Could not parse as JSON", + "piped data failed JSON parse", + span, + )))) + } + }, + _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Expected string values from pipeline", + "expects strings from pipeline", + span, + )))), }) .boxed()) } diff --git a/src/commands/from_toml.rs b/src/commands/from_toml.rs index 59610a8ffd..2021f503af 100644 --- a/src/commands/from_toml.rs +++ b/src/commands/from_toml.rs @@ -1,5 +1,5 @@ -use crate::object::{Primitive, Value, Dictionary, DataDescriptor}; use crate::object::base::OF64; +use crate::object::{DataDescriptor, Dictionary, Primitive, Value}; use crate::prelude::*; fn convert_toml_value_to_nu_value(v: &toml::Value) -> Value { @@ -8,31 +8,50 @@ fn convert_toml_value_to_nu_value(v: &toml::Value) -> Value { toml::Value::Integer(n) => Value::Primitive(Primitive::Int(*n)), toml::Value::Float(n) => Value::Primitive(Primitive::Float(OF64::from(*n))), toml::Value::String(s) => Value::Primitive(Primitive::String(s.clone())), - toml::Value::Array(a) => Value::List(a.iter().map(|x| convert_toml_value_to_nu_value(x)).collect()), + toml::Value::Array(a) => Value::List( + a.iter() + .map(|x| convert_toml_value_to_nu_value(x)) + .collect(), + ), toml::Value::Datetime(dt) => Value::Primitive(Primitive::String(dt.to_string())), toml::Value::Table(t) => { let mut collected = Dictionary::default(); for (k, v) in t.iter() { - collected.add(DataDescriptor::from(k.clone()), convert_toml_value_to_nu_value(v)); + collected.add( + DataDescriptor::from(k.clone()), + convert_toml_value_to_nu_value(v), + ); } Value::Object(collected) } } } -pub fn from_toml_string_to_value(s: String) -> Value { - let v: toml::Value = s.parse::().unwrap(); - convert_toml_value_to_nu_value(&v) +pub fn from_toml_string_to_value(s: String) -> Result> { + let v: toml::Value = s.parse::()?; + Ok(convert_toml_value_to_nu_value(&v)) } pub fn from_toml(args: CommandArgs) -> Result { let out = args.input; + let span = args.name_span; Ok(out - .map(|a| match a { - Value::Primitive(Primitive::String(s)) => { - ReturnValue::Value(from_toml_string_to_value(s)) - } - _ => ReturnValue::Value(Value::Primitive(Primitive::String("".to_string()))), + .map(move |a| match a { + Value::Primitive(Primitive::String(s)) => match from_toml_string_to_value(s) { + Ok(x) => ReturnValue::Value(x), + Err(_) => { + ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Could not parse as TOML", + "piped data failed TOML parse", + span, + )))) + } + }, + _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Expected string values from pipeline", + "expects strings from pipeline", + span, + )))), }) .boxed()) } diff --git a/src/commands/from_xml.rs b/src/commands/from_xml.rs index 5d82d2657d..d203005382 100644 --- a/src/commands/from_xml.rs +++ b/src/commands/from_xml.rs @@ -46,13 +46,9 @@ fn from_document_to_value(d: &roxmltree::Document) -> Value { from_node_to_value(&d.root_element()) } -pub fn from_xml_string_to_value(s: String) -> Value { - match roxmltree::Document::parse(&s) { - Ok(doc) => from_document_to_value(&doc), - Err(_) => Value::Error(Box::new(ShellError::string( - "Can't convert string from xml".to_string(), - ))), - } +pub fn from_xml_string_to_value(s: String) -> Result> { + let parsed = roxmltree::Document::parse(&s)?; + Ok(from_document_to_value(&parsed)) } pub fn from_xml(args: CommandArgs) -> Result { @@ -60,12 +56,19 @@ pub fn from_xml(args: CommandArgs) -> Result { let span = args.name_span; Ok(out .map(move |a| match a { - Value::Primitive(Primitive::String(s)) => { - ReturnValue::Value(from_xml_string_to_value(s)) - } + Value::Primitive(Primitive::String(s)) => match from_xml_string_to_value(s) { + Ok(x) => ReturnValue::Value(x), + Err(_) => { + ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Could not parse as XML", + "piped data failed XML parse", + span, + )))) + } + }, _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( - "Trying to convert XML from non-string".to_string(), - "given non-string", + "Expected string values from pipeline", + "expects strings from pipeline", span, )))), }) diff --git a/src/commands/from_yaml.rs b/src/commands/from_yaml.rs index 171ab3ae0d..ac725138d9 100644 --- a/src/commands/from_yaml.rs +++ b/src/commands/from_yaml.rs @@ -36,19 +36,31 @@ fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value) -> Value { } } -pub fn from_yaml_string_to_value(s: String) -> Value { - let v: serde_yaml::Value = serde_yaml::from_str(&s).unwrap(); - convert_yaml_value_to_nu_value(&v) +pub fn from_yaml_string_to_value(s: String) -> serde_yaml::Result { + let v: serde_yaml::Value = serde_yaml::from_str(&s)?; + Ok(convert_yaml_value_to_nu_value(&v)) } pub fn from_yaml(args: CommandArgs) -> Result { let out = args.input; + let span = args.name_span; Ok(out - .map(|a| match a { - Value::Primitive(Primitive::String(s)) => { - ReturnValue::Value(from_yaml_string_to_value(s)) - } - _ => ReturnValue::Value(Value::Primitive(Primitive::String("".to_string()))), + .map(move |a| match a { + Value::Primitive(Primitive::String(s)) => match from_yaml_string_to_value(s) { + Ok(x) => ReturnValue::Value(x), + Err(_) => { + ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Could not parse as YAML", + "piped data failed YAML parse", + span, + )))) + } + }, + _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Expected string values from pipeline", + "expects strings from pipeline", + span, + )))), }) .boxed()) } diff --git a/src/commands/open.rs b/src/commands/open.rs index 03abccf0fd..b085b13c3b 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -13,6 +13,8 @@ pub fn open(args: CommandArgs) -> Result { )); } + let span = args.name_span; + let cwd = args .env .lock() @@ -117,27 +119,67 @@ pub fn open(args: CommandArgs) -> Result { match file_extension { Some(x) if x == "toml" => { stream.push_back(ReturnValue::Value( - crate::commands::from_toml::from_toml_string_to_value(contents), + crate::commands::from_toml::from_toml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not open as TOML", + "could not open as TOML", + span, + ) + }, + )?, )); } Some(x) if x == "json" => { stream.push_back(ReturnValue::Value( - crate::commands::from_json::from_json_string_to_value(contents), + crate::commands::from_json::from_json_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not open as JSON", + "could not open as JSON", + span, + ) + }, + )?, )); } Some(x) if x == "xml" => { stream.push_back(ReturnValue::Value( - crate::commands::from_xml::from_xml_string_to_value(contents), + crate::commands::from_xml::from_xml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not open as XML", + "could not open as XML", + span, + ) + }, + )?, )); } Some(x) if x == "yml" => { stream.push_back(ReturnValue::Value( - crate::commands::from_yaml::from_yaml_string_to_value(contents), + crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not open as YAML", + "could not open as YAML", + span, + ) + }, + )?, )); } Some(x) if x == "yaml" => { stream.push_back(ReturnValue::Value( - crate::commands::from_yaml::from_yaml_string_to_value(contents), + crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err( + move |_| { + ShellError::maybe_labeled_error( + "Could not open as YAML", + "could not open as YAML", + span, + ) + }, + )?, )); } _ => { diff --git a/src/commands/split_column.rs b/src/commands/split_column.rs index c768ec6b83..429b3dab07 100644 --- a/src/commands/split_column.rs +++ b/src/commands/split_column.rs @@ -7,6 +7,7 @@ use log::trace; pub fn split_column(args: CommandArgs) -> Result { let input = args.input; + let span = args.name_span; let args = args.positional; Ok(input @@ -53,7 +54,11 @@ pub fn split_column(args: CommandArgs) -> Result { ReturnValue::Value(Value::Object(dict)) } } - _ => ReturnValue::Value(Value::Object(crate::object::Dictionary::default())), + _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Expected string values from pipeline", + "expects strings from pipeline", + span, + )))), }) .boxed()) } diff --git a/src/commands/split_row.rs b/src/commands/split_row.rs index fead9f8e1f..79b12adbfb 100644 --- a/src/commands/split_row.rs +++ b/src/commands/split_row.rs @@ -7,6 +7,7 @@ use log::trace; pub fn split_row(args: CommandArgs) -> Result { let input = args.input; + let span = args.name_span; let args = args.positional; let stream = input @@ -27,7 +28,14 @@ pub fn split_row(args: CommandArgs) -> Result { result } _ => { - let result = VecDeque::new(); + let mut result = VecDeque::new(); + result.push_back(ReturnValue::Value(Value::Error(Box::new( + ShellError::maybe_labeled_error( + "Expected string values from pipeline", + "expects strings from pipeline", + span, + ), + )))); result } }) diff --git a/src/commands/to_json.rs b/src/commands/to_json.rs index fa8aadd0a6..6d245c6e18 100644 --- a/src/commands/to_json.rs +++ b/src/commands/to_json.rs @@ -3,7 +3,15 @@ use crate::prelude::*; pub fn to_json(args: CommandArgs) -> Result { let out = args.input; + let span = args.name_span; Ok(out - .map(|a| ReturnValue::Value(Value::Primitive(Primitive::String(serde_json::to_string(&a).unwrap())))) + .map(move |a| match serde_json::to_string(&a) { + Ok(x) => ReturnValue::Value(Value::Primitive(Primitive::String(x))), + Err(_) => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Can not convert to JSON string", + "can not convert piped data to JSON string", + span, + )))), + }) .boxed()) } diff --git a/src/commands/to_toml.rs b/src/commands/to_toml.rs index ac0fe05a6d..497276223c 100644 --- a/src/commands/to_toml.rs +++ b/src/commands/to_toml.rs @@ -3,7 +3,15 @@ use crate::prelude::*; pub fn to_toml(args: CommandArgs) -> Result { let out = args.input; + let span = args.name_span; Ok(out - .map(|a| ReturnValue::Value(Value::Primitive(Primitive::String(toml::to_string(&a).unwrap())))) + .map(move |a| match toml::to_string(&a) { + Ok(x) => ReturnValue::Value(Value::Primitive(Primitive::String(x))), + Err(_) => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Can not convert to TOML string", + "can not convert piped data to TOML string", + span, + )))), + }) .boxed()) } diff --git a/src/commands/trim.rs b/src/commands/trim.rs index 2974b4e65b..35887f6521 100644 --- a/src/commands/trim.rs +++ b/src/commands/trim.rs @@ -6,13 +6,18 @@ use crate::prelude::*; pub fn trim(args: CommandArgs) -> Result { let input = args.input; + let span = args.name_span; Ok(input .map(move |v| match v { Value::Primitive(Primitive::String(s)) => { ReturnValue::Value(Value::Primitive(Primitive::String(s.trim().to_string()))) } - x => ReturnValue::Value(x), + _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error( + "Expected string values from pipeline", + "expects strings from pipeline", + span, + )))), }) .boxed()) }