Add support for wrapped generic lists to push and pop items

[ejsmith]

1822 added support for push and pop on interop objects that implement IList. JsonArray from System.Text.Json implements IList<T> and doesn't work.

My thoughts are that we implement a GenericIndexWrappedOperations that will have to use reflection to support adding and removing items.

I guess normal reflection in .NET has gotten fast enough where we don't need any special libraries, right?

[lahma]

I would probably strive towards something like:

• ListWrapper extends ObjectInstance or (ObjectWrapper) implements IObjectWrapper
  • has virtual (overridable) services like Get(int index) and Set(int index, object value), int Length { get; }
• GenericListWrapper extends ListWrapper, overridering methods and uses reflection to access generic instance

Current ArrayOperations IndexWrapperOperationsspecialization could just detect that wrapper is ListWrapper (or its derived type) and delegate to it.

We would need to detect that type is definitely not both dictionary and and a list and then allow creating specific wrapper. This is way I originally introduced ObjectWrapper.Create for such decisions.

Need to keep in mind that target could implement both dictionary and list interfaces and it shouldn't allowed to be wrapped to specific list wrapper unless it's meant to be used in int-indexing context. Probably most safe solution is to have allow-listed types like ArrayList, List<>, JsonArray, JArray etc no create the wrapper when it seems suitable.

[ejsmith]

I took at stab at an approach where I've added abilities to the TypeDescriptor and I'm using those from IndexWrappedOperations. I've got some reflection issues to resolve, but I think it can work. If you have any thoughts or you don't like this approach, please let me know.

[ejsmith]

@lahma Would you be able to take a look at this code? All but 1 test are passing. The one that is failing is trying to do a indexOf check on JsonArray and I think there is some sort of type coercion missing somewhere that I'm not able to track down. I'm also just interested in general if this is going to be an acceptable approach.

[ejsmith]

I ended up removing the indexOf test because I tried the same thing outside of Jint and it didn't work on JsonArray either. So that test was invalid.

[lahma]

With this approach there's the problem that the custom usage is only available via common array operations (push, pop) but that's of course basically what PR's title says. Now TypeDescriptor is no longer only describing the type and its capabilities but also operating on the types.

With the ListWrapper approach I suggested above a call to person.Cars[0].color could also use the faster custom object wrapper implementation for indexing without trying to figure out which one to use in which case - List should only allow indexing value with positive integer, otherwise call prototype (Array.prototype).

[ejsmith]

I get what you are saying and I definitely think this can be done a lot cleaner. I am just not familiar enough with everything yet. When I looked at taking your approach, it seemed like it was going to be a lot of work and I'd have a lot of learning to do.

TypeDescriptor already had behavior in it with TryGetValue and Remove and GetKeys.

I am going to try and do some more digging and see what I can come up with. It looks like I would need to derive from ObjectInstance and IObjectWrapper and then copy a lot of code from ObjectWrapper, but specialize it for just IList<T>? I guess this would be ListWrapper<T>? And then who would construct this wrapper object? Would we use reflection to automatically wrap IList<T> implementors?

[ejsmith]

