Make JsMap public (#1988)

* make JsSet and JsMap members new as they indicate a specific usage scenario
sebastienros · Oct 26, 2024 · 1461bf1 · 1461bf1
1 parent 7d1de2d
commit 1461bf1
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 65 deletions.
diff --git a/Jint.Tests.PublicInterface/MapTests.cs 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
@@ -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
@@ -1,11 +1,12 @@
+using System.Collections;
 using System.Diagnostics.CodeAnalysis;
 using Jint.Native.Object;
 using Jint.Runtime;
 using Jint.Runtime.Descriptors;
 
 namespace Jint.Native;
 
-internal sealed class JsMap : ObjectInstance
+public sealed class JsMap : ObjectInstance, IEnumerable<KeyValuePair<JsValue, JsValue>>
 {
     private readonly Realm _realm;
     internal readonly OrderedDictionary<JsValue, JsValue> _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<KeyValuePair<JsValue, JsValue>> 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
@@ -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
@@ -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);
 
     /// <summary>
     /// https://tc39.es/ecma262/#sec-map-iterable
     /// </summary>
     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
@@ -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,15 +76,15 @@ 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;
     }
 
     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
@@ -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
@@ -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 };
-}
+}