From 74d9bc4d7a754fc4c2ae4216c86482b50bc1405b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 20:55:17 +0200 Subject: [PATCH 1/2] fix(runtime): exec/test at lastIndex > 0 keep the whole subject (#9429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `regex/exec.rs` searched `&str_data[search_start_byte..]` and re-based the reported offsets. Offsets survived; assertions did not. A slice invents context at its left edge (`^`, `\b`, `(? length` was a search clamped to the end rather than "no match" (RegExpBuiltinExec 12.a) — `/a*/g` with lastIndex 5 on "ab" returned ""@2. The guard compared byte offsets, and `utf16_index_to_byte` saturates at the payload length, so it could never fire; the bound is now the UTF-16 comparison it always had to be. Fixture demonstrated failing (72 diff lines) on a compiler built from unfixed origin/main, byte-identical to node after. Six runtime tests, each confirmed to fail against the pre-fix engine calls. --- .../9429-regexp-exec-lastindex-context.md | 49 ++++++ crates/perry-runtime/src/regex/exec.rs | 82 +++++----- crates/perry-runtime/src/regex/exec_array.rs | 30 ++-- .../perry-runtime/src/regex/match_string.rs | 5 +- crates/perry-runtime/src/regex/tests.rs | 154 ++++++++++++++++++ ..._gap_9429_regexp_exec_lastindex_context.ts | 133 +++++++++++++++ 6 files changed, 394 insertions(+), 59 deletions(-) create mode 100644 changelog.d/9429-regexp-exec-lastindex-context.md create mode 100644 test-files/test_gap_9429_regexp_exec_lastindex_context.ts diff --git a/changelog.d/9429-regexp-exec-lastindex-context.md b/changelog.d/9429-regexp-exec-lastindex-context.md new file mode 100644 index 0000000000..1da787d1f2 --- /dev/null +++ b/changelog.d/9429-regexp-exec-lastindex-context.md @@ -0,0 +1,49 @@ +**`exec`/`test` at a non-zero `lastIndex` now evaluate the pattern against the +whole subject instead of `subject.slice(lastIndex)`** — `^`, `$`, `\b` and both +lookaround directions get their real context back. No flag beyond `g`/`y` was +needed to see this: + +```js +const r = /^b/g; r.lastIndex = 1; r.exec("ab") // was "b", now null +const l = /(?<=a)b/g; l.lastIndex = 1; l.exec("ab") // was null, now "b" +``` + +The engine call sliced the subject at the start offset and then re-based every +reported range by the same amount. Offsets survived that round trip; assertions +did not. A slice invents context at its left edge — `^` and `\b` hold at +offset 0 of the slice, where the subject says they must not — and destroys it — +`(?<=a)` cannot see the character it needs, and `(? length` was not "no match" +(RegExpBuiltinExec step 12.a) but a search clamped to the end of the subject — +`/a*/g` with `lastIndex = 5` on `"ab"` returned an empty match at index 2 where +Node returns `null`. The bound could not be expressed where it was being +checked: it is a UTF-16 code-unit comparison, and `utf16_index_to_byte` +saturates at the payload length, so the byte-offset guard it replaced could +never fire. That also matters for astral subjects, where the code-unit length +and the scalar count differ. + +Pinned by six runtime tests — one per engine lane, plus the past-the-end bound +and the `test` routing — and by a fixture byte-compared against Node covering +`^`, `$`, `\b`, `\B`, lookbehind, negative lookbehind and lookahead at +`lastIndex` 0 / mid-subject / end / past-end, sticky and global, and seven +hand-driven `exec` sweeps that have to terminate. + +Two of those sweeps need #9408 (landed in #9427) as well as this fix, and are +the reason to read the pair together: `while ((m = /^/gm.exec("one\r\ntwo")))` +walks `[0, 4, 5]` — Node's answer — only with both. With #9408 alone the loop +never terminates, because `^` holds at the slice's left edge at every index; +with this fix alone it stops early at `[0, 5]`, because `(?m)` still sees LF +only. diff --git a/crates/perry-runtime/src/regex/exec.rs b/crates/perry-runtime/src/regex/exec.rs index 76ee03da26..3b18cbb660 100644 --- a/crates/perry-runtime/src/regex/exec.rs +++ b/crates/perry-runtime/src/regex/exec.rs @@ -44,61 +44,73 @@ pub extern "C" fn js_regexp_exec( let has_indices = (*re).has_indices; let use_last_index = global || sticky; let last_index = if use_last_index { last_index_read } else { 0 }; - let search_start_byte = if use_last_index && last_index > 0 { - super::exec_array::utf16_index_to_byte(str_data, last_index) - } else { - 0 - }; - if search_start_byte > str_data.len() { - if use_last_index { - set_last_index_throwing(re, 0); - } + // Spec RegExpBuiltinExec step 12.a: `lastIndex > length` is "no match" + // outright — NOT a search clamped to the end of the subject. The bound + // is in UTF-16 code units, the same unit `lastIndex` is stored in; + // comparing byte offsets can't express it because + // `utf16_index_to_byte` saturates at `str_data.len()`. + if use_last_index && last_index > (*s).utf16_len as usize { + set_last_index_throwing(re, 0); LAST_EXEC_INDEX.with(|idx| *idx.borrow_mut() = -1.0); LAST_EXEC_GROUPS.with(|g| *g.borrow_mut() = ptr::null_mut()); return ptr::null_mut(); } - let search_str = &str_data[search_start_byte..]; + let search_start_byte = if use_last_index && last_index > 0 { + super::exec_array::utf16_index_to_byte(str_data, last_index) + } else { + 0 + }; + + // #9429: search FROM `search_start_byte` in the whole subject rather + // than searching a `&str_data[search_start_byte..]` slice. Every + // zero-width assertion — `^`, `$`, `\b`, and both lookaround + // directions — is defined against the real subject, and a slice both + // invents context at its left edge (`^`/`\b` hold where they must not) + // and destroys it (`(?<=a)` fails where it must hold). All three + // engines expose a positional entry point with exactly these + // semantics, documented as such: `regex::Regex::captures_at`, + // `fancy_regex::Regex::captures_from_pos` and + // `regress::Regex::find_from`. Their reported offsets are absolute, so + // nothing downstream re-bases them. let owned = if let Some(repeat_matcher) = lookup_repeat_matcher(re) { repeat_matcher .regex - .find(search_str) - .filter(|matched| !sticky || matched.start() == 0) + .find_from(str_data, search_start_byte) + .next() + .filter(|matched| !sticky || matched.start() == search_start_byte) .map(|matched| { if use_last_index { set_last_index_throwing( re, - super::exec_array::byte_index_to_utf16_index( - str_data, - search_start_byte + matched.end(), - ), + super::exec_array::byte_index_to_utf16_index(str_data, matched.end()), ); } OwnedExecMatch::from_repeat_matcher( str_data, - search_start_byte, &repeat_matcher, &matched, has_indices, ) }) } else if let Some(fre) = lookup_fancy_regex(re) { - match fre.captures(search_str) { - Ok(Some(caps)) if !sticky || caps.get(0).is_some_and(|full| full.start() == 0) => { + match fre.captures_from_pos(str_data, search_start_byte) { + Ok(Some(caps)) + if !sticky + || caps + .get(0) + .is_some_and(|full| full.start() == search_start_byte) => + { let full = caps.get(0).expect("capture zero is the full match"); if use_last_index { set_last_index_throwing( re, - super::exec_array::byte_index_to_utf16_index( - str_data, - search_start_byte + full.end(), - ), + super::exec_array::byte_index_to_utf16_index(str_data, full.end()), ); } Some(OwnedExecMatch::from_fancy( str_data, - search_start_byte, &fre, &caps, has_indices, @@ -108,26 +120,22 @@ pub extern "C" fn js_regexp_exec( } } else { regex - .captures(search_str) - .filter(|caps| !sticky || caps.get(0).is_some_and(|full| full.start() == 0)) + .captures_at(str_data, search_start_byte) + .filter(|caps| { + !sticky + || caps + .get(0) + .is_some_and(|full| full.start() == search_start_byte) + }) .map(|caps| { let full = caps.get(0).expect("capture zero is the full match"); if use_last_index { set_last_index_throwing( re, - super::exec_array::byte_index_to_utf16_index( - str_data, - search_start_byte + full.end(), - ), + super::exec_array::byte_index_to_utf16_index(str_data, full.end()), ); } - OwnedExecMatch::from_standard( - str_data, - search_start_byte, - regex, - &caps, - has_indices, - ) + OwnedExecMatch::from_standard(str_data, regex, &caps, has_indices) }) }; diff --git a/crates/perry-runtime/src/regex/exec_array.rs b/crates/perry-runtime/src/regex/exec_array.rs index 8c9602f6ed..461136cdab 100644 --- a/crates/perry-runtime/src/regex/exec_array.rs +++ b/crates/perry-runtime/src/regex/exec_array.rs @@ -63,6 +63,12 @@ impl OwnedCapture { /// All subject-derived state needed to build one RegExp match result. This is /// deliberately owned/scalar-only: no `&str`, `regex::Match`, or `Captures` /// may survive into the allocation phase (#8449). +/// +/// Every offset here is absolute in `str_data`. The constructors used to take +/// a `search_start_byte` and re-base onto it, because `exec` searched a +/// `&str_data[lastIndex..]` slice; they no longer do, because `exec` searches +/// the whole subject from a position (#9429). Reintroducing a base offset +/// would mean the engine had been handed a slice again. pub(super) struct OwnedExecMatch { captures: Vec>, named: Vec<(String, usize)>, @@ -72,7 +78,6 @@ pub(super) struct OwnedExecMatch { impl OwnedExecMatch { pub(super) fn from_standard( str_data: &str, - search_start_byte: usize, regex: ®ex::Regex, caps: ®ex::Captures, has_indices: bool, @@ -81,12 +86,7 @@ impl OwnedExecMatch { .iter() .map(|capture| { capture.map(|m| { - OwnedCapture::from_range_with_indices( - str_data, - search_start_byte + m.start(), - search_start_byte + m.end(), - has_indices, - ) + OwnedCapture::from_range_with_indices(str_data, m.start(), m.end(), has_indices) }) }) .collect(); @@ -113,7 +113,6 @@ impl OwnedExecMatch { pub(super) fn from_fancy( str_data: &str, - search_start_byte: usize, regex: &fancy_regex::Regex, caps: &fancy_regex::Captures, has_indices: bool, @@ -121,12 +120,7 @@ impl OwnedExecMatch { let captures: Vec> = (0..caps.len()) .map(|index| { caps.get(index).map(|m| { - OwnedCapture::from_range_with_indices( - str_data, - search_start_byte + m.start(), - search_start_byte + m.end(), - has_indices, - ) + OwnedCapture::from_range_with_indices(str_data, m.start(), m.end(), has_indices) }) }) .collect(); @@ -153,7 +147,6 @@ impl OwnedExecMatch { pub(super) fn from_repeat_matcher( str_data: &str, - search_start_byte: usize, regex: &super::repeat_matcher::RepeatMatcherRegex, matched: ®ress::Match, has_indices: bool, @@ -164,8 +157,8 @@ impl OwnedExecMatch { capture.map(|range| { OwnedCapture::from_range_with_indices( str_data, - search_start_byte + range.start, - search_start_byte + range.end, + range.start, + range.end, has_indices, ) }) @@ -177,8 +170,7 @@ impl OwnedExecMatch { .enumerate() .filter_map(|(index, name)| name.as_ref().map(|name| (name.clone(), index + 1))) .collect(); - let match_index = - byte_index_to_utf16_index(str_data, search_start_byte + matched.start()) as f64; + let match_index = byte_index_to_utf16_index(str_data, matched.start()) as f64; Self { captures, named, diff --git a/crates/perry-runtime/src/regex/match_string.rs b/crates/perry-runtime/src/regex/match_string.rs index 7f3c38950a..c0fc98be26 100644 --- a/crates/perry-runtime/src/regex/match_string.rs +++ b/crates/perry-runtime/src/regex/match_string.rs @@ -103,7 +103,6 @@ pub extern "C" fn js_string_match( OwnedStringMatch::NonGlobal( OwnedExecMatch::from_repeat_matcher( str_data, - 0, &repeat_matcher, &matched, has_indices, @@ -128,7 +127,7 @@ pub extern "C" fn js_string_match( return ptr::null_mut(); }; OwnedStringMatch::NonGlobal( - OwnedExecMatch::from_fancy(str_data, 0, &fre, &caps, has_indices), + OwnedExecMatch::from_fancy(str_data, &fre, &caps, has_indices), has_indices, ) } @@ -147,7 +146,7 @@ pub extern "C" fn js_string_match( return ptr::null_mut(); }; OwnedStringMatch::NonGlobal( - OwnedExecMatch::from_standard(str_data, 0, regex, &caps, has_indices), + OwnedExecMatch::from_standard(str_data, regex, &caps, has_indices), has_indices, ) } diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index ed53eb979d..ce39d3fb34 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1269,3 +1269,157 @@ fn fancy_engine_accepts_ascii_word_boundary_markers() { Some("
\nhello\n
\n\n") ); } + +// ---- #9429: exec/test at a non-zero lastIndex see the WHOLE subject ------ + +/// One `exec` at `last_index`, as `(matched text, .index, lastIndex after)`. +/// `None` also asserts the spec's reset-to-0 on a failed stateful exec, so a +/// row that stops matching cannot quietly leave `lastIndex` behind. +fn exec_from( + pattern: &str, + flags: &str, + subject: &str, + last_index: usize, +) -> Option<(String, f64, usize)> { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + store_last_index_number(re, last_index); + let arr = js_regexp_exec(re, make_string(subject)); + if arr.is_null() { + assert_eq!( + regex_last_index_offset(re), + 0, + "{pattern}/{flags} @{last_index}: a failed stateful exec resets lastIndex" + ); + return None; + } + let text = match_capture_text(arr, 0).expect("capture zero always participates"); + Some(( + text, + js_regexp_exec_get_index(), + regex_last_index_offset(re), + )) +} + +fn hit(text: &str, index: f64, last_index: usize) -> Option<(String, f64, usize)> { + Some((text.to_string(), index, last_index)) +} + +#[test] +fn exec_at_last_index_holds_anchors_against_the_subject_not_a_slice() { + // Every row is a position where the SLICE and the SUBJECT disagree. + // `^` is start-of-subject: at lastIndex 1 of "ab" it must not hold, even + // though it would hold at offset 0 of the slice "b". + assert_eq!(exec_from("^b", "g", "ab", 1), None); + assert_eq!(exec_from("^b", "g", "ab", 0), None); + assert_eq!(exec_from("^a", "g", "ab", 0), hit("a", 0.0, 1)); + assert_eq!(exec_from("^a", "g", "ab", 1), None); + // Under `m` it holds after a LineTerminator IN THE SUBJECT — index 2 of + // "a\nb" regardless of where the scan was told to start. + assert_eq!(exec_from("^b", "gm", "a\nb", 0), hit("b", 2.0, 3)); + assert_eq!(exec_from("^b", "gm", "a\nb", 1), hit("b", 2.0, 3)); + assert_eq!(exec_from("^b", "gm", "a\nb", 2), hit("b", 2.0, 3)); + // `\b`/`\B` read the character BEFORE the start position. + assert_eq!(exec_from(r"\bb", "g", "ab", 1), None); + assert_eq!(exec_from(r"\Bb", "g", "ab", 1), hit("b", 1.0, 2)); + assert_eq!(exec_from(r"\bb", "g", "a b", 1), hit("b", 2.0, 3)); + assert_eq!(exec_from(r"\Bb", "g", "a b", 1), None); + // `$` at the very end still matches the empty string there. + assert_eq!(exec_from("$", "g", "ab", 2), hit("", 2.0, 2)); +} + +#[test] +fn exec_at_last_index_keeps_lookaround_context() { + // The `regex` crate has no lookaround, so these run on the fancy-regex + // fallback — assert the lane, or the rows below could pass on a different + // engine than the one this fix touches. + let looky = js_regexp_new(make_string("(?<=a)b"), make_string("g")); + assert!( + lookup_fancy_regex(looky).is_some(), + "lookbehind must select the fancy-regex lane" + ); + + // Lookbehind is destroyed by a slice: the `a` is to the LEFT of the start. + assert_eq!(exec_from("(?<=a)b", "g", "ab", 0), hit("b", 1.0, 2)); + assert_eq!(exec_from("(?<=a)b", "g", "ab", 1), hit("b", 1.0, 2)); + assert_eq!(exec_from("(?<=a)b", "g", "ab", 2), None); + assert_eq!(exec_from("(?<=ab)c", "g", "abc", 2), hit("c", 2.0, 3)); + // …and a NEGATIVE lookbehind is wrong the other way: a slice makes it hold. + assert_eq!(exec_from("(? 0` must evaluate the pattern against +// the FULL subject, not against `subject.slice(lastIndex)`. Slicing destroys +// the context every zero-width assertion depends on, and it is wrong in both +// directions: `^`/`\b`/`(? " + value); +} + +// ---- `^` (no `m`): only position 0 of the SUBJECT ------------------------ +show("^b g@0 ab", ex(/^b/g, "ab", 0)); +show("^b g@1 ab", ex(/^b/g, "ab", 1)); +show("^b g@2 ab", ex(/^b/g, "ab", 2)); +show("^b y@1 ab", ex(/^b/y, "ab", 1)); +show("^a g@0 ab", ex(/^a/g, "ab", 0)); +show("^a g@1 ab", ex(/^a/g, "ab", 1)); + +// ---- `^` under /m: after a LineTerminator in the SUBJECT ---------------- +show("^b gm@0 a\\nb", ex(/^b/gm, "a\nb", 0)); +show("^b gm@1 a\\nb", ex(/^b/gm, "a\nb", 1)); +show("^b gm@2 a\\nb", ex(/^b/gm, "a\nb", 2)); +show("^n gm@1 a\\nb", ex(/^n/gm, "a\nb", 1)); + +// ---- `$` ----------------------------------------------------------------- +show("b$ g@0 ab", ex(/b$/g, "ab", 0)); +show("b$ g@1 ab", ex(/b$/g, "ab", 1)); +show("$ g@2 ab", ex(/$/g, "ab", 2)); +show("a$ gm@0 a\\nb", ex(/a$/gm, "a\nb", 0)); +show("a$ gm@1 a\\nb", ex(/a$/gm, "a\nb", 1)); + +// ---- `\b` / `\B` --------------------------------------------------------- +show("\\bb g@1 ab", ex(/\bb/g, "ab", 1)); +show("\\bb g@1 a b", ex(/\bb/g, "a b", 1)); +show("\\bb g@2 a b", ex(/\bb/g, "a b", 2)); +show("\\Bb g@1 ab", ex(/\Bb/g, "ab", 1)); +show("\\Bb g@1 a b", ex(/\Bb/g, "a b", 1)); +show("\\bb y@1 ab", ex(/\bb/y, "ab", 1)); +show("\\bb y@2 a b", ex(/\bb/y, "a b", 2)); + +// ---- lookbehind (fancy-regex lane) -------------------------------------- +show("(?<=a)b g@0 ab", ex(/(?<=a)b/g, "ab", 0)); +show("(?<=a)b g@1 ab", ex(/(?<=a)b/g, "ab", 1)); +show("(?<=a)b g@2 ab", ex(/(?<=a)b/g, "ab", 2)); +show("(?<=a)b y@1 ab", ex(/(?<=a)b/y, "ab", 1)); +show("(? 24) { at.push(-1); break; } + } + return JSON.stringify(at); +} +show("sweep ^/gm one\\ntwo", anchorSweep(/^/gm, "one\ntwo")); +show("sweep ^/gm a\\nb\\nc", anchorSweep(/^/gm, "a\nb\nc")); +show("sweep $/gm one\\ntwo", anchorSweep(/$/gm, "one\ntwo")); +show("sweep \\b/g ab cd", anchorSweep(/\b/g, "ab cd")); +show("sweep (?<=,)/g a,b,c", anchorSweep(/(?<=,)/g, "a,b,c")); +// Needs #9408 (in #9427) as well as this fix — see the header note. +show("sweep ^/gm one\\r\\ntwo", anchorSweep(/^/gm, "one\r\ntwo")); +show("sweep ^/gm a\\u2028b", anchorSweep(/^/gm, "a\u2028b")); + +// ---- a plain global exec loop still walks the whole subject -------------- +function execSweep(re: RegExp, s: string): string { + const out: string[] = []; + let m: RegExpExecArray | null; + let guard = 0; + while ((m = re.exec(s)) !== null) { + out.push(m[0] + "@" + m.index); + if (m[0] === "") re.lastIndex = re.lastIndex + 1; + if (++guard > 24) { out.push("!"); break; } + } + return JSON.stringify(out); +} +show("sweep \\w+/g ab cd ef", execSweep(/\w+/g, "ab cd ef")); +show("sweep ^\\w/gm ab\\ncd", execSweep(/^\w/gm, "ab\ncd")); +show("sweep (?<=a)./g ab ac", execSweep(/(?<=a)./g, "ab ac")); +show("sweep .(?=b)/g ab cb", execSweep(/.(?=b)/g, "ab cb")); From 2eac7d0973dece6497839bb279247cea2bc81637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 22:05:33 +0200 Subject: [PATCH 2/2] fix(runtime): a global scan keeps the empty match at a match's end (#9430) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ECMAScript's RegExpExec loop keeps a zero-width match that lands exactly where the previous match ended and then advances one code unit. Rust's iterators discard it and re-search one character right — `regex_automata`'s `Searcher::try_advance`, and `fancy_regex`'s `Matches::next_with`, which says so in its own doc comment. Every global operation was built on those iterators, so `"a".match(/a*/g)` was ["a"] where node gives ["a",""]. The rule fires at EVERY such position, not only the last: `"aXa" .match(/a*/g)` was ["a","a"] against node's ["a","","a",""]. The issue's "trailing empty match" is the visible half of a general divergence. New `regex::global_scan` holds the ECMAScript loop once and takes a starting byte offset instead of a slice. Every global site routes through it — match, matchAll, replace/replaceAll with a string, with $, and with a callback — on the linear and the fancy lanes. `regress` already implements the ECMAScript rule (`next_start` steps right only when `end == pos`), so its iterators stay, with a test pinning that lane as a control. `Regex::replace_all` leaves the string-replacement path because it drives the crate's iterator internally. Positional rather than sliced also fixes matchAll's half of #9429: it searched `&subject[lastIndex..]`, losing assertion context. `test_parity_regex_replace_fn_lookahead` diverged from node because of this, invisibly — it is scored against a stored expected file holding "OK", not against node, and its assertion encoded the Rust iterator's answer. Corrected to node's, so both runtimes now print OK. Fixture demonstrated failing on a compiler built from unfixed origin/main, byte-identical to node after. Four runtime tests, each confirmed to fail when the scan loop is reverted to the Rust rule. --- changelog.d/9430-global-scan-empty-match.md | 57 ++++++++ crates/perry-runtime/src/regex.rs | 52 ++++--- crates/perry-runtime/src/regex/global_scan.rs | 133 ++++++++++++++++++ crates/perry-runtime/src/regex/match_all.rs | 25 ++-- .../perry-runtime/src/regex/match_string.rs | 21 +-- .../perry-runtime/src/regex/replace_expand.rs | 22 ++- crates/perry-runtime/src/regex/tests.rs | 121 ++++++++++++++++ ...est_gap_9430_global_scan_trailing_empty.ts | 118 ++++++++++++++++ .../test_parity_regex_replace_fn_lookahead.ts | 9 +- 9 files changed, 513 insertions(+), 45 deletions(-) create mode 100644 changelog.d/9430-global-scan-empty-match.md create mode 100644 crates/perry-runtime/src/regex/global_scan.rs create mode 100644 test-files/test_gap_9430_global_scan_trailing_empty.ts diff --git a/changelog.d/9430-global-scan-empty-match.md b/changelog.d/9430-global-scan-empty-match.md new file mode 100644 index 0000000000..c64bd32d06 --- /dev/null +++ b/changelog.d/9430-global-scan-empty-match.md @@ -0,0 +1,57 @@ +**A global regex scan no longer drops the empty match that sits where the +previous match ended** — `"a".match(/a*/g)` is `["a",""]`, `"a".replace(/a*/g, +"<>")` is `"<><>"`, and the same for `matchAll` and every `replace` form. + +```js +"a".match(/a*/g) // was ["a"] now ["a",""] +"aXa".match(/a*/g) // was ["a","a"] now ["a","","a",""] +"ab".match(/b*/g) // was ["","b"] now ["","b",""] +"a".replace(/a*/g, "<>") // was "<>" now "<><>" +``` + +ECMAScript's `RegExp.prototype [ @@match ]` loop keeps a zero-width match at +the previous match's end and *then* advances one code unit +(`AdvanceStringIndex`). Rust's iterators do the opposite: both +`regex_automata`'s `Searcher::try_advance` and `fancy_regex`'s +`Matches::next_with` — the latter documented as "adapted from the `regex` +crate … ignores empty matches immediately after a match" — discard it and +re-search one character to the right. Every global operation was built on +those iterators, so every one inherited the rule. + +**The reported symptom understated it.** The rule fires wherever an empty +match lands on a previous match's end, not only at the end of the subject, so +interior matches were lost too: `"aXa".match(/a*/g)` was missing *two* of +Node's four elements, and `"a1b22".match(/\d*/g)` three of five. + +One `global_scan` module now holds the ECMAScript loop, and every global site +goes through it: `String#match`, `matchAll`, `replace`/`replaceAll` with a +string replacement, with a `$` replacement, and with a callback — on +both the linear `regex` lane and the `fancy_regex` lookaround/backreference +lane. `regress`, the third engine, already stepped one position past a +zero-width match, which is the ECMAScript rule; its iterators are used +unchanged, and a test pins that lane as the control. `Regex::replace_all` is +gone from the string-replacement path for the same reason — it runs the +crate's iterator internally. + +The scan takes a starting byte offset rather than a slice, which also gives +`matchAll` the #9429 treatment: it used to search +`&subject[lastIndex..]`, so a `matchAll` on a regex with a non-zero +`lastIndex` evaluated `^`, `\b` and lookbehind against the wrong left edge. + +**`test_parity_regex_replace_fn_lookahead` diverged from Node because of +this**, exactly as #9430 recorded — and the runner could not see it, because +that test is scored against a stored `expected/…txt` holding `OK` rather than +against Node. Its `/[a-z]+|(?=\.)/g` assertion asked for `["ab","cd"]`, which +is the Rust iterator's answer; Node has always produced `["ab","","cd"]` and +thrown. The assertion now reads Node's answer, so both runtimes print `OK`. + +**Found while fixing, NOT fixed here:** `split` by a pattern only fancy-regex +can compile does not run `RegExp.prototype [ @@split ]` at all — the fallback +walks `find_iter` and slices between matches. It therefore emits a trailing +`""` the spec's `q < size` bound never reaches (`"a,b,".split(/(?<=,)/)` → +`["a,","b,",""]` vs Node's `["a,","b,"]`) and splices no captured groups +(`"aXbXc".split(/((?<=a)X)/)` → `["a","bXc"]` vs Node's `["a","X","bXc"]`). +That is a lane gap rather than a scan gap — the `regex` lane runs the spec +algorithm in `spec_regex_split` and is correct — so it is excluded from this +fixture with a comment, and the runtime test `fancy_lookbehind_split` +currently pins the wrong answer. diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 9dda8b6f92..c78cfe5106 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -36,6 +36,8 @@ mod escape; #[cfg(feature = "regex-engine")] mod exec_array; #[cfg(feature = "regex-engine")] +mod global_scan; +#[cfg(feature = "regex-engine")] mod grammar; #[cfg(feature = "regex-engine")] mod lazy; @@ -1418,14 +1420,18 @@ unsafe fn replace_regex_str_fancy( repl_str: &str, ) -> *mut StringHeader { let has_named_groups = fre.capture_names().any(|n| n.is_some()); - let mut captures_list: Vec = Vec::new(); - let mut iter = fre.captures_iter(str_data); - while let Some(Ok(caps)) = iter.next() { - captures_list.push(caps); - if !global { - break; + // #9430: the ECMAScript scan for the global form. fancy-regex's own + // iterator drops a zero-width match that lands where the previous match + // ended, so `"a".replace(/(?<=x)?a*/g, …)`-shaped patterns lost their + // trailing (and every interior) empty replacement. + let captures_list: Vec = if global { + global_scan::fancy_captures(fre, str_data, 0) + } else { + match fre.captures(str_data) { + Ok(Some(caps)) => vec![caps], + Ok(None) | Err(_) => Vec::new(), } - } + }; let mut result = String::new(); let mut last_end = 0usize; for caps in &captures_list { @@ -1507,19 +1513,29 @@ pub extern "C" fn js_string_replace_regex( // Route through a JS-aware expander (closure form) so `$&` / `` $` `` / // `$'` — which the regex crate's native `$` syntax doesn't support — // are substituted per match. `$$`, `$n`, and `$` are handled too. - let result = if global { - regex - .replace_all(str_data, |caps: ®ex::Captures| { - expand_js_replacement(repl_str, caps, str_data, has_named_groups) - }) - .to_string() + // #9430: `Regex::replace_all` runs the crate's own match iterator, + // whose empty-match rule is not ECMAScript's. Drive the ECMAScript + // scan and splice the replacements here instead; the non-global form + // is the same loop over a one-element list. + let captures_list: Vec = if global { + global_scan::std_captures(regex, str_data, 0) } else { - regex - .replace(str_data, |caps: ®ex::Captures| { - expand_js_replacement(repl_str, caps, str_data, has_named_groups) - }) - .to_string() + regex.captures(str_data).into_iter().collect() }; + let mut result = String::with_capacity(str_data.len()); + let mut last_end = 0usize; + for caps in &captures_list { + let full = caps.get(0).expect("capture zero is the full match"); + result.push_str(&str_data[last_end..full.start()]); + result.push_str(&expand_js_replacement( + repl_str, + caps, + str_data, + has_named_groups, + )); + last_end = full.end(); + } + result.push_str(&str_data[last_end..]); finish_replace_bytes(result.as_bytes()) } diff --git a/crates/perry-runtime/src/regex/global_scan.rs b/crates/perry-runtime/src/regex/global_scan.rs new file mode 100644 index 0000000000..15b126db7d --- /dev/null +++ b/crates/perry-runtime/src/regex/global_scan.rs @@ -0,0 +1,133 @@ +//! ECMAScript's global-scan loop, shared by every operation that walks a +//! subject with a `g` regex: `String.prototype.match`, `matchAll`, `replace` +//! and `replaceAll`. +//! +//! **Why this exists (#9430).** ECMAScript's `RegExpExec` loop and Rust's +//! match iterators disagree about one position. Where a zero-width match +//! lands exactly at the end of the previous match, Rust +//! (`regex_automata::util::iter::Searcher::try_advance`, and `fancy_regex`'s +//! `Matches::next_with`, which is documented as "adapted from the `regex` +//! crate … ignores empty matches immediately after a match") throws that +//! match away and re-searches one character to the right. ECMAScript keeps +//! it and *then* advances one code unit — the `matchStr is ""` branch of +//! `RegExp.prototype [ @@match ]`. So `"a".match(/a*/g)` is `["a", ""]` in +//! JavaScript and `["a"]` under a Rust iterator; and because the rule fires +//! at every such position, not only the last one, `"aXa".match(/a*/g)` is +//! `["a", "", "a", ""]` in JavaScript and `["a", "a"]` under a Rust iterator. +//! +//! `regress` — the third engine, reached for quantified captures — already +//! implements the ECMAScript rule (`next_start` steps one position right +//! only when `end == pos`), so its iterators are used unchanged. This module +//! exists to give the other two the same semantics, and its walk starts at a +//! caller-supplied byte offset so `matchAll` can honour `lastIndex` without +//! slicing the subject (#9429). + +/// `AdvanceStringIndex` for a zero-width match that ended at `end`. +/// +/// ECMAScript advances one UTF-16 code unit; Rust `&str` offsets can only +/// name whole scalars, so an astral character is stepped over in one go +/// rather than in two halves. That is the same non-`u` code-unit gap #9218 +/// and #9409 record for `.` and `split(/(?:)/)` — the engines match Unicode +/// scalars, and no position exists here for the second half of a surrogate +/// pair. Stepping by a whole scalar keeps the walk on boundaries the engines +/// can be re-entered at, which is what the previous Rust-iterator behaviour +/// did as well, so nothing regresses on that axis. +pub(super) fn advance_past_empty(haystack: &str, end: usize) -> usize { + let mut next = end + 1; + while next < haystack.len() && !haystack.is_char_boundary(next) { + next += 1; + } + next +} + +/// Walk `haystack` from `start` the way `RegExpExec` does, collecting one `T` +/// per match. `find_at` must return the first match at or after the offset it +/// is given, as `(match start, match end, payload)`, or `None` to end the +/// scan. +/// +/// The cursor advances to the match end, or one code unit past it when the +/// match is empty, so it is strictly increasing and the loop terminates. The +/// `start > haystack.len()` bound is what makes a trailing empty match the +/// LAST one rather than the first of an infinite series. +pub(super) fn scan(haystack: &str, start: usize, mut find_at: F) -> Vec +where + F: FnMut(usize) -> Option<(usize, usize, T)>, +{ + let mut out = Vec::new(); + let mut cursor = start; + while cursor <= haystack.len() { + let Some((match_start, match_end, item)) = find_at(cursor) else { + break; + }; + out.push(item); + cursor = if match_end == match_start { + advance_past_empty(haystack, match_end) + } else { + match_end + }; + } + out +} + +/// Full-match byte ranges for the linear `regex` engine. +pub(super) fn std_ranges(re: ®ex::Regex, haystack: &str, start: usize) -> Vec<(usize, usize)> { + scan(haystack, start, |cursor| { + re.find_at(haystack, cursor).map(|matched| { + ( + matched.start(), + matched.end(), + (matched.start(), matched.end()), + ) + }) + }) +} + +/// Captures for the linear `regex` engine. +pub(super) fn std_captures<'h>( + re: ®ex::Regex, + haystack: &'h str, + start: usize, +) -> Vec> { + scan(haystack, start, |cursor| { + let caps = re.captures_at(haystack, cursor)?; + let full = caps.get(0).expect("capture zero is the full match"); + Some((full.start(), full.end(), caps)) + }) +} + +/// Full-match byte ranges for the `fancy_regex` fallback (lookaround / +/// backreferences). A scan error ends the walk, matching the +/// `while let Some(Ok(..))` shape these call sites used before. +pub(super) fn fancy_ranges( + re: &fancy_regex::Regex, + haystack: &str, + start: usize, +) -> Vec<(usize, usize)> { + scan(haystack, start, |cursor| { + match re.find_from_pos(haystack, cursor) { + Ok(Some(matched)) => Some(( + matched.start(), + matched.end(), + (matched.start(), matched.end()), + )), + Ok(None) | Err(_) => None, + } + }) +} + +/// Captures for the `fancy_regex` fallback. +pub(super) fn fancy_captures<'h>( + re: &fancy_regex::Regex, + haystack: &'h str, + start: usize, +) -> Vec> { + scan(haystack, start, |cursor| { + match re.captures_from_pos(haystack, cursor) { + Ok(Some(caps)) => { + let full = caps.get(0).expect("capture zero is the full match"); + Some((full.start(), full.end(), caps)) + } + Ok(None) | Err(_) => None, + } + }) +} diff --git a/crates/perry-runtime/src/regex/match_all.rs b/crates/perry-runtime/src/regex/match_all.rs index fdd01c5660..202e744609 100644 --- a/crates/perry-runtime/src/regex/match_all.rs +++ b/crates/perry-runtime/src/regex/match_all.rs @@ -89,17 +89,22 @@ unsafe fn materialize_match_all_results( // Rust data. The fancy-regex fallback (lookbehind/backreferences) is // needed because the never-match placeholder in `regex_ptr` would yield // an empty iterator otherwise. + // The scan starts AT `search_start` inside the whole subject — never on a + // `&str_data[search_start..]` slice, which would strip the context every + // zero-width assertion reads (#9429) — and follows the ECMAScript + // empty-match rule rather than a Rust iterator's (#9430). let str_data = string_as_str(s); let search_start = utf16_index_to_byte(str_data, start_char_index); - let search_str = &str_data[search_start..]; let mut owned: Vec = Vec::new(); if let Some(repeat_matcher) = super::lookup_repeat_matcher(re) { - for matched in repeat_matcher.regex.find_iter(search_str) { + // `regress`'s own iterator is positional and already advances one + // position past a zero-width match, which is the ECMAScript rule. + for matched in repeat_matcher.regex.find_from(str_data, search_start) { owned.push(OwnedMatchAllData { groups: matched .groups() - .map(|group| group.map(|range| search_str[range].to_string())) + .map(|group| group.map(|range| str_data[range].to_string())) .collect(), named: repeat_matcher .capture_names @@ -111,13 +116,12 @@ unsafe fn materialize_match_all_results( name.clone(), matched .group(index + 1) - .map(|range| search_str[range].to_string()), + .map(|range| str_data[range].to_string()), ) }) }) .collect(), - match_index: byte_index_to_utf16_index(str_data, search_start + matched.start()) - as f64, + match_index: byte_index_to_utf16_index(str_data, matched.start()) as f64, }); } } else if let Some(fre) = super::lookup_fancy_regex(re) { @@ -126,8 +130,7 @@ unsafe fn materialize_match_all_results( .enumerate() .filter_map(|(i, name)| name.map(|n| (i, n.to_string()))) .collect(); - let mut it = fre.captures_iter(search_str); - while let Some(Ok(caps)) = it.next() { + for caps in super::global_scan::fancy_captures(&fre, str_data, search_start) { owned.push(OwnedMatchAllData { groups: (0..caps.len()) .map(|j| caps.get(j).map(|m| m.as_str().to_string())) @@ -138,7 +141,7 @@ unsafe fn materialize_match_all_results( .collect(), match_index: caps .get(0) - .map(|m| byte_index_to_utf16_index(str_data, search_start + m.start()) as f64) + .map(|m| byte_index_to_utf16_index(str_data, m.start()) as f64) .unwrap_or(start_char_index as f64), }); } @@ -149,7 +152,7 @@ unsafe fn materialize_match_all_results( .enumerate() .filter_map(|(i, name)| name.map(|n| (i, n.to_string()))) .collect(); - for caps in regex.captures_iter(search_str) { + for caps in super::global_scan::std_captures(regex, str_data, search_start) { owned.push(OwnedMatchAllData { groups: (0..caps.len()) .map(|j| caps.get(j).map(|m| m.as_str().to_string())) @@ -160,7 +163,7 @@ unsafe fn materialize_match_all_results( .collect(), match_index: caps .get(0) - .map(|m| byte_index_to_utf16_index(str_data, search_start + m.start()) as f64) + .map(|m| byte_index_to_utf16_index(str_data, m.start()) as f64) .unwrap_or(start_char_index as f64), }); } diff --git a/crates/perry-runtime/src/regex/match_string.rs b/crates/perry-runtime/src/regex/match_string.rs index c0fc98be26..9b06e60bd8 100644 --- a/crates/perry-runtime/src/regex/match_string.rs +++ b/crates/perry-runtime/src/regex/match_string.rs @@ -112,11 +112,14 @@ pub extern "C" fn js_string_match( } } else if let Some(fre) = lookup_fancy_regex(re) { if global { - let matches: Vec = fre - .find_iter(str_data) - .filter_map(Result::ok) - .map(|m| OwnedCapture::from_range(str_data, m.start(), m.end())) - .collect(); + // #9430: the ECMAScript scan, not fancy-regex's iterator — + // the latter drops a zero-width match that lands where the + // previous match ended. + let matches: Vec = + super::global_scan::fancy_ranges(&fre, str_data, 0) + .into_iter() + .map(|(start, end)| OwnedCapture::from_range(str_data, start, end)) + .collect(); if matches.is_empty() { return ptr::null_mut(); } @@ -132,9 +135,11 @@ pub extern "C" fn js_string_match( ) } } else if global { - let matches: Vec = regex - .find_iter(str_data) - .map(|m| OwnedCapture::from_range(str_data, m.start(), m.end())) + // #9430: see the fancy branch above; `regex`'s iterator has the + // same non-ECMAScript empty-match rule. + let matches: Vec = super::global_scan::std_ranges(regex, str_data, 0) + .into_iter() + .map(|(start, end)| OwnedCapture::from_range(str_data, start, end)) .collect(); if matches.is_empty() { return ptr::null_mut(); diff --git a/crates/perry-runtime/src/regex/replace_expand.rs b/crates/perry-runtime/src/regex/replace_expand.rs index 7b38134399..4b87e5a0fd 100644 --- a/crates/perry-runtime/src/regex/replace_expand.rs +++ b/crates/perry-runtime/src/regex/replace_expand.rs @@ -248,8 +248,17 @@ pub(super) unsafe fn replace_regex_fn_fancy( // `OwnedMatchData`). let str_data = string_as_str(s_handle.get_raw_const_ptr::()); let mut matches: Vec = Vec::new(); - let mut iter = fre.captures_iter(str_data); - while let Some(Ok(caps)) = iter.next() { + // #9430: the ECMAScript scan for the global form — fancy-regex's iterator + // would drop every zero-width match sitting at a previous match's end. + let scanned: Vec = if global { + super::global_scan::fancy_captures(fre, str_data, 0) + } else { + match fre.captures(str_data) { + Ok(Some(caps)) => vec![caps], + Ok(None) | Err(_) => Vec::new(), + } + }; + for caps in &scanned { let full_match = caps.get(0).unwrap(); matches.push(OwnedMatchData { start: full_match.start(), @@ -263,9 +272,6 @@ pub(super) unsafe fn replace_regex_fn_fancy( .map(|(gi, n)| (n.clone(), caps.get(*gi).map(|m| m.as_str().to_string()))) .collect(), }); - if !global { - break; - } } replace_fn_run_matches(s_handle, &matches, closure_ptr, has_named_groups) @@ -408,8 +414,9 @@ pub extern "C" fn js_string_replace_regex_fn( .collect(), }); }; + // #9430: the ECMAScript scan, not `Regex::captures_iter`. if global { - for caps in regex.captures_iter(str_data) { + for caps in super::global_scan::std_captures(regex, str_data, 0) { push_caps(caps); } } else if let Some(caps) = regex.captures(str_data) { @@ -489,8 +496,9 @@ pub extern "C" fn js_string_replace_regex_named( let mut result = String::new(); let mut last_end = 0usize; + // #9430: the ECMAScript scan, not `Regex::captures_iter`. let captures_list: Vec = if global { - regex.captures_iter(str_data).collect() + super::global_scan::std_captures(regex, str_data, 0) } else { match regex.captures(str_data) { Some(caps) => vec![caps], diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index ce39d3fb34..af40de4e86 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1423,3 +1423,124 @@ fn stateful_test_reports_the_same_answer_as_exec() { assert_eq!(js_regexp_test(plain, make_string("ab")), 0); assert_eq!(regex_last_index_offset(plain), 1, "plain test leaves it be"); } + +// ---- #9430: a global scan keeps the empty match at a match's end --------- + +/// `subject.match(/pattern/flags)` for a global regex, as plain strings. +fn global_match_list(pattern: &str, flags: &str, subject: &str) -> Vec { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + let arr = js_string_match(make_string(subject), re); + if arr.is_null() { + return Vec::new(); + } + let len = unsafe { (*arr).length }; + (0..len) + .map(|index| match_capture_text(arr, index).expect("a match list holds only strings")) + .collect() +} + +fn replace_all_with(pattern: &str, flags: &str, subject: &str, repl: &str) -> String { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + let out = js_string_replace_regex(make_string(subject), re, make_string(repl)); + string_as_str(out).to_string() +} + +#[test] +fn ecmascript_scan_keeps_an_empty_match_where_the_previous_one_ended() { + // The scan loop's contract, pinned without an engine: an empty match at + // the previous match's end is KEPT, and the cursor then advances one + // position — Rust's iterators drop it and advance instead. + // + // The finder below is `/a*/` over "aXa" written out by hand. + let subject = "aXa"; + let seen = super::global_scan::scan(subject, 0, |cursor| { + // `a*` matches the empty string anywhere, so its leftmost match from + // `cursor` always STARTS at `cursor` and runs over the `a`s there. + let mut end = cursor; + while subject.as_bytes().get(end) == Some(&b'a') { + end += 1; + } + Some((cursor, end, (cursor, end))) + }); + assert_eq!(seen, vec![(0, 1), (1, 1), (2, 3), (3, 3)]); + + // The bound is what terminates the walk: without `cursor > len` ending it, + // the trailing empty match would repeat forever. + let empties = super::global_scan::scan("ab", 0, |cursor| Some((cursor, cursor, cursor))); + assert_eq!(empties, vec![0, 1, 2]); + + // A zero-width step never lands inside a scalar. + assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 0), 1); + assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 1), 5); + assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 5), 6); + assert_eq!(super::global_scan::advance_past_empty("ab", 2), 3); +} + +#[test] +fn global_match_keeps_the_trailing_and_interior_empty_matches() { + // The linear `regex` lane. + let plain = js_regexp_new(make_string("a*"), make_string("g")); + assert!( + lookup_fancy_regex(plain).is_none() && lookup_repeat_matcher(plain).is_none(), + "`a*` must stay on the linear engine" + ); + assert_eq!(global_match_list("a*", "g", "a"), vec!["a", ""]); + assert_eq!(global_match_list("a*", "g", "aa"), vec!["aa", ""]); + assert_eq!(global_match_list("b*", "g", "ab"), vec!["", "b", ""]); + // Not only the trailing one: the empty match at index 1 is interior. + assert_eq!(global_match_list("a*", "g", "aXa"), vec!["a", "", "a", ""]); + assert_eq!(global_match_list("x*", "g", "abc"), vec!["", "", "", ""]); + assert_eq!(global_match_list("a*", "g", ""), vec![""]); + // A pattern that cannot match empty is unchanged. + assert_eq!(global_match_list("a+", "g", "aXa"), vec!["a", "a"]); +} + +#[test] +fn global_match_keeps_empty_matches_on_the_fancy_lane() { + // A possibly-empty pattern the linear engine cannot compile. + let looky = js_regexp_new(make_string("a*(?!x)"), make_string("g")); + assert!( + lookup_fancy_regex(looky).is_some(), + "a lookahead must select the fancy-regex lane" + ); + assert_eq!(global_match_list("a*(?!x)", "g", "a"), vec!["a", ""]); + assert_eq!( + global_match_list("a*(?!x)", "g", "aXa"), + vec!["a", "", "a", ""] + ); + assert_eq!(global_match_list("(?<=,)", "g", "a,b,"), vec!["", ""]); +} + +#[test] +fn global_match_on_the_regress_lane_is_unchanged() { + // `regress`'s iterator already implements the ECMAScript rule; this is the + // control that says so, and that nothing routed it elsewhere. + let quantified = js_regexp_new(make_string("(a)*"), make_string("g")); + assert!( + lookup_repeat_matcher(quantified).is_some(), + "a quantified capture must select the regress lane" + ); + assert_eq!(global_match_list("(a)*", "g", "a"), vec!["a", ""]); + assert_eq!( + global_match_list("(a)*", "g", "aXa"), + vec!["a", "", "a", ""] + ); +} + +#[test] +fn global_replace_substitutes_at_every_empty_match() { + assert_eq!(replace_all_with("a*", "g", "a", "<>"), "<><>"); + assert_eq!(replace_all_with("a*", "g", "aXa", "-"), "--X--"); + assert_eq!(replace_all_with("b*", "g", "ab", "-"), "-a--"); + assert_eq!(replace_all_with("x*", "g", "abc", "-"), "-a-b-c-"); + assert_eq!(replace_all_with("a*", "g", "aXa", "[$&]"), "[a][]X[a][]"); + // The non-global form still replaces exactly one match. + assert_eq!(replace_all_with("a*", "", "aXa", "-"), "-Xa"); + // Fancy lane. + assert_eq!(replace_all_with("a*(?!x)", "g", "a", "<>"), "<><>"); + assert_eq!(replace_all_with("(?<=a)", "g", "aba", "!"), "a!ba!"); + // Named-group substitution takes its own scan path. + let named = js_regexp_new(make_string("(?a)*"), make_string("g")); + let out = js_string_replace_regex_named(make_string("a"), named, make_string("[$]")); + assert_eq!(string_as_str(out), "[a][]"); +} diff --git a/test-files/test_gap_9430_global_scan_trailing_empty.ts b/test-files/test_gap_9430_global_scan_trailing_empty.ts new file mode 100644 index 0000000000..618d7eb56f --- /dev/null +++ b/test-files/test_gap_9430_global_scan_trailing_empty.ts @@ -0,0 +1,118 @@ +// #9430 — a global scan must follow ECMAScript's `RegExpExec` loop: an empty +// match is kept even when it sits exactly where the previous match ended, and +// the scan then advances one code unit. Rust's `regex` and `fancy-regex` +// iterators use the opposite rule ("ignore an empty match immediately after a +// match"), which silently drops the trailing empty match of `"a".match(/a*/g)` +// — and every interior one too. +// +// Every row below uses only the `g` flag (plus whatever engine the pattern +// selects). No `m`, so #9408's LineTerminator gap cannot reach these. + +function show(label: string, value: unknown): void { + console.log(label + " => " + JSON.stringify(value)); +} + +// ---- String.prototype.match, global ------------------------------------- +show("match a* / a", "a".match(/a*/g)); +show("match a* / aXa", "aXa".match(/a*/g)); +show("match b* / ab", "ab".match(/b*/g)); +show("match x* / abc", "abc".match(/x*/g)); +show("match a* / (empty)", "".match(/a*/g)); +show("match a* / aa", "aa".match(/a*/g)); +show("match a*/ ab a", "ab a".match(/a*/g)); +show("match (?:) / abc", "abc".match(/(?:)/g)); +show("match a|/ ab", "ab".match(/a|/g)); +show("match \\d*/ a1b22", "a1b22".match(/\d*/g)); +show("match a+ / aXa", "aXa".match(/a+/g)); + +// ---- matchAll ------------------------------------------------------------ +function allOf(s: string, re: RegExp): string[] { + const out: string[] = []; + for (const m of s.matchAll(re)) out.push(JSON.stringify(m[0]) + "@" + m.index); + return out; +} +show("matchAll a* / a", allOf("a", /a*/g)); +show("matchAll b* / ab", allOf("ab", /b*/g)); +show("matchAll a* / aXa", allOf("aXa", /a*/g)); +show("matchAll x* / abc", allOf("abc", /x*/g)); +show("matchAll (a)* / a", allOf("a", /(a)*/g)); +show("matchAll (?<=,) / a,b,", allOf("a,b,", /(?<=,)/g)); +show("matchAll (?=b) / abb", allOf("abb", /(?=b)/g)); + +// matchAll honours a non-zero lastIndex on the source regex. +const mall = /a*/g; +mall.lastIndex = 1; +show("matchAll a* / aXa @1", (() => { + const out: string[] = []; + for (const m of "aXa".matchAll(mall)) out.push(JSON.stringify(m[0]) + "@" + m.index); + return out; +})()); +const mall2 = /(?<=a)/g; +mall2.lastIndex = 1; +show("matchAll (?<=a) / ab @1", (() => { + const out: string[] = []; + for (const m of "ab".matchAll(mall2)) out.push(JSON.stringify(m[0]) + "@" + m.index); + return out; +})()); + +// ---- replace with a string replacement ---------------------------------- +show("replace a*->{} / a", "a".replace(/a*/g, "<>")); +show("replace a*->{} / aXa", "aXa".replace(/a*/g, "-")); +show("replace b*->{} / ab", "ab".replace(/b*/g, "-")); +show("replace x*->{} / abc", "abc".replace(/x*/g, "-")); +show("replace a*->$& / aXa", "aXa".replace(/a*/g, "[$&]")); +show("replace (?:)->- / ab", "ab".replace(/(?:)/g, "-")); +show("replaceAll a*->- / a", "a".replaceAll(/a*/g, "-")); +show("replace (?<=a)->! / aba", "aba".replace(/(?<=a)/g, "!")); +show("replace (?=b)->! / abb", "abb".replace(/(?=b)/g, "!")); +show("replace (a)*->[$1] / a", "a".replace(/(a)*/g, "[$1]")); +show("replace (?a)*->[$] / a", "a".replace(/(?a)*/g, "[$]")); + +// ---- replace with a callback -------------------------------------------- +function collect(s: string, re: RegExp): string[] { + const out: string[] = []; + s.replace(re, (m: string, ...rest: unknown[]) => { + out.push(JSON.stringify(m) + "@" + rest[rest.length - 2]); + return m; + }); + return out; +} +show("cb a* / a", collect("a", /a*/g)); +show("cb a* / aXa", collect("aXa", /a*/g)); +show("cb b* / ab", collect("ab", /b*/g)); +show("cb (?=b) / abb", collect("abb", /(?=b)/g)); +show("cb (?<=,) / a,b,", collect("a,b,", /(?<=,)/g)); +show("cb (a)* / aXa", collect("aXa", /(a)*/g)); +show("cb [a-z]+|(?=\\.) / ab.cd", collect("ab.cd", /[a-z]+|(?=\.)/g)); +show("cbOut a* / a", "a".replace(/a*/g, (m: string) => "[" + m + "]")); +show("cbOut b* / ab", "ab".replace(/b*/g, (m: string) => "[" + m + "]")); + +// ---- split: the spec's `e == p` skip must stay unchanged ---------------- +show("split a* / a", "a".split(/a*/)); +show("split a* / aXa", "aXa".split(/a*/)); +show("split b* / ab", "ab".split(/b*/)); +show("split (?:) / abc", "abc".split(/(?:)/)); +show("split x* / abc", "abc".split(/x*/)); +show("split , / a,b,", "a,b,".split(/,/)); +// EXCLUDED, a third and unrelated root cause: `split` by a pattern only +// fancy-regex can compile (lookaround / backreferences) does not run the +// spec's `RegExp.prototype [ @@split ]` algorithm at all. That fallback walks +// `find_iter` and slices between the matches, so it emits a trailing `""` the +// spec's `q < size` bound never reaches — `"a,b,".split(/(?<=,)/)` is +// `["a,","b,",""]` here and `["a,","b,"]` in Node — and it splices no captured +// groups: `"aXbXc".split(/((?<=a)X)/)` is `["a","bXc"]` here and +// `["a","X","bXc"]` in Node. The `regex`-engine rows above take +// `spec_regex_split`, which is correct, so this is a lane gap, not a scan gap. +// (The runtime test `fancy_lookbehind_split` currently pins the wrong answer.) +// #9427 widens its reach: rewriting a `/m` anchor into lookaround moves those +// patterns onto the same lane, so `"a\r\nb".split(/^/gm)` takes it too and gains +// a spurious LEADING "" — `["","a\r","\n","b"]` against Node's `["a\r","\n","b"]`. +show("split a*,2 / aXa", "aXa".split(/a*/, 2)); +show("split ,* / ab,", "ab,".split(/,*/)); +show("split \\d* / a1b", "a1b".split(/\d*/)); +show("split (a)* / bab", "bab".split(/(a)*/)); + +// ---- non-global controls: one match, no scan --------------------------- +show("match a* nog / aXa", "aXa".match(/a*/)); +show("replace a* nog / aXa", "aXa".replace(/a*/, "-")); +show("search a* / Xa", "Xa".search(/a*/)); diff --git a/test-files/test_parity_regex_replace_fn_lookahead.ts b/test-files/test_parity_regex_replace_fn_lookahead.ts index 48eec4a721..158c2cbbc2 100644 --- a/test-files/test_parity_regex_replace_fn_lookahead.ts +++ b/test-files/test_parity_regex_replace_fn_lookahead.ts @@ -3,8 +3,15 @@ function run(re: RegExp, s: string): string[] { s.replace(re, (m: string) => { out.push(m); return m; }); return out; } +// #9430: the alternation's zero-width `(?=\.)` branch matches at index 2 — +// exactly where the `[a-z]+` match before it ended — and ECMAScript's scan +// loop KEEPS that match and then advances one code unit. This assertion used +// to read '["ab","cd"]', which is what a Rust match iterator produces (it +// discards an empty match sitting at the previous match's end) and what Perry +// therefore printed; Node has always thrown here. The stored expected-output +// file says `OK`, so the divergence was invisible to the runner. const a = run(/[a-z]+|(?=\.)/g, "ab.cd"); -if (JSON.stringify(a) !== '["ab","cd"]') throw new Error("A: " + JSON.stringify(a)); +if (JSON.stringify(a) !== '["ab","","cd"]') throw new Error("A: " + JSON.stringify(a)); const d = run(/(?=\.)/g, "a.b.c"); if (JSON.stringify(d) !== '["",""]') throw new Error("D: " + JSON.stringify(d));