Browse Source

LibJS: Keep RegExp.exec() results in correct order

By using regex::AllFlags::SkipTrimEmptyMatches we get a null string for
unmatched capture groups, which we then turn into an undefined entry in
the result array instead of putting all matches first and appending
undefined for the remaining number of capture groups - e.g. for

    /foo(ba((r)|(z)))/.exec("foobaz")

we now return

    ["foobaz", "baz", "z", undefined, "z"]

and not [

    ["foobaz", "baz", "z", "z", undefined]

Fixes part of #6042.

Also happens to fix selecting an element by ID using jQuery's $("#foo").
Linus Groh 4 years ago
parent
commit
e46fa3ac8b

+ 2 - 2
Userland/Libraries/LibJS/Runtime/RegExpObject.cpp

@@ -39,8 +39,8 @@ static Flags options_from(const String& flags, VM& vm, GlobalObject& global_obje
     Flags options {
     Flags options {
         // JS regexps are all 'global' by default as per our definition, but the "global" flag enables "stateful".
         // JS regexps are all 'global' by default as per our definition, but the "global" flag enables "stateful".
         // FIXME: Enable 'BrowserExtended' only if in a browser context.
         // FIXME: Enable 'BrowserExtended' only if in a browser context.
-        { (regex::ECMAScriptFlags)regex::AllFlags::Global | ECMAScriptFlags::BrowserExtended },
-        {},
+        .effective_flags = { (regex::ECMAScriptFlags)regex::AllFlags::Global | (regex::ECMAScriptFlags)regex::AllFlags::SkipTrimEmptyMatches | regex::ECMAScriptFlags::BrowserExtended },
+        .declared_flags = {},
     };
     };
 
 
     for (auto ch : flags) {
     for (auto ch : flags) {

+ 3 - 4
Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp

@@ -1,6 +1,6 @@
 /*
 /*
  * Copyright (c) 2020, Matthew Olsson <matthewcolsson@gmail.com>
  * Copyright (c) 2020, Matthew Olsson <matthewcolsson@gmail.com>
- * Copyright (c) 2020, Linus Groh <mail@linusgroh.de>
+ * Copyright (c) 2020-2021, Linus Groh <mail@linusgroh.de>
  * All rights reserved.
  * All rights reserved.
  *
  *
  * Redistribution and use in source and binary forms, with or without
  * Redistribution and use in source and binary forms, with or without
@@ -194,10 +194,9 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::exec)
 
 
     for (size_t i = 0; i < result.n_capture_groups; ++i) {
     for (size_t i = 0; i < result.n_capture_groups; ++i) {
         auto capture_value = js_undefined();
         auto capture_value = js_undefined();
-        if (result.capture_group_matches[0].size() > i) {
-            auto& capture = result.capture_group_matches[0][i];
+        auto& capture = result.capture_group_matches[0][i + 1];
+        if (!capture.view.is_null())
             capture_value = js_string(vm, capture.view.to_string());
             capture_value = js_string(vm, capture.view.to_string());
-        }
         array->indexed_properties().put(array, i + 1, capture_value);
         array->indexed_properties().put(array, i + 1, capture_value);
     }
     }
 
 

+ 10 - 0
Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.exec.js

@@ -28,6 +28,16 @@ test("basic unnamed captures", () => {
     expect(res[2]).toBe(undefined);
     expect(res[2]).toBe(undefined);
     expect(res.groups).toBe(undefined);
     expect(res.groups).toBe(undefined);
     expect(res.index).toBe(0);
     expect(res.index).toBe(0);
+
+    re = /(foo)?(bar)/;
+    res = re.exec("bar");
+
+    expect(res.length).toBe(3);
+    expect(res[0]).toBe("bar");
+    expect(res[1]).toBe(undefined);
+    expect(res[2]).toBe("bar");
+    expect(res.groups).toBe(undefined);
+    expect(res.index).toBe(0);
 });
 });
 
 
 test("basic named captures", () => {
 test("basic named captures", () => {