Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: String.prototype.replace substitutions, closes #610 #629

Merged
merged 9 commits into from
Sep 1, 2020
+157 −34
Merged

Fix: String.prototype.replace substitutions, closes #610 #629

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24eda41
Fix: String.prototype.replace for multiple capture groups, closes #610
RageKnify Aug 13, 2020
eb813fd
Test: Add test for #629, based on #610
RageKnify Aug 13, 2020
3b57355
Merge branch 'master' into fix/string-replace
RageKnify Aug 24, 2020
33c07e6
Fix: String.prototype.replace capture groups per spec, see #629
RageKnify Aug 24, 2020
81cf31d
Test: Improve coverage for String.prototype.replace
RageKnify Aug 24, 2020
a8222ce
Refactor: String.prototype.replace for better readability
RageKnify Aug 24, 2020
6cbc366
Fix: String.prototype.replace to not panic if no match
RageKnify Aug 25, 2020
1ca0828
Refactor: Cleanup String.prototype.replace
RageKnify Sep 1, 2020
54df208
Refactor: Cleanup String.prototype.replace
RageKnify Sep 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 86 additions & 34 deletions boa/src/builtins/string/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,10 @@ impl String {

let regex_body = Self::get_regex_string(args.get(0).expect("Value needed"));
let re = Regex::new(&regex_body).expect("unable to convert regex to regex object");
let mat = re.find(&primitive_val).expect("unable to find value");
let mat = match re.find(&primitive_val) {
Some(mat) => mat,
None => return Ok(Value::from(primitive_val)),
};
let caps = re
.captures(&primitive_val)
.expect("unable to get capture groups from text");
Expand All @@ -479,39 +482,88 @@ impl String {
match replace_object {
Value::String(val) => {
// https://tc39.es/ecma262/#table-45
let mut result = val.to_string();
let re = Regex::new(r"\$(\d)").unwrap();

if val.find("$$").is_some() {
result = val.replace("$$", "$")
}

if val.find("$`").is_some() {
let start_of_match = mat.start();
let slice = &primitive_val[..start_of_match];
result = val.replace("$`", slice);
}

if val.find("$'").is_some() {
let end_of_match = mat.end();
let slice = &primitive_val[end_of_match..];
result = val.replace("$'", slice);
}

if val.find("$&").is_some() {
// get matched value
let matched = caps.get(0).expect("cannot get matched value");
result = val.replace("$&", matched.as_str());
}

// Capture $1, $2, $3 etc
if re.is_match(&result) {
let mat_caps = re.captures(&result).unwrap();
let group_str = mat_caps.get(1).unwrap().as_str();
let group_int = group_str.parse::<usize>().unwrap();
result = re
.replace(result.as_str(), caps.get(group_int).unwrap().as_str())
.to_string()
let mut result = StdString::new();
let mut chars = val.chars().peekable();

let m = caps.len();

while let Some(first) = chars.next() {
if first == '$' {
let second = chars.next();
let second_is_digit = second.map_or(false, |ch| ch.is_digit(10));
// we use peek so that it is still in the iterator if not used
let third = if second_is_digit { chars.peek() } else { None };
let third_is_digit = third.map_or(false, |ch| ch.is_digit(10));

match (second, third) {
(Some('$'), _) => {
// $$
result.push('$');
}
(Some('&'), _) => {
// $&
let matched = caps.get(0).expect("cannot get matched value");
Razican marked this conversation as resolved.
Show resolved Hide resolved
result.push_str(matched.as_str());
}
(Some('`'), _) => {
// $`
let start_of_match = mat.start();
result.push_str(&primitive_val[..start_of_match]);
}
(Some('\''), _) => {
// $'
let end_of_match = mat.end();
result.push_str(&primitive_val[end_of_match..]);
}
(Some(second), Some(third))
if second_is_digit && third_is_digit =>
{
// $nn
let tens = second.to_digit(10).unwrap() as usize;
let units = third.to_digit(10).unwrap() as usize;
let nn = 10 * tens + units;
if nn == 0 || nn > m {
[Some(first), Some(second), chars.next()]
.iter()
.flatten()
.for_each(|ch: &char| result.push(*ch));
HalidOdat marked this conversation as resolved.
Show resolved Hide resolved
} else {
let group = match caps.get(nn) {
Some(text) => text.as_str(),
None => "",
};
result.push_str(group);
chars.next(); // consume third
}
}
(Some(second), _) if second_is_digit => {
// $n
let n = second.to_digit(10).unwrap() as usize;
if n == 0 || n > m {
result.push(first);
result.push(second);
} else {
let group = match caps.get(n) {
Some(text) => text.as_str(),
None => "",
};
result.push_str(group);
}
}
(Some('<'), _) => {
// $<
todo!("named capture groups")
}
_ => {
// $?, ? is none of the above
// we can consume second because it isn't $
result.push(first);
second.iter().for_each(|ch: &char| result.push(*ch));
}
HalidOdat marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
result.push(first);
}
}

result
Expand Down
68 changes: 68 additions & 0 deletions boa/src/builtins/string/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,74 @@ fn replace() {
assert_eq!(forward(&mut engine, "a"), "\"2bc\"");
}

#[test]
fn replace_no_match() {
let realm = Realm::create();
let mut engine = Interpreter::new(realm);
let init = r#"
var a = "abc";
a = a.replace(/d/, "$&$&");
"#;

forward(&mut engine, init);

assert_eq!(forward(&mut engine, "a"), "\"abc\"");
}

#[test]
fn replace_with_capture_groups() {
let realm = Realm::create();
let mut engine = Interpreter::new(realm);
let init = r#"
var re = /(\w+)\s(\w+)/;
var a = "John Smith";
a = a.replace(re, '$2, $1');
a
"#;

forward(&mut engine, init);

assert_eq!(forward(&mut engine, "a"), "\"Smith, John\"");
}

#[test]
fn replace_with_tenth_capture_group() {
let realm = Realm::create();
let mut engine = Interpreter::new(realm);
let init = r#"
var re = /(\d)(\d)(\d)(\d)(\d)(\d)(\d)(\d)(\d)(\d)/;
var a = "0123456789";
let res = a.replace(re, '$10');
"#;

forward(&mut engine, init);

assert_eq!(forward(&mut engine, "res"), "\"9\"");
}

#[test]
fn replace_substitutions() {
let realm = Realm::create();
let mut engine = Interpreter::new(realm);
let init = r#"
var re = / two /;
var a = "one two three";
var dollar = a.replace(re, " $$ ");
var matched = a.replace(re, "$&$&");
var start = a.replace(re, " $` ");
var end = a.replace(re, " $' ");
var no_sub = a.replace(re, " $_ ");
"#;

forward(&mut engine, init);

assert_eq!(forward(&mut engine, "dollar"), "\"one $ three\"");
assert_eq!(forward(&mut engine, "matched"), "\"one two two three\"");
assert_eq!(forward(&mut engine, "start"), "\"one one three\"");
assert_eq!(forward(&mut engine, "end"), "\"one three three\"");
assert_eq!(forward(&mut engine, "no_sub"), "\"one $_ three\"");
}

#[test]
fn replace_with_function() {
let realm = Realm::create();
Expand Down