Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RegExp.prototype[Symbol.match] and String.prototype.match #1836

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Fix RegExp.prototype[Symbol.match] and String.prototype.match #1836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3371fc
NativeRegExp: Implement Symbol.match
balajirrao Feb 14, 2025
eba34cf
Fix String.prototype.match
balajirrao Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/NativeString.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,6 @@ public Object execIdCall(
return ScriptRuntime.wrapBoolean(
(id == Id_equals) ? s1.equals(s2) : s1.equalsIgnoreCase(s2));
}

case Id_match:
case Id_search:
case Id_replace:
case Id_replaceAll:
Expand Down Expand Up @@ -864,6 +862,9 @@ else if (Normalizer.Form.NFC.name().equals(formStr))
case SymbolId_iterator:
return new NativeStringIterator(scope, requireObjectCoercible(cx, thisObj, f));

case Id_match:
return js_match(f, cx, scope, thisObj, args);

case Id_matchAll:
{
// See ECMAScript spec 22.1.3.14
Expand Down Expand Up @@ -1041,6 +1042,49 @@ private ScriptableObject defaultIndexPropertyDescriptor(Object value) {
return desc;
}

/*
*
* See ECMA 22.1.3.13
*
*/
private static Object js_match(
IdFunctionObject f, Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
Object o = requireObjectCoercible(cx, thisObj, f);
Object regexp = args.length > 0 ? args[0] : Undefined.instance;
RegExpProxy regExpProxy = ScriptRuntime.checkRegExpProxy(cx);
if (regexp != null && !Undefined.isUndefined(regexp)) {
Object matcher = ScriptRuntime.getObjectElem(regexp, SymbolKey.MATCH, cx, scope);
// If method is not undefined, it should be a Callable
if (matcher != null && !Undefined.isUndefined(matcher)) {
if (!(matcher instanceof Callable)) {
throw ScriptRuntime.notFunctionError(
regexp, matcher, SymbolKey.MATCH.getName());
}
return ((Callable) matcher)
.call(cx, scope, ScriptRuntime.toObject(scope, regexp), new Object[] {o});
}
}

String s = ScriptRuntime.toString(o);
String regexpToString = Undefined.isUndefined(regexp) ? "" : ScriptRuntime.toString(regexp);

String flags = null;

// Not standard; Done for backward compatibility
if (cx.getLanguageVersion() < Context.VERSION_1_6 && args.length > 1) {
flags = ScriptRuntime.toString(args[1]);
}

Object compiledRegExp = regExpProxy.compileRegExp(cx, regexpToString, flags);
Scriptable rx = regExpProxy.wrapRegExp(cx, scope, compiledRegExp);

Object method = ScriptRuntime.getObjectElem(rx, SymbolKey.MATCH, cx, scope);
if (!(method instanceof Callable)) {
throw ScriptRuntime.notFunctionError(rx, method, SymbolKey.MATCH.getName());
}
return ((Callable) method).call(cx, scope, rx, new Object[] {s});
}

/*
*
* See ECMA 15.5.4.6. Uses Java String.indexOf()
Expand Down
55 changes: 54 additions & 1 deletion rhino/src/main/java/org/mozilla/javascript/regexp/NativeRegExp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import java.io.Serializable;
import org.mozilla.javascript.AbstractEcmaObjectOperations;
import org.mozilla.javascript.Callable;
import org.mozilla.javascript.Constructable;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.Function;
Expand Down Expand Up @@ -2662,6 +2663,13 @@ protected Object getInstanceIdValue(int id) {
return super.getInstanceIdValue(id);
}

private void setLastIndex(ScriptableObject thisObj, Object value) {
if ((thisObj.getAttributes("lastIndex") & READONLY) != 0) {
throw ScriptRuntime.typeErrorById("msg.modify.readonly", "lastIndex");
}
ScriptableObject.putProperty(thisObj, "lastIndex", value);
}

private void setLastIndex(Object value) {
if ((lastIndexAttr & READONLY) != 0) {
throw ScriptRuntime.typeErrorById("msg.modify.readonly", "lastIndex");
Expand Down Expand Up @@ -2784,7 +2792,7 @@ public Object execIdCall(
return realThis(thisObj, f).execSub(cx, scope, args, PREFIX);

case SymbolId_match:
return realThis(thisObj, f).execSub(cx, scope, args, MATCH);
return js_SymbolMatch(cx, scope, thisObj, args);

case SymbolId_matchAll:
return js_SymbolMatchAll(cx, scope, thisObj, args);
Expand All @@ -2797,6 +2805,51 @@ public Object execIdCall(
throw new IllegalArgumentException(String.valueOf(id));
}

public static Object regExpExec(
Scriptable regexp, String string, Context cx, Scriptable scope) {
// See ECMAScript spec 22.2.7.1
Object execMethod = ScriptRuntime.getObjectProp(regexp, "exec", cx, scope);
if (execMethod instanceof Callable) {
return ((Callable) execMethod).call(cx, scope, regexp, new Object[] {string});
}
return NativeRegExp.js_exec(cx, scope, regexp, new Object[] {string});
}

private Object js_SymbolMatch(
Context cx, Scriptable scope, Scriptable thisScriptable, Object[] args) {
// See ECMAScript spec 22.2.6.8
var thisObj = ScriptableObject.ensureScriptableObject(thisScriptable);

String string = ScriptRuntime.toString(args.length > 0 ? args[0] : Undefined.instance);
String flags = ScriptRuntime.toString(ScriptRuntime.getObjectProp(thisObj, "flags", cx));
boolean fullUnicode = flags.indexOf('u') != -1 || flags.indexOf('v') != -1;

if (flags.indexOf('g') == -1) return regExpExec(thisObj, string, cx, scope);

setLastIndex(thisObj, ScriptRuntime.zeroObj);
Scriptable result = cx.newArray(scope, 0);
int i = 0;
while (true) {
Object match = regExpExec(thisObj, string, cx, scope);
if (match == null) {
if (i == 0) return null;
else return result;
}

String matchStr =
ScriptRuntime.toString(ScriptRuntime.getObjectIndex(match, 0, cx, scope));
result.put(i++, result, matchStr);

if (matchStr.isEmpty()) {
long thisIndex =
ScriptRuntime.toLength(
ScriptRuntime.getObjectProp(thisObj, "lastIndex", cx));
long nextIndex = ScriptRuntime.advanceStringIndex(string, thisIndex, fullUnicode);
setLastIndex(thisObj, nextIndex);
}
}
}

static Object js_exec(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
return realThis(thisObj, "exec").execSub(cx, scope, args, MATCH);
}
Expand Down
12 changes: 1 addition & 11 deletions rhino/src/main/java/org/mozilla/javascript/regexp/NativeRegExpStringIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

package org.mozilla.javascript.regexp;

import org.mozilla.javascript.Callable;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.ES6Iterator;
import org.mozilla.javascript.ScriptRuntime;
Expand Down Expand Up @@ -66,7 +65,7 @@ protected boolean isDone(Context cx, Scriptable scope) {
return true;
}

next = regExpExec(cx, scope);
next = NativeRegExp.regExpExec(regexp, string, cx, scope);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please pop down to line 98 and get rid of the private method "regExpExec" that's no longer used?

if (next == null) {
// Done! Point ii of the spec
next = Undefined.instance;
Expand Down Expand Up @@ -95,15 +94,6 @@ protected Object nextValue(Context cx, Scriptable scope) {
return next;
}

private Object regExpExec(Context cx, Scriptable scope) {
// See ECMAScript spec 22.2.7.1
Object execMethod = ScriptRuntime.getObjectProp(regexp, "exec", cx);
if (execMethod instanceof Callable) {
return ((Callable) execMethod).call(cx, scope, regexp, new Object[] {string});
}
return NativeRegExp.js_exec(cx, scope, regexp, new Object[] {string});
}

@Override
protected String getTag() {
return ITERATOR_TAG;
Expand Down
4 changes: 2 additions & 2 deletions rhino/src/test/java/org/mozilla/javascript/tests/NativeRegExpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ public void matchGlobal() throws Exception {
/**
* @throws Exception if an error occurs
*/
// TODO @Test
@Test
public void matchGlobalSymbol() throws Exception {
final String script =
"var result = /a/g[Symbol.match]('aaba');\n"
Expand Down Expand Up @@ -500,7 +500,7 @@ public void matchStickyAndGlobal() throws Exception {
/**
* @throws Exception if an error occurs
*/
// TODO @Test
@Test
public void matchStickyAndGlobalSymbol() throws Exception {
final String script =
"var result = /a/yg[Symbol.match]('aaba');\n"
Expand Down
22 changes: 2 additions & 20 deletions tests/testsrc/test262.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1578,7 +1578,7 @@ built-ins/Reflect 13/153 (8.5%)
set/return-false-if-receiver-is-not-writable.js
set/return-false-if-target-is-not-writable.js

built-ins/RegExp 1150/1854 (62.03%)
built-ins/RegExp 1134/1854 (61.17%)
CharacterClassEscapes 24/24 (100.0%)
dotall 4/4 (100.0%)
escape 20/20 (100.0%)
Expand Down Expand Up @@ -1667,30 +1667,14 @@ built-ins/RegExp 1150/1854 (62.03%)
prototype/Symbol.matchAll/this-tostring-flags-throws.js
prototype/Symbol.match/builtin-infer-unicode.js
prototype/Symbol.match/builtin-success-g-set-lastindex.js
prototype/Symbol.match/builtin-success-g-set-lastindex-err.js
prototype/Symbol.match/builtin-success-u-return-val-groups.js
prototype/Symbol.match/coerce-global.js
prototype/Symbol.match/exec-err.js
prototype/Symbol.match/exec-invocation.js
prototype/Symbol.match/exec-return-type-invalid.js
prototype/Symbol.match/exec-return-type-valid.js
prototype/Symbol.match/flags-tostring-error.js
prototype/Symbol.match/g-coerce-result-err.js
prototype/Symbol.match/g-get-exec-err.js
prototype/Symbol.match/g-get-result-err.js
prototype/Symbol.match/g-init-lastindex.js
prototype/Symbol.match/g-match-empty-advance-lastindex.js
prototype/Symbol.match/g-match-empty-coerce-lastindex-err.js
prototype/Symbol.match/g-match-empty-set-lastindex-err.js
prototype/Symbol.match/g-success-return-val.js
prototype/Symbol.match/get-exec-err.js
prototype/Symbol.match/get-flags-err.js
prototype/Symbol.match/get-global-err.js
prototype/Symbol.match/get-unicode-error.js
prototype/Symbol.match/not-a-constructor.js
prototype/Symbol.match/this-val-non-regexp.js
prototype/Symbol.match/u-advance-after-empty.js
prototype/Symbol.match/y-fail-global-return.js
prototype/Symbol.replace/arg-1-coerce.js
prototype/Symbol.replace/arg-1-coerce-err.js
prototype/Symbol.replace/arg-2-coerce.js
Expand Down Expand Up @@ -2024,7 +2008,7 @@ built-ins/SetIteratorPrototype 0/11 (0.0%)

~built-ins/SharedArrayBuffer

built-ins/String 92/1182 (7.78%)
built-ins/String 90/1182 (7.61%)
fromCharCode/not-a-constructor.js
fromCodePoint/not-a-constructor.js
prototype/charAt/not-a-constructor.js
Expand All @@ -2048,11 +2032,9 @@ built-ins/String 92/1182 (7.78%)
prototype/matchAll/not-a-constructor.js
prototype/matchAll/regexp-matchAll-invocation.js
prototype/matchAll/regexp-prototype-matchAll-invocation.js
prototype/match/cstm-matcher-get-err.js
prototype/match/cstm-matcher-invocation.js
prototype/match/duplicate-named-groups-properties.js
prototype/match/duplicate-named-indices-groups-properties.js
prototype/match/invoke-builtin-match.js
prototype/match/not-a-constructor.js
prototype/normalize/not-a-constructor.js
prototype/padEnd/not-a-constructor.js
Expand Down