Struct properties that implement IList<T> are unable to hold values on Deserialization

[jozkee]

Properties of a custom struct type that implements IList<T> are unable to read values, this is because when we try to add the element into the list, we do it on a copy of said property which is returned by jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue).

This issue is probably duplicated also on custom structs that implement Dictionary or IDictionary.
This issue does not occur when the value type list is the root object.

I would wait to see if the new converter refactor indirectly fixes this issue.
cc @steveharter, @layomia, @ahsonkhan

Repro:

[Fact]
public static void StructListPropertyDoesNotHoldElements()
{
    string json = $"{{\"StructList\":[10,20,30,40,50,60,70,80]}}";
    StructListWrapper wrapper = JsonSerializer.Deserialize<StructListWrapper>(json);
    Assert.Equal(8, wrapper.StructList.Count);
}

private class StructListWrapper
{
    public StructList<int> StructList { get; set; }
}

private struct StructList<T> : IList<T>
{
    private List<T> _list;
    public T this[int index]
    {
        get {
            InitializeIfNull();
            return _list[index];
        }
        set {
            InitializeIfNull();
            _list[index] = value;
        }
    }

    public int Count => _list == null? 0: _list.Count;

    public bool IsReadOnly => false;

    private void InitializeIfNull()
    {
        if (_list == null) {
            _list = new List<T>();
        }
    }

    public void Add(T item)
    {
        InitializeIfNull();
        _list.Add(item);
    }

    public void Clear()
    {
        _list.Clear();
    }

    public bool Contains(T item)
    {
        return _list.Contains(item);
    }

    public void CopyTo(T[] array, int arrayIndex)
    {
        throw new NotImplementedException();
    }

    public IEnumerator<T> GetEnumerator()
    {
        return _list.GetEnumerator();
    }

    public int IndexOf(T item)
    {
        return _list.IndexOf(item);
    }

    public void Insert(int index, T item)
    {
        _list.Insert(index, item);
    }

    public bool Remove(T item)
    {
        return _list.Remove(item);
    }

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

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

[layomia]

@jozkee can you verify if this was fixed in #2259?

[jozkee]

@layomia this was not fixed on the converters refactor; see #2259 (comment).

[RaymondHuy]

@jozkee @layomia I have just had a look on this issue. My approach is to add a new property for storing elements of value collection type into IListOfTConverter class:
[MaybeNull]
private TCollection listStruct;
Inside the CreateCollection method, I will create an instance of this property if the TypeToConvert is value type and also modify the Add method:

        protected override void Add(in TElement value, ref ReadStack state)
        {
            if (listStruct != null)
            {
                listStruct.Add(value);
                state.Current.ReturnValue = listStruct;
            }
            else ((TCollection)state.Current.ReturnValue!).Add(value);
        }

Do you think it is a good approach ? If it's the point, I can make a PR for it :)

[wzchua]

How about Unsafe.Unbox when it is a valuetype?

[devsko]

@wzchua Unsafe.Unbox unfortunately is not available in net461 / netstandard
edit: but the package can be referenced 👍