From 8fbc3f5a7d8ccee5162d0c3a50f69d2282a8fde2 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 18:42:36 -0800 Subject: [PATCH 01/10] Add String.prototype.split --- boa/src/builtins/string/mod.rs | 59 +++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index a9bdc22f29b..7da8c7482d2 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -15,7 +15,7 @@ mod tests; use crate::property::DataDescriptor; use crate::{ - builtins::{string::string_iterator::StringIterator, BuiltIn, RegExp}, + builtins::{string::string_iterator::StringIterator, Array, BuiltIn, RegExp}, object::{ConstructorBuilder, Object, ObjectData}, property::Attribute, value::{RcString, Value}, @@ -105,6 +105,7 @@ impl BuiltIn for String { .method(Self::to_uppercase, "toUpperCase", 0) .method(Self::substring, "substring", 2) .method(Self::substr, "substr", 2) + .method(Self::split, "split", 2) .method(Self::value_of, "valueOf", 0) .method(Self::match_all, "matchAll", 1) .method(Self::replace, "replace", 2) @@ -1165,6 +1166,62 @@ impl String { } } + /// String.prototype.split() + /// + /// The `split()` method divides a String into an ordered list of substrings, puts these substrings into an array, and returns the array. + /// + /// The division is done by searching for a pattern; where the pattern is provided as the first parameter in the method's call. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-string.prototype.split + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split + pub(crate) fn split(this: &Value, args: &[Value], context: &mut Context) -> Result { + let string = this.to_string(context)?; + + let separator = if args.is_empty() { + None + } else { + let arg0 = args + .get(0) + .expect("failed to get argument for String method"); + + if arg0.is_null_or_undefined() { + None + } else { + arg0.to_string(context).ok() + } + }; + + let limit = if args.len() < 2 { + std::u32::MAX as usize + } else { + args.get(1) + .expect("Could not get the limit argument") + .to_integer(context)? as usize + }; + + let values: Vec = match separator { + None if limit == 0 => vec![], + None => vec![Value::from(string)], + Some(separator) if separator == "" => string + .encode_utf16() + .map(|cp| Value::from(std::string::String::from_utf16_lossy(&[cp]))) + .take(limit) + .collect(), + Some(separator) => string + .split(separator.as_str()) + .map(|v| Value::from(v)) + .take(limit) + .collect(), + }; + + let new = Array::new_array(context)?; + Array::construct_array(&new, &values, context) + } + /// String.prototype.valueOf() /// /// The `valueOf()` method returns the primitive value of a `String` object. From 918434c0506a64fc6d05ee45ef7c9bab41a3a539 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 18:57:20 -0800 Subject: [PATCH 02/10] Add unit test --- boa/src/builtins/string/tests.rs | 52 ++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs index a4f9409e380..c5a49a4216c 100644 --- a/boa/src/builtins/string/tests.rs +++ b/boa/src/builtins/string/tests.rs @@ -563,6 +563,58 @@ fn trim_end() { assert_eq!(forward(&mut context, "' Hello '.trimEnd()"), "\" Hello\""); } +#[test] +fn split() { + let mut context = Context::new(); + assert_eq!( + forward(&mut context, "'Hello'.split()"), + forward(&mut context, "['Hello']") + ); + assert_eq!( + forward(&mut context, "'Hello'.split(null)"), + forward(&mut context, "['Hello']") + ); + assert_eq!( + forward(&mut context, "'Hello'.split(undefined)"), + forward(&mut context, "['Hello']") + ); + assert_eq!( + forward(&mut context, "'Hello'.split('')"), + forward(&mut context, "['H','e','l','l','o']") + ); + + assert_eq!( + forward(&mut context, "'x1x2'.split('x')"), + forward(&mut context, "['','1','2']") + ); + assert_eq!( + forward(&mut context, "'x1x2x'.split('x')"), + forward(&mut context, "['','1','2','']") + ); + + assert_eq!( + forward(&mut context, "'x1x2x'.split('x', 0)"), + forward(&mut context, "[]") + ); + assert_eq!( + forward(&mut context, "'x1x2x'.split('x', 2)"), + forward(&mut context, "['','1']") + ); + assert_eq!( + forward(&mut context, "'x1x2x'.split('x', 10)"), + forward(&mut context, "['','1','2','']") + ); + + assert_eq!( + forward(&mut context, "'Hello'.split(null, 0)"), + forward(&mut context, "[]") + ); + assert_eq!( + forward(&mut context, "'Hello'.split(undefined, 0)"), + forward(&mut context, "[]") + ); +} + #[test] fn index_of_with_no_arguments() { let mut context = Context::new(); From 1c5302e9672789365a6ac528ab34472cd8021e06 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 19:03:11 -0800 Subject: [PATCH 03/10] Fix clippy --- boa/src/builtins/string/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 7da8c7482d2..c5b0cc30aff 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1213,7 +1213,7 @@ impl String { .collect(), Some(separator) => string .split(separator.as_str()) - .map(|v| Value::from(v)) + .map(&Value::from) .take(limit) .collect(), }; From c2de7bba6553f542ff327a7727f8276cb07dd7df Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 19:59:46 -0800 Subject: [PATCH 04/10] Add more tests --- boa/src/builtins/string/tests.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs index c5a49a4216c..abea994540a 100644 --- a/boa/src/builtins/string/tests.rs +++ b/boa/src/builtins/string/tests.rs @@ -613,6 +613,29 @@ fn split() { forward(&mut context, "'Hello'.split(undefined, 0)"), forward(&mut context, "[]") ); + + assert_eq!( + forward(&mut context, "''.split()"), + forward(&mut context, "['']") + ); + assert_eq!( + forward(&mut context, "''.split(undefined)"), + forward(&mut context, "['']") + ); + assert_eq!( + forward(&mut context, "''.split('')"), + forward(&mut context, "[]") + ); + assert_eq!( + forward(&mut context, "''.split('1')"), + forward(&mut context, "['']") + ); + + // TODO: Support invalid code point in string + assert_eq!( + forward(&mut context, "'𝟘𝟙𝟚𝟛'.split('')"), + forward(&mut context, "['�','�','�','�','�','�','�','�']") + ); } #[test] From 7ee639b8772cf7d4c0ca9a73bba872321dd8ef2c Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 20:00:35 -0800 Subject: [PATCH 05/10] Add support for separator with @@split method --- boa/src/builtins/string/mod.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index c5b0cc30aff..afd39bda16f 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1186,11 +1186,27 @@ impl String { } else { let arg0 = args .get(0) - .expect("failed to get argument for String method"); + .expect("Failed to get argument for String method"); if arg0.is_null_or_undefined() { None } else { + if let Some(splitter) = + arg0.get_property(context.well_known_symbols().split_symbol()) + { + let splitter = splitter.as_data_descriptor().unwrap().value(); + if splitter.is_function() { + let arguments = vec![ + Value::from(string), + args.get(1) + .map(|x| x.to_owned()) + .unwrap_or(Value::Undefined), + ]; + return context.call(&splitter, this, &arguments); + } else if !splitter.is_undefined() { + return Err(Value::from("separator[Symbol.split] is not a function")); + } + } arg0.to_string(context).ok() } }; From 0cc01a62c913bf499d9015b5b02ff623b7ea0098 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 21:14:17 -0800 Subject: [PATCH 06/10] Add todo comments --- boa/src/builtins/string/mod.rs | 1 + boa/src/builtins/string/tests.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index afd39bda16f..35494a1c5e5 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1224,6 +1224,7 @@ impl String { None => vec![Value::from(string)], Some(separator) if separator == "" => string .encode_utf16() + // TODO: Support keeping invalid code point in string .map(|cp| Value::from(std::string::String::from_utf16_lossy(&[cp]))) .take(limit) .collect(), diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs index abea994540a..4027d700604 100644 --- a/boa/src/builtins/string/tests.rs +++ b/boa/src/builtins/string/tests.rs @@ -631,7 +631,7 @@ fn split() { forward(&mut context, "['']") ); - // TODO: Support invalid code point in string + // TODO: Support keeping invalid code point in string assert_eq!( forward(&mut context, "'𝟘𝟙𝟚𝟛'.split('')"), forward(&mut context, "['�','�','�','�','�','�','�','�']") From f842431a73167525c225ef2721f86e17598db434 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Fri, 1 Jan 2021 21:33:32 -0800 Subject: [PATCH 07/10] Add more unit tests --- boa/src/builtins/string/mod.rs | 4 ++- boa/src/builtins/string/tests.rs | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 35494a1c5e5..61df1df14d9 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1204,7 +1204,9 @@ impl String { ]; return context.call(&splitter, this, &arguments); } else if !splitter.is_undefined() { - return Err(Value::from("separator[Symbol.split] is not a function")); + return Err(Value::from( + "TypeError: separator[Symbol.split] is not a function", + )); } } arg0.to_string(context).ok() diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs index 4027d700604..e94d0636aff 100644 --- a/boa/src/builtins/string/tests.rs +++ b/boa/src/builtins/string/tests.rs @@ -605,6 +605,11 @@ fn split() { forward(&mut context, "['','1','2','']") ); + assert_eq!( + forward(&mut context, "'x1x2x'.split(1)"), + forward(&mut context, "['x','x2x']") + ); + assert_eq!( forward(&mut context, "'Hello'.split(null, 0)"), forward(&mut context, "[]") @@ -638,6 +643,49 @@ fn split() { ); } +#[test] +fn split_with_symbol_split_method() { + assert_eq!( + forward( + &mut Context::new(), + r#" + let sep = {}; + sep[Symbol.split] = function(s, limit) { return s + limit.toString(); }; + 'hello'.split(sep, 10) + "# + ), + "\"hello10\"" + ); + + assert_eq!( + forward( + &mut Context::new(), + r#" + let sep = {}; + sep[Symbol.split] = undefined; + 'hello'.split(sep) + "# + ), + "[ \"hello\" ]" + ); + + assert_eq!( + forward( + &mut Context::new(), + r#" + try { + let sep = {}; + sep[Symbol.split] = 10; + 'hello'.split(sep, 10); + } catch(e) { + e.toString() + } + "# + ), + "\"TypeError: separator[Symbol.split] is not a function\"" + ); +} + #[test] fn index_of_with_no_arguments() { let mut context = Context::new(); From 424dad5a7b975b12f0a675d02f1b81ecad220a59 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Sat, 2 Jan 2021 12:56:40 -0800 Subject: [PATCH 08/10] Add RequireObjectCoercible call --- boa/src/builtins/string/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 61df1df14d9..b15e960297f 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1179,6 +1179,7 @@ impl String { /// [spec]: https://tc39.es/ecma262/#sec-string.prototype.split /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split pub(crate) fn split(this: &Value, args: &[Value], context: &mut Context) -> Result { + let this = this.require_object_coercible(context)?; let string = this.to_string(context)?; let separator = if args.is_empty() { From ecd0fe8f61d62eede3152710583f12eebff8f304 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Sat, 2 Jan 2021 15:58:50 -0800 Subject: [PATCH 09/10] Fix nit --- boa/src/builtins/string/mod.rs | 62 ++++++++++++++++------------------ 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index b15e960297f..6300ec7b23f 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1182,50 +1182,46 @@ impl String { let this = this.require_object_coercible(context)?; let string = this.to_string(context)?; - let separator = if args.is_empty() { - None - } else { - let arg0 = args - .get(0) - .expect("Failed to get argument for String method"); - - if arg0.is_null_or_undefined() { - None - } else { - if let Some(splitter) = - arg0.get_property(context.well_known_symbols().split_symbol()) - { - let splitter = splitter.as_data_descriptor().unwrap().value(); - if splitter.is_function() { + let separator = args.get(0).filter(|value| !value.is_null_or_undefined()); + + if let Some(result) = separator + .and_then(|separator| separator.as_object()) + .and_then(|separator| { + let key = context.well_known_symbols().split_symbol(); + + match separator.get_method(context, key) { + Ok(splitter) => splitter.map(|splitter| { let arguments = vec![ - Value::from(string), + Value::from(string.clone()), args.get(1) .map(|x| x.to_owned()) .unwrap_or(Value::Undefined), ]; - return context.call(&splitter, this, &arguments); - } else if !splitter.is_undefined() { - return Err(Value::from( - "TypeError: separator[Symbol.split] is not a function", - )); - } + splitter.call(this, &arguments, context) + }), + Err(_) => Some(Err( + context.construct_type_error("separator[Symbol.split] is not a function") + )), } - arg0.to_string(context).ok() - } - }; + }) + { + return result; + } - let limit = if args.len() < 2 { - std::u32::MAX as usize - } else { - args.get(1) - .expect("Could not get the limit argument") - .to_integer(context)? as usize - }; + let separator = separator + .map(|separator| separator.to_string(context)) + .transpose()?; + + let limit = args + .get(1) + .map(|arg| arg.to_integer(context).map(|limit| limit as usize)) + .transpose()? + .unwrap_or(std::u32::MAX as usize); let values: Vec = match separator { None if limit == 0 => vec![], None => vec![Value::from(string)], - Some(separator) if separator == "" => string + Some(separator) if separator.is_empty() => string .encode_utf16() // TODO: Support keeping invalid code point in string .map(|cp| Value::from(std::string::String::from_utf16_lossy(&[cp]))) From 689e3c18711757c5666f6efcccab1267a7aef627 Mon Sep 17 00:00:00 2001 From: Jevan Chan Date: Sat, 2 Jan 2021 17:00:52 -0800 Subject: [PATCH 10/10] Minor nit --- boa/src/builtins/string/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 6300ec7b23f..6cf9c511ea9 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -1191,13 +1191,13 @@ impl String { match separator.get_method(context, key) { Ok(splitter) => splitter.map(|splitter| { - let arguments = vec![ + let arguments = &[ Value::from(string.clone()), args.get(1) .map(|x| x.to_owned()) .unwrap_or(Value::Undefined), ]; - splitter.call(this, &arguments, context) + splitter.call(this, arguments, context) }), Err(_) => Some(Err( context.construct_type_error("separator[Symbol.split] is not a function")