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.

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

add Object.hasOwn (#1052) #1157

Merged
merged 5 commits into from Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
20 changes: 20 additions & 0 deletions src/org/mozilla/javascript/NativeObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ protected void fillConstructorProperties(IdFunctionObject ctor) {
addIdFunctionProperty(ctor, OBJECT_TAG, ConstructorId_entries, "entries", 1);
addIdFunctionProperty(ctor, OBJECT_TAG, ConstructorId_fromEntries, "fromEntries", 1);
addIdFunctionProperty(ctor, OBJECT_TAG, ConstructorId_values, "values", 1);
addIdFunctionProperty(ctor, OBJECT_TAG, ConstructorId_hasOwn, "hasOwn", 1);
}
addIdFunctionProperty(ctor, OBJECT_TAG, ConstructorId_keys, "keys", 1);
addIdFunctionProperty(
Expand Down Expand Up @@ -472,6 +473,24 @@ public Object execIdCall(
}
return cx.newArray(scope, ids);
}
case ConstructorId_hasOwn:
{
Object arg = args.length < 1 ? Undefined.instance : args[0];
ScriptableObject obj = ensureScriptableObject(arg);
Object str = args.length < 2 ? Undefined.instance : args[1];
boolean result;
Copy link
Contributor

Choose a reason for hiding this comment

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

I read the spec.
https://tc39.es/proposal-accessible-object-hasownproperty/

Object.hasOwn calls the HasOwnProperty operation in the same way as Object.prototype.hasOwnProperty. Could you make this common? I think it is better to write it in ScriptRuntime or ScriptableObject.

boolean result;
Object arg = args.length < 1 ? Undefined.instance : args[0];
if (arg instanceof Symbol) {
result = ensureSymbolScriptable(thisObj).has((Symbol) arg, thisObj);
} else {
StringIdOrIndex s = ScriptRuntime.toStringIdOrIndex(cx, arg);
if (s.stringId == null) {
result = thisObj.has(s.index, thisObj);
} else {
result = thisObj.has(s.stringId, thisObj);
}
}
return ScriptRuntime.wrapBoolean(result);

Copy link
Collaborator

@p-bakker p-bakker Jan 18, 2022

Choose a reason for hiding this comment

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

Good point!

The correct location to put it however is org.mozilla.javascript.AbstractEcmaObjectOperations, as the spec defines the HasOwnProperty operation as one of the Abstract Operations on Objects (section 7.3.13).

Do check the notes at the top of the file as to how to add stuff: https://github.com/mozilla/rhino/blob/master/src/org/mozilla/javascript/AbstractEcmaObjectOperations.java#L8

Also possibly see where else the new function ought to be used and see if you can refactor that existing code to use the new function as well

Copy link
Author

Choose a reason for hiding this comment

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

I changed 'hasOwnProperty' and 'hasOwn' to call the function 'hasOwnPropertyInObject' with the ec9b150 commit. I'll have to find more codes to handle the same process here.

if (str instanceof Symbol) {
result = obj.has((Symbol) str, obj);
} else {
StringIdOrIndex s = ScriptRuntime.toStringIdOrIndex(cx, str);
if (s.stringId == null) {
result = obj.has(s.index, obj);
} else {
result = obj.has(s.stringId, obj);
}
}
return result;
}
case ConstructorId_getOwnPropertyNames:
{
Object arg = args.length < 1 ? Undefined.instance : args[0];
Expand Down Expand Up @@ -998,6 +1017,7 @@ protected int findPrototypeId(String s) {
ConstructorId_entries = -18,
ConstructorId_fromEntries = -19,
ConstructorId_values = -20,
ConstructorId_hasOwn = -21,
Id_constructor = 1,
Id_toString = 2,
Id_toLocaleString = 3,
Expand Down
112 changes: 112 additions & 0 deletions testsrc/org/mozilla/javascript/tests/es2022/NativeObjectTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package org.mozilla.javascript.tests.es2022;

import static org.junit.Assert.assertEquals;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.ScriptableObject;

public class NativeObjectTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Is there a test for not following the prototype chain?
    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwn
Object.hasOwn(example, 'toString');         // returns false
  1. How is Symbol handled in the spec?


private Context cx;
private ScriptableObject scope;

@Before
public void setUp() {
cx = Context.enter();
cx.setLanguageVersion(Context.VERSION_ES6);
scope = cx.initStandardObjects();
}

@After
public void tearDown() { Context.exit(); }

@Test
public void testHasStringOwn() {
Object result = cx.evaluateString(
scope,
"let result = Object.hasOwn({ test: '123' }, 'test');\n" +
"'result = ' + result;",
"test",
1,
null
);

assertEquals("result = true", result);
}

@Test
public void testHasUndefinedOwn() {
Object result = cx.evaluateString(
scope,
"let result = Object.hasOwn({ test: undefined }, 'test');\n" +
"'result = ' + result;",
"test",
1,
null
);

assertEquals("result = true", result);
}

@Test
public void testHasNullOwn() {
Object result = cx.evaluateString(
scope,
"let result = Object.hasOwn({ test: null }, 'test');\n" +
"'result = ' + result;",
"test",
1,
null
);

assertEquals("result = true", result);
}

@Test
public void testHasArrayPropertyOwn() {
Object result = cx.evaluateString(
scope,
"let dessert = [\"cake\", \"coffee\", \"chocolate\"];\n" +
"let result = Object.hasOwn(dessert, 2);\n" +
"'result = ' + result;",
"test",
1,
null
);

assertEquals("result = true", result);
}

@Test
public void testHasNoOwn() {
Object result = cx.evaluateString(
scope,
"let result = Object.hasOwn({ cake: 123 }, 'test');\n" +
"'result = ' + result;",
"test",
1,
null
);

assertEquals("result = false", result);
}

@Test
public void testCreateHasOwn() {
Object result = cx.evaluateString(
scope,
"var foo = Object.create(null);\n" +
"foo.prop = 'test';\n" +
"var result = Object.hasOwn(foo, 'prop')\n" +
"'result = ' + result;",
"test",
1,
null
);

assertEquals("result = true", result);
}
}