Do you think something like this is on the right path?

    public class ArrayLikeWrapper : ObjectInstance, IObjectWrapper
    {
        private static readonly Type _genericListType = typeof(IList<>);
        private static readonly Type _genericReadonlyListType = typeof(IReadOnlyList<>);

        private readonly ICollection? _collection;
        private readonly IList? _list;

        internal ArrayLikeWrapper(Engine engine, object? target)
            : base(engine)
        {
            if (target == null)
            {
                ExceptionHelper.ThrowArgumentNullException(nameof(target));
            }

            Target = target;
            _collection = target as ICollection;
            _list = target as IList;
            Prototype = engine.Intrinsics.Array.PrototypeObject;
        }

        public static ArrayLikeWrapper Create(Engine engine, object? target)
        {
            if (target == null)
            {
                ExceptionHelper.ThrowArgumentNullException(nameof(target));
            }

            if (target is IList list)
                return new ArrayLikeWrapper(engine, list);

            if (target is IList<object> objectList)
                return new ArrayLikeWrapper<object>(engine, objectList);

#if NET8_0_OR_GREATER
            if (target is JsonArray array)
                return new ArrayLikeWrapper<JsonNode?>(engine, array);
#endif

            var type = target.GetType();
            if (type.IsGenericType && type.GetGenericTypeDefinition() == _genericListType)
            {
                var arrayItemType = type.GenericTypeArguments[0];
                var arrayWrapperType = typeof(ArrayLikeWrapper<>).MakeGenericType(arrayItemType);
                return (ArrayLikeWrapper)Activator.CreateInstance(arrayWrapperType, engine, target)!;
            }

            if (type.IsGenericType && type.GetGenericTypeDefinition() == _genericReadonlyListType)
            {
                var arrayItemType = type.GenericTypeArguments[0];
                var arrayWrapperType = typeof(ReadOnlyArrayLikeWrapper<>).MakeGenericType(arrayItemType);
                return (ArrayLikeWrapper)Activator.CreateInstance(arrayWrapperType, engine, target)!;
            }

            return new ArrayLikeWrapper(engine, target);
        }

        public object Target { get; }
        public virtual int Length => _collection?.Count ?? 0;

        public virtual object? Get(int index)
        {
            if (_list is not null && index >= 0 && index < _list.Count)
            {
                return _list[index];
            }

            return null;
        }

        public virtual void Set(int index, object value)
        {
            EnsureCapacity(index);

            if (_list is not null)
            {
                _list[index] = value;
            }
        }

        public virtual void Add(object value)
        {
            _list?.Add(value);
        }

        public virtual void RemoveAt(int index)
        {
            _list?.RemoveAt(index);
        }

        public virtual void EnsureCapacity(int capacity)
        {
            if (_list is null)
            {
                return;
            }

            while (_list.Count < capacity)
            {
                _list.Add(null);
            }
        }

        public override object ToObject()
        {
            return Target;
        }
    }

    public class ArrayLikeWrapper<T> : ArrayLikeWrapper
    {
        private readonly IList<T> _list;

        public ArrayLikeWrapper(Engine engine, IList<T> target)
            : base(engine, target)
        {
            _list = target;
        }

        public override int Length => _list.Count;

        public override object? Get(int index)
        {
            if (index >= 0 && index < _list.Count)
            {
                return _list[index];
            }

            return null;
        }

        public override void Set(int index, object value)
        {
            EnsureCapacity(index);
            _list[index] = (T)value;
        }

        public override void Add(object value)
        {
            _list.Add((T)value);
        }

        public override void RemoveAt(int index)
        {
            _list.RemoveAt(index);
        }

        public override void EnsureCapacity(int capacity)
        {
            while (_list.Count < capacity)
            {
                _list.Add(default!);
            }
        }
    }

    public class ReadOnlyArrayLikeWrapper<T> : ArrayLikeWrapper
    {
        private readonly IReadOnlyList<T> _list;

        public ReadOnlyArrayLikeWrapper(Engine engine, IReadOnlyList<T> target)
            : base(engine, target)
        {
            _list = target;
        }

        public override int Length => _list.Count;

        public override object? Get(int index)
        {
            if (index >= 0 && index < _list.Count)
            {
                return _list[index];
            }

            return null;
        }

        public override void Set(int index, object value)
        {
            ExceptionHelper.ThrowNotSupportedException();
        }

        public override void Add(object value)
        {
            ExceptionHelper.ThrowNotSupportedException();
        }

        public override void RemoveAt(int index)
        {
            ExceptionHelper.ThrowNotSupportedException();
        }

        public override void EnsureCapacity(int capacity)
        {
            ExceptionHelper.ThrowNotSupportedException();
        }
    }

[ejsmith]

Trying this approach here: #1828