From 322c8a3995e3fab7c77b78b60348337e24a2d9e2 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Sat, 12 Jun 2021 23:58:03 +0300 Subject: [PATCH] LibJS: Add the MapIterator built-in and the key/values/entries methods While this implementation should be complete it is based on HashMap's iterator, which currently follows bucket-order instead of the required insertion order. This can be simply fixed by replacing the underlying HashMap member in Map with an enhanced one that maintains a linked list in insertion order. --- Userland/Libraries/LibJS/CMakeLists.txt | 2 + Userland/Libraries/LibJS/Forward.h | 1 + .../Libraries/LibJS/Runtime/GlobalObject.cpp | 1 + .../Libraries/LibJS/Runtime/MapIterator.cpp | 35 ++++++++++ .../Libraries/LibJS/Runtime/MapIterator.h | 39 +++++++++++ .../LibJS/Runtime/MapIteratorPrototype.cpp | 67 +++++++++++++++++++ .../LibJS/Runtime/MapIteratorPrototype.h | 25 +++++++ .../Libraries/LibJS/Runtime/MapPrototype.cpp | 35 ++++++++++ .../Libraries/LibJS/Runtime/MapPrototype.h | 3 + .../builtins/Map/Map.prototype.entries.js | 26 +++++++ .../Tests/builtins/Map/Map.prototype.keys.js | 26 +++++++ .../builtins/Map/Map.prototype.values.js | 26 +++++++ 12 files changed, 286 insertions(+) create mode 100644 Userland/Libraries/LibJS/Runtime/MapIterator.cpp create mode 100644 Userland/Libraries/LibJS/Runtime/MapIterator.h create mode 100644 Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.cpp create mode 100644 Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.h create mode 100644 Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.entries.js create mode 100644 Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.keys.js create mode 100644 Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.values.js diff --git a/Userland/Libraries/LibJS/CMakeLists.txt b/Userland/Libraries/LibJS/CMakeLists.txt index 3b2fdb5845..0e2bb053eb 100644 --- a/Userland/Libraries/LibJS/CMakeLists.txt +++ b/Userland/Libraries/LibJS/CMakeLists.txt @@ -57,6 +57,8 @@ set(SOURCES Runtime/LexicalEnvironment.cpp Runtime/Map.cpp Runtime/MapConstructor.cpp + Runtime/MapIterator.cpp + Runtime/MapIteratorPrototype.cpp Runtime/MapPrototype.cpp Runtime/MarkedValueList.cpp Runtime/MathObject.cpp diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h index e8350fd584..5329370c15 100644 --- a/Userland/Libraries/LibJS/Forward.h +++ b/Userland/Libraries/LibJS/Forward.h @@ -74,6 +74,7 @@ #define JS_ENUMERATE_ITERATOR_PROTOTYPES \ __JS_ENUMERATE(Iterator, iterator) \ __JS_ENUMERATE(ArrayIterator, array_iterator) \ + __JS_ENUMERATE(MapIterator, map_iterator) \ __JS_ENUMERATE(SetIterator, set_iterator) \ __JS_ENUMERATE(StringIterator, string_iterator) diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index 688a077142..08fe603084 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include diff --git a/Userland/Libraries/LibJS/Runtime/MapIterator.cpp b/Userland/Libraries/LibJS/Runtime/MapIterator.cpp new file mode 100644 index 0000000000..e7196183c6 --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/MapIterator.cpp @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2021, Idan Horowitz + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +namespace JS { + +MapIterator* MapIterator::create(GlobalObject& global_object, Map& map, Object::PropertyKind iteration_kind) +{ + return global_object.heap().allocate(global_object, *global_object.map_iterator_prototype(), map, iteration_kind); +} + +MapIterator::MapIterator(Object& prototype, Map& map, Object::PropertyKind iteration_kind) + : Object(prototype) + , m_map(map) + , m_iteration_kind(iteration_kind) + , m_iterator(map.entries().begin()) +{ +} + +MapIterator::~MapIterator() +{ +} + +void MapIterator::visit_edges(Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(&m_map); +} + +} diff --git a/Userland/Libraries/LibJS/Runtime/MapIterator.h b/Userland/Libraries/LibJS/Runtime/MapIterator.h new file mode 100644 index 0000000000..39674b37d1 --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/MapIterator.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2021, Idan Horowitz + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace JS { + +class MapIterator final : public Object { + JS_OBJECT(MapIterator, Object); + +public: + static MapIterator* create(GlobalObject&, Map& map, Object::PropertyKind iteration_kind); + + explicit MapIterator(Object& prototype, Map& map, Object::PropertyKind iteration_kind); + virtual ~MapIterator() override; + + Map& map() const { return m_map; } + bool done() const { return m_done; } + Object::PropertyKind iteration_kind() const { return m_iteration_kind; } + +private: + friend class MapIteratorPrototype; + + virtual void visit_edges(Cell::Visitor&) override; + + Map& m_map; + bool m_done { false }; + Object::PropertyKind m_iteration_kind; + HashMap::IteratorType m_iterator; +}; + +} diff --git a/Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.cpp b/Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.cpp new file mode 100644 index 0000000000..274c0f3751 --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.cpp @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2020, Matthew Olsson + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include + +namespace JS { + +MapIteratorPrototype::MapIteratorPrototype(GlobalObject& global_object) + : Object(*global_object.iterator_prototype()) +{ +} + +void MapIteratorPrototype::initialize(GlobalObject& global_object) +{ + auto& vm = this->vm(); + Object::initialize(global_object); + + define_native_function(vm.names.next, next, 0, Attribute::Configurable | Attribute::Writable); + define_property(vm.well_known_symbol_to_string_tag(), js_string(global_object.heap(), "Map Iterator"), Attribute::Configurable); +} + +MapIteratorPrototype::~MapIteratorPrototype() +{ +} + +JS_DEFINE_NATIVE_FUNCTION(MapIteratorPrototype::next) +{ + auto this_value = vm.this_value(global_object); + if (!this_value.is_object() || !is(this_value.as_object())) { + vm.throw_exception(global_object, ErrorType::NotA, "Map Iterator"); + return {}; + } + + auto& map_iterator = static_cast(this_value.as_object()); + if (map_iterator.done()) + return create_iterator_result_object(global_object, js_undefined(), true); + + auto& map = map_iterator.map(); + if (map_iterator.m_iterator == map.entries().end()) { + map_iterator.m_done = true; + return create_iterator_result_object(global_object, js_undefined(), true); + } + + auto iteration_kind = map_iterator.iteration_kind(); + + auto entry = *map_iterator.m_iterator; + ++map_iterator.m_iterator; + if (iteration_kind == Object::PropertyKind::Key) + return create_iterator_result_object(global_object, entry.key, false); + else if (iteration_kind == Object::PropertyKind::Value) + return create_iterator_result_object(global_object, entry.value, false); + + auto* entry_array = Array::create(global_object); + entry_array->define_property(0, entry.key); + entry_array->define_property(1, entry.value); + return create_iterator_result_object(global_object, entry_array, false); +} + +} diff --git a/Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.h b/Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.h new file mode 100644 index 0000000000..f35ffb374b --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.h @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2020, Matthew Olsson + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace JS { + +class MapIteratorPrototype final : public Object { + JS_OBJECT(MapIteratorPrototype, Object) + +public: + MapIteratorPrototype(GlobalObject&); + virtual void initialize(GlobalObject&) override; + virtual ~MapIteratorPrototype() override; + +private: + JS_DECLARE_NATIVE_FUNCTION(next); +}; + +} diff --git a/Userland/Libraries/LibJS/Runtime/MapPrototype.cpp b/Userland/Libraries/LibJS/Runtime/MapPrototype.cpp index b5fb37757f..e50bbe5dfc 100644 --- a/Userland/Libraries/LibJS/Runtime/MapPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/MapPrototype.cpp @@ -5,6 +5,7 @@ */ #include +#include #include namespace JS { @@ -22,13 +23,17 @@ void MapPrototype::initialize(GlobalObject& global_object) define_native_function(vm.names.clear, clear, 0, attr); define_native_function(vm.names.delete_, delete_, 1, attr); + define_native_function(vm.names.entries, entries, 0, attr); define_native_function(vm.names.forEach, for_each, 1, attr); define_native_function(vm.names.get, get, 1, attr); define_native_function(vm.names.has, has, 1, attr); + define_native_function(vm.names.keys, keys, 0, attr); define_native_function(vm.names.set, set, 2, attr); + define_native_function(vm.names.values, values, 0, attr); define_native_accessor(vm.names.size, size_getter, {}, Attribute::Configurable); + define_property(vm.well_known_symbol_iterator(), Object::get(vm.names.entries), attr); define_property(vm.well_known_symbol_to_string_tag(), js_string(global_object.heap(), vm.names.Map), Attribute::Configurable); } @@ -67,6 +72,16 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::delete_) return Value(map->entries().remove(vm.argument(0))); } +// 24.1.3.4 Map.prototype.entries ( ), https://tc39.es/ecma262/#sec-map.prototype.entries +JS_DEFINE_NATIVE_FUNCTION(MapPrototype::entries) +{ + auto* map = typed_this(vm, global_object); + if (!map) + return {}; + + return MapIterator::create(global_object, *map, Object::PropertyKind::KeyAndValue); +} + // 24.1.3.5 Map.prototype.forEach ( callbackfn [ , thisArg ] ), https://tc39.es/ecma262/#sec-map.prototype.foreach JS_DEFINE_NATIVE_FUNCTION(MapPrototype::for_each) { @@ -108,6 +123,16 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::has) return Value(entries.find(vm.argument(0)) != entries.end()); } +// 24.1.3.8 Map.prototype.keys ( ), https://tc39.es/ecma262/#sec-map.prototype.keys +JS_DEFINE_NATIVE_FUNCTION(MapPrototype::keys) +{ + auto* map = typed_this(vm, global_object); + if (!map) + return {}; + + return MapIterator::create(global_object, *map, Object::PropertyKind::Key); +} + // 24.1.3.9 Map.prototype.set ( key, value ), https://tc39.es/ecma262/#sec-map.prototype.set JS_DEFINE_NATIVE_FUNCTION(MapPrototype::set) { @@ -121,6 +146,16 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::set) return map; } +// 24.1.3.11 Map.prototype.values ( ), https://tc39.es/ecma262/#sec-map.prototype.values +JS_DEFINE_NATIVE_FUNCTION(MapPrototype::values) +{ + auto* map = typed_this(vm, global_object); + if (!map) + return {}; + + return MapIterator::create(global_object, *map, Object::PropertyKind::Value); +} + // 24.1.3.10 get Map.prototype.size, https://tc39.es/ecma262/#sec-get-map.prototype.size JS_DEFINE_NATIVE_GETTER(MapPrototype::size_getter) { diff --git a/Userland/Libraries/LibJS/Runtime/MapPrototype.h b/Userland/Libraries/LibJS/Runtime/MapPrototype.h index ae7b95ba96..2a6d7dda61 100644 --- a/Userland/Libraries/LibJS/Runtime/MapPrototype.h +++ b/Userland/Libraries/LibJS/Runtime/MapPrototype.h @@ -23,10 +23,13 @@ private: JS_DECLARE_NATIVE_FUNCTION(clear); JS_DECLARE_NATIVE_FUNCTION(delete_); + JS_DECLARE_NATIVE_FUNCTION(entries); JS_DECLARE_NATIVE_FUNCTION(for_each); JS_DECLARE_NATIVE_FUNCTION(get); JS_DECLARE_NATIVE_FUNCTION(has); + JS_DECLARE_NATIVE_FUNCTION(keys); JS_DECLARE_NATIVE_FUNCTION(set); + JS_DECLARE_NATIVE_FUNCTION(values); JS_DECLARE_NATIVE_GETTER(size_getter); }; diff --git a/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.entries.js b/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.entries.js new file mode 100644 index 0000000000..b811fc1b9b --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.entries.js @@ -0,0 +1,26 @@ +test("length", () => { + expect(Map.prototype.entries.length).toBe(0); +}); + +test("basic functionality", () => { + const original = [ + ["a", 0], + ["b", 1], + ["c", 2], + ]; + const a = new Map(original); + const it = a.entries(); + // FIXME: This test should be rewritten once we have proper iteration order + const first = it.next(); + expect(first.done).toBeFalse(); + expect(a.has(first.value[0])).toBeTrue(); + const second = it.next(); + expect(second.done).toBeFalse(); + expect(a.has(second.value[0])).toBeTrue(); + const third = it.next(); + expect(third.done).toBeFalse(); + expect(a.has(third.value[0])).toBeTrue(); + expect(it.next()).toEqual({ value: undefined, done: true }); + expect(it.next()).toEqual({ value: undefined, done: true }); + expect(it.next()).toEqual({ value: undefined, done: true }); +}); diff --git a/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.keys.js b/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.keys.js new file mode 100644 index 0000000000..2734be1308 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.keys.js @@ -0,0 +1,26 @@ +test("length", () => { + expect(Map.prototype.keys.length).toBe(0); +}); + +test("basic functionality", () => { + const original = [ + ["a", 0], + ["b", 1], + ["c", 2], + ]; + const a = new Map(original); + const it = a.keys(); + // FIXME: This test should be rewritten once we have proper iteration order + const first = it.next(); + expect(first.done).toBeFalse(); + expect(a.has(first.value)).toBeTrue(); + const second = it.next(); + expect(second.done).toBeFalse(); + expect(a.has(second.value)).toBeTrue(); + const third = it.next(); + expect(third.done).toBeFalse(); + expect(a.has(third.value)).toBeTrue(); + expect(it.next()).toEqual({ value: undefined, done: true }); + expect(it.next()).toEqual({ value: undefined, done: true }); + expect(it.next()).toEqual({ value: undefined, done: true }); +}); diff --git a/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.values.js b/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.values.js new file mode 100644 index 0000000000..a6aec10da2 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/builtins/Map/Map.prototype.values.js @@ -0,0 +1,26 @@ +test("length", () => { + expect(Map.prototype.values.length).toBe(0); +}); + +test("basic functionality", () => { + const original = [ + ["a", 0], + ["b", 1], + ["c", 2], + ]; + const a = new Map(original); + const it = a.values(); + // FIXME: This test should be rewritten once we have proper iteration order + const first = it.next(); + expect(first.done).toBeFalse(); + expect([0, 1, 2].includes(first.value)).toBeTrue(); + const second = it.next(); + expect(second.done).toBeFalse(); + expect([0, 1, 2].includes(second.value)).toBeTrue(); + const third = it.next(); + expect(third.done).toBeFalse(); + expect([0, 1, 2].includes(third.value)).toBeTrue(); + expect(it.next()).toEqual({ value: undefined, done: true }); + expect(it.next()).toEqual({ value: undefined, done: true }); + expect(it.next()).toEqual({ value: undefined, done: true }); +});