Cleanup ObjectInstance public API (#1720)

* Hide CanPut() * Hide DefinePropertyOrThrow() * Hide DeletePropertyOrThrow() * Hide IsArray() from ObjectInstance and add JsValueExtensions.IsArray() * Hide SetPrototypeOf() and allow set via Prototype property * Make IsLooselyEqual() protected * Hide IsArrayLike * Only expose Length property for JsArray and JsTypedArray * Remove Invoke
sebastienros · Jan 5, 2024 · 69b33ac · 69b33ac
1 parent 7ad9db7
commit 69b33ac
Show file tree

Hide file tree

Showing 30 changed files with 71 additions and 70 deletions.
diff --git a/Jint.Tests.PublicInterface/RavenApiUsageTests.cs b/Jint.Tests.PublicInterface/RavenApiUsageTests.cs
@@ -231,7 +231,7 @@ public override bool Equals(JsString obj)
         };
     }
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected override bool IsLooselyEqual(JsValue value)
     {
         return value switch
         {

diff --git a/Jint.Tests/Runtime/Debugger/DebugHandlerTests.cs b/Jint.Tests/Runtime/Debugger/DebugHandlerTests.cs
@@ -34,7 +34,7 @@ public void AvoidsPauseRecursion()
                     var obj = info.CurrentScopeChain[0].GetBindingValue("obj") as ObjectInstance;
                     var prop = obj.GetOwnProperty("name");
                     // This is where reentrance would occur:
-                    var value = prop.Get.Invoke(engine);
+                    var value = engine.Invoke(prop.Get);
                     didPropertyAccess = true;
                 }
                 return StepMode.Into;

diff --git a/Jint.Tests/Runtime/EngineTests.cs b/Jint.Tests/Runtime/EngineTests.cs
@@ -832,7 +832,7 @@ public void ShouldInvokeAFunctionValue()
 
             var add = _engine.GetValue("add");
 
-            Assert.Equal(3, add.Invoke(_engine, 1, 2));
+            Assert.Equal(3, _engine.Invoke(add, 1, 2));
         }
 
         [Fact]
@@ -844,7 +844,7 @@ public void ShouldAllowInvokeAFunctionValueWithNullValueAsArgument()
 
             var add = _engine.GetValue("get");
             string str = null;
-            Assert.Equal(Native.JsValue.Null, add.Invoke(_engine, str));
+            Assert.Equal(Native.JsValue.Null, _engine.Invoke(add, str));
         }
 
 
@@ -857,7 +857,7 @@ public void ShouldNotInvokeNonFunctionValue()
 
             var x = _engine.GetValue("x");
 
-            var exception = Assert.Throws<JavaScriptException>(() => x.Invoke(_engine, 1, 2));
+            var exception = Assert.Throws<JavaScriptException>(() => _engine.Invoke(x, 1, 2));
             Assert.Equal("Can only invoke functions", exception.Message);
         }
 
@@ -2575,7 +2575,7 @@ public void ShouldReturnCorrectConcatenatedStrings()
                 }");
 
             var concat = _engine.GetValue("concat");
-            var result = concat.Invoke(_engine, "concat", "well", "done").ToObject() as string;
+            var result = _engine.Invoke(concat, "concat", "well", "done").ToObject() as string;
             Assert.Equal("concatwelldone", result);
         }
 
@@ -2709,10 +2709,10 @@ public void ShouldSupportDefaultsInFunctionParameters()
             ");
 
             var function = _engine.GetValue("f");
-            var result = function.Invoke(_engine, 3).ToString();
+            var result = _engine.Invoke(function, 3).ToString();
             Assert.Equal("15", result);
 
-            result = function.Invoke(_engine, 3, JsValue.Undefined).ToString();
+            result = _engine.Invoke(function, 3, JsValue.Undefined).ToString();
             Assert.Equal("15", result);
         }
 

