From 18f96c82b1d0f9a687b8b2eac898df58c6acf57f Mon Sep 17 00:00:00 2001 From: tofpie Date: Sat, 12 Dec 2020 10:22:48 +0100 Subject: [PATCH 1/8] Handle space argument for JSON.stringify() --- boa/src/builtins/json/mod.rs | 61 +++++++++++++++++++-- boa/src/builtins/json/tests.rs | 96 ++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 4 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 9567bd085c7..6b8a58c5f97 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -19,7 +19,8 @@ use crate::{ property::{Attribute, DataDescriptor, PropertyKey}, BoaProfiler, Context, Result, Value, }; -use serde_json::{self, Value as JSONValue}; +use serde::Serialize; +use serde_json::{self, ser::PrettyFormatter, Serializer, Value as JSONValue}; #[cfg(test)] mod tests; @@ -144,6 +145,34 @@ impl Json { Some(replacer) if replacer.is_object() => replacer, _ => return Ok(Value::from(object.to_json(context)?.to_string())), }; + const SPACE_INDENT: &str = " "; + let space = match args.get(2) { + Some(indent) if indent.is_number() => { + if let Some(indent_length) = indent.as_number() { + let indent_length = if indent_length > 10.0 { + 10.0 + } else { + indent_length + }; + let indent_length = if indent_length < 0.0 || indent_length.is_nan() { + 0.0 + } else { + indent_length + }; + &SPACE_INDENT[..indent_length as usize] + } else { + "" + } + } + Some(space) if space.is_string() => { + if let Some(space) = space.as_string() { + &space[..std::cmp::min(space.len(), 10)] + } else { + "" + } + } + _ => "", + }; let replacer_as_object = replacer .as_object() @@ -172,7 +201,10 @@ impl Json { ), ); } - Ok(Value::from(object_to_return.to_json(context)?.to_string())) + Ok(Value::from(json_to_pretty_string( + &object_to_return.to_json(context)?, + space, + ))) }) .ok_or_else(Value::undefined)? } else if replacer_as_object.is_array() { @@ -195,9 +227,30 @@ impl Json { obj_to_return.insert(field.to_string(context)?.to_string(), value); } } - Ok(Value::from(JSONValue::Object(obj_to_return).to_string())) + Ok(Value::from(json_to_pretty_string( + &JSONValue::Object(obj_to_return), + space, + ))) } else { - Ok(Value::from(object.to_json(context)?.to_string())) + Ok(Value::from(json_to_pretty_string( + &object.to_json(context)?, + space, + ))) } } } + +fn json_to_pretty_string(json: &JSONValue, space: &str) -> String { + if space.len() == 0 { + return json.to_string(); + } + let formatter = PrettyFormatter::with_indent(space.as_bytes()); + let mut writer = Vec::with_capacity(128); + let mut serializer = Serializer::with_formatter(&mut writer, formatter); + json.serialize(&mut serializer) + .expect("JSON serialization failed"); + unsafe { + // The serde json serializer always produce correct UTF-8 + String::from_utf8_unchecked(writer) + } +} diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 22e06e5dd65..a895355cfa5 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -207,6 +207,102 @@ fn json_stringify_fractional_numbers() { assert_eq!(actual, expected); } +#[test] +fn json_stringify_pretty_print() { + let mut context = Context::new(); + + let actual = forward(&mut context, r#"JSON.stringify({a: "b", b: "c"}, {}, 4)"#); + let expected = forward( + &mut context, + r#"'{ + "a": "b", + "b": "c" +}'"#, + ); + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_pretty_print_four_spaces() { + let mut context = Context::new(); + + let actual = forward(&mut context, r#"JSON.stringify({a: "b", b: "c"}, {}, 4.3)"#); + let expected = forward( + &mut context, + r#"'{ + "a": "b", + "b": "c" +}'"#, + ); + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_pretty_print_twenty_spaces() { + let mut context = Context::new(); + + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, ["a", "b"], 20)"#, + ); + let expected = forward( + &mut context, + r#"'{ + "a": "b", + "b": "c" +}'"#, + ); + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_pretty_print_bad_space_argument() { + let mut context = Context::new(); + + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, ["a", "b"], [])"#, + ); + let expected = forward(&mut context, r#"'{"a":"b","b":"c"}'"#); + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_pretty_print_with_string() { + let mut context = Context::new(); + + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, {}, "abcd")"#, + ); + let expected = forward( + &mut context, + r#"'{ +abcd"a": "b", +abcd"b": "c" +}'"#, + ); + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_pretty_print_with_too_long_string() { + let mut context = Context::new(); + + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, {}, "abcdefghijklmn")"#, + ); + let expected = forward( + &mut context, + r#"'{ +abcdefghij"a": "b", +abcdefghij"b": "c" +}'"#, + ); + assert_eq!(actual, expected); +} + #[test] fn json_parse_array_with_reviver() { let mut context = Context::new(); From cf073a266524efa3bdda3e593ccb87db2b8c0b70 Mon Sep 17 00:00:00 2001 From: tofpie Date: Sat, 12 Dec 2020 11:52:58 +0100 Subject: [PATCH 2/8] Add feature serde for bench --- boa/Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/boa/Cargo.toml b/boa/Cargo.toml index 9c95f143133..76a2aae405f 100644 --- a/boa/Cargo.toml +++ b/boa/Cargo.toml @@ -50,11 +50,14 @@ bench = false [[bench]] name = "parser" harness = false +required-features = ["serde"] [[bench]] name = "exec" harness = false +required-features = ["serde"] [[bench]] name = "full" harness = false +required-features = ["serde"] From 01183395e447e8286e43cfb23b96392f0e813207 Mon Sep 17 00:00:00 2001 From: tofpie Date: Sat, 12 Dec 2020 11:56:19 +0100 Subject: [PATCH 3/8] Fix clippy error --- boa/src/builtins/json/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 6b8a58c5f97..b07b7f95395 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -241,7 +241,7 @@ impl Json { } fn json_to_pretty_string(json: &JSONValue, space: &str) -> String { - if space.len() == 0 { + if space.is_empty() { return json.to_string(); } let formatter = PrettyFormatter::with_indent(space.as_bytes()); From cc5c35911af691aa571385a78ffa4e8f2eb0019f Mon Sep 17 00:00:00 2001 From: tofpie Date: Sat, 12 Dec 2020 17:56:22 +0100 Subject: [PATCH 4/8] Fix issue with early return when replacer is undefined --- boa/src/builtins/json/mod.rs | 13 +++++++++---- boa/src/builtins/json/tests.rs | 5 ++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index b07b7f95395..a92851cb62d 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -141,10 +141,6 @@ impl Json { None => return Ok(Value::undefined()), Some(obj) => obj, }; - let replacer = match args.get(1) { - Some(replacer) if replacer.is_object() => replacer, - _ => return Ok(Value::from(object.to_json(context)?.to_string())), - }; const SPACE_INDENT: &str = " "; let space = match args.get(2) { Some(indent) if indent.is_number() => { @@ -173,6 +169,15 @@ impl Json { } _ => "", }; + let replacer = match args.get(1) { + Some(replacer) if replacer.is_object() => replacer, + _ => { + return Ok(Value::from(json_to_pretty_string( + &object.to_json(context)?, + space, + ))) + } + }; let replacer_as_object = replacer .as_object() diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index a895355cfa5..7b201f498a8 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -226,7 +226,10 @@ fn json_stringify_pretty_print() { fn json_stringify_pretty_print_four_spaces() { let mut context = Context::new(); - let actual = forward(&mut context, r#"JSON.stringify({a: "b", b: "c"}, {}, 4.3)"#); + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, undefined, 4.3)"#, + ); let expected = forward( &mut context, r#"'{ From 788b5a3c7b251c0a3e80b45f99567ddb5675b878 Mon Sep 17 00:00:00 2001 From: tofpie <75836434+tofpie@users.noreply.github.com> Date: Sat, 12 Dec 2020 18:10:50 +0100 Subject: [PATCH 5/8] Use undefined as replacer in all tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Borges --- boa/src/builtins/json/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 7b201f498a8..9298f71f856 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -211,7 +211,7 @@ fn json_stringify_fractional_numbers() { fn json_stringify_pretty_print() { let mut context = Context::new(); - let actual = forward(&mut context, r#"JSON.stringify({a: "b", b: "c"}, {}, 4)"#); + let actual = forward(&mut context, r#"JSON.stringify({a: "b", b: "c"}, undefined, 4)"#); let expected = forward( &mut context, r#"'{ @@ -276,7 +276,7 @@ fn json_stringify_pretty_print_with_string() { let actual = forward( &mut context, - r#"JSON.stringify({a: "b", b: "c"}, {}, "abcd")"#, + r#"JSON.stringify({a: "b", b: "c"}, undefined, "abcd")"#, ); let expected = forward( &mut context, @@ -294,7 +294,7 @@ fn json_stringify_pretty_print_with_too_long_string() { let actual = forward( &mut context, - r#"JSON.stringify({a: "b", b: "c"}, {}, "abcdefghijklmn")"#, + r#"JSON.stringify({a: "b", b: "c"}, undefined, "abcdefghijklmn")"#, ); let expected = forward( &mut context, From ca9b3ba86343baf4edac4151e072953fc94b2248 Mon Sep 17 00:00:00 2001 From: tofpie Date: Sat, 12 Dec 2020 18:21:17 +0100 Subject: [PATCH 6/8] Fix formatting --- boa/src/builtins/json/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 9298f71f856..c18d9b2211c 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -211,7 +211,10 @@ fn json_stringify_fractional_numbers() { fn json_stringify_pretty_print() { let mut context = Context::new(); - let actual = forward(&mut context, r#"JSON.stringify({a: "b", b: "c"}, undefined, 4)"#); + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, undefined, 4)"#, + ); let expected = forward( &mut context, r#"'{ From 4aa00edfbf198aa3aafe02f34671bffc9da080e7 Mon Sep 17 00:00:00 2001 From: tofpie Date: Mon, 14 Dec 2020 18:10:49 +0100 Subject: [PATCH 7/8] Add support for String and Number object in space argument --- boa/src/builtins/json/mod.rs | 70 ++++++++++++++++++---------------- boa/src/builtins/json/tests.rs | 34 +++++++++++++---- 2 files changed, 63 insertions(+), 41 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index a92851cb62d..5e819dc259d 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -142,39 +142,43 @@ impl Json { Some(obj) => obj, }; const SPACE_INDENT: &str = " "; - let space = match args.get(2) { - Some(indent) if indent.is_number() => { - if let Some(indent_length) = indent.as_number() { - let indent_length = if indent_length > 10.0 { - 10.0 - } else { - indent_length - }; - let indent_length = if indent_length < 0.0 || indent_length.is_nan() { - 0.0 - } else { - indent_length - }; - &SPACE_INDENT[..indent_length as usize] - } else { - "" - } - } - Some(space) if space.is_string() => { - if let Some(space) = space.as_string() { - &space[..std::cmp::min(space.len(), 10)] - } else { - "" - } - } - _ => "", + let space = args + .get(2) + .and_then(|value| match value { + Value::String(s) => Some(s.clone()), + Value::Object(ref object) => object.borrow().as_string(), + _ => None, + }) + .unwrap_or_default(); + let space_len = args + .get(2) + .and_then(|value| match value { + Value::Rational(f) => Some(*f), + Value::Integer(i) => Some((*i).into()), + Value::Object(ref object) => object.borrow().as_number(), + _ => None, + }) + .unwrap_or_default(); + let space_len = if space_len < 0.0 || space_len.is_nan() { + 0 + } else if space_len > 10.0 { + 10 + } else { + space_len as usize }; + + let gap = if space_len > 0 { + &SPACE_INDENT[..space_len] + } else { + &space[..std::cmp::min(space.len(), 10)] + }; + let replacer = match args.get(1) { Some(replacer) if replacer.is_object() => replacer, _ => { return Ok(Value::from(json_to_pretty_string( &object.to_json(context)?, - space, + gap, ))) } }; @@ -208,7 +212,7 @@ impl Json { } Ok(Value::from(json_to_pretty_string( &object_to_return.to_json(context)?, - space, + gap, ))) }) .ok_or_else(Value::undefined)? @@ -234,22 +238,22 @@ impl Json { } Ok(Value::from(json_to_pretty_string( &JSONValue::Object(obj_to_return), - space, + gap, ))) } else { Ok(Value::from(json_to_pretty_string( &object.to_json(context)?, - space, + gap, ))) } } } -fn json_to_pretty_string(json: &JSONValue, space: &str) -> String { - if space.is_empty() { +fn json_to_pretty_string(json: &JSONValue, gap: &str) -> String { + if gap.is_empty() { return json.to_string(); } - let formatter = PrettyFormatter::with_indent(space.as_bytes()); + let formatter = PrettyFormatter::with_indent(gap.as_bytes()); let mut writer = Vec::with_capacity(128); let mut serializer = Serializer::with_formatter(&mut writer, formatter); json.serialize(&mut serializer) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index c18d9b2211c..c08c5bba84e 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -261,6 +261,24 @@ fn json_stringify_pretty_print_twenty_spaces() { assert_eq!(actual, expected); } +#[test] +fn json_stringify_pretty_print_with_number_object() { + let mut context = Context::new(); + + let actual = forward( + &mut context, + r#"JSON.stringify({a: "b", b: "c"}, undefined, new Number(10))"#, + ); + let expected = forward( + &mut context, + r#"'{ + "a": "b", + "b": "c" +}'"#, + ); + assert_eq!(actual, expected); +} + #[test] fn json_stringify_pretty_print_bad_space_argument() { let mut context = Context::new(); @@ -274,36 +292,36 @@ fn json_stringify_pretty_print_bad_space_argument() { } #[test] -fn json_stringify_pretty_print_with_string() { +fn json_stringify_pretty_print_with_too_long_string() { let mut context = Context::new(); let actual = forward( &mut context, - r#"JSON.stringify({a: "b", b: "c"}, undefined, "abcd")"#, + r#"JSON.stringify({a: "b", b: "c"}, undefined, "abcdefghijklmn")"#, ); let expected = forward( &mut context, r#"'{ -abcd"a": "b", -abcd"b": "c" +abcdefghij"a": "b", +abcdefghij"b": "c" }'"#, ); assert_eq!(actual, expected); } #[test] -fn json_stringify_pretty_print_with_too_long_string() { +fn json_stringify_pretty_print_with_string_object() { let mut context = Context::new(); let actual = forward( &mut context, - r#"JSON.stringify({a: "b", b: "c"}, undefined, "abcdefghijklmn")"#, + r#"JSON.stringify({a: "b", b: "c"}, undefined, new String("abcd"))"#, ); let expected = forward( &mut context, r#"'{ -abcdefghij"a": "b", -abcdefghij"b": "c" +abcd"a": "b", +abcd"b": "c" }'"#, ); assert_eq!(actual, expected); From 4aec630fdf3c2187a666904d83dff7190aa9faf9 Mon Sep 17 00:00:00 2001 From: tofpie Date: Wed, 16 Dec 2020 15:18:45 +0100 Subject: [PATCH 8/8] Use an approach that follows the spec more strictly --- boa/src/builtins/json/mod.rs | 55 ++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 5e819dc259d..000b3378fa1 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -17,6 +17,7 @@ use crate::{ builtins::BuiltIn, object::ObjectInitializer, property::{Attribute, DataDescriptor, PropertyKey}, + value::IntegerOrInfinity, BoaProfiler, Context, Result, Value, }; use serde::Serialize; @@ -142,36 +143,36 @@ impl Json { Some(obj) => obj, }; const SPACE_INDENT: &str = " "; - let space = args - .get(2) - .and_then(|value| match value { - Value::String(s) => Some(s.clone()), - Value::Object(ref object) => object.borrow().as_string(), - _ => None, - }) - .unwrap_or_default(); - let space_len = args - .get(2) - .and_then(|value| match value { - Value::Rational(f) => Some(*f), - Value::Integer(i) => Some((*i).into()), - Value::Object(ref object) => object.borrow().as_number(), - _ => None, - }) - .unwrap_or_default(); - let space_len = if space_len < 0.0 || space_len.is_nan() { - 0 - } else if space_len > 10.0 { - 10 + let gap = if let Some(space) = args.get(2) { + let space = if let Some(space_obj) = space.as_object() { + if let Some(space) = space_obj.borrow().as_number() { + Value::from(space) + } else if let Some(space) = space_obj.borrow().as_string() { + Value::from(space) + } else { + space.clone() + } + } else { + space.clone() + }; + if space.is_number() { + let space_mv = match space.to_integer_or_infinity(context)? { + IntegerOrInfinity::NegativeInfinity => 0, + IntegerOrInfinity::PositiveInfinity => 10, + IntegerOrInfinity::Integer(i) if i < 1 => 0, + IntegerOrInfinity::Integer(i) => std::cmp::min(i, 10) as usize, + }; + Value::from(&SPACE_INDENT[..space_mv]) + } else if let Some(string) = space.as_string() { + Value::from(&string[..std::cmp::min(string.len(), 10)]) + } else { + Value::from("") + } } else { - space_len as usize + Value::from("") }; - let gap = if space_len > 0 { - &SPACE_INDENT[..space_len] - } else { - &space[..std::cmp::min(space.len(), 10)] - }; + let gap = &gap.to_string(context)?; let replacer = match args.get(1) { Some(replacer) if replacer.is_object() => replacer,