From 9fc266349a8e03cd2880f3ff6cee4a6eb73de4cc Mon Sep 17 00:00:00 2001 From: Ken Lyon Date: Thu, 24 Oct 2024 10:48:53 -0700 Subject: [PATCH 1/3] Make JsSet public --- Jint/Native/JsSet.cs | 18 +++++++++--------- Jint/Runtime/OrderedSet.cs | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Jint/Native/JsSet.cs b/Jint/Native/JsSet.cs index af4ac74bb6..df858f2565 100644 --- a/Jint/Native/JsSet.cs +++ b/Jint/Native/JsSet.cs @@ -5,7 +5,7 @@ namespace Jint.Native; -internal sealed class JsSet : ObjectInstance +public sealed class JsSet : ObjectInstance { internal readonly OrderedSet _set; @@ -47,17 +47,17 @@ 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 Remove(JsValue value) => _set.Remove(value); - internal void Clear() => _set.Clear(); + public void Clear() => _set.Clear(); - internal bool Has(JsValue key) => _set.Contains(key); + public bool Has(JsValue key) => _set.Contains(key); - internal bool SetDelete(JsValue key) => _set.Remove(key); + public bool SetDelete(JsValue key) => _set.Remove(key); - internal void ForEach(ICallable callable, JsValue thisArg) + public void ForEach(ICallable callable, JsValue thisArg) { var args = _engine._jsValueArrayPool.RentArray(3); args[2] = this; @@ -73,7 +73,7 @@ internal void ForEach(ICallable callable, JsValue thisArg) _engine._jsValueArrayPool.ReturnArray(args); } - internal ObjectInstance Entries() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructEntryIterator(this); + public ObjectInstance Entries() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructEntryIterator(this); - internal ObjectInstance Values() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructValueIterator(this); + public ObjectInstance Values() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructValueIterator(this); } diff --git a/Jint/Runtime/OrderedSet.cs b/Jint/Runtime/OrderedSet.cs index da1e93287d..06d7b489c7 100644 --- a/Jint/Runtime/OrderedSet.cs +++ b/Jint/Runtime/OrderedSet.cs @@ -1,6 +1,6 @@ namespace Jint.Runtime; -internal sealed class OrderedSet +public sealed class OrderedSet { internal List _list; internal HashSet _set; From d2301a742605e8b6cfe191c990085be71c150d2f Mon Sep 17 00:00:00 2001 From: Ken Lyon Date: Thu, 24 Oct 2024 13:33:38 -0700 Subject: [PATCH 2/3] Some code review changes --- Jint/Native/JsSet.cs | 10 ++++------ Jint/Native/Set/SetPrototype.cs | 6 +++--- Jint/Runtime/OrderedSet.cs | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/Jint/Native/JsSet.cs b/Jint/Native/JsSet.cs index df858f2565..5a68b66a8d 100644 --- a/Jint/Native/JsSet.cs +++ b/Jint/Native/JsSet.cs @@ -13,7 +13,7 @@ public sealed class JsSet : ObjectInstance { } - public JsSet(Engine engine, OrderedSet set) : base(engine) + internal JsSet(Engine engine, OrderedSet set) : base(engine) { _set = set; _prototype = _engine.Realm.Intrinsics.Set.PrototypeObject; @@ -49,15 +49,13 @@ protected override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out public void Add(JsValue value) => _set.Add(value); - public void Remove(JsValue value) => _set.Remove(value); - public void Clear() => _set.Clear(); public bool Has(JsValue key) => _set.Contains(key); public bool SetDelete(JsValue key) => _set.Remove(key); - public void ForEach(ICallable callable, JsValue thisArg) + internal void ForEach(ICallable callable, JsValue thisArg) { var args = _engine._jsValueArrayPool.RentArray(3); args[2] = this; @@ -73,7 +71,7 @@ public void ForEach(ICallable callable, JsValue thisArg) _engine._jsValueArrayPool.ReturnArray(args); } - public ObjectInstance Entries() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructEntryIterator(this); + internal ObjectInstance Entries() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructEntryIterator(this); - public ObjectInstance Values() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructValueIterator(this); + internal ObjectInstance Values() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructValueIterator(this); } diff --git a/Jint/Native/Set/SetPrototype.cs b/Jint/Native/Set/SetPrototype.cs index d70e03fd6a..6b3b0f0278 100644 --- a/Jint/Native/Set/SetPrototype.cs +++ b/Jint/Native/Set/SetPrototype.cs @@ -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.SetDelete(e); index--; } } @@ -144,7 +144,7 @@ private JsSet Difference(JsValue thisObject, JsValue[] arguments) nextValue = JsNumber.PositiveZero; } - resultSetData.Remove(nextValue); + resultSetData.SetDelete(nextValue); } return resultSetData; @@ -308,7 +308,7 @@ private JsSet SymmetricDifference(JsValue thisObject, JsValue[] arguments) { if (inResult) { - resultSetData.Remove(nextValue); + resultSetData.SetDelete(nextValue); } } else diff --git a/Jint/Runtime/OrderedSet.cs b/Jint/Runtime/OrderedSet.cs index 06d7b489c7..da1e93287d 100644 --- a/Jint/Runtime/OrderedSet.cs +++ b/Jint/Runtime/OrderedSet.cs @@ -1,6 +1,6 @@ namespace Jint.Runtime; -public sealed class OrderedSet +internal sealed class OrderedSet { internal List _list; internal HashSet _set; From df7893217743422a04b47ce95df736e9bb0093d3 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sat, 26 Oct 2024 08:44:47 +0300 Subject: [PATCH 3/3] Use intrinsics to construct, add public API test, IEnumerable, SetDelete => Remove --- .../Jint.Tests.PublicInterface.csproj | 1 + Jint.Tests.PublicInterface/SetTests.cs | 40 +++++++++++++++++++ Jint/Native/JsSet.cs | 11 +++-- Jint/Native/Set/SetConstructor.cs | 36 ++++++++++------- Jint/Native/Set/SetPrototype.cs | 8 ++-- Jint/Runtime/Intrinsics.cs | 2 +- Jint/Runtime/OrderedSet.cs | 8 +++- 7 files changed, 83 insertions(+), 23 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 5a68b66a8d..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,11 +6,11 @@ namespace Jint.Native; -public 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)) { } @@ -53,7 +54,7 @@ protected override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out public bool Has(JsValue key) => _set.Contains(key); - public bool SetDelete(JsValue key) => _set.Remove(key); + public bool Remove(JsValue key) => _set.Remove(key); internal void ForEach(ICallable callable, JsValue thisArg) { @@ -74,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 6b3b0f0278..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; } @@ -119,7 +119,7 @@ private JsSet Difference(JsValue thisObject, JsValue[] arguments) var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); if (inOther) { - resultSetData.SetDelete(e); + resultSetData.Remove(e); index--; } } @@ -144,7 +144,7 @@ private JsSet Difference(JsValue thisObject, JsValue[] arguments) nextValue = JsNumber.PositiveZero; } - resultSetData.SetDelete(nextValue); + resultSetData.Remove(nextValue); } return resultSetData; @@ -308,7 +308,7 @@ private JsSet SymmetricDifference(JsValue thisObject, JsValue[] arguments) { if (inResult) { - resultSetData.SetDelete(nextValue); + resultSetData.Remove(nextValue); } } else 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(); }