Make JsSet public (#1987)

* Make JsSet public * Some code review changes * Use intrinsics to construct, add public API test, IEnumerable, SetDelete => Remove --------- Co-authored-by: Marko Lahma <[email protected]>
sebastienros · Oct 26, 2024 · 7d1de2d · 7d1de2d
1 parent 7381086
commit 7d1de2d
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 26 deletions.
diff --git a/Jint.Tests.PublicInterface/Jint.Tests.PublicInterface.csproj b/Jint.Tests.PublicInterface/Jint.Tests.PublicInterface.csproj
@@ -14,6 +14,7 @@
 
   <ItemGroup>
     <PackageReference Include="Acornima.Extras" />
+    <PackageReference Include="FluentAssertions" />
     <PackageReference Include="Flurl.Http.Signed" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" />
     <PackageReference Include="MongoDB.Bson.signed" />

diff --git a/Jint.Tests.PublicInterface/SetTests.cs b/Jint.Tests.PublicInterface/SetTests.cs
@@ -0,0 +1,40 @@
+using FluentAssertions;
+using Jint.Native;
+
+namespace Jint.Tests.Runtime;
+
+public class SetTests
+{
+    [Fact]
+    public void ConConstructSet()
+    {
+        var engine = new Engine();
+
+        var set = engine.Intrinsics.Set.Construct();
+        set.Add(42);
+        set.Add("foo");
+        set.Size.Should().Be(2);
+
+        set.Should().ContainInOrder(42, "foo");
+
+        set.Has(42).Should().BeTrue();
+        set.Has("foo").Should().BeTrue();
+        set.Has(24).Should().BeFalse();
+
+        engine.SetValue("s", set);
+        engine.Evaluate("s.size").Should().Be((JsNumber) 2);
+        engine.Evaluate("s.has(42)").Should().Be(JsBoolean.True);
+        engine.Evaluate("s.has('foo')").Should().Be(JsBoolean.True);
+        engine.Evaluate("s.has(24)").Should().Be(JsBoolean.False);
+
+        set.Remove(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);
+
+        set.Clear();
+        set.Should().BeEmpty();
+        set.Size.Should().Be(0);
+        engine.Evaluate("s.size").Should().Be((JsNumber) 0);
+    }
+}
diff --git a/Jint/Native/JsSet.cs b/Jint/Native/JsSet.cs
@@ -1,19 +1,20 @@
+using System.Collections;
 using System.Diagnostics.CodeAnalysis;
 using Jint.Native.Object;
 using Jint.Runtime;
 using Jint.Runtime.Descriptors;
 
 namespace Jint.Native;
 
-internal sealed class JsSet : ObjectInstance
+public sealed class JsSet : ObjectInstance, IEnumerable<JsValue>
 {
     internal readonly OrderedSet<JsValue> _set;
 
-    public JsSet(Engine engine) : this(engine, new OrderedSet<JsValue>(SameValueZeroComparer.Instance))
+    internal JsSet(Engine engine) : this(engine, new OrderedSet<JsValue>(SameValueZeroComparer.Instance))
     {
     }
 
-    public JsSet(Engine engine, OrderedSet<JsValue> set) : base(engine)
+    internal JsSet(Engine engine, OrderedSet<JsValue> set) : base(engine)
     {
         _set = set;
         _prototype = _engine.Realm.Intrinsics.Set.PrototypeObject;
@@ -47,15 +48,13 @@ protected override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out
         return base.TryGetProperty(property, out descriptor);
     }
 
-    internal void Add(JsValue value) => _set.Add(value);
+    public void Add(JsValue value) => _set.Add(value);
 
-    internal void Remove(JsValue value) => _set.Remove(value);
+    public void Clear() => _set.Clear();
 
-    internal void Clear() => _set.Clear();
+    public bool Has(JsValue key) => _set.Contains(key);
 
-    internal bool Has(JsValue key) => _set.Contains(key);
-
-    internal bool SetDelete(JsValue key) => _set.Remove(key);
+    public bool Remove(JsValue key) => _set.Remove(key);
 
     internal void ForEach(ICallable callable, JsValue thisArg)
     {
@@ -76,4 +75,8 @@ internal void ForEach(ICallable callable, JsValue thisArg)
     internal ObjectInstance Entries() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructEntryIterator(this);
 
     internal ObjectInstance Values() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructValueIterator(this);
+
+    public IEnumerator<JsValue> GetEnumerator() => _set.GetEnumerator();
+
+    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 }
diff --git a/Jint/Native/Set/SetConstructor.cs b/Jint/Native/Set/SetConstructor.cs
@@ -8,7 +8,7 @@
 
 namespace Jint.Native.Set;
 
-internal sealed class SetConstructor : Constructor
+public sealed class SetConstructor : Constructor
 {
     private static readonly JsString _functionName = new("Set");
 
@@ -37,25 +37,14 @@ protected override void Initialize()
         SetSymbols(symbols);
     }
 
-    private static JsValue Species(JsValue thisObject, JsValue[] arguments)
-    {
-        return thisObject;
-    }
+    public JsSet Construct() => ConstructSet(this);
 
     /// <summary>
     /// https://tc39.es/ecma262/#sec-set-iterable
     /// </summary>
     public override ObjectInstance Construct(JsValue[] arguments, JsValue newTarget)
     {
-        if (newTarget.IsUndefined())
-        {
-            ExceptionHelper.ThrowTypeError(_engine.Realm);
-        }
-
-        var set = OrdinaryCreateFromConstructor(
-            newTarget,
-            static intrinsics => intrinsics.Set.PrototypeObject,
-            static (Engine engine, Realm _, object? _) => new JsSet(engine));
+        var set = ConstructSet(newTarget);
 
         if (arguments.Length > 0 && !arguments[0].IsNullOrUndefined())
         {
@@ -92,4 +81,23 @@ public override ObjectInstance Construct(JsValue[] arguments, JsValue newTarget)
 
         return set;
     }
+
+    private JsSet ConstructSet(JsValue newTarget)
+    {
+        if (newTarget.IsUndefined())
+        {
+            ExceptionHelper.ThrowTypeError(_engine.Realm);
+        }
+
+        var set = OrdinaryCreateFromConstructor(
+            newTarget,
+            static intrinsics => intrinsics.Set.PrototypeObject,
+            static (Engine engine, Realm _, object? _) => new JsSet(engine));
+        return set;
+    }
+
+    private static JsValue Species(JsValue thisObject, JsValue[] arguments)
+    {
+        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.SetDelete(arguments.At(0))
+        return set.Remove(arguments.At(0))
             ? JsBoolean.True
             : JsBoolean.False;
     }

diff --git a/Jint/Runtime/Intrinsics.cs b/Jint/Runtime/Intrinsics.cs
@@ -198,7 +198,7 @@ internal Intrinsics(Engine engine, Realm realm)
     internal MapIteratorPrototype MapIteratorPrototype =>
         _mapIteratorPrototype ??= new MapIteratorPrototype(_engine, _realm, IteratorPrototype);
 
-    internal SetConstructor Set =>
+    public SetConstructor Set =>
         _set ??= new SetConstructor(_engine, _realm, Function.PrototypeObject, Object.PrototypeObject);
 
     internal SetIteratorPrototype SetIteratorPrototype =>

diff --git a/Jint/Runtime/OrderedSet.cs b/Jint/Runtime/OrderedSet.cs
@@ -1,6 +1,8 @@
+using System.Collections;
+
 namespace Jint.Runtime;
 
-internal sealed class OrderedSet<T>
+internal sealed class OrderedSet<T> : IEnumerable<T>
 {
     internal List<T> _list;
     internal HashSet<T> _set;
@@ -61,4 +63,8 @@ public bool Remove(T item)
         _set.Remove(item);
         return _list.Remove(item);
     }
+
+    public IEnumerator<T> GetEnumerator() => _list.GetEnumerator();
+
+    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 }