replace/replaceAll mishandle $ substitution patterns and insert undefined for an unmatched group #341

Version

v1.2.3 / master @ 5473bfb

What happens

A capture group that does not participate in the match puts the text undefined into the
output:

import MagicString from 'magic-string'

new MagicString('ac').replace(/a(b)?/, '[$1]').toString()
// '[undefined]c'
'ac'.replace(/a(b)?/, '[$1]')
// '[]c'

Nothing is thrown. An optional group that happens not to match is ordinary, so this
silently writes undefined into generated code.

The rest of the table

_replaceRegexp links to the MDN table of $ patterns, and implements three of its six
entries. Every other entry is either wrong or absent:

// `$0` names no group, so it is literal - it expands to the whole match instead
new MagicString('abcabc').replace(/(a)/, '[$0]').toString() // '[a]bcabc'
'abcabc'.replace(/(a)/, '[$0]')                             // '[$0]bcabc'

// `$nn` never falls back to `$n`, so it stays literal when there is no group nn
new MagicString('ab').replace(/(a)/, '[$12]').toString()    // '[$12]b'
'ab'.replace(/(a)/, '[$12]')                                // '[a2]b'

// `$<name>` is not recognised
new MagicString('ab').replace(/(?<first>a)/, '[$<first>]').toString() // '[$<first>]b'
'ab'.replace(/(?<first>a)/, '[$<first>]')                             // '[a]b'

// `` $` `` and `$'` are not recognised
new MagicString('abcabc').replace(/b/, '$`').toString()  // 'a$`cabc'
'abcabc'.replace(/b/, '$`')                              // 'aacabc'
new MagicString('abcabc').replace(/b/, '$\'').toString() // "a$'cabc"
'abcabc'.replace(/b/, '$\'')                             // 'acabccabc'

A string search value expands nothing at all

_replaceString and _replaceAllString pass the replacement through untouched, so the
same replacement behaves differently depending only on whether the pattern is a string or
an equivalent regexp:

new MagicString('abcabc').replace('b', '$$').toString() // 'a$$cabc'
new MagicString('abcabc').replace(/b/, '$$').toString() // 'a$cabc'
'abcabc'.replace('b', '$$')                             // 'a$cabc'  <- both should be this

$&, $` and $' are affected the same way. $n and $<name> are correctly literal
for a string search value, because there are no capture groups of either kind — that part
String.prototype.replace also does.

Cause

The substitution is one pass of replacement.replace(/\$(\$|&|\d+)/g, ...):

const num = +i
if (num < match.length)
  return match[+i]
return `$${i}`

• for a group that did not participate, match[n] is undefined. It is returned from the
  replacer and coerced to "undefined" when concatenated.
• the same guard admits i === '0', and match[0] is the whole match, so $0 expands to
  it rather than staying literal.
• \d+ is greedy, so $12 is always read as index 12. String.prototype.replace prefers
  the two-digit reading only when that group exists, and otherwise falls back to one digit,
  which is why $12 is group 1 followed by a literal 2 when the pattern has one group.
• $` , $' and $<name> are not in the pattern, so they were never candidates.

Scope

Comparing against String.prototype.replace/replaceAll as the reference, over random
sources, patterns (0–3 groups, named groups, optional groups, zero-length matches, global
and not) and replacement strings assembled from the substitution tokens: 32,902 of
136,000 comparisons disagree on master. Every disagreement is one of the cases above.

The undefined case is the only one that corrupts output silently. The others leak a $
sequence into the output verbatim, or — for $0 — substitute text that was not asked for.