Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions lib/Runtime/Library/JavascriptBuiltInFunctionList.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ BUILTIN(JavascriptObject, LookupGetter, EntryLookupGetter, FunctionInfo::ErrorOn
BUILTIN(JavascriptObject, LookupSetter, EntryLookupSetter, FunctionInfo::ErrorOnNew)
BUILTIN(JavascriptObject, Is, EntryIs, FunctionInfo::ErrorOnNew)
BUILTIN(JavascriptObject, Assign, EntryAssign, FunctionInfo::ErrorOnNew)
BUILTIN(JavascriptObject, Values, EntryValues, FunctionInfo::ErrorOnNew)
BUILTIN(JavascriptObject, Entries, EntryEntries, FunctionInfo::ErrorOnNew)
BUILTIN(ObjectPrototypeObject, __proto__getter, Entry__proto__getter, FunctionInfo::ErrorOnNew | FunctionInfo::DoNotProfile)
BUILTIN(ObjectPrototypeObject, __proto__setter, Entry__proto__setter, FunctionInfo::ErrorOnNew | FunctionInfo::DoNotProfile)
BUILTIN(JavascriptRegExp, NewInstance, NewInstance, FunctionInfo::SkipDefaultNewObject)
Expand Down
14 changes: 14 additions & 0 deletions lib/Runtime/Library/JavascriptLibrary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3536,6 +3536,14 @@ namespace Js
library->AddFunctionToLibraryObject(objectConstructor, PropertyIds::assign, &JavascriptObject::EntryInfo::Assign, 2));
}

if (scriptContext->GetConfig()->IsES7BuiltinsEnabled())
{
scriptContext->SetBuiltInLibraryFunction(JavascriptObject::EntryInfo::Values.GetOriginalEntryPoint(),
library->AddFunctionToLibraryObject(objectConstructor, PropertyIds::values, &JavascriptObject::EntryInfo::Values, 1));
scriptContext->SetBuiltInLibraryFunction(JavascriptObject::EntryInfo::Entries.GetOriginalEntryPoint(),
library->AddFunctionToLibraryObject(objectConstructor, PropertyIds::entries, &JavascriptObject::EntryInfo::Entries, 1));
}

objectConstructor->SetHasNoEnumerableProperties(true);
}

Expand Down Expand Up @@ -6042,6 +6050,12 @@ namespace Js
REG_OBJECTS_LIB_FUNC(assign, JavascriptObject::EntryAssign);
}

if (config.IsES7BuiltinsEnabled())
{
REG_OBJECTS_LIB_FUNC(values, JavascriptObject::EntryValues);
REG_OBJECTS_LIB_FUNC(entries, JavascriptObject::EntryEntries);
}

return hr;
}

Expand Down
79 changes: 79 additions & 0 deletions lib/Runtime/Library/JavascriptObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,85 @@ namespace Js
return JavascriptOperators::GetOwnEnumerablePropertyNames(object, scriptContext);
}

Var JavascriptObject::GetValuesOrEntries(RecyclableObject* object, bool valuesToReturn, ScriptContext* scriptContext)
{
Assert(object != nullptr);
Assert(scriptContext != nullptr);
JavascriptArray* valuesArray = scriptContext->GetLibrary()->CreateArray(0);

Var ownKeysVar = JavascriptOperators::GetOwnEnumerablePropertyNames(object, scriptContext);
JavascriptArray* ownKeysResult = nullptr;
if (JavascriptArray::Is(ownKeysVar))
{
ownKeysResult = JavascriptArray::FromVar(ownKeysVar);
}
else
{
return valuesArray;
}

uint32 length = ownKeysResult->GetLength();

Var nextKey;
const PropertyRecord* propertyRecord = nullptr;
PropertyId propertyId;
for (uint32 i = 0, index = 0; i < length; i++)
{
nextKey = ownKeysResult->DirectGetItem(i);
Assert(JavascriptString::Is(nextKey));

BOOL propertyKeyResult = JavascriptConversion::ToPropertyKey(nextKey, scriptContext, &propertyRecord);
Assert(propertyKeyResult);
propertyId = propertyRecord->GetPropertyId();
Assert(propertyId != Constants::NoProperty);

Var value = JavascriptOperators::GetProperty(object, propertyId, scriptContext);
if (!valuesToReturn)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i don't see anywhere you're checking enumerability in the loop (in either function) - if, during iteration, an unvisited property is deleted or made non-enumerable, it should be skipped. See tc39/test262#453 for the test262 tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Having properties deleted and their attributes changed during enumeration would be an excellent unit test to add.

Copy link
Collaborator

Choose a reason for hiding this comment

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

{
// For Object.entries each entry is key, value pair
JavascriptArray* entry = scriptContext->GetLibrary()->CreateArray(2);
entry->DirectSetItemAt(0, CrossSite::MarshalVar(scriptContext, nextKey));
entry->DirectSetItemAt(1, CrossSite::MarshalVar(scriptContext, value));
value = entry;
}
valuesArray->DirectSetItemAt(index++, CrossSite::MarshalVar(scriptContext, value));
}

return valuesArray;
}

