Obsolete and cleanup some ObjectInstance access patterns (#1809)

Jint/Engine.cs

@@ -597,56 +597,31 @@ namespace Jint
                     var v = baseObj.Get(property, reference.ThisValue);
                     return v;
                 }
-                else
-                {
-                    // check if we are accessing a string, boxing operation can be costly to do index access
-                    // we have good chance to have fast path with integer or string indexer
-                    ObjectInstance? o = null;
-                    if ((property._type & (InternalTypes.String | InternalTypes.Integer)) != InternalTypes.Empty
-                        && baseValue is JsString s
-                        && TryHandleStringValue(property, s, ref o, out var jsValue))
-                    {
-                        return jsValue;
-                    }
-
-                    if (o is null)
-                    {
-                        o = Runtime.TypeConverter.ToObject(Realm, baseValue);
-                    }
-
-                    if (reference.IsPrivateReference)
-                    {
-                        return o.PrivateGet((PrivateName) reference.ReferencedName);
-                    }
-
-                    var desc = o.GetProperty(property);
-                    if (desc == PropertyDescriptor.Undefined)
-                    {
-                        return JsValue.Undefined;
-                    }
 
-                    if (desc.IsDataDescriptor())
-                    {
-                        return desc.Value;
-                    }
+                // check if we are accessing a string, boxing operation can be costly to do index access
+                // we have good chance to have fast path with integer or string indexer
+                ObjectInstance? o = null;
+                if ((property._type & (InternalTypes.String | InternalTypes.Integer)) != InternalTypes.Empty
+                    && baseValue is JsString s
+                    && TryHandleStringValue(property, s, ref o, out var jsValue))
+                {
+                    return jsValue;
+                }
 
-                    var getter = desc.Get!;
-                    if (getter.IsUndefined())
-                    {
-                        return JsValue.Undefined;
-                    }
+                if (o is null)
+                {
+                    o = Runtime.TypeConverter.ToObject(Realm, baseValue);
+                }
 
-                    var callable = (ICallable) getter;
-                    return callable.Call(baseValue, Arguments.Empty);
+                if (reference.IsPrivateReference)
+                {
+                    return o.PrivateGet((PrivateName) reference.ReferencedName);
                 }
-            }
 
-            var record = baseValue as Environment;
-            if (record is null)
-            {
-                ExceptionHelper.ThrowArgumentException();
+                return o.Get(property, reference.ThisValue);
             }
 
+            var record = (Environment) baseValue;
             var bindingValue = record.GetBindingValue(reference.ReferencedName.ToString(), reference.Strict);
 
             if (returnReferenceToPool)

Jint/Native/Array/ArrayInstance.cs

@@ -292,17 +292,6 @@ namespace Jint.Native.Array
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private JsNumber GetJsNumberLength() => _length is null ? JsNumber.PositiveZero : (JsNumber) _length._value!;
 