diff --git a/Jint.Tests/Runtime/FunctionTests.cs b/Jint.Tests/Runtime/FunctionTests.cs
@@ -111,7 +111,7 @@ public void CanInvokeConstructorsFromEngine()
         Assert.Equal("abc", instanceFromFunction.Get("a"));
         Assert.Equal(123, instanceFromFunction.Get("b"));
 
-        var arrayInstance = (ArrayInstance) _engine.Construct("Array", "abc", 123).AsObject();
+        var arrayInstance = (JsArray) _engine.Construct("Array", "abc", 123).AsObject();
         Assert.Equal((uint) 2, arrayInstance.Length);
         Assert.Equal("abc", arrayInstance[0]);
         Assert.Equal(123, arrayInstance[1]);

diff --git a/Jint.Tests/Runtime/InteropTests.cs b/Jint.Tests/Runtime/InteropTests.cs
@@ -2358,7 +2358,7 @@ public void ShouldNotResolveToPrimitiveSymbol()
             ");
 
             Assert.NotNull(c.ToString());
-            Assert.Equal((uint) 0, c.As<ObjectInstance>().Length);
+            Assert.Equal((uint) 0, c.AsObject().GetLength());
         }
 
         private class DictionaryWrapper
@@ -2887,7 +2887,7 @@ public void CanConfigureCustomObjectTypeForJsToClrConversion()
         {
             var engine = new Engine(options =>
             {
-                options.Interop.CreateClrObject = oi => new Dictionary<string, object>((int) oi.Length);
+                options.Interop.CreateClrObject = oi => new Dictionary<string, object>();
             });
 
             object capture = null;

diff --git a/Jint.Tests/Runtime/RegExpTests.cs b/Jint.Tests/Runtime/RegExpTests.cs
@@ -35,7 +35,7 @@ public void CanNotBreakEngineWithLongRunningRegExp()
     public void PreventsInfiniteLoop()
     {
         var engine = new Engine();
-        var result = (ArrayInstance) engine.Evaluate("'x'.match(/|/g);");
+        var result = (JsArray) engine.Evaluate("'x'.match(/|/g);");
         Assert.Equal((uint) 2, result.Length);
         Assert.Equal("", result[0]);
         Assert.Equal("", result[1]);

diff --git a/Jint/JsValueExtensions.cs b/Jint/JsValueExtensions.cs
@@ -29,6 +29,14 @@ public static bool IsUndefined(this JsValue value)
         return value._type == InternalTypes.Undefined;
     }
 
+
+    [Pure]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static bool IsArray(this JsValue value)
+    {
+        return value is JsArray;
+    }
+
     [Pure]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     internal static bool IsNullOrUndefined(this JsValue value)

diff --git a/Jint/Native/Array/ArrayInstance.cs b/Jint/Native/Array/ArrayInstance.cs
@@ -65,9 +65,9 @@ private void InitializePrototypeAndValidateCapacity(Engine engine, uint capacity
             }
         }
 
-        public sealed override bool IsArrayLike => true;
+        internal sealed override bool IsArrayLike => true;
 
-        public sealed override bool IsArray() => true;
+        internal sealed override bool IsArray() => true;
 
         internal sealed override bool HasOriginalIterator
             => ReferenceEquals(Get(GlobalSymbolRegistry.Iterator), _constructor?.PrototypeObject._originalIteratorFunction);
@@ -287,7 +287,7 @@ private bool DefineOwnProperty(uint index, PropertyDescriptor desc)
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal uint GetLength() => (uint) GetJsNumberLength()._value;
+        internal override uint GetLength() => (uint) GetJsNumberLength()._value;
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private JsNumber GetJsNumberLength() => _length is null ? JsNumber.PositiveZero : (JsNumber) _length._value!;
@@ -1267,8 +1267,6 @@ internal JsArray Map(JsValue[] arguments)
             return false;
         }
 