Var JavascriptObject::EntryValues(RecyclableObject* function, CallInfo callInfo, ...)
{
PROBE_STACK(function->GetScriptContext(), Js::Constants::MinStackDefault);

ARGUMENTS(args, callInfo);
ScriptContext* scriptContext = function->GetScriptContext();

Assert(!(callInfo.Flags & CallFlags_New));
CHAKRATEL_LANGSTATS_INC_BUILTINCOUNT(ObjectValuesCount);

Var tempVar = args.Info.Count < 2 ? scriptContext->GetLibrary()->GetUndefined() : args[1];
RecyclableObject *object = RecyclableObject::FromVar(JavascriptOperators::ToObject(tempVar, scriptContext));

return GetValuesOrEntries(object, true /*valuesToReturn*/, scriptContext);
}

Var JavascriptObject::EntryEntries(RecyclableObject* function, CallInfo callInfo, ...)
{
PROBE_STACK(function->GetScriptContext(), Js::Constants::MinStackDefault);

ARGUMENTS(args, callInfo);
ScriptContext* scriptContext = function->GetScriptContext();

Assert(!(callInfo.Flags & CallFlags_New));
CHAKRATEL_LANGSTATS_INC_BUILTINCOUNT(ObjectEntriesCount);

Var tempVar = args.Info.Count < 2 ? scriptContext->GetLibrary()->GetUndefined() : args[1];
RecyclableObject *object = RecyclableObject::FromVar(JavascriptOperators::ToObject(tempVar, scriptContext));

return GetValuesOrEntries(object, false /*valuesToReturn*/, scriptContext);
}

Var JavascriptObject::CreateOwnSymbolPropertiesHelper(RecyclableObject* object, ScriptContext* scriptContext)
{
return CreateKeysHelper(object, scriptContext, TRUE, true /*includeSymbolsOnly */, false, true /*includeSpecialProperties*/);
Expand Down
8 changes: 7 additions & 1 deletion lib/Runtime/Library/JavascriptObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ namespace Js
static FunctionInfo LookupSetter;
static FunctionInfo Is;
static FunctionInfo Assign;
static FunctionInfo Values;
static FunctionInfo Entries;
};

static Var NewInstance(RecyclableObject* function, CallInfo callInfo, ...);
Expand Down Expand Up @@ -77,7 +79,8 @@ namespace Js
static Var EntryLookupSetter(RecyclableObject* function, CallInfo callInfo, ...);
static Var EntryIs(RecyclableObject* function, CallInfo callInfo, ...);
static Var EntryAssign(RecyclableObject* function, CallInfo callInfo, ...);

static Var EntryValues(RecyclableObject* function, CallInfo callInfo, ...);
static Var EntryEntries(RecyclableObject* function, CallInfo callInfo, ...);

static Var GetPrototypeOf(RecyclableObject* obj, ScriptContext* scriptContext);
static BOOL ChangePrototype(RecyclableObject* object, RecyclableObject* newPrototype, bool validate, ScriptContext* scriptContext);
Expand All @@ -91,6 +94,9 @@ namespace Js
static Var GetOwnPropertyDescriptorHelper(RecyclableObject* obj, Var propertyKey, ScriptContext* scriptContext);
static BOOL GetOwnPropertyDescriptorHelper(RecyclableObject* obj, PropertyId propertyId, ScriptContext* scriptContext, PropertyDescriptor& propertyDescriptor);

