From ad495608c71ec73f3ba51d24b7577d89677b6409 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Sat, 16 May 2020 17:00:15 -0500 Subject: [PATCH 01/31] remove undefined values from objects --- boa/src/builtins/json/tests.rs | 11 +++++++++++ boa/src/builtins/value/mod.rs | 12 +++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index ba8535884ee..25458d3b251 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -16,3 +16,14 @@ fn json_sanity() { "true" ); } + +#[test] +fn json_stringify_remove_undefined_values_from_objects() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + + let actual = forward(&mut engine, r#"JSON.stringify({ aaa: undefined, bbb: 'ccc' })"#); + let expected = r#"{"bbb":"ccc"}"#; + + assert_eq!(actual, expected); +} diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index 0d3f86672cb..6e2194e6bf8 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -720,12 +720,18 @@ impl ValueData { Self::Null | Self::Symbol(_) | Self::Undefined => JSONValue::Null, Self::Boolean(b) => JSONValue::Bool(b), Self::Object(ref obj) => { - let new_obj = obj + let mut new_obj = Map::new(); + obj .borrow() .properties .iter() - .map(|(k, _)| (k.clone(), self.get_field(k.as_str()).to_json())) - .collect::>(); + .for_each(|(k, _)| { + let key = k.clone(); + let value = self.get_field(k.to_string()); + if !value.is_undefined() { + new_obj.insert(key, value.to_json()); + } + }); JSONValue::Object(new_obj) } Self::String(ref str) => JSONValue::String(str.clone()), From 38987ca6eb3aa36031e30da3b44caf5c9bf80e95 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Sat, 16 May 2020 21:50:04 -0500 Subject: [PATCH 02/31] remove function values from objects --- boa/src/builtins/json/tests.rs | 12 ++++++++++++ boa/src/builtins/value/mod.rs | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 25458d3b251..2a19462be77 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -27,3 +27,15 @@ fn json_stringify_remove_undefined_values_from_objects() { assert_eq!(actual, expected); } + +#[test] +fn json_stringify_remove_function_values_from_objects() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + + let actual = forward(&mut engine, r#"JSON.stringify({ aaa: () => {}, bbb: 'ccc' })"#); + let expected = r#"{"bbb":"ccc"}"#; + + assert_eq!(actual, expected); +} + diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index 6e2194e6bf8..de81a003816 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -728,7 +728,7 @@ impl ValueData { .for_each(|(k, _)| { let key = k.clone(); let value = self.get_field(k.to_string()); - if !value.is_undefined() { + if !value.is_undefined() && !value.is_function() { new_obj.insert(key, value.to_json()); } }); From 414cc3095ef2c6398735e36342254241edbe35e4 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 11 May 2020 23:28:38 -0500 Subject: [PATCH 03/31] json stringify "whitelist" replacer --- boa/src/builtins/json/mod.rs | 35 +++++++++++++++++++++++++++++++--- boa/src/builtins/json/tests.rs | 17 +++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 7a5e80458c6..f380c1b6304 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -15,9 +15,11 @@ use crate::builtins::{ function::make_builtin_fn, - value::{ResultValue, Value}, + value::{ResultValue, Value, ValueData}, + object::ObjectKind }; use crate::exec::Interpreter; +use std::ops::Deref; use serde_json::{self, Value as JSONValue}; #[cfg(test)] @@ -66,8 +68,35 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// [spec]: https://tc39.es/ecma262/#sec-json.stringify /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue { - let obj = args.get(0).expect("cannot get argument for JSON.stringify"); - let json = obj.to_json().to_string(); + let object = args.get(0).expect("cannot get argument for JSON.stringify"); + let object_to_return = Value::new_object(None); + if let Some(arg) = args.get(1) { + match arg.data() { + ValueData::Object(ref obj) => { + let derefed_obj = (*obj).deref(); + let borrowed_derefed_obj = derefed_obj.borrow(); + if borrowed_derefed_obj.kind == ObjectKind::Array { + for (key, value) in borrowed_derefed_obj.properties.iter() { + if let Some(Value(x)) = &value.value { + if key != "length" { + object_to_return.set_property( + x.to_string(), + object.get_property(&x.to_string()).unwrap() + ); + } + } + } + return Ok(Value::from(object_to_return.to_json().to_string())); + } else { + panic!("replacer only supports arrays at this time"); + } + } + _ => { + panic!("replacer only supports arrays at this time"); + } + } + } + let json = object.to_json().to_string(); Ok(Value::from(json)) } diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 2a19462be77..a6853300098 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -39,3 +39,20 @@ fn json_stringify_remove_function_values_from_objects() { assert_eq!(actual, expected); } +#[test] +fn json_stringify_replacer_array() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let actual = forward( + &mut engine, + r#"JSON.stringify({aaa: 'bbb', bbb: 'ccc', ccc: 'ddd'}, ['aaa', 'bbb'])"# + ); + let expected = forward( + &mut engine, + r#"'{"aaa":"bbb","bbb":"ccc"}'"# + ); + assert_eq!( + actual, + expected + ); +} From c9b3ff67e798e97345a9342601f675697349ed19 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 14 May 2020 00:29:21 -0500 Subject: [PATCH 04/31] run rust format --- boa/src/builtins/json/mod.rs | 6 +++--- boa/src/builtins/json/tests.rs | 12 +++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index f380c1b6304..98496372463 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -15,12 +15,12 @@ use crate::builtins::{ function::make_builtin_fn, + object::ObjectKind, value::{ResultValue, Value, ValueData}, - object::ObjectKind }; use crate::exec::Interpreter; -use std::ops::Deref; use serde_json::{self, Value as JSONValue}; +use std::ops::Deref; #[cfg(test)] mod tests; @@ -81,7 +81,7 @@ pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultVa if key != "length" { object_to_return.set_property( x.to_string(), - object.get_property(&x.to_string()).unwrap() + object.get_property(&x.to_string()).unwrap(), ); } } diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index a6853300098..a7cdb0afbd9 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -45,14 +45,8 @@ fn json_stringify_replacer_array() { let mut engine = Interpreter::new(realm); let actual = forward( &mut engine, - r#"JSON.stringify({aaa: 'bbb', bbb: 'ccc', ccc: 'ddd'}, ['aaa', 'bbb'])"# - ); - let expected = forward( - &mut engine, - r#"'{"aaa":"bbb","bbb":"ccc"}'"# - ); - assert_eq!( - actual, - expected + r#"JSON.stringify({aaa: 'bbb', bbb: 'ccc', ccc: 'ddd'}, ['aaa', 'bbb'])"#, ); + let expected = forward(&mut engine, r#"'{"aaa":"bbb","bbb":"ccc"}'"#); + assert_eq!(actual, expected); } From 1ff51d93675d4cc85258a3984f11744d575e4d24 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 14 May 2020 12:32:19 -0500 Subject: [PATCH 05/31] prefer if let over single case match --- boa/src/builtins/json/mod.rs | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 98496372463..9856626cb34 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -71,27 +71,22 @@ pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultVa let object = args.get(0).expect("cannot get argument for JSON.stringify"); let object_to_return = Value::new_object(None); if let Some(arg) = args.get(1) { - match arg.data() { - ValueData::Object(ref obj) => { - let derefed_obj = (*obj).deref(); - let borrowed_derefed_obj = derefed_obj.borrow(); - if borrowed_derefed_obj.kind == ObjectKind::Array { - for (key, value) in borrowed_derefed_obj.properties.iter() { - if let Some(Value(x)) = &value.value { - if key != "length" { - object_to_return.set_property( - x.to_string(), - object.get_property(&x.to_string()).unwrap(), - ); - } + if let ValueData::Object(ref obj) = arg.data() { + let derefed_obj = (*obj).deref(); + let borrowed_derefed_obj = derefed_obj.borrow(); + if borrowed_derefed_obj.kind == ObjectKind::Array { + for (key, value) in borrowed_derefed_obj.properties.iter() { + if let Some(Value(x)) = &value.value { + if key != "length" { + object_to_return.set_property( + x.to_string(), + object.get_property(&x.to_string()).unwrap(), + ); } } - return Ok(Value::from(object_to_return.to_json().to_string())); - } else { - panic!("replacer only supports arrays at this time"); } - } - _ => { + return Ok(Value::from(object_to_return.to_json().to_string())); + } else { panic!("replacer only supports arrays at this time"); } } From 3585ed76c1b1a436acb6edb88d42e8d425f9c2c9 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 14 May 2020 12:52:38 -0500 Subject: [PATCH 06/31] exit early when replacer and space arguments are ommitted --- boa/src/builtins/json/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 9856626cb34..4f717d99116 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -69,6 +69,11 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue { let object = args.get(0).expect("cannot get argument for JSON.stringify"); + if args.len() == 1 { + let json = object.to_json().to_string(); + return Ok(Value::from(json)); + } + let object_to_return = Value::new_object(None); if let Some(arg) = args.get(1) { if let ValueData::Object(ref obj) = arg.data() { @@ -87,12 +92,11 @@ pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultVa } return Ok(Value::from(object_to_return.to_json().to_string())); } else { - panic!("replacer only supports arrays at this time"); + unimplemented!("replacer only supports arrays at this time"); } } } - let json = object.to_json().to_string(); - Ok(Value::from(json)) + panic!("cannot get replacer for JSON.stringify"); } /// Create a new `JSON` object. From c94cb36120747571ab9efd0ebe7089105918ecaa Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 14 May 2020 21:22:50 -0500 Subject: [PATCH 07/31] add test for array of numbers (works with existing implementation) --- boa/src/builtins/json/tests.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index a7cdb0afbd9..c5f1cb5a4df 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -40,7 +40,7 @@ fn json_stringify_remove_function_values_from_objects() { } #[test] -fn json_stringify_replacer_array() { +fn json_stringify_replacer_array_strings() { let realm = Realm::create(); let mut engine = Interpreter::new(realm); let actual = forward( @@ -50,3 +50,15 @@ fn json_stringify_replacer_array() { let expected = forward(&mut engine, r#"'{"aaa":"bbb","bbb":"ccc"}'"#); assert_eq!(actual, expected); } + +#[test] +fn json_stringify_replacer_array_numbers() { + let realm = Realm::create(); + let mut engine = Executor::new(realm); + let actual = forward( + &mut engine, + r#"JSON.stringify({ 0: 'aaa', 1: 'bbb', 2: 'ccc'}, [1, 2])"#, + ); + let expected = forward(&mut engine, r#"'{"1":"bbb","2":"ccc"}'"#); + assert_eq!(actual, expected); +} From 32f4e3918096704bc5287938e35258ce715b4689 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 14 May 2020 23:16:39 -0500 Subject: [PATCH 08/31] WIP: to_json returns undefined to null --- boa/src/builtins/json/mod.rs | 27 ++++++++++++++++++++------- boa/src/builtins/json/tests.rs | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 4f717d99116..82615277459 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -16,6 +16,7 @@ use crate::builtins::{ function::make_builtin_fn, object::ObjectKind, + property::Property, value::{ResultValue, Value, ValueData}, }; use crate::exec::Interpreter; @@ -67,7 +68,7 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// /// [spec]: https://tc39.es/ecma262/#sec-json.stringify /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify -pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue { +pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) -> ResultValue { let object = args.get(0).expect("cannot get argument for JSON.stringify"); if args.len() == 1 { let json = object.to_json().to_string(); @@ -78,9 +79,9 @@ pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultVa if let Some(arg) = args.get(1) { if let ValueData::Object(ref obj) = arg.data() { let derefed_obj = (*obj).deref(); - let borrowed_derefed_obj = derefed_obj.borrow(); - if borrowed_derefed_obj.kind == ObjectKind::Array { - for (key, value) in borrowed_derefed_obj.properties.iter() { + let borrowed_derefed_replacer = derefed_obj.borrow(); + if borrowed_derefed_replacer.kind == ObjectKind::Array { + for (key, value) in borrowed_derefed_replacer.properties.iter() { if let Some(Value(x)) = &value.value { if key != "length" { object_to_return.set_property( @@ -90,10 +91,22 @@ pub fn stringify(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultVa } } } - return Ok(Value::from(object_to_return.to_json().to_string())); - } else { - unimplemented!("replacer only supports arrays at this time"); + } else if borrowed_derefed_replacer.kind == ObjectKind::Function { + if let ValueData::Object(ref obj) = object.data() { + let derefed_obj = (*obj).deref(); + let borrowed_deref_obj = derefed_obj.borrow(); + for (key, val) in borrowed_deref_obj.properties.iter() { + if let Some(value) = &val.value { + let mut this_arg = object.clone(); + object_to_return.set_property( + String::from(key), + Property::default().value(interpreter.call(arg, &mut this_arg, &[Value::string(key), Value::from(value)]).unwrap()), + ); + } + } + } } + return Ok(Value::from(object_to_return.to_json().to_string())); } } panic!("cannot get replacer for JSON.stringify"); diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index c5f1cb5a4df..0d5a31ed514 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -62,3 +62,21 @@ fn json_stringify_replacer_array_numbers() { let expected = forward(&mut engine, r#"'{"1":"bbb","2":"ccc"}'"#); assert_eq!(actual, expected); } + +#[test] +fn json_stringify_replacer_function() { + let realm = Realm::create(); + let mut engine = Executor::new(realm); + let actual = forward( + &mut engine, + r#"JSON.stringify({ aaa: 1, bbb: 2}, (key, value) => { + if (key === 'aaa') { + return undefined; + } + + return value; + })"#, + ); + let expected = forward(&mut engine, r#"'{"bbb":2}'"#); + assert_eq!(actual, expected); +} From 5756eecd7328704c32c91f7d8855472f8934a71a Mon Sep 17 00:00:00 2001 From: Nick Little Date: Sun, 17 May 2020 21:49:43 -0500 Subject: [PATCH 09/31] fix rust format --- boa/src/builtins/json/mod.rs | 10 +++++++++- boa/src/builtins/json/tests.rs | 14 ++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 82615277459..180532a9634 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -100,7 +100,15 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - let mut this_arg = object.clone(); object_to_return.set_property( String::from(key), - Property::default().value(interpreter.call(arg, &mut this_arg, &[Value::string(key), Value::from(value)]).unwrap()), + Property::default().value( + interpreter + .call( + arg, + &mut this_arg, + &[Value::string(key), Value::from(value)], + ) + .unwrap(), + ), ); } } diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 0d5a31ed514..667227a1f7f 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -22,7 +22,10 @@ fn json_stringify_remove_undefined_values_from_objects() { let realm = Realm::create(); let mut engine = Interpreter::new(realm); - let actual = forward(&mut engine, r#"JSON.stringify({ aaa: undefined, bbb: 'ccc' })"#); + let actual = forward( + &mut engine, + r#"JSON.stringify({ aaa: undefined, bbb: 'ccc' })"#, + ); let expected = r#"{"bbb":"ccc"}"#; assert_eq!(actual, expected); @@ -33,7 +36,10 @@ fn json_stringify_remove_function_values_from_objects() { let realm = Realm::create(); let mut engine = Interpreter::new(realm); - let actual = forward(&mut engine, r#"JSON.stringify({ aaa: () => {}, bbb: 'ccc' })"#); + let actual = forward( + &mut engine, + r#"JSON.stringify({ aaa: () => {}, bbb: 'ccc' })"#, + ); let expected = r#"{"bbb":"ccc"}"#; assert_eq!(actual, expected); @@ -54,7 +60,7 @@ fn json_stringify_replacer_array_strings() { #[test] fn json_stringify_replacer_array_numbers() { let realm = Realm::create(); - let mut engine = Executor::new(realm); + let mut engine = Interpreter::new(realm); let actual = forward( &mut engine, r#"JSON.stringify({ 0: 'aaa', 1: 'bbb', 2: 'ccc'}, [1, 2])"#, @@ -66,7 +72,7 @@ fn json_stringify_replacer_array_numbers() { #[test] fn json_stringify_replacer_function() { let realm = Realm::create(); - let mut engine = Executor::new(realm); + let mut engine = Interpreter::new(realm); let actual = forward( &mut engine, r#"JSON.stringify({ aaa: 1, bbb: 2}, (key, value) => { From cd66600d787a6e0581b5647be6c4597c0174accd Mon Sep 17 00:00:00 2001 From: Nick Little Date: Sun, 17 May 2020 22:13:35 -0500 Subject: [PATCH 10/31] add test to remove symbol values from objects --- boa/src/builtins/json/tests.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 667227a1f7f..86b03e89af2 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -45,6 +45,20 @@ fn json_stringify_remove_function_values_from_objects() { assert_eq!(actual, expected); } +#[test] +fn json_stringify_remove_symbols_from_objects() { + let realm = Realm::create(); + let mut engine = Executor::new(realm); + + let actual = forward( + &mut engine, + r#"JSON.stringify({ aaa: Symbol(), bbb: 'ccc' })"#, + ); + let expected = r#"{"bbb":"ccc"}"#; + + assert_eq!(actual, expected); +} + #[test] fn json_stringify_replacer_array_strings() { let realm = Realm::create(); From a14aa735fde888e00379d67aea3454ba5c84e3a0 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 18 May 2020 18:40:46 -0500 Subject: [PATCH 11/31] ignore json stringify tests with symbols (there is a bug) --- boa/src/builtins/json/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 86b03e89af2..65204eca999 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -46,6 +46,8 @@ fn json_stringify_remove_function_values_from_objects() { } #[test] +#[ignore] +// there is a bug for setting a symbol as a field's value fn json_stringify_remove_symbols_from_objects() { let realm = Realm::create(); let mut engine = Executor::new(realm); From d17a4558985d39eff95e6ccdfa8ab1dc1822293c Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 18 May 2020 20:54:04 -0500 Subject: [PATCH 12/31] handle json stringification for arrays --- boa/src/builtins/json/tests.rs | 42 +++++++++++++++++++++++++++++++++- boa/src/builtins/value/mod.rs | 30 +++++++++++++++++------- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 65204eca999..9c6dbf61e54 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -50,7 +50,7 @@ fn json_stringify_remove_function_values_from_objects() { // there is a bug for setting a symbol as a field's value fn json_stringify_remove_symbols_from_objects() { let realm = Realm::create(); - let mut engine = Executor::new(realm); + let mut engine = Interpreter::new(realm); let actual = forward( &mut engine, @@ -102,3 +102,43 @@ fn json_stringify_replacer_function() { let expected = forward(&mut engine, r#"'{"bbb":2}'"#); assert_eq!(actual, expected); } + +#[test] +fn json_stringify_arrays() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let actual = forward(&mut engine, r#"JSON.stringify(['a', 'b'])"#); + let expected = forward(&mut engine, r#"'["a","b"]'"#); + + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_object_array() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let actual = forward(&mut engine, r#"JSON.stringify([{a: 'b'}, {b: 'c'}])"#); + let expected = forward(&mut engine, r#"'[{"a":"b"},{"b":"c"}]'"#); + + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_array_converts_undefined_to_null() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let actual = forward(&mut engine, r#"JSON.stringify([undefined])"#); + let expected = forward(&mut engine, r#"'[null]'"#); + + assert_eq!(actual, expected); +} + +#[test] +fn json_stringify_array_converts_function_to_null() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + let actual = forward(&mut engine, r#"JSON.stringify([() => {}])"#); + let expected = forward(&mut engine, r#"'[null]'"#); + + assert_eq!(actual, expected); +} diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index de81a003816..b5646488363 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -717,22 +717,33 @@ impl ValueData { /// Conversts the `Value` to `JSON`. pub fn to_json(&self) -> JSONValue { match *self { - Self::Null | Self::Symbol(_) | Self::Undefined => JSONValue::Null, + Self::Null => JSONValue::Null, Self::Boolean(b) => JSONValue::Bool(b), Self::Object(ref obj) => { - let mut new_obj = Map::new(); - obj - .borrow() - .properties - .iter() - .for_each(|(k, _)| { + if obj.borrow().kind == ObjectKind::Array { + let mut arr: Vec = Vec::new(); + obj.borrow().properties.iter().for_each(|(k, _)| { + if k != "length" { + let value = self.get_field(k.to_string()); + if value.is_undefined() || value.is_function() { + arr.push(JSONValue::Null); + } else { + arr.push(self.get_field(k.to_string()).to_json()); + } + } + }); + JSONValue::Array(arr) + } else { + let mut new_obj = Map::new(); + obj.borrow().properties.iter().for_each(|(k, _)| { let key = k.clone(); let value = self.get_field(k.to_string()); if !value.is_undefined() && !value.is_function() { new_obj.insert(key, value.to_json()); } }); - JSONValue::Object(new_obj) + JSONValue::Object(new_obj) + } } Self::String(ref str) => JSONValue::String(str.clone()), Self::Rational(num) => JSONValue::Number( @@ -743,6 +754,9 @@ impl ValueData { // TODO: throw TypeError panic!("TypeError: \"BigInt value can't be serialized in JSON\""); } + Self::Symbol(_) | Self::Undefined => { + panic!("Symbols and Undefined JSON Values depend on parent type"); + } } } From d58ad968877d2d4c18c9d80be643d349d7cc06ed Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 18 May 2020 21:07:38 -0500 Subject: [PATCH 13/31] exit early --- boa/src/builtins/json/mod.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 180532a9634..50f1472d696 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -82,13 +82,15 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - let borrowed_derefed_replacer = derefed_obj.borrow(); if borrowed_derefed_replacer.kind == ObjectKind::Array { for (key, value) in borrowed_derefed_replacer.properties.iter() { + if key == "length" { + continue; + } + if let Some(Value(x)) = &value.value { - if key != "length" { - object_to_return.set_property( - x.to_string(), - object.get_property(&x.to_string()).unwrap(), - ); - } + object_to_return.set_property( + x.to_string(), + object.get_property(&x.to_string()).unwrap(), + ); } } } else if borrowed_derefed_replacer.kind == ObjectKind::Function { From 8538005a3f2f005e6aa50094359f97935c2f9897 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 18 May 2020 21:20:27 -0500 Subject: [PATCH 14/31] refactor JSON.stringify function --- boa/src/builtins/json/mod.rs | 66 +++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 50f1472d696..d4ed1b00da0 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -78,43 +78,47 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - let object_to_return = Value::new_object(None); if let Some(arg) = args.get(1) { if let ValueData::Object(ref obj) = arg.data() { - let derefed_obj = (*obj).deref(); - let borrowed_derefed_replacer = derefed_obj.borrow(); - if borrowed_derefed_replacer.kind == ObjectKind::Array { - for (key, value) in borrowed_derefed_replacer.properties.iter() { - if key == "length" { - continue; - } + let replacer = (*obj).deref().borrow(); + match replacer.kind { + ObjectKind::Array => { + for (key, value) in replacer.properties.iter() { + if key == "length" { + continue; + } - if let Some(Value(x)) = &value.value { - object_to_return.set_property( - x.to_string(), - object.get_property(&x.to_string()).unwrap(), - ); - } - } - } else if borrowed_derefed_replacer.kind == ObjectKind::Function { - if let ValueData::Object(ref obj) = object.data() { - let derefed_obj = (*obj).deref(); - let borrowed_deref_obj = derefed_obj.borrow(); - for (key, val) in borrowed_deref_obj.properties.iter() { - if let Some(value) = &val.value { - let mut this_arg = object.clone(); + if let Some(Value(x)) = &value.value { object_to_return.set_property( - String::from(key), - Property::default().value( - interpreter - .call( - arg, - &mut this_arg, - &[Value::string(key), Value::from(value)], - ) - .unwrap(), - ), + x.to_string(), + object + .get_property(&x.to_string()) + .expect("failed to get property from object"), ); } } } + ObjectKind::Function => { + if let ValueData::Object(ref obj) = object.data() { + let object_to_stringify = (*obj).deref().borrow(); + for (key, val) in object_to_stringify.properties.iter() { + if let Some(value) = &val.value { + let mut this_arg = object.clone(); + object_to_return.set_property( + String::from(key), + Property::default().value( + interpreter + .call( + arg, + &mut this_arg, + &[Value::string(key), Value::from(value)], + ) + .expect("failed to get returned value from replacer function"), + ), + ); + } + } + } + } + _ => panic!("JSON.stringify replacer can only be an array or function"), } return Ok(Value::from(object_to_return.to_json().to_string())); } From 0320e173184ac2fb2a9619f76056205b419e4481 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 18 May 2020 21:24:19 -0500 Subject: [PATCH 15/31] add test that needs to be addressed once symbols are fixed --- boa/src/builtins/json/tests.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 9c6dbf61e54..77d875b2cfb 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -142,3 +142,14 @@ fn json_stringify_array_converts_function_to_null() { assert_eq!(actual, expected); } + +#[test] +#[ignore] +fn json_stringify_array_converts_symbol_to_null() { + let realm = Realm::create(); + let mut engine = Executor::new(realm); + let actual = forward(&mut engine, r#"JSON.stringify([Symbol()])"#); + let expected = forward(&mut engine, r#"'[null]'"#); + + assert_eq!(actual, expected); +} From 30b0bf9d41e50d602df05dc0c456d413487687a1 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 19 May 2020 23:01:18 -0500 Subject: [PATCH 16/31] refactor JSON.stringify --- boa/src/builtins/json/mod.rs | 114 +++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 45 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index d4ed1b00da0..8ef3f8869f2 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -70,60 +70,84 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) -> ResultValue { let object = args.get(0).expect("cannot get argument for JSON.stringify"); + if args.len() == 1 { - let json = object.to_json().to_string(); - return Ok(Value::from(json)); + return Ok(Value::from(object.to_json().to_string())); } - let object_to_return = Value::new_object(None); - if let Some(arg) = args.get(1) { - if let ValueData::Object(ref obj) = arg.data() { - let replacer = (*obj).deref().borrow(); - match replacer.kind { - ObjectKind::Array => { - for (key, value) in replacer.properties.iter() { - if key == "length" { - continue; - } + let replacer = args.get(1).expect("cannot get JSON.stringify replacer"); + if !replacer.is_object() { + return Ok(Value::from(object.to_json().to_string())); + } - if let Some(Value(x)) = &value.value { - object_to_return.set_property( - x.to_string(), - object - .get_property(&x.to_string()) - .expect("failed to get property from object"), - ); - } - } + let replacer_as_object = replacer + .as_object() + .expect("JSON.stringify replacer was an object"); + if replacer_as_object.is_callable() { + let object_to_return = Value::new_object(None); + if let ValueData::Object(ref obj) = object.data() { + let object_to_stringify = (*obj).deref().borrow(); + for (key, val) in object_to_stringify.properties.iter() { + if let Some(value) = &val.value { + let mut this_arg = object.clone(); + object_to_return.set_property( + String::from(key), + Property::default().value( + interpreter + .call( + replacer, + &mut this_arg, + &[Value::string(key), Value::from(value)], + ) + .expect("failed to get returned value from replacer function"), + ), + ); } - ObjectKind::Function => { - if let ValueData::Object(ref obj) = object.data() { - let object_to_stringify = (*obj).deref().borrow(); - for (key, val) in object_to_stringify.properties.iter() { - if let Some(value) = &val.value { - let mut this_arg = object.clone(); - object_to_return.set_property( - String::from(key), - Property::default().value( - interpreter - .call( - arg, - &mut this_arg, - &[Value::string(key), Value::from(value)], - ) - .expect("failed to get returned value from replacer function"), - ), - ); - } - } - } + } + } + Ok(Value::from(object_to_return.to_json().to_string())) + } else if replacer_as_object.kind == ObjectKind::Array { + let mut obj_to_return = + serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); + let fields = replacer_as_object + .properties + .iter() + .filter_map(|(key, prop)| { + if key == "length" { + None + } else { + // TODO: this should be the abstract operation `Get` + prop.value.as_ref().map(Value::to_string) } - _ => panic!("JSON.stringify replacer can only be an array or function"), + }); + for field in fields { + if let Some(value) = object + .get_property(&field) + .map(|prop| prop.value.as_ref().map(|v| v.to_json())) + .flatten() + { + obj_to_return.insert(field.to_string(), value); } - return Ok(Value::from(object_to_return.to_json().to_string())); } + Ok(Value::from(JSONValue::Object(obj_to_return).to_string())) + } else { + Ok(Value::from(object.to_json().to_string())) } - panic!("cannot get replacer for JSON.stringify"); + + // let object_to_return = Value::new_object(None); + // if let Some(arg) = args.get(1) { + // if let ValueData::Object(ref obj) = arg.data() { + // let replacer = (*obj).deref().borrow(); + // match replacer.kind { + // ObjectKind::Function => { + + // } + // _ => panic!("JSON.stringify replacer can only be an array or function"), + // } + // return Ok(Value::from(object_to_return.to_json().to_string())); + // } + // } + // panic!("cannot get replacer for JSON.stringify"); } /// Create a new `JSON` object. From 0eba1625c64f77ee4f6f32ce29c97255bf76a2de Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 19 May 2020 23:46:21 -0500 Subject: [PATCH 17/31] use option in function replacer --- boa/src/builtins/json/mod.rs | 44 +++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 8ef3f8869f2..b88cee7a41a 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -84,28 +84,30 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - .as_object() .expect("JSON.stringify replacer was an object"); if replacer_as_object.is_callable() { - let object_to_return = Value::new_object(None); - if let ValueData::Object(ref obj) = object.data() { - let object_to_stringify = (*obj).deref().borrow(); - for (key, val) in object_to_stringify.properties.iter() { - if let Some(value) = &val.value { - let mut this_arg = object.clone(); - object_to_return.set_property( - String::from(key), - Property::default().value( - interpreter - .call( - replacer, - &mut this_arg, - &[Value::string(key), Value::from(value)], - ) - .expect("failed to get returned value from replacer function"), - ), - ); + object + .as_object() + .map(|obj| { + let object_to_return = Value::new_object(None); + for (key, val) in obj.properties.iter() { + if let Some(value) = &val.value { + let mut this_arg = object.clone(); + object_to_return.set_property( + String::from(key), + Property::default().value( + interpreter + .call( + replacer, + &mut this_arg, + &[Value::string(key), Value::from(value)], + ) + .expect("failed to get returned value from replacer function"), + ), + ); + } } - } - } - Ok(Value::from(object_to_return.to_json().to_string())) + Value::from(object_to_return.to_json().to_string()) + }) + .ok_or_else(Value::undefined) } else if replacer_as_object.kind == ObjectKind::Array { let mut obj_to_return = serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); From 139911088d848bceec49fb5de1161f16d6b86a03 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 19 May 2020 23:54:04 -0500 Subject: [PATCH 18/31] remove some comments and unused imports --- boa/src/builtins/json/mod.rs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index b88cee7a41a..0361059b090 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -17,11 +17,10 @@ use crate::builtins::{ function::make_builtin_fn, object::ObjectKind, property::Property, - value::{ResultValue, Value, ValueData}, + value::{ResultValue, Value}, }; use crate::exec::Interpreter; use serde_json::{self, Value as JSONValue}; -use std::ops::Deref; #[cfg(test)] mod tests; @@ -135,21 +134,6 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - } else { Ok(Value::from(object.to_json().to_string())) } - - // let object_to_return = Value::new_object(None); - // if let Some(arg) = args.get(1) { - // if let ValueData::Object(ref obj) = arg.data() { - // let replacer = (*obj).deref().borrow(); - // match replacer.kind { - // ObjectKind::Function => { - - // } - // _ => panic!("JSON.stringify replacer can only be an array or function"), - // } - // return Ok(Value::from(object_to_return.to_json().to_string())); - // } - // } - // panic!("cannot get replacer for JSON.stringify"); } /// Create a new `JSON` object. From 44ed70ffa4c78eb00c7df654e8b174c56385df71 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Sat, 23 May 2020 11:41:44 -0500 Subject: [PATCH 19/31] address PR comments --- boa/src/builtins/json/mod.rs | 39 ++++++++++++++++------------------- boa/src/builtins/value/mod.rs | 4 ++-- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 0361059b090..8025357a409 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -87,22 +87,20 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - .as_object() .map(|obj| { let object_to_return = Value::new_object(None); - for (key, val) in obj.properties.iter() { - if let Some(value) = &val.value { - let mut this_arg = object.clone(); - object_to_return.set_property( - String::from(key), - Property::default().value( - interpreter - .call( - replacer, - &mut this_arg, - &[Value::string(key), Value::from(value)], - ) - .expect("failed to get returned value from replacer function"), - ), - ); - } + for (key, val) in obj.properties.iter().filter(|(_, v)|v.value.is_some()) { + let mut this_arg = object.clone(); + object_to_return.set_property( + key.to_owned(), + Property::default().value( + interpreter + .call( + replacer, + &mut this_arg, + &[Value::string(key), Value::from(val.value.as_ref())], + ) + .expect("failed to get returned value from replacer function"), + ), + ); } Value::from(object_to_return.to_json().to_string()) }) @@ -112,18 +110,17 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); let fields = replacer_as_object .properties - .iter() - .filter_map(|(key, prop)| { + .keys() + .filter_map(|key| { if key == "length" { None } else { - // TODO: this should be the abstract operation `Get` - prop.value.as_ref().map(Value::to_string) + Some(replacer.get_field_slice(&key.to_string())) } }); for field in fields { if let Some(value) = object - .get_property(&field) + .get_property(&field.to_string()) .map(|prop| prop.value.as_ref().map(|v| v.to_json())) .flatten() { diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index b5646488363..06cde028813 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -722,7 +722,7 @@ impl ValueData { Self::Object(ref obj) => { if obj.borrow().kind == ObjectKind::Array { let mut arr: Vec = Vec::new(); - obj.borrow().properties.iter().for_each(|(k, _)| { + obj.borrow().properties.keys().for_each(|k| { if k != "length" { let value = self.get_field(k.to_string()); if value.is_undefined() || value.is_function() { @@ -735,7 +735,7 @@ impl ValueData { JSONValue::Array(arr) } else { let mut new_obj = Map::new(); - obj.borrow().properties.iter().for_each(|(k, _)| { + obj.borrow().properties.keys().for_each(|k| { let key = k.clone(); let value = self.get_field(k.to_string()); if !value.is_undefined() && !value.is_function() { From 235d6292c1768c3a530a6eb6c3da05ab0cdba9d3 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Mon, 25 May 2020 15:57:40 -0500 Subject: [PATCH 20/31] use new get_field --- boa/src/builtins/json/mod.rs | 2 +- boa/src/builtins/json/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 8025357a409..8238633b037 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -115,7 +115,7 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - if key == "length" { None } else { - Some(replacer.get_field_slice(&key.to_string())) + Some(replacer.get_field(key.to_string())) } }); for field in fields { diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 77d875b2cfb..06420bf7390 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -147,7 +147,7 @@ fn json_stringify_array_converts_function_to_null() { #[ignore] fn json_stringify_array_converts_symbol_to_null() { let realm = Realm::create(); - let mut engine = Executor::new(realm); + let mut engine = Interpreter::new(realm); let actual = forward(&mut engine, r#"JSON.stringify([Symbol()])"#); let expected = forward(&mut engine, r#"'[null]'"#); From fadb75c8756fd73757754f97025e068551c9aaed Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 26 May 2020 09:12:31 -0500 Subject: [PATCH 21/31] use filter map --- boa/src/builtins/json/mod.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 8238633b037..59590aafbf4 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -87,17 +87,19 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - .as_object() .map(|obj| { let object_to_return = Value::new_object(None); - for (key, val) in obj.properties.iter().filter(|(_, v)|v.value.is_some()) { + for (key, val) in obj.properties.iter().filter_map(|(key, v)| { + if let Some(value) = &v.value { + Some((key, value)) + } else { + None + } + }) { let mut this_arg = object.clone(); object_to_return.set_property( key.to_owned(), Property::default().value( interpreter - .call( - replacer, - &mut this_arg, - &[Value::string(key), Value::from(val.value.as_ref())], - ) + .call(replacer, &mut this_arg, &[Value::string(key), val.clone()]) .expect("failed to get returned value from replacer function"), ), ); @@ -108,16 +110,13 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - } else if replacer_as_object.kind == ObjectKind::Array { let mut obj_to_return = serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); - let fields = replacer_as_object - .properties - .keys() - .filter_map(|key| { - if key == "length" { - None - } else { - Some(replacer.get_field(key.to_string())) - } - }); + let fields = replacer_as_object.properties.keys().filter_map(|key| { + if key == "length" { + None + } else { + Some(replacer.get_field(key.to_string())) + } + }); for field in fields { if let Some(value) = object .get_property(&field.to_string()) From 600e560fb9f5f4eeb79511ce0771beac93b870c8 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 26 May 2020 13:24:24 -0500 Subject: [PATCH 22/31] propogate errors --- boa/src/builtins/json/mod.rs | 7 +++---- boa/src/builtins/json/tests.rs | 12 ++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 59590aafbf4..81e3be4ceb4 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -99,14 +99,13 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - key.to_owned(), Property::default().value( interpreter - .call(replacer, &mut this_arg, &[Value::string(key), val.clone()]) - .expect("failed to get returned value from replacer function"), + .call(replacer, &mut this_arg, &[Value::string(key), val.clone()])?, ), ); } - Value::from(object_to_return.to_json().to_string()) + Ok(Value::from(object_to_return.to_json().to_string())) }) - .ok_or_else(Value::undefined) + .ok_or_else(Value::undefined)? } else if replacer_as_object.kind == ObjectKind::Array { let mut obj_to_return = serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 06420bf7390..736a0de679b 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -153,3 +153,15 @@ fn json_stringify_array_converts_symbol_to_null() { assert_eq!(actual, expected); } + #[test] + fn json_stringify_function_replacer_propogate_error() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + + let actual = forward(&mut engine, r#"JSON.stringify({x: 1}, (key, value) => { + throw new TypeError("type error") + })"#); + let expected = forward(&mut engine, r#"'Error: undefined'"#); + + assert_eq!(actual, expected); + } \ No newline at end of file From 19e86323cbcd5229f45accd5d6d726da12bdf7b9 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 26 May 2020 13:27:35 -0500 Subject: [PATCH 23/31] format --- boa/src/builtins/json/mod.rs | 9 +++++---- boa/src/builtins/json/tests.rs | 21 ++++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 81e3be4ceb4..6f2df536b66 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -97,10 +97,11 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - let mut this_arg = object.clone(); object_to_return.set_property( key.to_owned(), - Property::default().value( - interpreter - .call(replacer, &mut this_arg, &[Value::string(key), val.clone()])?, - ), + Property::default().value(interpreter.call( + replacer, + &mut this_arg, + &[Value::string(key), val.clone()], + )?), ); } Ok(Value::from(object_to_return.to_json().to_string())) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 736a0de679b..4421b32bf02 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -153,15 +153,18 @@ fn json_stringify_array_converts_symbol_to_null() { assert_eq!(actual, expected); } - #[test] - fn json_stringify_function_replacer_propogate_error() { - let realm = Realm::create(); - let mut engine = Interpreter::new(realm); +#[test] +fn json_stringify_function_replacer_propogate_error() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); - let actual = forward(&mut engine, r#"JSON.stringify({x: 1}, (key, value) => { + let actual = forward( + &mut engine, + r#"JSON.stringify({x: 1}, (key, value) => { throw new TypeError("type error") - })"#); - let expected = forward(&mut engine, r#"'Error: undefined'"#); + })"#, + ); + let expected = forward(&mut engine, r#"'Error: undefined'"#); - assert_eq!(actual, expected); - } \ No newline at end of file + assert_eq!(actual, expected); +} From c39ba92a73dfe2263ddd5e68120327b1d2c54d21 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Tue, 26 May 2020 13:44:43 -0500 Subject: [PATCH 24/31] assert that something is thrown --- boa/src/builtins/json/tests.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 4421b32bf02..cd5519bd707 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -160,11 +160,17 @@ fn json_stringify_function_replacer_propogate_error() { let actual = forward( &mut engine, - r#"JSON.stringify({x: 1}, (key, value) => { - throw new TypeError("type error") - })"#, + r#" + let thrown = 0; + try { + JSON.stringify({x: 1}, (key, value) => { throw 1 }) + } catch (err) { + thrown = err; + } + thrown + "#, ); - let expected = forward(&mut engine, r#"'Error: undefined'"#); + let expected = forward(&mut engine, r#"1"#); assert_eq!(actual, expected); } From 766c7dc7e9869260b33539d07c3eb030ed3c1ced Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 28 May 2020 07:18:22 -0500 Subject: [PATCH 25/31] return undefined when no argument passed in to JSON.stringify --- boa/src/builtins/json/mod.rs | 5 ++++- boa/src/builtins/json/tests.rs | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 6f2df536b66..c32a8ce0b2f 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -68,7 +68,10 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// [spec]: https://tc39.es/ecma262/#sec-json.stringify /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) -> ResultValue { - let object = args.get(0).expect("cannot get argument for JSON.stringify"); + let object = match args.get(0) { + Some(obj) => obj, + None => return Ok(Value::undefined()), + }; if args.len() == 1 { return Ok(Value::from(object.to_json().to_string())); diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index cd5519bd707..761b98c44a6 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -174,3 +174,14 @@ fn json_stringify_function_replacer_propogate_error() { assert_eq!(actual, expected); } + +#[test] +fn json_stringify_with_no_args() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + + let actual = forward(&mut engine, r#"JSON.stringify()"#); + let expected = forward(&mut engine, r#"undefined"#); + + assert_eq!(actual, expected); +} From 0c9b795de6e74e13b18ad214a7015ab0fc908cd2 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Thu, 28 May 2020 07:25:06 -0500 Subject: [PATCH 26/31] use pattern matching to get replacer / exit early --- boa/src/builtins/json/mod.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index c32a8ce0b2f..0fda3d3d7ea 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -73,14 +73,10 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - None => return Ok(Value::undefined()), }; - if args.len() == 1 { - return Ok(Value::from(object.to_json().to_string())); - } - - let replacer = args.get(1).expect("cannot get JSON.stringify replacer"); - if !replacer.is_object() { - return Ok(Value::from(object.to_json().to_string())); - } + let replacer = match args.get(1) { + Some(replacer) if replacer.is_object() => replacer, + _ => return Ok(Value::from(object.to_json().to_string())), + }; let replacer_as_object = replacer .as_object() From f25778ae9ac552f6d97a941099729731703579d4 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Fri, 29 May 2020 08:00:14 -0500 Subject: [PATCH 27/31] use if let instead of match --- boa/src/builtins/json/mod.rs | 105 +++++++++++++++++------------------ 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 0fda3d3d7ea..02e4255a739 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -68,66 +68,65 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// [spec]: https://tc39.es/ecma262/#sec-json.stringify /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) -> ResultValue { - let object = match args.get(0) { - Some(obj) => obj, - None => return Ok(Value::undefined()), - }; + if let Some(object) = args.get(0) { + let replacer = match args.get(1) { + Some(replacer) if replacer.is_object() => replacer, + _ => return Ok(Value::from(object.to_json().to_string())), + }; - let replacer = match args.get(1) { - Some(replacer) if replacer.is_object() => replacer, - _ => return Ok(Value::from(object.to_json().to_string())), - }; - - let replacer_as_object = replacer - .as_object() - .expect("JSON.stringify replacer was an object"); - if replacer_as_object.is_callable() { - object + let replacer_as_object = replacer .as_object() - .map(|obj| { - let object_to_return = Value::new_object(None); - for (key, val) in obj.properties.iter().filter_map(|(key, v)| { - if let Some(value) = &v.value { - Some((key, value)) - } else { - None + .expect("JSON.stringify replacer was an object"); + if replacer_as_object.is_callable() { + object + .as_object() + .map(|obj| { + let object_to_return = Value::new_object(None); + for (key, val) in obj.properties.iter().filter_map(|(key, v)| { + if let Some(value) = &v.value { + Some((key, value)) + } else { + None + } + }) { + let mut this_arg = object.clone(); + object_to_return.set_property( + key.to_owned(), + Property::default().value(interpreter.call( + replacer, + &mut this_arg, + &[Value::string(key), val.clone()], + )?), + ); } - }) { - let mut this_arg = object.clone(); - object_to_return.set_property( - key.to_owned(), - Property::default().value(interpreter.call( - replacer, - &mut this_arg, - &[Value::string(key), val.clone()], - )?), - ); + Ok(Value::from(object_to_return.to_json().to_string())) + }) + .ok_or_else(Value::undefined)? + } else if replacer_as_object.kind == ObjectKind::Array { + let mut obj_to_return = + serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); + let fields = replacer_as_object.properties.keys().filter_map(|key| { + if key == "length" { + None + } else { + Some(replacer.get_field(key.to_string())) + } + }); + for field in fields { + if let Some(value) = object + .get_property(&field.to_string()) + .map(|prop| prop.value.as_ref().map(|v| v.to_json())) + .flatten() + { + obj_to_return.insert(field.to_string(), value); } - Ok(Value::from(object_to_return.to_json().to_string())) - }) - .ok_or_else(Value::undefined)? - } else if replacer_as_object.kind == ObjectKind::Array { - let mut obj_to_return = - serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); - let fields = replacer_as_object.properties.keys().filter_map(|key| { - if key == "length" { - None - } else { - Some(replacer.get_field(key.to_string())) - } - }); - for field in fields { - if let Some(value) = object - .get_property(&field.to_string()) - .map(|prop| prop.value.as_ref().map(|v| v.to_json())) - .flatten() - { - obj_to_return.insert(field.to_string(), value); } + Ok(Value::from(JSONValue::Object(obj_to_return).to_string())) + } else { + Ok(Value::from(object.to_json().to_string())) } - Ok(Value::from(JSONValue::Object(obj_to_return).to_string())) } else { - Ok(Value::from(object.to_json().to_string())) + Ok(Value::undefined()) } } From d39a397365cfa5bb94545767655d7bf1e76d678b Mon Sep 17 00:00:00 2001 From: Nick Little Date: Fri, 29 May 2020 12:37:13 -0500 Subject: [PATCH 28/31] cover other conditions where JSON.stringify should return undefined --- boa/src/builtins/json/mod.rs | 10 ++++++---- boa/src/builtins/json/tests.rs | 10 +++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 02e4255a739..82102112721 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -68,7 +68,11 @@ pub fn parse(_: &mut Value, args: &[Value], _: &mut Interpreter) -> ResultValue /// [spec]: https://tc39.es/ecma262/#sec-json.stringify /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) -> ResultValue { - if let Some(object) = args.get(0) { + let object = match args.get(0) { + Some(obj) if obj.is_symbol() || obj.is_function() => return Ok(Value::undefined()), + 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().to_string())), @@ -125,9 +129,7 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - } else { Ok(Value::from(object.to_json().to_string())) } - } else { - Ok(Value::undefined()) - } + } /// Create a new `JSON` object. diff --git a/boa/src/builtins/json/tests.rs b/boa/src/builtins/json/tests.rs index 761b98c44a6..e5e419540aa 100644 --- a/boa/src/builtins/json/tests.rs +++ b/boa/src/builtins/json/tests.rs @@ -176,12 +176,16 @@ fn json_stringify_function_replacer_propogate_error() { } #[test] -fn json_stringify_with_no_args() { +fn json_stringify_return_undefined() { let realm = Realm::create(); let mut engine = Interpreter::new(realm); - let actual = forward(&mut engine, r#"JSON.stringify()"#); + let actual_no_args = forward(&mut engine, r#"JSON.stringify()"#); + let actual_function = forward(&mut engine, r#"JSON.stringify(() => {})"#); + let actual_symbol = forward(&mut engine, r#"JSON.stringify(Symbol())"#); let expected = forward(&mut engine, r#"undefined"#); - assert_eq!(actual, expected); + assert_eq!(actual_no_args, expected); + assert_eq!(actual_function, expected); + assert_eq!(actual_symbol, expected); } From f6cdd1c13dfbe4e9342b557dc11a387b61f0e945 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Fri, 29 May 2020 13:27:20 -0500 Subject: [PATCH 29/31] run formatter --- boa/src/builtins/json/mod.rs | 101 +++++++++++++++++------------------ 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 82102112721..9f81b899acb 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -73,63 +73,62 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - 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().to_string())), - }; + let replacer = match args.get(1) { + Some(replacer) if replacer.is_object() => replacer, + _ => return Ok(Value::from(object.to_json().to_string())), + }; - let replacer_as_object = replacer + let replacer_as_object = replacer + .as_object() + .expect("JSON.stringify replacer was an object"); + if replacer_as_object.is_callable() { + object .as_object() - .expect("JSON.stringify replacer was an object"); - if replacer_as_object.is_callable() { - object - .as_object() - .map(|obj| { - let object_to_return = Value::new_object(None); - for (key, val) in obj.properties.iter().filter_map(|(key, v)| { - if let Some(value) = &v.value { - Some((key, value)) - } else { - None - } - }) { - let mut this_arg = object.clone(); - object_to_return.set_property( - key.to_owned(), - Property::default().value(interpreter.call( - replacer, - &mut this_arg, - &[Value::string(key), val.clone()], - )?), - ); + .map(|obj| { + let object_to_return = Value::new_object(None); + for (key, val) in obj.properties.iter().filter_map(|(key, v)| { + if let Some(value) = &v.value { + Some((key, value)) + } else { + None } - Ok(Value::from(object_to_return.to_json().to_string())) - }) - .ok_or_else(Value::undefined)? - } else if replacer_as_object.kind == ObjectKind::Array { - let mut obj_to_return = - serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); - let fields = replacer_as_object.properties.keys().filter_map(|key| { - if key == "length" { - None - } else { - Some(replacer.get_field(key.to_string())) - } - }); - for field in fields { - if let Some(value) = object - .get_property(&field.to_string()) - .map(|prop| prop.value.as_ref().map(|v| v.to_json())) - .flatten() - { - obj_to_return.insert(field.to_string(), value); + }) { + let mut this_arg = object.clone(); + object_to_return.set_property( + key.to_owned(), + Property::default().value(interpreter.call( + replacer, + &mut this_arg, + &[Value::string(key), val.clone()], + )?), + ); } + Ok(Value::from(object_to_return.to_json().to_string())) + }) + .ok_or_else(Value::undefined)? + } else if replacer_as_object.kind == ObjectKind::Array { + let mut obj_to_return = + serde_json::Map::with_capacity(replacer_as_object.properties.len() - 1); + let fields = replacer_as_object.properties.keys().filter_map(|key| { + if key == "length" { + None + } else { + Some(replacer.get_field(key.to_string())) + } + }); + for field in fields { + if let Some(value) = object + .get_property(&field.to_string()) + .map(|prop| prop.value.as_ref().map(|v| v.to_json())) + .flatten() + { + obj_to_return.insert(field.to_string(), value); } - Ok(Value::from(JSONValue::Object(obj_to_return).to_string())) - } else { - Ok(Value::from(object.to_json().to_string())) } - + Ok(Value::from(JSONValue::Object(obj_to_return).to_string())) + } else { + Ok(Value::from(object.to_json().to_string())) + } } /// Create a new `JSON` object. From a61a567a938136ce79bf8bb2de4528cc8c5128e4 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Fri, 29 May 2020 17:08:20 -0500 Subject: [PATCH 30/31] mark code as unreachable --- boa/src/builtins/value/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/value/mod.rs b/boa/src/builtins/value/mod.rs index 06cde028813..ec682c7b09e 100644 --- a/boa/src/builtins/value/mod.rs +++ b/boa/src/builtins/value/mod.rs @@ -755,7 +755,7 @@ impl ValueData { panic!("TypeError: \"BigInt value can't be serialized in JSON\""); } Self::Symbol(_) | Self::Undefined => { - panic!("Symbols and Undefined JSON Values depend on parent type"); + unreachable!("Symbols and Undefined JSON Values depend on parent type"); } } } From 5a32e0217f12f34c688962aa8dad0cff3671a048 Mon Sep 17 00:00:00 2001 From: Nick Little Date: Fri, 29 May 2020 22:04:20 -0500 Subject: [PATCH 31/31] use option mapping over if let --- boa/src/builtins/json/mod.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 9f81b899acb..ad7cd378fd0 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -86,13 +86,11 @@ pub fn stringify(_: &mut Value, args: &[Value], interpreter: &mut Interpreter) - .as_object() .map(|obj| { let object_to_return = Value::new_object(None); - for (key, val) in obj.properties.iter().filter_map(|(key, v)| { - if let Some(value) = &v.value { - Some((key, value)) - } else { - None - } - }) { + for (key, val) in obj + .properties + .iter() + .filter_map(|(k, v)| v.value.as_ref().map(|value| (k, value))) + { let mut this_arg = object.clone(); object_to_return.set_property( key.to_owned(),