diff --git a/Cargo.lock b/Cargo.lock index d06959454a..354e8319fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5104,6 +5104,7 @@ version = "1.0.112" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4d1bd37ce2324cf3bf85e5a25f96eb4baf0d5aa6eba43e7ae8958870c4ec48ed" dependencies = [ + "indexmap", "itoa", "ryu", "serde", diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml index 991a8bdb2e..cca111bc51 100644 --- a/crates/nu-command/Cargo.toml +++ b/crates/nu-command/Cargo.toml @@ -75,7 +75,7 @@ roxmltree = "0.18" rusqlite = { version = "0.29", features = ["bundled", "backup", "chrono"], optional = true } same-file = "1.0" serde = { version = "1.0", features = ["derive"] } -serde_json = "1.0" +serde_json = { version = "1.0", features = ["preserve_order"] } serde_urlencoded = "0.7" serde_yaml = "0.9" sha2 = "0.10" diff --git a/crates/nu-command/src/formats/from/json.rs b/crates/nu-command/src/formats/from/json.rs index d55fa6c09a..af6dc13357 100644 --- a/crates/nu-command/src/formats/from/json.rs +++ b/crates/nu-command/src/formats/from/json.rs @@ -22,6 +22,7 @@ impl Command for FromJson { Signature::build("from json") .input_output_types(vec![(Type::String, Type::Any)]) .switch("objects", "treat each line as a separate value", Some('o')) + .switch("strict", "follow the json specification exactly", Some('s')) .category(Category::Formats) } @@ -42,6 +43,14 @@ impl Command for FromJson { "b" => Value::test_list(vec![Value::test_int(1), Value::test_int(2)]), })), }, + Example { + example: r#"'{ "a": 1, "b": 2 }' | from json -s"#, + description: "Parse json strictly which will error on comments and trailing commas", + result: Some(Value::test_record(record! { + "a" => Value::test_int(1), + "b" => Value::test_int(2), + })), + }, ] } @@ -59,52 +68,61 @@ impl Command for FromJson { return Ok(PipelineData::new_with_metadata(metadata, span)); } + let strict = call.has_flag(engine_state, stack, "strict")?; + // TODO: turn this into a structured underline of the nu_json error if call.has_flag(engine_state, stack, "objects")? { - let converted_lines: Vec = string_input - .lines() - .filter_map(move |x| { - if x.trim() == "" { - None - } else { - match convert_string_to_value(x.to_string(), span) { - Ok(v) => Some(v), - Err(error) => Some(Value::error(error, span)), - } - } - }) - .collect(); + let lines = string_input.lines().filter(|line| !line.trim().is_empty()); + + let converted_lines: Vec<_> = if strict { + lines + .map(|line| { + convert_string_to_value_strict(line, span) + .unwrap_or_else(|err| Value::error(err, span)) + }) + .collect() + } else { + lines + .map(|line| { + convert_string_to_value(line, span) + .unwrap_or_else(|err| Value::error(err, span)) + }) + .collect() + }; + Ok(converted_lines .into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone())) + } else if strict { + Ok(convert_string_to_value_strict(&string_input, span)? + .into_pipeline_data_with_metadata(metadata)) } else { - Ok(convert_string_to_value(string_input, span)? + Ok(convert_string_to_value(&string_input, span)? .into_pipeline_data_with_metadata(metadata)) } } } -fn convert_nujson_to_value(value: &nu_json::Value, span: Span) -> Value { +fn convert_nujson_to_value(value: nu_json::Value, span: Span) -> Value { match value { - nu_json::Value::Array(array) => { - let v: Vec = array - .iter() + nu_json::Value::Array(array) => Value::list( + array + .into_iter() .map(|x| convert_nujson_to_value(x, span)) - .collect(); - - Value::list(v, span) - } - nu_json::Value::Bool(b) => Value::bool(*b, span), - nu_json::Value::F64(f) => Value::float(*f, span), - nu_json::Value::I64(i) => Value::int(*i, span), + .collect(), + span, + ), + nu_json::Value::Bool(b) => Value::bool(b, span), + nu_json::Value::F64(f) => Value::float(f, span), + nu_json::Value::I64(i) => Value::int(i, span), nu_json::Value::Null => Value::nothing(span), nu_json::Value::Object(k) => Value::record( - k.iter() - .map(|(k, v)| (k.clone(), convert_nujson_to_value(v, span))) + k.into_iter() + .map(|(k, v)| (k, convert_nujson_to_value(v, span))) .collect(), span, ), nu_json::Value::U64(u) => { - if *u > i64::MAX as u64 { + if u > i64::MAX as u64 { Value::error( ShellError::CantConvert { to_type: "i64 sized integer".into(), @@ -115,22 +133,22 @@ fn convert_nujson_to_value(value: &nu_json::Value, span: Span) -> Value { span, ) } else { - Value::int(*u as i64, span) + Value::int(u as i64, span) } } - nu_json::Value::String(s) => Value::string(s.clone(), span), + nu_json::Value::String(s) => Value::string(s, span), } } // Converts row+column to a Span, assuming bytes (1-based rows) fn convert_row_column_to_span(row: usize, col: usize, contents: &str) -> Span { let mut cur_row = 1; - let mut cur_col = 0; + let mut cur_col = 1; for (offset, curr_byte) in contents.bytes().enumerate() { if curr_byte == b'\n' { cur_row += 1; - cur_col = 0; + cur_col = 1; } if cur_row >= row && cur_col >= col { return Span::new(offset, offset); @@ -142,22 +160,21 @@ fn convert_row_column_to_span(row: usize, col: usize, contents: &str) -> Span { Span::new(contents.len(), contents.len()) } -fn convert_string_to_value(string_input: String, span: Span) -> Result { - let result: Result = nu_json::from_str(&string_input); - match result { - Ok(value) => Ok(convert_nujson_to_value(&value, span)), +fn convert_string_to_value(string_input: &str, span: Span) -> Result { + match nu_json::from_str(string_input) { + Ok(value) => Ok(convert_nujson_to_value(value, span)), Err(x) => match x { nu_json::Error::Syntax(_, row, col) => { let label = x.to_string(); - let label_span = convert_row_column_to_span(row, col, &string_input); + let label_span = convert_row_column_to_span(row, col, string_input); Err(ShellError::GenericError { error: "Error while parsing JSON text".into(), msg: "error parsing JSON text".into(), span: Some(span), help: None, inner: vec![ShellError::OutsideSpannedLabeledError { - src: string_input, + src: string_input.into(), error: "Error while parsing JSON text".into(), msg: label, span: label_span, @@ -174,6 +191,35 @@ fn convert_string_to_value(string_input: String, span: Span) -> Result Result { + match serde_json::from_str(string_input) { + Ok(value) => Ok(convert_nujson_to_value(value, span)), + Err(err) => Err(if err.is_syntax() { + let label = err.to_string(); + let label_span = convert_row_column_to_span(err.line(), err.column(), string_input); + ShellError::GenericError { + error: "Error while parsing JSON text".into(), + msg: "error parsing JSON text".into(), + span: Some(span), + help: None, + inner: vec![ShellError::OutsideSpannedLabeledError { + src: string_input.into(), + error: "Error while parsing JSON text".into(), + msg: label, + span: label_span, + }], + } + } else { + ShellError::CantConvert { + to_type: format!("structured json data ({err})"), + from_type: "string".into(), + span, + help: None, + } + }), + } +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-command/tests/format_conversions/json.rs b/crates/nu-command/tests/format_conversions/json.rs index 2ca0e67828..1938d6d66e 100644 --- a/crates/nu-command/tests/format_conversions/json.rs +++ b/crates/nu-command/tests/format_conversions/json.rs @@ -43,6 +43,32 @@ fn from_json_text_to_table() { }) } +#[test] +fn from_json_text_to_table_strict() { + Playground::setup("filter_from_json_test_1_strict", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContentToBeTrimmed( + "katz.txt", + r#" + { + "katz": [ + {"name": "Yehuda", "rusty_luck": 1}, + {"name": "JT", "rusty_luck": 1}, + {"name": "Andres", "rusty_luck": 1}, + {"name":"GorbyPuff", "rusty_luck": 1} + ] + } + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), + "open katz.txt | from json -s | get katz | get rusty_luck | length " + ); + + assert_eq!(actual.out, "4"); + }) +} + #[test] fn from_json_text_recognizing_objects_independently_to_table() { Playground::setup("filter_from_json_test_2", |dirs, sandbox| { @@ -70,6 +96,33 @@ fn from_json_text_recognizing_objects_independently_to_table() { }) } +#[test] +fn from_json_text_recognizing_objects_independently_to_table_strict() { + Playground::setup("filter_from_json_test_2_strict", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContentToBeTrimmed( + "katz.txt", + r#" + {"name": "Yehuda", "rusty_luck": 1} + {"name": "JT", "rusty_luck": 1} + {"name": "Andres", "rusty_luck": 1} + {"name":"GorbyPuff", "rusty_luck": 3} + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + open katz.txt + | from json -o -s + | where name == "GorbyPuff" + | get rusty_luck.0 + "# + )); + + assert_eq!(actual.out, "3"); + }) +} + #[test] fn table_to_json_text() { Playground::setup("filter_to_json_test", |dirs, sandbox| { @@ -99,6 +152,35 @@ fn table_to_json_text() { }) } +#[test] +fn table_to_json_text_strict() { + Playground::setup("filter_to_json_test_strict", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContentToBeTrimmed( + "sample.txt", + r#" + JonAndrehudaTZ,3 + GorbyPuff,100 + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + open sample.txt + | lines + | split column "," name luck + | select name + | to json + | from json -s + | get 0 + | get name + "# + )); + + assert_eq!(actual.out, "JonAndrehudaTZ"); + }) +} + #[test] fn top_level_values_from_json() { for (value, type_name) in [("null", "nothing"), ("true", "bool"), ("false", "bool")] { @@ -109,6 +191,28 @@ fn top_level_values_from_json() { } } +#[test] +fn top_level_values_from_json_strict() { + for (value, type_name) in [("null", "nothing"), ("true", "bool"), ("false", "bool")] { + let actual = nu!(r#""{}" | from json -s | to json"#, value); + assert_eq!(actual.out, value); + let actual = nu!(r#""{}" | from json -s | describe"#, value); + assert_eq!(actual.out, type_name); + } +} + +#[test] +fn strict_parsing_fails_on_comment() { + let actual = nu!(r#"'{ "a": 1, /* comment */ "b": 2 }' | from json -s"#); + assert!(actual.err.contains("error parsing JSON text")); +} + +#[test] +fn strict_parsing_fails_on_trailing_comma() { + let actual = nu!(r#"'{ "a": 1, "b": 2, }' | from json -s"#); + assert!(actual.err.contains("error parsing JSON text")); +} + #[test] fn ranges_to_json_as_array() { let value = r#"[ 1, 2, 3]"#; diff --git a/crates/nu-json/src/de.rs b/crates/nu-json/src/de.rs index 76fff8c2b3..6fcfbbf7d7 100644 --- a/crates/nu-json/src/de.rs +++ b/crates/nu-json/src/de.rs @@ -26,15 +26,6 @@ pub struct Deserializer> { state: State, } -// macro_rules! try_or_invalid { -// ($self_:expr, $e:expr) => { -// match $e { -// Some(v) => v, -// None => { return Err($self_.error(ErrorCode::InvalidNumber)); } -// } -// } -// } - impl Deserializer where Iter: Iterator,