// Param valuesToReturn should be set to true when we are looking for values from an object otherwise entries will be returned
static Var GetValuesOrEntries(RecyclableObject* object, bool valuesToReturn, ScriptContext* scriptContext);

// Presently used in the projection as a mechanism of calling the general object prototype toString.
static JavascriptString* ToStringInternal(Var thisArg, ScriptContext* scriptContext)
{
Expand Down
7 changes: 7 additions & 0 deletions test/es6/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,13 @@
<tags>BugFix</tags>
</default>
</test>
<test>
<default>
<files>valuesAndEntries.js</files>
<compile-flags>-ES7Builtins -args summary -endargs</compile-flags>
<tags>BugFix</tags>
</default>
</test>
<test>
<default>
<files>OS_5403724.js</files>
Expand Down
140 changes: 140 additions & 0 deletions test/es6/valuesAndEntries.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");

var tests = [
{
name: "Object.values/entries should exist and constructed properly",
body: function () {
assert.isTrue(Object.hasOwnProperty('values'), "Object should have a values method");
assert.isTrue(Object.hasOwnProperty('entries'), "Object should have a entries method");
assert.areEqual(1, Object.values.length, "values method takes one argument");
assert.areEqual(1, Object.entries.length, "entries method takes one argument");
assert.areEqual("values", Object.values.name, "values.name is 'values'");
assert.areEqual("entries", Object.entries.name, "entries.name is 'entries'");

var descriptor = Object.getOwnPropertyDescriptor(Object, 'values');
assert.isTrue(descriptor.writable, "writable(values) must be true");
assert.isFalse(descriptor.enumerable, "enumerable(values) must be false");
assert.isTrue(descriptor.configurable, "configurable(values) must be true");

descriptor = Object.getOwnPropertyDescriptor(Object, 'entries');
assert.isTrue(descriptor.writable, "writable(entries) must be true");
assert.isFalse(descriptor.enumerable, "enumerable(entries) must be false");
assert.isTrue(descriptor.configurable, "configurable(entries) must be true");
}
},
{
name: "Object.values/entries basic syntax",
body: function () {
assert.throws(function () { eval("Object.values();"); }, TypeError, "Calling values method without any arguments throws an error", "Object expected");
assert.throws(function () { eval("Object.values(undefined);"); }, TypeError, "Calling values method on undefined throws an error", "Object expected");
assert.throws(function () { eval("Object.values(null);"); }, TypeError, "Calling values method on null throws an error", "Object expected");
assert.isTrue(Array.isArray(Object.values({})), "calling values method on an object returns an array");
assert.areEqual(0, Object.values({}).length, "calling values method on an empty object returns an empty array");

assert.throws(function () { eval("Object.entries();"); }, TypeError, "Calling entries method without any arguments throws an error", "Object expected");
assert.throws(function () { eval("Object.entries(undefined);"); }, TypeError, "Calling entries method on undefined throws an error", "Object expected");
assert.throws(function () { eval("Object.entries(null);"); }, TypeError, "Calling entries method on null throws an error", "Object expected");
assert.isTrue(Array.isArray(Object.entries({})), "calling entries method on an object returns an array");
assert.areEqual(0, Object.entries({}).length, "calling entries method on an empty object returns an empty array");
}
},
{
name: "Object.values/entries functionality",
body: function () {
var a1 = {prop1:10, prop2:20};
var values = Object.values(a1);
assert.areEqual(2, values.length, "calling values on an object with two properties will returned an array of 2 elements");
assert.areEqual(10, values[0], "First element of the returned values array should be 10");
assert.areEqual(20, values[1], "Second element of the returned values array should be 20");

var entries = Object.entries(a1);
assert.areEqual(2, entries.length, "calling entries on an object with two properties will returned an array of 2 elements");
assert.isTrue(Array.isArray(entries[0]) && Array.isArray(entries[1]), "each element itself an array of key, value pair");
assert.areEqual(["prop1", 10], entries[0], "First element of the returned entry array should be ['prop1', 10]");
assert.areEqual(["prop2", 20], entries[1], "Second element of the returned entry array should be ['prop2', 20]");

var a2 = {prop3 : 30};
a2[2] = 40;
a2["prop4"] = 50;
Object.defineProperty(a2, "prop5", { value: 60, enumerable: true});
Object.defineProperty(a2, "prop6", { value: 70, enumerable: false});
Object.defineProperty(a2, 'prop7', { enumerable: true, get: function () { return 80;}});
var sym = Symbol('prop8');
a2[sym] = 90;

values = Object.values(a2);
assert.areEqual(5, values.length, "values method returns an array of 5 elements, symbol and non-enumerable should be excluded");
assert.areEqual([40,30,50,60,80], values, "values method returns an array and matches correctly");

entries = Object.entries(a2);
assert.areEqual(5, entries.length, "entries method returns an array of 5 elements, symbol and non-enumerable should be excluded");
assert.areEqual("2,40,prop3,30,prop4,50,prop5,60,prop7,80", entries.toString(), "entries method returns an array and matches correctly");
}
},
{
name: "Object.values/entries with proxy",
body: function () {
var obj1 = {prop1:10};
var proxy1 = new Proxy(obj1, { });
var values = Object.values(proxy1);
assert.areEqual(1, values.length, "values - Proxy object on an object with one property returns an array of 1 element");
assert.areEqual(10, values[0], "values - Proxy object on an object with one property returns correct element");

var entries = Object.entries(proxy1);
assert.areEqual(1, entries.length, "entries - Proxy object on an object with one property returns an array of 1 element");
assert.areEqual(["prop1", 10], entries[0], "entries - Proxy object on an object with one property returns correct element");

var obj2 = {};
Object.defineProperty(obj2, "prop2", { value: 20, enumerable: true });
Object.defineProperty(obj2, "prop3", { get: function () { return 30; }, enumerable: true });
var proxy2 = new Proxy(obj2, {
getOwnPropertyDescriptor: function (target, propert) {
return Reflect.getOwnPropertyDescriptor(target, propert);
}
});

values = Object.values(proxy2);
assert.areEqual(2, values.length, "values - exhibiting a Proxy trapping getOwnPropertyDescriptor returns an aray to 2 elements");
assert.areEqual(20, values[0], "values - a Proxy trapping getOwnPropertyDescriptor matching the first element");
assert.areEqual(30, values[1], "values - a Proxy trapping getOwnPropertyDescriptor matching the second element");

entries = Object.entries(proxy2);
assert.areEqual(2, entries.length, "values - exhibiting a Proxy trapping getOwnPropertyDescriptor returns an aray to 2 elements");
assert.areEqual(["prop2", 20], entries[0], "entries - a Proxy trapping getOwnPropertyDescriptor matching the first element");
assert.areEqual(["prop3", 30], entries[1], "entries - a Proxy trapping getOwnPropertyDescriptor matching the second element");

var obj3 = {};
var count = 0;
var proxy3 = new Proxy(obj3, {
get: function (target, property, receiver) {
return count++ * 5;
},
getOwnPropertyDescriptor: function (target, property) {
return {configurable: true, enumerable: true};
},

ownKeys: function (target) {
return ["prop0", "prop1", Symbol("prop2"), Symbol("prop5")];
}
});

values = Object.values(proxy3);
assert.areEqual(2, values.length, "values - exhibiting a Proxy with get and ownKeys traps - returns 2 elements");
assert.areEqual(0, values[0], "values - exhibiting a Proxy with get and ownKeys traps - matching first element");
assert.areEqual(5, values[1], "values - exhibiting a Proxy with get and ownKeys traps - matching second element");

entries = Object.entries(proxy3);
assert.areEqual(2, entries.length, "entries - exhibiting a Proxy with get and ownKeys trap - returns 2 elements");
assert.areEqual(["prop0", 10], entries[0], "entries - exhibiting a Proxy with get and ownKeys trap - matching first element");
assert.areEqual(["prop1", 15], entries[1], "entries - exhibiting a Proxy with get and ownKeys trap - matching second element");

}
}
];

testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });