From decc48f8534618bf9023ee409624e19c3243aa42 Mon Sep 17 00:00:00 2001 From: Denis Pushkarev Date: Sat, 28 Sep 2019 21:48:29 +0700 Subject: [PATCH] `String#{ matchAll, replaceAll }` throws a error on non-global regex argument per https://github.com/tc39/proposal-string-replaceall/pull/24 and decision from TC39 meetings --- CHANGELOG.md | 1 + packages/core-js-compat/src/data.js | 7 +++-- .../core-js/modules/es.string.match-all.js | 22 ++++++++++---- .../modules/esnext.string.replace-all.js | 29 +++++++------------ .../modules/esnext.symbol.replace-all.js | 3 +- tests/commonjs.js | 8 ++--- tests/compat/tests.js | 6 +++- tests/pure/es.string.match-all.js | 15 +--------- tests/pure/esnext.string.replace-all.js | 4 +-- tests/tests/es.string.match-all.js | 15 +--------- tests/tests/esnext.string.replace-all.js | 4 +-- 11 files changed, 48 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75139eba89c0..9d53f3e7d142 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Changelog ##### Unreleased +- **`String#{ matchAll, replaceAll }` throws an error on non-global regex argument per [this PR](https://github.com/tc39/proposal-string-replaceall/pull/24) and the decision from TC39 meetings. It's a breaking change, but because it's a breaking change in the ES spec, it's added at the minor release** - Added [iterator helpers stage 2 proposal](https://github.com/tc39/proposal-iterator-helpers): - `Iterator` - `Iterator.from` diff --git a/packages/core-js-compat/src/data.js b/packages/core-js-compat/src/data.js index 80c90fa792d6..27188b452acc 100644 --- a/packages/core-js-compat/src/data.js +++ b/packages/core-js-compat/src/data.js @@ -848,9 +848,10 @@ const data = { safari: '10.0', }, 'es.string.match-all': { - chrome: '73', - firefox: '67', - safari: '13', + // Early implementations does not throw an error on non-global regex + // chrome: '73', + // firefox: '67', + // safari: '13', }, 'es.string.pad-end': { edge: '15', diff --git a/packages/core-js/modules/es.string.match-all.js b/packages/core-js/modules/es.string.match-all.js index 25d07435cea1..5a6d50a00ee4 100644 --- a/packages/core-js/modules/es.string.match-all.js +++ b/packages/core-js/modules/es.string.match-all.js @@ -6,8 +6,10 @@ var toLength = require('../internals/to-length'); var aFunction = require('../internals/a-function'); var anObject = require('../internals/an-object'); var classof = require('../internals/classof'); -var getFlags = require('../internals/regexp-flags'); +var isRegExp = require('../internals/is-regexp'); +var getRegExpFlags = require('../internals/regexp-flags'); var createNonEnumerableProperty = require('../internals/create-non-enumerable-property'); +var fails = require('../internals/fails'); var wellKnownSymbol = require('../internals/well-known-symbol'); var speciesConstructor = require('../internals/species-constructor'); var advanceStringIndex = require('../internals/advance-string-index'); @@ -21,6 +23,11 @@ var setInternalState = InternalStateModule.set; var getInternalState = InternalStateModule.getterFor(REGEXP_STRING_ITERATOR); var RegExpPrototype = RegExp.prototype; var regExpBuiltinExec = RegExpPrototype.exec; +var nativeMatchAll = ''.matchAll; + +var WORKS_WITH_NON_GLOBAL_REGEX = !!nativeMatchAll && !fails(function () { + 'a'.matchAll(/./); +}); var regExpExec = function (R, S) { var exec = R.exec; @@ -64,7 +71,7 @@ var $matchAll = function (string) { C = speciesConstructor(R, RegExp); flagsValue = R.flags; if (flagsValue === undefined && R instanceof RegExp && !('flags' in RegExpPrototype)) { - flagsValue = getFlags.call(R); + flagsValue = getRegExpFlags.call(R); } flags = flagsValue === undefined ? '' : String(flagsValue); matcher = new C(C === RegExp ? R.source : R, flags); @@ -76,15 +83,20 @@ var $matchAll = function (string) { // `String.prototype.matchAll` method // https://github.com/tc39/proposal-string-matchall -$({ target: 'String', proto: true }, { +$({ target: 'String', proto: true, forced: WORKS_WITH_NON_GLOBAL_REGEX }, { matchAll: function matchAll(regexp) { var O = requireObjectCoercible(this); - var S, matcher, rx; + var flags, S, matcher, rx; if (regexp != null) { + if (isRegExp(regexp)) { + flags = String('flags' in RegExpPrototype ? regexp.flags : getRegExpFlags.call(regexp)); + if (!~flags.indexOf('g')) throw TypeError('`.matchAll` does not allow non-global regexes'); + } + if (WORKS_WITH_NON_GLOBAL_REGEX) return nativeMatchAll.apply(O, arguments); matcher = regexp[MATCH_ALL]; if (matcher === undefined && IS_PURE && classof(regexp) == 'RegExp') matcher = $matchAll; if (matcher != null) return aFunction(matcher).call(regexp, O); - } + } else if (WORKS_WITH_NON_GLOBAL_REGEX) return nativeMatchAll.apply(O, arguments); S = String(O); rx = new RegExp(regexp, 'g'); return IS_PURE ? $matchAll.call(rx, S) : rx[MATCH_ALL](S); diff --git a/packages/core-js/modules/esnext.string.replace-all.js b/packages/core-js/modules/esnext.string.replace-all.js index 329d3003a59f..5a883a509860 100644 --- a/packages/core-js/modules/esnext.string.replace-all.js +++ b/packages/core-js/modules/esnext.string.replace-all.js @@ -1,38 +1,31 @@ 'use strict'; var $ = require('../internals/export'); -var createNonEnumerableProperty = require('../internals/create-non-enumerable-property'); var requireObjectCoercible = require('../internals/require-object-coercible'); -var anObject = require('../internals/an-object'); var isRegExp = require('../internals/is-regexp'); var getRegExpFlags = require('../internals/regexp-flags'); -var speciesConstructor = require('../internals/species-constructor'); var wellKnownSymbol = require('../internals/well-known-symbol'); var IS_PURE = require('../internals/is-pure'); -var REPLACE_ALL = wellKnownSymbol('replaceAll'); +var REPLACE = wellKnownSymbol('replace'); var RegExpPrototype = RegExp.prototype; -var $replaceAll = function (string, replaceValue) { - var rx = anObject(this); - var flags = String('flags' in RegExpPrototype ? rx.flags : getRegExpFlags.call(rx)); - if (!~flags.indexOf('g')) { - rx = new (speciesConstructor(rx, RegExp))(rx.source, flags + 'g'); - } - return String(string).replace(rx, replaceValue); -}; - // `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 replacer, string, searchString, template, result, index; + var IS_REG_EXP, flags, replacer, string, searchString, template, result, index; if (searchValue != null) { - replacer = searchValue[REPLACE_ALL]; + IS_REG_EXP = isRegExp(searchValue); + if (IS_REG_EXP) { + flags = String('flags' in RegExpPrototype ? searchValue.flags : getRegExpFlags.call(searchValue)); + if (!~flags.indexOf('g')) throw TypeError('`.replaceAll` does not allow non-global regexes'); + } + replacer = searchValue[REPLACE]; if (replacer !== undefined) { return replacer.call(searchValue, O, replaceValue); - } else if (IS_PURE && isRegExp(searchValue)) { - return $replaceAll.call(searchValue, O, replaceValue); + } else if (IS_PURE && IS_REG_EXP) { + return String(O).replace(searchValue, replaceValue); } } string = String(O); @@ -49,5 +42,3 @@ $({ target: 'String', proto: true }, { return result; } }); - -IS_PURE || REPLACE_ALL in RegExpPrototype || createNonEnumerableProperty(RegExpPrototype, REPLACE_ALL, $replaceAll); diff --git a/packages/core-js/modules/esnext.symbol.replace-all.js b/packages/core-js/modules/esnext.symbol.replace-all.js index c103887233de..82cbd293d9c3 100644 --- a/packages/core-js/modules/esnext.symbol.replace-all.js +++ b/packages/core-js/modules/esnext.symbol.replace-all.js @@ -1,5 +1,4 @@ +// TODO: remove from `core-js@4` var defineWellKnownSymbol = require('../internals/define-well-known-symbol'); -// `Symbol.replaceAll` well-known symbol -// https://tc39.github.io/proposal-string-replaceall/ defineWellKnownSymbol('replaceAll'); diff --git a/tests/commonjs.js b/tests/commonjs.js index d9ecc417f352..180005f5829f 100644 --- a/tests/commonjs.js +++ b/tests/commonjs.js @@ -221,7 +221,7 @@ for (const _PATH of ['../packages/core-js-pure', '../packages/core-js']) { ok(load('features/string/trim-end')(' a ') === ' a'); ok(load('features/string/trim-left')(' a ') === 'a '); ok(load('features/string/trim-right')(' a ') === ' a'); - ok('next' in load('features/string/match-all')('a', /./)); + ok('next' in load('features/string/match-all')('a', /./g)); ok(typeof load('features/string/replace-all') === 'function'); ok('next' in load('features/string/iterator')('qwe')); ok(load('features/string/virtual/code-point-at').call('a', 0) === 97); @@ -250,7 +250,7 @@ for (const _PATH of ['../packages/core-js-pure', '../packages/core-js']) { ok(load('features/string/virtual/trim-end').call(' a ') === ' a'); ok(load('features/string/virtual/trim-left').call(' a ') === 'a '); ok(load('features/string/virtual/trim-right').call(' a ') === ' a'); - ok('next' in load('features/string/virtual/match-all').call('a', /./)); + ok('next' in load('features/string/virtual/match-all').call('a', /./g)); ok(typeof load('features/string/virtual/replace-all') === 'function'); ok(load('features/string/virtual').at.call('a', 0) === 'a'); ok('next' in load('features/string/virtual/iterator').call('qwe')); @@ -559,7 +559,7 @@ for (const _PATH of ['../packages/core-js-pure', '../packages/core-js']) { ok(load('stable/string/ends-with')('qwe', 'we')); ok(load('stable/string/includes')('qwe', 'w')); ok(load('stable/string/repeat')('q', 3) === 'qqq'); - ok('next' in load('stable/string/match-all')('a', /./)); + ok('next' in load('stable/string/match-all')('a', /./g)); ok(load('stable/string/starts-with')('qwe', 'qw')); ok(typeof load('stable/string/anchor') === 'function'); ok(typeof load('stable/string/big') === 'function'); @@ -1626,7 +1626,7 @@ load('features/typed-array/to-string'); load('features/typed-array/values'); ok(typeof load('features/typed-array').Uint32Array === 'function'); ok(typeof load('es/string/match') === 'function'); -ok('next' in load('es/string/match-all')('a', /./)); +ok('next' in load('es/string/match-all')('a', /./g)); ok(typeof load('es/string/replace') === 'function'); ok(typeof load('es/string/search') === 'function'); ok(load('es/string/split')('a s d', ' ').length === 3); diff --git a/tests/compat/tests.js b/tests/compat/tests.js index 994b86fb7aa0..54814ed85cc2 100644 --- a/tests/compat/tests.js +++ b/tests/compat/tests.js @@ -766,7 +766,11 @@ GLOBAL.tests = { return ''.match(O) == 7 && execCalled; }, 'es.string.match-all': function () { - return String.prototype.matchAll; + try { + 'a'.matchAll(/./); + } catch (error) { + return 'a'.matchAll(/./g); + } }, 'es.string.pad-end': function () { return String.prototype.padEnd && !WEBKIT_STRING_PAD_BUG; diff --git a/tests/pure/es.string.match-all.js b/tests/pure/es.string.match-all.js index 9e05012bbc3b..16f888745f65 100644 --- a/tests/pure/es.string.match-all.js +++ b/tests/pure/es.string.match-all.js @@ -69,20 +69,7 @@ QUnit.test('String#matchAll', assert => { value: undefined, done: true, }); - iterator = matchAll('1111a2b3cccc', /(\d)(\D)/); - assert.isIterator(iterator); - assert.isIterable(iterator); - assert.deepEqual(iterator.next(), { - value: assign(['1a', '1', 'a'], { - input: '1111a2b3cccc', - index: 3, - }), - done: false, - }); - assert.deepEqual(iterator.next(), { - value: undefined, - done: true, - }); + assert.throws(() => matchAll('1111a2b3cccc', /(\d)(\D)/), TypeError); iterator = matchAll('1111a2b3cccc', '(\\d)(\\D)'); assert.isIterator(iterator); assert.isIterable(iterator); diff --git a/tests/pure/esnext.string.replace-all.js b/tests/pure/esnext.string.replace-all.js index a597c9b2f88b..54d5f825ba97 100644 --- a/tests/pure/esnext.string.replace-all.js +++ b/tests/pure/esnext.string.replace-all.js @@ -16,7 +16,7 @@ QUnit.test('String#replaceAll', assert => { return 'c'; }), 'aca'); const searcher = { - [Symbol.replaceAll](O, replaceValue) { + [Symbol.replace](O, replaceValue) { assert.same(this, searcher, '`this` is `searcher`'); assert.same(String(O), 'aba', '`O` is `aba`'); assert.same(String(replaceValue), 'c', '`replaceValue` is `c`'); @@ -29,7 +29,7 @@ QUnit.test('String#replaceAll', assert => { assert.throws(() => replaceAll(null, 'a', 'b'), TypeError); assert.throws(() => replaceAll(undefined, 'a', 'b'), TypeError); } - assert.same(replaceAll('b.b.b.b.b', /\./, 'a'), 'babababab'); + assert.throws(() => replaceAll('b.b.b.b.b', /\./, 'a'), TypeError); assert.same(replaceAll('b.b.b.b.b', /\./g, 'a'), 'babababab'); const object = {}; assert.same(replaceAll('[object Object]', object, 'a'), 'a'); diff --git a/tests/tests/es.string.match-all.js b/tests/tests/es.string.match-all.js index 967c560113a5..98b3194da9a7 100644 --- a/tests/tests/es.string.match-all.js +++ b/tests/tests/es.string.match-all.js @@ -69,20 +69,7 @@ QUnit.test('String#matchAll', assert => { value: undefined, done: true, }); - iterator = '1111a2b3cccc'.matchAll(/(\d)(\D)/); - assert.isIterator(iterator); - assert.isIterable(iterator); - assert.deepEqual(iterator.next(), { - value: assign(['1a', '1', 'a'], { - input: '1111a2b3cccc', - index: 3, - }), - done: false, - }); - assert.deepEqual(iterator.next(), { - value: undefined, - done: true, - }); + assert.throws(() => '1111a2b3cccc'.matchAll(/(\d)(\D)/), TypeError); iterator = '1111a2b3cccc'.matchAll('(\\d)(\\D)'); assert.isIterator(iterator); assert.isIterable(iterator); diff --git a/tests/tests/esnext.string.replace-all.js b/tests/tests/esnext.string.replace-all.js index 91f9612d7f54..2441467e7c81 100644 --- a/tests/tests/esnext.string.replace-all.js +++ b/tests/tests/esnext.string.replace-all.js @@ -18,7 +18,7 @@ QUnit.test('String#replaceAll', assert => { return 'c'; }), 'aca'); const searcher = { - [Symbol.replaceAll](O, replaceValue) { + [Symbol.replace](O, replaceValue) { assert.same(this, searcher, '`this` is `searcher`'); assert.same(String(O), 'aba', '`O` is `aba`'); assert.same(String(replaceValue), 'c', '`replaceValue` is `c`'); @@ -31,7 +31,7 @@ QUnit.test('String#replaceAll', assert => { assert.throws(() => replaceAll.call(null, 'a', 'b'), TypeError); assert.throws(() => replaceAll.call(undefined, 'a', 'b'), TypeError); } - assert.same('b.b.b.b.b'.replaceAll(/\./, 'a'), 'babababab'); + assert.throws(() => 'b.b.b.b.b'.replaceAll(/\./, 'a'), TypeError); assert.same('b.b.b.b.b'.replaceAll(/\./g, 'a'), 'babababab'); const object = {}; assert.same('[object Object]'.replaceAll(object, 'a'), 'a');