From 1461bf14bc03c76aba6d8b2fb61f46ca62d2caa3 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sat, 26 Oct 2024 19:44:33 +0300 Subject: [PATCH] Make JsMap public (#1988) * make JsSet and JsMap members new as they indicate a specific usage scenario --- Jint.Tests.PublicInterface/MapTests.cs | 43 +++++++++++++++++++++ Jint.Tests.PublicInterface/SetTests.cs | 2 +- Jint/Native/JsMap.cs | 53 ++++++++++---------------- Jint/Native/JsSet.cs | 2 +- Jint/Native/Map/MapConstructor.cs | 47 ++++++++++++----------- Jint/Native/Map/MapPrototype.cs | 6 +-- Jint/Native/Set/SetPrototype.cs | 8 ++-- Jint/Runtime/Intrinsics.cs | 4 +- 8 files changed, 100 insertions(+), 65 deletions(-) create mode 100644 Jint.Tests.PublicInterface/MapTests.cs diff --git a/Jint.Tests.PublicInterface/MapTests.cs b/Jint.Tests.PublicInterface/MapTests.cs new file mode 100644 index 0000000000..42e96ac954 --- /dev/null +++ b/Jint.Tests.PublicInterface/MapTests.cs @@ -0,0 +1,43 @@ +using FluentAssertions; +using Jint.Native; + +namespace Jint.Tests.Runtime; + +public class MapTests +{ + [Fact] + public void ConConstructMap() + { + var engine = new Engine(); + + var map = engine.Intrinsics.Map.Construct(); + map.Set(42, "the meaning of life"); + map.Set("foo", "bar"); + map.Size.Should().Be(2); + + map.Has(42).Should().BeTrue(); + map.Has("foo").Should().BeTrue(); + map.Has(24).Should().BeFalse(); + + map.Get(42).Should().Be((JsString) "the meaning of life"); + map.Get("foo").Should().Be((JsString) "bar"); + map.Get(24).Should().Be(JsValue.Undefined); + + engine.SetValue("m", map); + engine.Evaluate("m.size").Should().Be((JsNumber) 2); + engine.Evaluate("m.has(42)").Should().Be(JsBoolean.True); + engine.Evaluate("m.has('foo')").Should().Be(JsBoolean.True); + engine.Evaluate("m.has(24)").Should().Be(JsBoolean.False); + + map.Should().Contain((JsNumber) 42, (JsString) "the meaning of life"); + map.Remove(42).Should().BeTrue(); + map.Has(42).Should().BeFalse(); + engine.Evaluate("m.has(42)").Should().Be(JsBoolean.False); + engine.Evaluate("m.size").Should().Be((JsNumber) 1); + + map.Clear(); + map.Should().BeEmpty(); + map.Size.Should().Be(0); + engine.Evaluate("m.size").Should().Be((JsNumber) 0); + } +} diff --git a/Jint.Tests.PublicInterface/SetTests.cs b/Jint.Tests.PublicInterface/SetTests.cs index dada59965c..839bbc0899 100644 --- a/Jint.Tests.PublicInterface/SetTests.cs +++ b/Jint.Tests.PublicInterface/SetTests.cs @@ -27,7 +27,7 @@ public void ConConstructSet() engine.Evaluate("s.has('foo')").Should().Be(JsBoolean.True); engine.Evaluate("s.has(24)").Should().Be(JsBoolean.False); - set.Remove(42).Should().BeTrue(); + set.Delete(42).Should().BeTrue(); set.Has(42).Should().BeFalse(); engine.Evaluate("s.has(42)").Should().Be(JsBoolean.False); engine.Evaluate("s.size").Should().Be((JsNumber) 1); diff --git a/Jint/Native/JsMap.cs b/Jint/Native/JsMap.cs index eb3fd11523..7050ba5b4e 100644 --- a/Jint/Native/JsMap.cs +++ b/Jint/Native/JsMap.cs @@ -1,3 +1,4 @@ +using System.Collections; using System.Diagnostics.CodeAnalysis; using Jint.Native.Object; using Jint.Runtime; @@ -5,7 +6,7 @@ namespace Jint.Native; -internal sealed class JsMap : ObjectInstance +public sealed class JsMap : ObjectInstance, IEnumerable> { private readonly Realm _realm; internal readonly OrderedDictionary _map; @@ -37,22 +38,25 @@ protected override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out return base.TryGetProperty(property, out descriptor); } - internal void Clear() - { - _map.Clear(); - } + public int Size => _map.Count; - internal bool Has(JsValue key) - { - return _map.ContainsKey(key); - } + public void Clear() => _map.Clear(); - internal bool MapDelete(JsValue key) + public bool Has(JsValue key) => _map.ContainsKey(key); + + public bool Remove(JsValue key) => _map.Remove(key); + + public new JsValue Get(JsValue key) { - return _map.Remove(key); + if (!_map.TryGetValue(key, out var value)) + { + return Undefined; + } + + return value; } - internal void MapSet(JsValue key, JsValue value) + public new void Set(JsValue key, JsValue value) { if (key is JsNumber number && number.IsNegativeZero()) { @@ -76,28 +80,13 @@ internal void ForEach(ICallable callable, JsValue thisArg) _engine._jsValueArrayPool.ReturnArray(args); } - internal JsValue MapGet(JsValue key) - { - if (!_map.TryGetValue(key, out var value)) - { - return Undefined; - } + internal ObjectInstance Iterator() => _realm.Intrinsics.MapIteratorPrototype.ConstructEntryIterator(this); - return value; - } + internal ObjectInstance Keys() => _realm.Intrinsics.MapIteratorPrototype.ConstructKeyIterator(this); - internal ObjectInstance Iterator() - { - return _realm.Intrinsics.MapIteratorPrototype.ConstructEntryIterator(this); - } + internal ObjectInstance Values() => _realm.Intrinsics.MapIteratorPrototype.ConstructValueIterator(this); - internal ObjectInstance Keys() - { - return _realm.Intrinsics.MapIteratorPrototype.ConstructKeyIterator(this); - } + public IEnumerator> GetEnumerator() => _map.GetEnumerator(); - internal ObjectInstance Values() - { - return _realm.Intrinsics.MapIteratorPrototype.ConstructValueIterator(this); - } + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/Jint/Native/JsSet.cs b/Jint/Native/JsSet.cs index ba0130fe31..f6335aa56e 100644 --- a/Jint/Native/JsSet.cs +++ b/Jint/Native/JsSet.cs @@ -54,7 +54,7 @@ protected override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out public bool Has(JsValue key) => _set.Contains(key); - public bool Remove(JsValue key) => _set.Remove(key); + public new bool Delete(JsValue key) => _set.Remove(key); internal void ForEach(ICallable callable, JsValue thisArg) { diff --git a/Jint/Native/Map/MapConstructor.cs b/Jint/Native/Map/MapConstructor.cs index 95bfbea9a9..fb19d2c36a 100644 --- a/Jint/Native/Map/MapConstructor.cs +++ b/Jint/Native/Map/MapConstructor.cs @@ -11,7 +11,7 @@ namespace Jint.Native.Map; -internal sealed class MapConstructor : Constructor +public sealed class MapConstructor : Constructor { private static readonly JsString _functionName = new("Map"); @@ -33,28 +33,34 @@ internal MapConstructor( protected override void Initialize() { const PropertyFlag PropertyFlags = PropertyFlag.Writable | PropertyFlag.Configurable; - var properties = new PropertyDictionary(1, checkExistingKeys: false) - { - ["groupBy"] = new(new ClrFunction(Engine, "groupBy", GroupBy, 2, PropertyFlag.Configurable), PropertyFlags), - }; + var properties = new PropertyDictionary(1, checkExistingKeys: false) { ["groupBy"] = new(new ClrFunction(Engine, "groupBy", GroupBy, 2, PropertyFlag.Configurable), PropertyFlags), }; SetProperties(properties); - var symbols = new SymbolDictionary(1) - { - [GlobalSymbolRegistry.Species] = new GetSetPropertyDescriptor(get: new ClrFunction(_engine, "get [Symbol.species]", Species, 0, PropertyFlag.Configurable), set: Undefined, PropertyFlag.Configurable) - }; + var symbols = new SymbolDictionary(1) { [GlobalSymbolRegistry.Species] = new GetSetPropertyDescriptor(get: new ClrFunction(_engine, "get [Symbol.species]", Species, 0, PropertyFlag.Configurable), set: Undefined, PropertyFlag.Configurable) }; SetSymbols(symbols); } - private static JsValue Species(JsValue thisObject, JsValue[] arguments) - { - return thisObject; - } + public JsMap Construct() => ConstructMap(this); /// /// https://tc39.es/ecma262/#sec-map-iterable /// public override ObjectInstance Construct(JsValue[] arguments, JsValue newTarget) + { + var map = ConstructMap(newTarget); + + if (arguments.Length > 0 && !arguments[0].IsNullOrUndefined()) + { + var adder = ((ObjectInstance) map).Get("set"); + var iterator = arguments.At(0).GetIterator(_realm); + + IteratorProtocol.AddEntriesFromIterable(map, iterator, adder); + } + + return map; + } + + private JsMap ConstructMap(JsValue newTarget) { if (newTarget.IsUndefined()) { @@ -66,14 +72,6 @@ public override ObjectInstance Construct(JsValue[] arguments, JsValue newTarget) static intrinsics => intrinsics.Map.PrototypeObject, static (Engine engine, Realm realm, object? _) => new JsMap(engine, realm)); - if (arguments.Length > 0 && !arguments[0].IsNullOrUndefined()) - { - var adder = map.Get("set"); - var iterator = arguments.At(0).GetIterator(_realm); - - IteratorProtocol.AddEntriesFromIterable(map, iterator, adder); - } - return map; } @@ -89,9 +87,14 @@ private JsValue GroupBy(JsValue thisObject, JsValue[] arguments) var map = (JsMap) Construct(_realm.Intrinsics.Map); foreach (var pair in grouping) { - map.MapSet(pair.Key, pair.Value); + map.Set(pair.Key, pair.Value); } return map; } + + private static JsValue Species(JsValue thisObject, JsValue[] arguments) + { + return thisObject; + } } diff --git a/Jint/Native/Map/MapPrototype.cs b/Jint/Native/Map/MapPrototype.cs index a6140512cd..17c77f5cbd 100644 --- a/Jint/Native/Map/MapPrototype.cs +++ b/Jint/Native/Map/MapPrototype.cs @@ -63,7 +63,7 @@ private JsValue Size(JsValue thisObject, JsValue[] arguments) private JsValue Get(JsValue thisObject, JsValue[] arguments) { var map = AssertMapInstance(thisObject); - return map.MapGet(arguments.At(0)); + return map.Get(arguments.At(0)); } private JsValue Clear(JsValue thisObject, JsValue[] arguments) @@ -76,7 +76,7 @@ private JsValue Clear(JsValue thisObject, JsValue[] arguments) private JsValue Delete(JsValue thisObject, JsValue[] arguments) { var map = AssertMapInstance(thisObject); - return map.MapDelete(arguments.At(0)) + return map.Remove(arguments.At(0)) ? JsBoolean.True : JsBoolean.False; } @@ -84,7 +84,7 @@ private JsValue Delete(JsValue thisObject, JsValue[] arguments) private JsValue Set(JsValue thisObject, JsValue[] arguments) { var map = AssertMapInstance(thisObject); - map.MapSet(arguments.At(0), arguments.At(1)); + map.Set(arguments.At(0), arguments.At(1)); return thisObject; } diff --git a/Jint/Native/Set/SetPrototype.cs b/Jint/Native/Set/SetPrototype.cs index 4d2f4b3852..a88e29cc4d 100644 --- a/Jint/Native/Set/SetPrototype.cs +++ b/Jint/Native/Set/SetPrototype.cs @@ -85,7 +85,7 @@ private JsValue Clear(JsValue thisObject, JsValue[] arguments) private JsBoolean Delete(JsValue thisObject, JsValue[] arguments) { var set = AssertSetInstance(thisObject); - return set.Remove(arguments.At(0)) + return set.Delete(arguments.At(0)) ? JsBoolean.True : JsBoolean.False; } @@ -119,7 +119,7 @@ private JsSet Difference(JsValue thisObject, JsValue[] arguments) var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); if (inOther) { - resultSetData.Remove(e); + resultSetData.Delete(e); index--; } } @@ -144,7 +144,7 @@ private JsSet Difference(JsValue thisObject, JsValue[] arguments) nextValue = JsNumber.PositiveZero; } - resultSetData.Remove(nextValue); + resultSetData.Delete(nextValue); } return resultSetData; @@ -308,7 +308,7 @@ private JsSet SymmetricDifference(JsValue thisObject, JsValue[] arguments) { if (inResult) { - resultSetData.Remove(nextValue); + resultSetData.Delete(nextValue); } } else diff --git a/Jint/Runtime/Intrinsics.cs b/Jint/Runtime/Intrinsics.cs index f90abbd5f0..6a3f20ddfb 100644 --- a/Jint/Runtime/Intrinsics.cs +++ b/Jint/Runtime/Intrinsics.cs @@ -192,7 +192,7 @@ internal Intrinsics(Engine engine, Realm realm) public Float64ArrayConstructor Float64Array => _float64Array ??= new Float64ArrayConstructor(_engine, _realm, TypedArray, TypedArray.PrototypeObject); - internal MapConstructor Map => + public MapConstructor Map => _map ??= new MapConstructor(_engine, _realm, Function.PrototypeObject, Object.PrototypeObject); internal MapIteratorPrototype MapIteratorPrototype => @@ -293,4 +293,4 @@ internal Intrinsics(Engine engine, Realm realm) internal ThrowTypeError ThrowTypeError => _throwTypeError ??= new ThrowTypeError(_engine, _engine.Realm) { _prototype = _engine.Realm.Intrinsics.Function.PrototypeObject }; -} \ No newline at end of file +}