-        public sealed override uint Length => GetLength();
-
         internal sealed override bool IsIntegerIndexedArray => true;
 
         public JsValue this[uint index]

diff --git a/Jint/Native/Array/ArrayIteratorPrototype.cs b/Jint/Native/Array/ArrayIteratorPrototype.cs
@@ -61,7 +61,7 @@ public ArrayIterator(Engine engine, JsArray array, ArrayIteratorType kind) : bas
 
         public override bool TryIteratorStep(out ObjectInstance nextItem)
         {
-            var len = _array.Length;
+            var len = _array.GetLength();
             var position = _position;
             if (!_closed && position < len)
             {
@@ -109,7 +109,7 @@ public override bool TryIteratorStep(out ObjectInstance nextItem)
             if (_typedArray is not null)
             {
                 _typedArray._viewedArrayBuffer.AssertNotDetached();
-                len = _typedArray.Length;
+                len = _typedArray.GetLength();
             }
             else
             {

diff --git a/Jint/Native/Array/ArrayOperations.cs b/Jint/Native/Array/ArrayOperations.cs
@@ -291,7 +291,7 @@ public override uint GetLength()
             {
                 if (!_target.IsConcatSpreadable)
                 {
-                    return _target.Length;
+                    return _target.GetLength();
                 }
 
                 var descValue = _target.Get(CommonProperties.Length);
@@ -317,7 +317,7 @@ public override void EnsureCapacity(ulong capacity)
 
             public override bool TryGetValue(ulong index, out JsValue value)
             {
-                if (index < _target.Length)
+                if (index < _target.GetLength())
                 {
                     value = _target[(int) index];
                     return true;

diff --git a/Jint/Native/Array/ArrayPrototype.cs b/Jint/Native/Array/ArrayPrototype.cs
@@ -547,7 +547,7 @@ private JsValue FlatMap(JsValue thisObject, JsValue[] arguments)
             var mapperFunction = arguments.At(0);
             var thisArg = arguments.At(1);
 
-            var sourceLen = O.Length;
+            var sourceLen = O.GetLength();
 
             if (!mapperFunction.IsCallable)
             {
@@ -608,7 +608,7 @@ private JsValue FlatMap(JsValue thisObject, JsValue[] arguments)
                             : depth - 1;
 
                         var objectInstance = (ObjectInstance) element;
-                        var elementLen = objectInstance.Length;
+                        var elementLen = objectInstance.GetLength();
                         targetIndex = FlattenIntoArray(target, objectInstance, elementLen, targetIndex, newDepth);
                     }
                     else
@@ -866,7 +866,7 @@ private JsValue FindLastIndex(JsValue thisObject, JsValue[] arguments)
         private JsValue At(JsValue thisObject, JsValue[] arguments)
         {
             var target = TypeConverter.ToObject(_realm, thisObject);
-            var len = target.Length;
+            var len = target.GetLength();
             var relativeIndex = TypeConverter.ToInteger(arguments.At(0));
 
             ulong actualIndex;

diff --git a/Jint/Native/JsArray.cs b/Jint/Native/JsArray.cs
@@ -24,7 +24,9 @@ public JsArray(Engine engine, uint capacity = 0, uint length = 0) : base(engine,
     public JsArray(Engine engine, JsValue[] items) : base(engine, items)
     {
     }
-
+
+    public uint Length => GetLength();
+
     private sealed class JsArrayDebugView
     {
         private readonly JsArray _array;
@@ -39,7 +41,7 @@ public JsValue[] Values
         {
             get
             {
-                var values = new JsValue[_array.Length];
+                var values = new JsValue[_array.GetLength()];
                 var i = 0;
                 foreach (var value in _array)
                 {

diff --git a/Jint/Native/JsBigInt.cs b/Jint/Native/JsBigInt.cs
@@ -65,7 +65,7 @@ public override string ToString()
         return TypeConverter.ToString(_value);
     }
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected internal override bool IsLooselyEqual(JsValue value)
     {
         if (value is JsBigInt bigInt)
         {

diff --git a/Jint/Native/JsBoolean.cs b/Jint/Native/JsBoolean.cs
@@ -28,7 +28,7 @@ public override string ToString()
         return _value ? "true" : "false";
     }
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected internal override bool IsLooselyEqual(JsValue value)
     {
         if (value is JsBoolean jsBoolean)
         {

diff --git a/Jint/Native/JsNull.cs b/Jint/Native/JsNull.cs
@@ -13,7 +13,7 @@ internal JsNull() : base(Types.Null)
 
     public override string ToString() => "null";
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected internal override bool IsLooselyEqual(JsValue value)
     {
         return ReferenceEquals(Null, value) || ReferenceEquals(Undefined, value);
     }

diff --git a/Jint/Native/JsNumber.cs b/Jint/Native/JsNumber.cs
@@ -251,7 +251,7 @@ internal bool IsNegativeInfinity()
         return double.IsNegativeInfinity(_value);
     }
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected internal override bool IsLooselyEqual(JsValue value)
     {
         if (value is JsNumber jsNumber)
         {

diff --git a/Jint/Native/JsString.cs b/Jint/Native/JsString.cs
@@ -307,7 +307,7 @@ public virtual bool Equals(JsString? other)
         return string.Equals(_value, other.ToString(), StringComparison.Ordinal);
     }
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected internal override bool IsLooselyEqual(JsValue value)
     {
         if (value is JsString jsString)
         {

diff --git a/Jint/Native/JsUndefined.cs b/Jint/Native/JsUndefined.cs
@@ -13,7 +13,7 @@ internal JsUndefined() : base(Types.Undefined)
 
     public override string ToString() => "undefined";
 
-    public override bool IsLooselyEqual(JsValue value)
+    protected internal override bool IsLooselyEqual(JsValue value)
     {
         return ReferenceEquals(Undefined, value) || ReferenceEquals(Null, value);
     }

diff --git a/Jint/Native/JsValue.cs b/Jint/Native/JsValue.cs
@@ -32,7 +32,7 @@ internal JsValue(InternalTypes type)
         }
 
         [Pure]
-        public virtual bool IsArray() => false;
+        internal virtual bool IsArray() => false;
 
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         internal virtual bool IsIntegerIndexedArray => false;
@@ -177,18 +177,6 @@ public static JsValue FromObjectWithType(Engine engine, object? value, Type? typ
         /// </summary>
         internal virtual bool ToBoolean() => _type > InternalTypes.Null;
 
-        /// <summary>
-        /// Invoke the current value as function.
-        /// </summary>
-        /// <param name="engine">The engine handling the invoke.</param>
-        /// <param name="arguments">The arguments of the function call.</param>
-        /// <returns>The value returned by the function call.</returns>
-        [Obsolete("Should use Engine.Invoke when direct invoking is needed.")]
-        public JsValue Invoke(Engine engine, params JsValue[] arguments)
-        {
-            return engine.Invoke(this, arguments);
-        }
-
         /// <summary>
         /// https://tc39.es/ecma262/#sec-getv
         /// </summary>
@@ -315,7 +303,7 @@ public override string ToString()
         /// <summary>
         /// https://tc39.es/ecma262/#sec-islooselyequal
         /// </summary>
-        public virtual bool IsLooselyEqual(JsValue value)
+        protected internal virtual bool IsLooselyEqual(JsValue value)
         {
             if (ReferenceEquals(this, value))
             {

diff --git a/Jint/Native/Json/JsonSerializer.cs b/Jint/Native/Json/JsonSerializer.cs
@@ -84,7 +84,7 @@ private void SetupReplacer(JsValue replacer)
                 if (oi.IsArray())
                 {
                     _propertyList = new List<JsValue>();
-                    var len = oi.Length;
+                    var len = oi.GetLength();
                     var k = 0;
                     while (k < len)
                     {