-        protected sealed override void AddProperty(JsValue property, PropertyDescriptor descriptor)
-        {
-            if (CommonProperties.Length.Equals(property ))
-            {
-                _length = descriptor;
-                return;
-            }
-
-            base.AddProperty(property, descriptor);
-        }
-
         protected sealed override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out PropertyDescriptor? descriptor)
         {
             if (CommonProperties.Length.Equals(property))
@@ -436,7 +425,7 @@ namespace Jint.Native.Array
         {
             if (!TryGetValue(index, out var value))
             {
-                value = UnwrapJsValue(Prototype?.GetProperty(JsString.Create(index)) ?? PropertyDescriptor.Undefined);
+                value = Prototype?.Get(JsString.Create(index)) ?? Undefined;
             }
 
             return value;

Jint/Native/JsArguments.cs

@@ -153,7 +153,7 @@ namespace Jint.Native
             }
 
             // property is an accessor or inherited
-            var desc = GetProperty(property);
+            var desc = GetOwnProperty(property);
 
             if (desc.IsAccessorDescriptor())
             {

Jint/Native/Object/ObjectInstance.cs

@@ -179,13 +179,6 @@ namespace Jint.Native.Object
             _properties[property] = value;
         }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal void SetDataProperty(string property, JsValue value)
-        {
-            _properties ??= new PropertyDictionary();
-            _properties[property] = new PropertyDescriptor(value, PropertyFlag.ConfigurableEnumerableWritable);
-        }
-
         [MethodImpl(MethodImplOptions.NoInlining)]
         private void SetPropertyUnlikely(JsValue property, PropertyDescriptor value)
         {
@@ -326,6 +319,7 @@ namespace Jint.Native.Object
 
         internal virtual IEnumerable<JsValue> GetInitialOwnStringPropertyKeys() => System.Linq.Enumerable.Empty<JsValue>();
 
+        [Obsolete("Will be removed")]
         protected virtual void AddProperty(JsValue property, PropertyDescriptor descriptor)
         {
             SetProperty(property, descriptor);
@@ -453,9 +447,7 @@ namespace Jint.Native.Object
             SetProperty(property, desc);
         }
 
-        /// <summary>
-        /// http://www.ecma-international.org/ecma-262/5.1/#sec-8.12.2
-        /// </summary>
+        [Obsolete("Use Get or GetOwnProperty")]
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public PropertyDescriptor GetProperty(JsValue property)
         {
@@ -473,13 +465,8 @@ namespace Jint.Native.Object
         {
             value = Undefined;
             var desc = GetOwnProperty(property);
-            if (desc != null && desc != PropertyDescriptor.Undefined)
+            if (desc != PropertyDescriptor.Undefined)
             {
-                if (desc == PropertyDescriptor.Undefined)
-                {
-                    return false;
-                }
-
                 var descValue = desc.Value;
                 if (desc.WritableSet && !ReferenceEquals(descValue, null))
                 {
@@ -500,12 +487,7 @@ namespace Jint.Native.Object
                 return true;
             }
 
-            if (ReferenceEquals(Prototype, null))
-            {
-                return false;
-            }
-
-            return Prototype.TryGetValue(property, out value);
+            return Prototype?.TryGetValue(property, out value) == true;
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -654,7 +636,7 @@ namespace Jint.Native.Object
                 return Extensible;
             }
 
-            var inherited = Prototype.GetProperty(property);
+            var inherited = Prototype.GetOwnProperty(property);
 
             if (inherited == PropertyDescriptor.Undefined)
             {
@@ -1345,7 +1327,7 @@ namespace Jint.Native.Object
         }
 
         /// <summary>
-        /// https://tc39.es/ecma262/#sec-createdatapropertyorthrow
+        /// https://tc39.es/ecma262/#sec-createdataproperty
         /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool CreateDataProperty(JsValue p, JsValue v)
@@ -1353,9 +1335,8 @@ namespace Jint.Native.Object
             return DefineOwnProperty(p, new PropertyDescriptor(v, PropertyFlag.ConfigurableEnumerableWritable));
         }
 
-
         /// <summary>
-        /// https://tc39.es/ecma262/#sec-createdataproperty
+        /// https://tc39.es/ecma262/#sec-createdatapropertyorthrow
         /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal bool CreateDataPropertyOrThrow(JsValue p, JsValue v)

Jint/Runtime/Environments/GlobalEnvironment.cs

@@ -240,7 +240,7 @@ namespace Jint.Runtime.Environments
             }
             else
             {
-                desc = _global.GetProperty(name.Name);
+                desc = _global.GetOwnProperty(name.Name);
             }
 
             if (strict && desc == PropertyDescriptor.Undefined)

Jint/Runtime/Environments/ObjectEnvironment.cs

@@ -88,8 +88,7 @@ namespace Jint.Runtime.Environments
                 return false;
             }
 
-            var desc = _bindingObject.GetProperty(name.Value);
-            value = ObjectInstance.UnwrapJsValue(desc, _bindingObject);
+            value = _bindingObject.Get(name.Value);
             return true;
         }
 
@@ -154,13 +153,13 @@ namespace Jint.Runtime.Environments
 
         internal override JsValue GetBindingValue(Key name, bool strict)
         {
-            var desc = _bindingObject.GetProperty(name.Name);
-            if (strict && desc == PropertyDescriptor.Undefined)
+            JsValue value;
+            if (!_bindingObject.TryGetValue(name.Name, out value) && strict)
             {
-                ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name);
+                ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name.Name);
             }
 
-            return ObjectInstance.UnwrapJsValue(desc, _bindingObject);
+            return value;
         }
 
         internal override bool DeleteBinding(Key name) => _bindingObject.Delete(name.Name);

Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs

@@ -223,7 +223,7 @@ namespace Jint.Runtime.Interpreter.Expressions
                                 ExceptionHelper.ThrowTypeError(engine.Realm, $"Cannot delete property '{referencedName}' of {o}");
                             }
 
-                            if (StrictModeScope.IsStrictModeCode && !r.Base.AsObject().GetProperty(referencedName).Configurable)
+                            if (StrictModeScope.IsStrictModeCode && !r.Base.AsObject().GetOwnProperty(referencedName).Configurable)
                             {
                                 ExceptionHelper.ThrowTypeError(engine.Realm, $"Cannot delete property '{referencedName}' of {o}");
                             }