From a803d9226fbc87a9e1c906d5a7d592f083eaaa14 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 25 Aug 2022 10:53:47 -0400 Subject: [PATCH] LibJS: Always access RegExp flags by its "flags" property This is a normative change in the ECMA-262 spec. See: https://github.com/tc39/ecma262/commit/35b7eb2 Note there is a bit of weirdness between the mainline spec and the set notation proposal as the latter has not been updated with this change. For now, this implements what the spec PR and other prototypes indicate how the proposal will behave. --- .../LibJS/Runtime/RegExpPrototype.cpp | 49 +++++++++---------- .../RegExp/RegExp.prototype.@@match.js | 27 ++++++++++ .../RegExp/RegExp.prototype.@@replace.js | 27 ++++++++++ 3 files changed, 78 insertions(+), 25 deletions(-) create mode 100644 Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@match.js create mode 100644 Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@replace.js diff --git a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp index f4f1d54718..7d1353ba87 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp @@ -497,35 +497,32 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::symbol_match) // 3. Let S be ? ToString(string). auto string = TRY(vm.argument(0).to_utf16_string(vm)); - // 4. Let global be ToBoolean(? Get(rx, "global")). - bool global = TRY(regexp_object->get(vm.names.global)).to_boolean(); + // 4. Let flags be ? ToString(? Get(rx, "flags")). + auto flags_value = TRY(regexp_object->get(vm.names.flags)); + auto flags = TRY(flags_value.to_string(vm)); - // 5. If global is false, then - if (!global) { + // 5. If flags does not contain "g", then + if (!flags.contains('g')) { // a. Return ? RegExpExec(rx, S). return TRY(regexp_exec(vm, *regexp_object, move(string))); } // 6. Else, - // a. Assert: global is true. + // a. If flags contains "u", let fullUnicode be true. Otherwise, let fullUnicode be false. + // FIXME: Update spec steps when https://github.com/tc39/ecma262/pull/2418 is rebased on the + // current ECMA-262 main branch. According to the PR, this is how this step will look: + bool full_unicode = flags.contains('u') || flags.contains('v'); - // b. Let fullUnicode be ToBoolean(? Get(rx, "unicodeSets")). - bool full_unicode = TRY(regexp_object->get(vm.names.unicodeSets)).to_boolean(); - - // c. If fullUnicode is false, set fullUnicode to ! ToBoolean(? Get(rx, "unicode")). - if (!full_unicode) - full_unicode = TRY(regexp_object->get(vm.names.unicode)).to_boolean(); - - // d. Perform ? Set(rx, "lastIndex", +0𝔽, true). + // b. Perform ? Set(rx, "lastIndex", +0𝔽, true). TRY(regexp_object->set(vm.names.lastIndex, Value(0), Object::ShouldThrowExceptions::Yes)); - // e. Let A be ! ArrayCreate(0). + // c. Let A be ! ArrayCreate(0). auto* array = MUST(Array::create(realm, 0)); - // f. Let n be 0. + // d. Let n be 0. size_t n = 0; - // g. Repeat, + // e. Repeat, while (true) { // i. Let result be ? RegExpExec(rx, S). auto result_value = TRY(regexp_exec(vm, *regexp_object, string)); @@ -631,20 +628,22 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::symbol_replace) replace_value = js_string(vm, move(replace_string)); } - // 7. Let global be ToBoolean(? Get(rx, "global")). - bool global = TRY(regexp_object->get(vm.names.global)).to_boolean(); + // 7. Let flags be ? ToString(? Get(rx, "flags")). + auto flags_value = TRY(regexp_object->get(vm.names.flags)); + auto flags = TRY(flags_value.to_string(vm)); + + // 8. If flags contains "g", let global be true. Otherwise, let global be false. + bool global = flags.contains('g'); bool full_unicode = false; // 8. If global is true, then if (global) { - // a. Let fullUnicode be ToBoolean(? Get(rx, "unicodeSets")). - full_unicode = TRY(regexp_object->get(vm.names.unicodeSets)).to_boolean(); + // a. If flags contains "u", let fullUnicode be true. Otherwise, let fullUnicode be false. + // FIXME: Update spec steps when https://github.com/tc39/ecma262/pull/2418 is rebased on the + // current ECMA-262 main branch. According to the PR, this is how this step will look: + full_unicode = flags.contains('u') || flags.contains('v'); - // b. If fullUnicode is false, set fullUnicode to ! ToBoolean(? Get(rx, "unicode")). - if (!full_unicode) - full_unicode = TRY(regexp_object->get(vm.names.unicode)).to_boolean(); - - // c. Perform ? Set(rx, "lastIndex", +0𝔽, true). + // b. Perform ? Set(rx, "lastIndex", +0𝔽, true). TRY(regexp_object->set(vm.names.lastIndex, Value(0), Object::ShouldThrowExceptions::Yes)); } diff --git a/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@match.js b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@match.js new file mode 100644 index 0000000000..b82574a574 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@match.js @@ -0,0 +1,27 @@ +describe("basic functionality", () => { + test("uses flags property instead of individual property lookups", () => { + let accessedFlags = false; + let accessedGlobal = false; + let accessedUnicode = false; + + class RegExp1 extends RegExp { + get flags() { + accessedFlags = true; + return "g"; + } + get global() { + accessedGlobal = true; + return false; + } + get unicode() { + accessedUnicode = true; + return false; + } + } + + RegExp.prototype[Symbol.match].call(new RegExp1("foo")); + expect(accessedFlags).toBeTrue(); + expect(accessedGlobal).toBeFalse(); + expect(accessedUnicode).toBeFalse(); + }); +}); diff --git a/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@replace.js b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@replace.js new file mode 100644 index 0000000000..2d3440ab06 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.@@replace.js @@ -0,0 +1,27 @@ +describe("basic functionality", () => { + test("uses flags property instead of individual property lookups", () => { + let accessedFlags = false; + let accessedGlobal = false; + let accessedUnicode = false; + + class RegExp1 extends RegExp { + get flags() { + accessedFlags = true; + return "g"; + } + get global() { + accessedGlobal = true; + return false; + } + get unicode() { + accessedUnicode = true; + return false; + } + } + + RegExp.prototype[Symbol.replace].call(new RegExp1("foo")); + expect(accessedFlags).toBeTrue(); + expect(accessedGlobal).toBeFalse(); + expect(accessedUnicode).toBeFalse(); + }); +});