diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index ba9e56be2f..57a0e51b34 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -235,29 +235,44 @@ Value Object::get_own_properties(PropertyKind kind, bool only_enumerable_propert ++property_index; } - for (auto& it : shape().property_table_ordered()) { - if (only_enumerable_properties && !it.value.attributes.is_enumerable()) - continue; - - if (return_type == GetOwnPropertyReturnType::StringOnly && it.key.is_symbol()) - continue; - if (return_type == GetOwnPropertyReturnType::SymbolOnly && it.key.is_string()) - continue; - + auto add_property_to_results = [&](auto& property) { if (kind == PropertyKind::Key) { - properties_array->define_property(property_index, it.key.to_value(vm())); + properties_array->define_property(property_index, property.key.to_value(vm())); } else if (kind == PropertyKind::Value) { - properties_array->define_property(property_index, get(it.key)); + properties_array->define_property(property_index, get(property.key)); } else { auto* entry_array = Array::create(global_object()); - entry_array->define_property(0, it.key.to_value(vm())); - entry_array->define_property(1, get(it.key)); + entry_array->define_property(0, property.key.to_value(vm())); + entry_array->define_property(1, get(property.key)); properties_array->define_property(property_index, entry_array); } - if (vm().exception()) - return {}; + }; - ++property_index; + // NOTE: Most things including for..in/of and Object.{keys,values,entries}() use StringOnly, and in those + // cases we won't be iterating the ordered property table twice. We can certainly improve this though. + if (return_type == GetOwnPropertyReturnType::All || return_type == GetOwnPropertyReturnType::StringOnly) { + for (auto& it : shape().property_table_ordered()) { + if (only_enumerable_properties && !it.value.attributes.is_enumerable()) + continue; + if (it.key.is_symbol()) + continue; + add_property_to_results(it); + if (vm().exception()) + return {}; + ++property_index; + } + } + if (return_type == GetOwnPropertyReturnType::All || return_type == GetOwnPropertyReturnType::SymbolOnly) { + for (auto& it : shape().property_table_ordered()) { + if (only_enumerable_properties && !it.value.attributes.is_enumerable()) + continue; + if (it.key.is_string()) + continue; + add_property_to_results(it); + if (vm().exception()) + return {}; + ++property_index; + } } return properties_array; diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index b5b6ba57ad..9ee5ac4c1e 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -74,6 +74,7 @@ public: }; enum class GetOwnPropertyReturnType { + All, StringOnly, SymbolOnly, }; @@ -96,7 +97,7 @@ public: virtual bool put(const PropertyName&, Value, Value receiver = {}); Value get_own_property(const PropertyName&, Value receiver) const; - Value get_own_properties(PropertyKind, bool only_enumerable_properties = false, GetOwnPropertyReturnType = GetOwnPropertyReturnType::StringOnly) const; + Value get_own_properties(PropertyKind, bool only_enumerable_properties = false, GetOwnPropertyReturnType = GetOwnPropertyReturnType::All) const; Value get_enumerable_own_property_names(PropertyKind) const; virtual Optional get_own_property_descriptor(const PropertyName&) const; Value get_own_property_descriptor_object(const PropertyName&) const; diff --git a/Userland/Libraries/LibJS/Tests/builtins/Reflect/Reflect.ownKeys.js b/Userland/Libraries/LibJS/Tests/builtins/Reflect/Reflect.ownKeys.js index b262775f1a..53ca14796f 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Reflect/Reflect.ownKeys.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Reflect/Reflect.ownKeys.js @@ -23,12 +23,14 @@ describe("normal behavior", () => { }); test("regular object with some properties has own keys", () => { - var objectOwnKeys = Reflect.ownKeys({ foo: "bar", bar: "baz", 0: 42 }); + var symbol = Symbol("symbol"); + var objectOwnKeys = Reflect.ownKeys({ foo: "bar", [symbol]: "qux", bar: "baz", 0: 42 }); expect(objectOwnKeys instanceof Array).toBeTrue(); - expect(objectOwnKeys).toHaveLength(3); + expect(objectOwnKeys).toHaveLength(4); expect(objectOwnKeys[0]).toBe("0"); expect(objectOwnKeys[1]).toBe("foo"); expect(objectOwnKeys[2]).toBe("bar"); + expect(objectOwnKeys[3]).toBe(symbol); }); test("empty array has only 'length' own key", () => { @@ -38,7 +40,7 @@ describe("normal behavior", () => { expect(arrayOwnKeys[0]).toBe("length"); }); - test("array with some values has 'lenght' and indices own keys", () => { + test("array with some values has 'length' and indices own keys", () => { var arrayOwnKeys = Reflect.ownKeys(["foo", [], 123, undefined]); expect(arrayOwnKeys instanceof Array).toBeTrue(); expect(arrayOwnKeys).toHaveLength(5);