From 7d1de2d6c65ea460b62a1ef531fd06a347bddbbc Mon Sep 17 00:00:00 2001 From: Ken Lyon Date: Fri, 25 Oct 2024 22:57:56 -0700 Subject: [PATCH] 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 --- .../Jint.Tests.PublicInterface.csproj | 1 + Jint.Tests.PublicInterface/SetTests.cs | 40 +++++++++++++++++++ Jint/Native/JsSet.cs | 21 +++++----- Jint/Native/Set/SetConstructor.cs | 36 ++++++++++------- Jint/Native/Set/SetPrototype.cs | 2 +- Jint/Runtime/Intrinsics.cs | 2 +- Jint/Runtime/OrderedSet.cs | 8 +++- 7 files changed, 84 insertions(+), 26 deletions(-) create mode 100644 Jint.Tests.PublicInterface/SetTests.cs diff --git a/Jint.Tests.PublicInterface/Jint.Tests.PublicInterface.csproj b/Jint.Tests.PublicInterface/Jint.Tests.PublicInterface.csproj index bf3c659d53..06e26b9cf9 100644 --- a/Jint.Tests.PublicInterface/Jint.Tests.PublicInterface.csproj +++ b/Jint.Tests.PublicInterface/Jint.Tests.PublicInterface.csproj @@ -14,6 +14,7 @@ + diff --git a/Jint.Tests.PublicInterface/SetTests.cs b/Jint.Tests.PublicInterface/SetTests.cs new file mode 100644 index 0000000000..dada59965c --- /dev/null +++ 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 index af4ac74bb6..ba0130fe31 100644 --- a/Jint/Native/JsSet.cs +++ b/Jint/Native/JsSet.cs @@ -1,3 +1,4 @@ +using System.Collections; using System.Diagnostics.CodeAnalysis; using Jint.Native.Object; using Jint.Runtime; @@ -5,15 +6,15 @@ namespace Jint.Native; -internal sealed class JsSet : ObjectInstance +public sealed class JsSet : ObjectInstance, IEnumerable { internal readonly OrderedSet _set; - public JsSet(Engine engine) : this(engine, new OrderedSet(SameValueZeroComparer.Instance)) + internal JsSet(Engine engine) : this(engine, new OrderedSet(SameValueZeroComparer.Instance)) { } - public JsSet(Engine engine, OrderedSet set) : base(engine) + internal JsSet(Engine engine, OrderedSet 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 GetEnumerator() => _set.GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/Jint/Native/Set/SetConstructor.cs b/Jint/Native/Set/SetConstructor.cs index b1ec294c49..36acb8a3ea 100644 --- 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); /// /// https://tc39.es/ecma262/#sec-set-iterable /// 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 index d70e03fd6a..4d2f4b3852 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.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 index 5dfac649f3..f90abbd5f0 100644 --- 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 index da1e93287d..4315962b4a 100644 --- a/Jint/Runtime/OrderedSet.cs +++ b/Jint/Runtime/OrderedSet.cs @@ -1,6 +1,8 @@ +using System.Collections; + namespace Jint.Runtime; -internal sealed class OrderedSet +internal sealed class OrderedSet : IEnumerable { internal List _list; internal HashSet _set; @@ -61,4 +63,8 @@ public bool Remove(T item) _set.Remove(item); return _list.Remove(item); } + + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); }