fix handling of special replacements patterns in String#replaceAll, c…

…lose #900
zloirock · Dec 30, 2020 · 18592d1 · 18592d1
1 parent 44ee579
commit 18592d1
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Changelog
 ##### Unreleased
+- Fixed handling of special replacements patterns in `String#replaceAll`, [#900](https://github.com/zloirock/core-js/issues/900)
 - Fixed iterators dependencies of `Promise.any` and `Promise.allSettled` entries
 
 ##### 3.8.1 - 2020.12.06

diff --git a/packages/core-js/internals/get-substitution.js b/packages/core-js/internals/get-substitution.js
@@ -0,0 +1,40 @@
+var toObject = require('../internals/to-object');
+
+var floor = Math.floor;
+var replace = ''.replace;
+var SUBSTITUTION_SYMBOLS = /\$([$&'`]|\d\d?|<[^>]*>)/g;
+var SUBSTITUTION_SYMBOLS_NO_NAMED = /\$([$&'`]|\d\d?)/g;
+
+// https://tc39.github.io/ecma262/#sec-getsubstitution
+module.exports = function (matched, str, position, captures, namedCaptures, replacement) {
+  var tailPos = position + matched.length;
+  var m = captures.length;
+  var symbols = SUBSTITUTION_SYMBOLS_NO_NAMED;
+  if (namedCaptures !== undefined) {
+    namedCaptures = toObject(namedCaptures);
+    symbols = SUBSTITUTION_SYMBOLS;
+  }
+  return replace.call(replacement, symbols, function (match, ch) {
+    var capture;
+    switch (ch.charAt(0)) {
+      case '$': return '$';
+      case '&': return matched;
+      case '`': return str.slice(0, position);
+      case "'": return str.slice(tailPos);
+      case '<':
+        capture = namedCaptures[ch.slice(1, -1)];
+        break;
+      default: // \d\d?
+        var n = +ch;
+        if (n === 0) return match;
+        if (n > m) {
+          var f = floor(n / 10);
+          if (f === 0) return match;
+          if (f <= m) return captures[f - 1] === undefined ? ch.charAt(1) : captures[f - 1] + ch.charAt(1);
+          return match;
+        }
+        capture = captures[n - 1];
+    }
+    return capture === undefined ? '' : capture;
+  });
+};
diff --git a/packages/core-js/modules/es.string.replace-all.js b/packages/core-js/modules/es.string.replace-all.js
@@ -3,18 +3,29 @@ var $ = require('../internals/export');
 var requireObjectCoercible = require('../internals/require-object-coercible');
 var isRegExp = require('../internals/is-regexp');
 var getRegExpFlags = require('../internals/regexp-flags');
+var getSubstitution = require('../internals/get-substitution');
 var wellKnownSymbol = require('../internals/well-known-symbol');
 var IS_PURE = require('../internals/is-pure');
 
 var REPLACE = wellKnownSymbol('replace');
 var RegExpPrototype = RegExp.prototype;
+var max = Math.max;
+
+var stringIndexOf = function (string, searchValue, fromIndex) {
+  if (fromIndex > string.length) return -1;
+  if (searchValue === '') return fromIndex;
+  return string.indexOf(searchValue, fromIndex);
+};
 
 // `String.prototype.replaceAll` method
 // https://github.com/tc39/proposal-string-replace-all
 $({ target: 'String', proto: true }, {
   replaceAll: function replaceAll(searchValue, replaceValue) {
     var O = requireObjectCoercible(this);
-    var IS_REG_EXP, flags, replacer, string, searchString, template, result, position, index;
+    var IS_REG_EXP, flags, replacer, string, searchString, functionalReplace, searchLength, advanceBy, replacement;
+    var position = 0;
+    var endOfLastMatch = 0;
+    var result = '';
     if (searchValue != null) {
       IS_REG_EXP = isRegExp(searchValue);
       if (IS_REG_EXP) {
@@ -33,17 +44,23 @@ $({ target: 'String', proto: true }, {
     }
     string = String(O);
     searchString = String(searchValue);
-    if (searchString === '') return replaceAll.call(string, /(?:)/g, replaceValue);
-    template = string.split(searchString);
-    if (typeof replaceValue !== 'function') {
-      return template.join(String(replaceValue));
+    functionalReplace = typeof replaceValue === 'function';
+    if (!functionalReplace) replaceValue = String(replaceValue);
+    searchLength = searchString.length;
+    advanceBy = max(1, searchLength);
+    position = stringIndexOf(string, searchString, 0);
+    while (position !== -1) {
+      if (functionalReplace) {
+        replacement = String(replaceValue(searchString, position, string));
+      } else {
+        replacement = getSubstitution(searchString, string, position, [], undefined, replaceValue);
+      }
+      result += string.slice(endOfLastMatch, position) + replacement;
+      endOfLastMatch = position + searchLength;
+      position = stringIndexOf(string, searchString, position + advanceBy);
     }
-    result = template[0];
-    position = result.length;
-    for (index = 1; index < template.length; index++) {
-      result += String(replaceValue(searchString, position, string));
-      position += searchString.length + template[index].length;
-      result += template[index];
+    if (endOfLastMatch < string.length) {
+      result += string.slice(endOfLastMatch);
     }
     return result;
   }

diff --git a/packages/core-js/modules/es.string.replace.js b/packages/core-js/modules/es.string.replace.js
@@ -1,18 +1,15 @@
 'use strict';
 var fixRegExpWellKnownSymbolLogic = require('../internals/fix-regexp-well-known-symbol-logic');
 var anObject = require('../internals/an-object');
-var toObject = require('../internals/to-object');
 var toLength = require('../internals/to-length');
 var toInteger = require('../internals/to-integer');
 var requireObjectCoercible = require('../internals/require-object-coercible');
 var advanceStringIndex = require('../internals/advance-string-index');
+var getSubstitution = require('../internals/get-substitution');
 var regExpExec = require('../internals/regexp-exec-abstract');
 
 var max = Math.max;
 var min = Math.min;
-var floor = Math.floor;
-var SUBSTITUTION_SYMBOLS = /\$([$&'`]|\d\d?|<[^>]*>)/g;
-var SUBSTITUTION_SYMBOLS_NO_NAMED = /\$([$&'`]|\d\d?)/g;
 
 var maybeToString = function (it) {
   return it === undefined ? it : String(it);
@@ -98,38 +95,4 @@ fixRegExpWellKnownSymbolLogic('replace', 2, function (REPLACE, nativeReplace, ma
       return accumulatedResult + S.slice(nextSourcePosition);
     }
   ];
-
-  // https://tc39.github.io/ecma262/#sec-getsubstitution
-  function getSubstitution(matched, str, position, captures, namedCaptures, replacement) {
-    var tailPos = position + matched.length;
-    var m = captures.length;
-    var symbols = SUBSTITUTION_SYMBOLS_NO_NAMED;
-    if (namedCaptures !== undefined) {
-      namedCaptures = toObject(namedCaptures);
-      symbols = SUBSTITUTION_SYMBOLS;
-    }
-    return nativeReplace.call(replacement, symbols, function (match, ch) {
-      var capture;
-      switch (ch.charAt(0)) {
-        case '$': return '$';
-        case '&': return matched;
-        case '`': return str.slice(0, position);
-        case "'": return str.slice(tailPos);
-        case '<':
-          capture = namedCaptures[ch.slice(1, -1)];
-          break;
-        default: // \d\d?
-          var n = +ch;
-          if (n === 0) return match;
-          if (n > m) {
-            var f = floor(n / 10);
-            if (f === 0) return match;
-            if (f <= m) return captures[f - 1] === undefined ? ch.charAt(1) : captures[f - 1] + ch.charAt(1);
-            return match;
-          }
-          capture = captures[n - 1];
-      }
-      return capture === undefined ? '' : capture;
-    });
-  }
 });
diff --git a/tests/pure/es.string.replace-all.js b/tests/pure/es.string.replace-all.js
@@ -26,6 +26,10 @@ QUnit.test('String#replaceAll', assert => {
   assert.same(replaceAll('aba', searcher, 'c'), 'foo');
   assert.same(replaceAll('aba', 'b'), 'aundefineda');
   assert.same(replaceAll('xxx', '', '_'), '_x_x_x_');
+  assert.same(replaceAll('121314', '1', '$$'), '$2$3$4', '$$');
+  assert.same(replaceAll('121314', '1', '$&'), '121314', '$&');
+  assert.same(replaceAll('121314', '1', '$`'), '212312134', '$`');
+  assert.same(replaceAll('121314', '1', '$\''), '213142314344', '$\'');
   if (STRICT) {
     assert.throws(() => replaceAll(null, 'a', 'b'), TypeError);
     assert.throws(() => replaceAll(undefined, 'a', 'b'), TypeError);

diff --git a/tests/tests/es.string.replace-all.js b/tests/tests/es.string.replace-all.js
@@ -28,6 +28,10 @@ QUnit.test('String#replaceAll', assert => {
   assert.same('aba'.replaceAll(searcher, 'c'), 'foo');
   assert.same('aba'.replaceAll('b'), 'aundefineda');
   assert.same('xxx'.replaceAll('', '_'), '_x_x_x_');
+  assert.same('121314'.replaceAll('1', '$$'), '$2$3$4', '$$');
+  assert.same('121314'.replaceAll('1', '$&'), '121314', '$&');
+  assert.same('121314'.replaceAll('1', '$`'), '212312134', '$`');
+  assert.same('121314'.replaceAll('1', '$\''), '213142314344', '$\'');
   if (STRICT) {
     assert.throws(() => replaceAll.call(null, 'a', 'b'), TypeError);
     assert.throws(() => replaceAll.call(undefined, 'a', 'b'), TypeError);