From f6d620aab78f6824af328672157efb389a5bedef Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Sun, 6 Oct 2024 23:36:15 +0200 Subject: [PATCH 1/7] replaceAll first attempt --- .../string_objects/string_prototype.rs | 138 +++++++++++++++++- 1 file changed, 131 insertions(+), 7 deletions(-) diff --git a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs index 9c80679b5..43e6d81e2 100644 --- a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs +++ b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::{collections::VecDeque, iter::repeat}; +use std::{cmp::max, collections::VecDeque, iter::repeat}; use small_string::SmallString; @@ -682,7 +682,7 @@ impl StringPrototype { let search_length = search_string.len(agent); // 8. Let position be StringIndexOf(s, searchString, 0). - let position = if let Some(position) = search_string.as_str(agent).find(s.as_str(agent)) + let position = if let Some(position) = s.as_str(agent).find(search_string.as_str(agent)) { position } else { @@ -708,7 +708,7 @@ impl StringPrototype { // 11. Let following be the substring of s from position + searchLength. // 12. If functionalReplace is true, let preceding = &s.as_str(agent)[0..position].to_owned(); - let following = &s.as_str(agent)[position..search_length].to_owned(); + let following = &s.as_str(agent)[position + search_length..].to_owned(); // 14. Return the string-concatenation of preceding, replacement, and following. let concatenated_result = format!("{}{}{}", preceding, result.as_str(agent), following); @@ -718,16 +718,140 @@ impl StringPrototype { // 6. If functionalReplace is false, Set replaceValue to ? ToString(replaceValue). let replace_string = to_string(agent, replace_value)?; // Everything are strings: `"foo".replace("o", "a")` => use rust's replace + let result = + s.as_str(agent) + .replacen(search_string.as_str(agent), replace_string.as_str(agent), 1); + Ok(String::from_string(agent, result).into_value()) + } + + /// ### [22.1.3.20 String.prototype.replaceAll ( searchValue, replaceValue )](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.replaceall) + fn replace_all(agent: &mut Agent, this_value: Value, args: ArgumentsList) -> JsResult { + // 1. Let O be ? RequireObjectCoercible(this value). + let o = require_object_coercible(agent, this_value)?; + + let search_value = args.get(0); + let replace_value = args.get(1); + + // 2. If searchValue is neither undefined nor null, then + if !search_value.is_null() && !search_value.is_undefined() { + // a. Let isRegExp be ? IsRegExp(searchValue). + let is_reg_exp = false; + + // b. If isRegExp is true, then + if is_reg_exp { + // i. Let flags be ? Get(searchValue, "flags"). + // ii. Perform ? RequireObjectCoercible(flags). + // iii. If ? ToString(flags) does not contain "g", throw a TypeError exception. + todo!(); + } + + // c. Let replacer be ? GetMethod(searchValue, %Symbol.replace%). + let symbol = WellKnownSymbolIndexes::Replace.into(); + let replacer = get_method(agent, search_value, symbol)?; + + // d. If replacer is not undefined, Return ? Call(replacer, searchValue, « O, replaceValue »). + if let Some(replacer) = replacer { + return call_function( + agent, + replacer, + search_value, + Some(ArgumentsList(&[o, replace_value])), + ); + } + } + + // 3. Let s be ? ToString(O). + let s = to_string(agent, o)?; + + // 4. Let searchString be ? ToString(searchValue). + let search_string = to_string(agent, search_value)?; + + // 5. Let functionalReplace be IsCallable(replaceValue). + if let Some(functional_replace) = is_callable(replace_value) { + // 7. Let searchLength be the length of searchString. + let search_length = search_string.len(agent); + + // 8. Let advanceBy be max(1, searchLength). + let advance_by = max(1, search_length); + + // 9. Let matchPositions be a new empty List. + let mut match_positions: Vec = vec![]; + + // 10. Let position be StringIndexOf(s, searchString, 0). + let search_str = search_string.as_str(agent).to_owned(); + let subject = s.as_str(agent).to_owned(); + + let mut position = if let Some(position) = subject.find(&search_str) { + position + } else { + // If position is not-found, return s. + return Ok(s.into_value()); + }; + + match_positions.push(position); + position += advance_by; + + // 11. Repeat, while position is not not-found, + loop { + if let Some(pos) = subject[position..].find(&search_str) { + match_positions.push(position + pos); + position += advance_by; + } else { + break; + }; + } + + // 12. Let endOfLastMatch be 0. + let mut end_of_last_match = 0; + + // 13. Let result be the empty String. + let mut result = "".to_string(); + + // 14. For each element p of matchPositions, do + for p in match_positions { + // a. Let preserved be the substring of string from endOfLastMatch to p. + let preserved = subject[end_of_last_match..p].to_owned(); + + // b. let replacement be ? ToString(? Call(replaceValue, undefined, « searchString, 𝔽(p), string »)). + let replacement = call_function( + agent, + functional_replace, + Value::Undefined, + Some(ArgumentsList(&[ + search_string.into_value(), + Number::from(position as u32).into_value(), + s.into_value(), + ])), + )?; + + // d. Set result to the string-concatenation of result, preserved, and replacement. + let replacement_str = replacement.to_string(agent)?; + let replacement_str = replacement_str.as_str(agent); + + let concat = format!("{}{}", &preserved, &replacement_str); + result.push_str(&concat); + end_of_last_match = p + search_length; + } + + // 15. If endOfLastMatch < the length of string, set result to the string-concatenation of result and the substring of string from endOfLastMatch. + if end_of_last_match < subject.len() { + let preserved = subject[end_of_last_match..].to_owned(); + result.push_str(&preserved); + } + + // 16. Return result. + return Ok(String::from_str(agent, &result).into_value()); + } + + // 6. If functionalReplace is false, Set replaceValue to ? ToString(replaceValue). + let replace_string = to_string(agent, replace_value)?; + // Everything are strings: `"foo".replaceAll("o", "a")` => use rust's replace let result = s .as_str(agent) .replace(search_string.as_str(agent), replace_string.as_str(agent)); Ok(String::from_string(agent, result).into_value()) } - fn replace_all(_agent: &mut Agent, _this_value: Value, _: ArgumentsList) -> JsResult { - todo!() - } - fn search(_agent: &mut Agent, _this_value: Value, _: ArgumentsList) -> JsResult { todo!() } From eafacf82ea8dce27ac9a4d43d7db8d0b521e2580 Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Sun, 6 Oct 2024 23:47:00 +0200 Subject: [PATCH 2/7] updated tests --- tests/expectations.json | 31 ------------------------------- tests/metrics.json | 8 ++++---- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/tests/expectations.json b/tests/expectations.json index 9e9d1f568..9be0c4beb 100644 --- a/tests/expectations.json +++ b/tests/expectations.json @@ -6193,16 +6193,10 @@ "built-ins/String/prototype/repeat/repeat-string-n-times.js": "TIMEOUT", "built-ins/String/prototype/replace/15.5.4.11-1.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A12.js": "CRASH", - "built-ins/String/prototype/replace/S15.5.4.11_A1_T10.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A1_T14.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A1_T16.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A1_T17.js": "CRASH", - "built-ins/String/prototype/replace/S15.5.4.11_A1_T4.js": "CRASH", - "built-ins/String/prototype/replace/S15.5.4.11_A1_T5.js": "CRASH", - "built-ins/String/prototype/replace/S15.5.4.11_A1_T6.js": "CRASH", - "built-ins/String/prototype/replace/S15.5.4.11_A1_T7.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A1_T8.js": "CRASH", - "built-ins/String/prototype/replace/S15.5.4.11_A1_T9.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A2_T1.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A2_T10.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A2_T2.js": "CRASH", @@ -6221,7 +6215,6 @@ "built-ins/String/prototype/replace/S15.5.4.11_A4_T3.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A4_T4.js": "CRASH", "built-ins/String/prototype/replace/S15.5.4.11_A5_T1.js": "CRASH", - "built-ins/String/prototype/replace/cstm-replace-is-null.js": "CRASH", "built-ins/String/prototype/replace/regexp-prototype-replace-v-flag.js": "CRASH", "built-ins/String/prototype/replaceAll/getSubstitution-0x0024-0x0024.js": "CRASH", "built-ins/String/prototype/replaceAll/getSubstitution-0x0024-0x0026.js": "CRASH", @@ -6231,17 +6224,8 @@ "built-ins/String/prototype/replaceAll/getSubstitution-0x0024.js": "CRASH", "built-ins/String/prototype/replaceAll/getSubstitution-0x0024N.js": "CRASH", "built-ins/String/prototype/replaceAll/getSubstitution-0x0024NN.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-call-abrupt.js": "CRASH", "built-ins/String/prototype/replaceAll/replaceValue-call-each-match-position.js": "CRASH", "built-ins/String/prototype/replaceAll/replaceValue-call-matching-empty.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-call-skip-no-match.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-call-tostring-abrupt.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-fn-skip-toString.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-tostring-abrupt.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-value-replaces-string.js": "CRASH", - "built-ins/String/prototype/replaceAll/replaceValue-value-tostring.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-empty-string-this-empty-string.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-empty-string.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-flags-no-g-throws.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-flags-null-undefined-throws.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-flags-toString-abrupt.js": "CRASH", @@ -6249,17 +6233,8 @@ "built-ins/String/prototype/replaceAll/searchValue-isRegExp-abrupt.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-replacer-RegExp-call-fn.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-replacer-RegExp-call.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-replacer-before-tostring.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-replacer-call-abrupt.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-replacer-call.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-replacer-is-null.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-replacer-method-abrupt.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-tostring-abrupt.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-tostring-regexp.js": "CRASH", - "built-ins/String/prototype/replaceAll/this-is-null-throws.js": "CRASH", - "built-ins/String/prototype/replaceAll/this-is-undefined-throws.js": "CRASH", - "built-ins/String/prototype/replaceAll/this-tostring-abrupt.js": "CRASH", - "built-ins/String/prototype/replaceAll/this-tostring.js": "CRASH", "built-ins/String/prototype/search/S15.5.4.12_A1.1_T1.js": "CRASH", "built-ins/String/prototype/search/S15.5.4.12_A1_T1.js": "CRASH", "built-ins/String/prototype/search/S15.5.4.12_A1_T10.js": "CRASH", @@ -18655,12 +18630,6 @@ "language/expressions/yield/star-rhs-unresolvable.js": "CRASH", "language/expressions/yield/star-string.js": "CRASH", "language/expressions/yield/star-throw-is-null.js": "CRASH", - "language/function-code/10.4.3-1-100-s.js": "CRASH", - "language/function-code/10.4.3-1-100gs.js": "CRASH", - "language/function-code/10.4.3-1-101-s.js": "CRASH", - "language/function-code/10.4.3-1-101gs.js": "CRASH", - "language/function-code/10.4.3-1-102-s.js": "CRASH", - "language/function-code/10.4.3-1-102gs.js": "CRASH", "language/function-code/eval-param-env-with-computed-key.js": "CRASH", "language/function-code/eval-param-env-with-prop-initializer.js": "CRASH", "language/global-code/decl-func-dup.js": "FAIL", diff --git a/tests/metrics.json b/tests/metrics.json index 4d199b9db..8eb9fc92b 100644 --- a/tests/metrics.json +++ b/tests/metrics.json @@ -1,11 +1,11 @@ { "results": { - "crash": 16842, - "fail": 7996, - "pass": 20331, + "crash": 16808, + "fail": 7999, + "pass": 20361, "skip": 40, "timeout": 3, "unresolved": 0 }, - "total": 45172 + "total": 45212 } \ No newline at end of file From d964099a8b059baa85140474f5693be1adc188a3 Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Mon, 7 Oct 2024 18:52:27 +0200 Subject: [PATCH 3/7] applied suggestions --- .../string_objects/string_prototype.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs index 43e6d81e2..795bf2c99 100644 --- a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs +++ b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs @@ -805,13 +805,10 @@ impl StringPrototype { let mut end_of_last_match = 0; // 13. Let result be the empty String. - let mut result = "".to_string(); + let mut result = std::string::String::with_capacity(subject.len()); // 14. For each element p of matchPositions, do for p in match_positions { - // a. Let preserved be the substring of string from endOfLastMatch to p. - let preserved = subject[end_of_last_match..p].to_owned(); - // b. let replacement be ? ToString(? Call(replaceValue, undefined, « searchString, 𝔽(p), string »)). let replacement = call_function( agent, @@ -824,23 +821,25 @@ impl StringPrototype { ])), )?; + // a. Let preserved be the substring of string from endOfLastMatch to p. + let preserved = &subject[end_of_last_match..p]; // d. Set result to the string-concatenation of result, preserved, and replacement. let replacement_str = replacement.to_string(agent)?; let replacement_str = replacement_str.as_str(agent); - - let concat = format!("{}{}", &preserved, &replacement_str); - result.push_str(&concat); + result.reserve(preserved.len() + replacement_str.len()); + result.push_str(preserved); + result.push_str(replacement_str); end_of_last_match = p + search_length; } // 15. If endOfLastMatch < the length of string, set result to the string-concatenation of result and the substring of string from endOfLastMatch. if end_of_last_match < subject.len() { - let preserved = subject[end_of_last_match..].to_owned(); - result.push_str(&preserved); + let preserved = &subject[end_of_last_match..]; + result.push_str(preserved); } // 16. Return result. - return Ok(String::from_str(agent, &result).into_value()); + return Ok(String::from_string(agent, result).into_value()); } // 6. If functionalReplace is false, Set replaceValue to ? ToString(replaceValue). From 770db35bc337b8de62980c7debb3f49485cbfad4 Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Mon, 7 Oct 2024 18:55:28 +0200 Subject: [PATCH 4/7] replace loop with while --- .../text_processing/string_objects/string_prototype.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs index 795bf2c99..61f99d95d 100644 --- a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs +++ b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs @@ -792,13 +792,9 @@ impl StringPrototype { position += advance_by; // 11. Repeat, while position is not not-found, - loop { - if let Some(pos) = subject[position..].find(&search_str) { - match_positions.push(position + pos); - position += advance_by; - } else { - break; - }; + while let Some(pos) = subject[position..].find(&search_str) { + match_positions.push(position + pos); + position += advance_by; } // 12. Let endOfLastMatch be 0. From 3e813cee9a4b25cb54098cd267e8cd1c358fbfef Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Fri, 11 Oct 2024 16:31:15 +0200 Subject: [PATCH 5/7] changes in string.prototype --- .../string_objects/string_prototype.rs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs index 61f99d95d..d00448e21 100644 --- a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs +++ b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs @@ -778,23 +778,19 @@ impl StringPrototype { let mut match_positions: Vec = vec![]; // 10. Let position be StringIndexOf(s, searchString, 0). - let search_str = search_string.as_str(agent).to_owned(); + let search_str = search_string.as_str(agent); let subject = s.as_str(agent).to_owned(); - - let mut position = if let Some(position) = subject.find(&search_str) { - position - } else { - // If position is not-found, return s. - return Ok(s.into_value()); - }; - - match_positions.push(position); - position += advance_by; + let mut position = 0; // 11. Repeat, while position is not not-found, - while let Some(pos) = subject[position..].find(&search_str) { + while let Some(pos) = subject[position..].find(search_str) { match_positions.push(position + pos); - position += advance_by; + position += advance_by + pos; + } + + // If none has found, return s. + if match_positions.len() == 0 { + return Ok(s.into_value()); } // 12. Let endOfLastMatch be 0. From 29f1fbf15e0948e0f7204ebbb211ab4eb51c4328 Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Fri, 11 Oct 2024 16:41:27 +0200 Subject: [PATCH 6/7] fix clippy issues --- .../builtins/text_processing/string_objects/string_prototype.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs index d00448e21..10cc83931 100644 --- a/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs +++ b/nova_vm/src/ecmascript/builtins/text_processing/string_objects/string_prototype.rs @@ -789,7 +789,7 @@ impl StringPrototype { } // If none has found, return s. - if match_positions.len() == 0 { + if match_positions.is_empty() { return Ok(s.into_value()); } From ccdd4d33070bbd5e2a41d55e741c3aab96be812e Mon Sep 17 00:00:00 2001 From: Oscar Otero Date: Fri, 11 Oct 2024 20:47:20 +0200 Subject: [PATCH 7/7] updated metrics --- tests/expectations.json | 1 - tests/metrics.json | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/expectations.json b/tests/expectations.json index ac513ac10..125d53a54 100644 --- a/tests/expectations.json +++ b/tests/expectations.json @@ -6231,7 +6231,6 @@ "built-ins/String/prototype/replaceAll/searchValue-isRegExp-abrupt.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-replacer-RegExp-call-fn.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-replacer-RegExp-call.js": "CRASH", - "built-ins/String/prototype/replaceAll/searchValue-replacer-is-null.js": "CRASH", "built-ins/String/prototype/replaceAll/searchValue-tostring-regexp.js": "CRASH", "built-ins/String/prototype/search/S15.5.4.12_A1.1_T1.js": "CRASH", "built-ins/String/prototype/search/S15.5.4.12_A1_T1.js": "CRASH", diff --git a/tests/metrics.json b/tests/metrics.json index 8eb9fc92b..e3b00d339 100644 --- a/tests/metrics.json +++ b/tests/metrics.json @@ -1,8 +1,8 @@ { "results": { - "crash": 16808, - "fail": 7999, - "pass": 20361, + "crash": 16279, + "fail": 8210, + "pass": 20679, "skip": 40, "timeout": 3, "unresolved": 0