Set values of array and immutable collection properties directly (#42420

)

* Set values of array and immutable collection properties directly

* Address review feedback

src/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableEnumerableConverter.cs

@@ -102,19 +102,16 @@ public static void RegisterImmutableCollection(Type immutableCollectionType, Typ
             options.TryAddCreateRangeDelegate(delegateKey, createRangeDelegate);
         }
 
-        public static bool IsImmutableEnumerable(Type type, out bool IsImmutableArray)
+        public static bool IsImmutableEnumerable(Type type)
         {
             if (!type.IsGenericType)
             {
-                IsImmutableArray = false;
                 return false;
             }
 
             switch (type.GetGenericTypeDefinition().FullName)
             {
                 case ImmutableArrayGenericTypeName:
-                    IsImmutableArray = true;
-                    return true;
                 case ImmutableListGenericTypeName:
                 case ImmutableListGenericInterfaceTypeName:
                 case ImmutableStackGenericTypeName:
@@ -124,10 +121,8 @@ public static bool IsImmutableEnumerable(Type type, out bool IsImmutableArray)
                 case ImmutableSortedSetGenericTypeName:
                 case ImmutableHashSetGenericTypeName:
                 case ImmutableSetGenericInterfaceTypeName:
-                    IsImmutableArray = false;
                     return true;
                 default:
-                    IsImmutableArray = false;
                     return false;
             }
         }

src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs

@@ -27,7 +27,6 @@ internal abstract class JsonPropertyInfo
         private JsonPropertyInfo _dictionaryValuePropertyPolicy;
 
         public bool CanBeNull { get; private set; }
-        public bool IsImmutableArray { get; private set; }
 
         public ClassType ClassType;
 
@@ -156,12 +155,10 @@ private void DetermineSerializationCapabilities()
                             DefaultImmutableDictionaryConverter.RegisterImmutableDictionary(RuntimePropertyType, ElementType, Options);
                             DictionaryConverter = s_jsonImmutableDictionaryConverter;
                         }
-                        else if (ClassType == ClassType.Enumerable && DefaultImmutableEnumerableConverter.IsImmutableEnumerable(RuntimePropertyType, out bool isImmutableArray))
+                        else if (ClassType == ClassType.Enumerable && DefaultImmutableEnumerableConverter.IsImmutableEnumerable(RuntimePropertyType))
                         {
                             DefaultImmutableEnumerableConverter.RegisterImmutableCollection(RuntimePropertyType, ElementType, Options);
                             EnumerableConverter = s_jsonImmutableEnumerableConverter;
-
-                            IsImmutableArray = isImmutableArray;
                         }
                     }
                 }

src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs

@@ -89,6 +89,7 @@ private static bool HandleEndArray(
             }
 
             IEnumerable value = ReadStackFrame.GetEnumerableValue(ref state.Current);
+            bool setPropertyDirectly = false;
 
             if (state.Current.TempEnumerableValues != null)
             {
@@ -103,6 +104,13 @@ private static bool HandleEndArray(
 
                 value = converter.CreateFromList(ref state, (IList)value, options);
                 state.Current.TempEnumerableValues = null;
+
+                // Since we used a converter, we just processed an array or an immutable collection. This means we created a new enumerable object.
+                // If we are processing an enumerable property, replace the current value of the property with the new instance.
+                if (state.Current.IsProcessingProperty(ClassType.Enumerable))
+                {
+                    setPropertyDirectly = true;
+                }
             }
             else if (state.Current.IsProcessingProperty(ClassType.Enumerable))
             {
@@ -132,7 +140,7 @@ private static bool HandleEndArray(
                 state.Pop();
             }
 
-            ApplyObjectToEnumerable(value, ref state);
+            ApplyObjectToEnumerable(value, ref state, setPropertyDirectly);
             return false;
         }
 
@@ -184,9 +192,7 @@ internal static void ApplyObjectToEnumerable(
                     JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
 
                     object currentEnumerable = jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue);
-                    if (currentEnumerable == null ||
-                        // ImmutableArray<T> is a struct, so default value won't be null.
-                        jsonPropertyInfo.IsImmutableArray)
+                    if (currentEnumerable == null)
                     {
                         jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value);
                     }
@@ -266,9 +272,7 @@ internal static void ApplyValueToEnumerable<TProperty>(
                     JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
 
                     object currentEnumerable = jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue);
-                    if (currentEnumerable == null ||
-                        // ImmutableArray<T> is a struct, so default value won't be null.
-                        jsonPropertyInfo.IsImmutableArray)
+                    if (currentEnumerable == null)
                     {
                         jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value);
                     }

src/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs

@@ -202,9 +202,8 @@ public static object CreateEnumerableValue(ref ReadStack state)
 
                 state.Current.TempEnumerableValues = converterList;
 
-                // Clear the value if present to ensure we don't confuse tempEnumerableValues with the collection.
-                if (!jsonPropertyInfo.IsPropertyPolicy &&
-                    !state.Current.JsonPropertyInfo.IsImmutableArray)
+                // Clear the value if present to ensure we don't confuse TempEnumerableValues with the collection.
+                if (!jsonPropertyInfo.IsPropertyPolicy && jsonPropertyInfo.CanBeNull)
                 {
                     jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, null);
                 }
@@ -242,8 +241,8 @@ public static object CreateDictionaryValue(ref ReadStack state)
 
                 state.Current.TempDictionaryValues = converterDictionary;
 
-                // Clear the value if present to ensure we don't confuse tempEnumerableValues with the collection.
-                if (!jsonPropertyInfo.IsPropertyPolicy)
+                // Clear the value if present to ensure we don't confuse TempDictionaryValues with the collection.
+                if (!jsonPropertyInfo.IsPropertyPolicy && jsonPropertyInfo.CanBeNull)
                 {
                     jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, null);
                 }

src/System.Text.Json/tests/Serialization/Array.ReadTests.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using Xunit;
 
@@ -487,5 +488,146 @@ public static void ClassWithMixedSettersIsParsed(string json)
             Assert.NotNull(parsedObject.ParsedChild3);
             Assert.True(parsedObject.ParsedChild3.SequenceEqual(new int[] { 3, 3 }));
         }
+
+        public class ClassWithNonNullEnumerableGetters
+        {
+            private string[] _array = null;
+            private List<string> _list = null;
+            private StringListWrapper _listWrapper = null;
+            // Immutable array is a struct.
+            private ImmutableArray<string> _immutableArray = default;
+            private ImmutableList<string> _immutableList = null;
+
+            public string[] Array
+            {
+                get => _array ?? new string[] { "-1" };
+                set { _array = value; }
+            }
+
+            public List<string> List
+            {
+                get => _list ?? new List<string> { "-1" };
+                set { _list = value; }
+            }
+
+            public StringListWrapper ListWrapper
+            {
+                get => _listWrapper ?? new StringListWrapper { "-1" };
+                set { _listWrapper = value; }
+            }
+
+            public ImmutableArray<string> MyImmutableArray
+            {
+                get => _immutableArray.IsDefault ? ImmutableArray.CreateRange(new List<string> { "-1" }) : _immutableArray;
+                set { _immutableArray = value; }
+            }
+
+            public ImmutableList<string> MyImmutableList
+            {
+                get => _immutableList ?? ImmutableList.CreateRange(new List<string> { "-1" });
+                set { _immutableList = value; }
+            }
+
+            internal object GetRawArray => _array;
+            internal object GetRawList => _list;
+            internal object GetRawListWrapper => _listWrapper;
+            internal object GetRawImmutableArray => _immutableArray;
+            internal object GetRawImmutableList => _immutableList;
+        }
+
+        [Fact]
+        public static void ClassWithNonNullEnumerableGettersIsParsed()
+        {
+            static void TestRoundTrip(ClassWithNonNullEnumerableGetters obj)
+            {
+                ClassWithNonNullEnumerableGetters roundtrip = JsonSerializer.Deserialize<ClassWithNonNullEnumerableGetters>(JsonSerializer.Serialize(obj));
+
+                if (obj.Array != null)
+                {
+                    Assert.Equal(obj.Array.Length, roundtrip.Array.Length);
+                    Assert.Equal(obj.List.Count, roundtrip.List.Count);
+                    Assert.Equal(obj.ListWrapper.Count, roundtrip.ListWrapper.Count);
+                    Assert.Equal(obj.MyImmutableArray.Length, roundtrip.MyImmutableArray.Length);
+                    Assert.Equal(obj.MyImmutableList.Count, roundtrip.MyImmutableList.Count);
+
+                    if (obj.Array.Length > 0)
+                    {
+                        Assert.Equal(obj.Array[0], roundtrip.Array[0]);
+                        Assert.Equal(obj.List[0], roundtrip.List[0]);
+                        Assert.Equal(obj.ListWrapper[0], roundtrip.ListWrapper[0]);
+                        Assert.Equal(obj.MyImmutableArray[0], roundtrip.MyImmutableArray[0]);
+                        Assert.Equal(obj.MyImmutableList[0], roundtrip.MyImmutableList[0]);
+                    }
+                }
+                else
+                {
+                    Assert.Null(obj.GetRawArray);
+                    Assert.Null(obj.GetRawList);
+                    Assert.Null(obj.GetRawListWrapper);
+                    Assert.Null(obj.GetRawImmutableList);
+                    Assert.Null(roundtrip.GetRawArray);
+                    Assert.Null(roundtrip.GetRawList);
+                    Assert.Null(roundtrip.GetRawListWrapper);
+                    Assert.Null(roundtrip.GetRawImmutableList);
+                    Assert.Equal(obj.GetRawImmutableArray, roundtrip.GetRawImmutableArray);
+                }
+            }
+
+            const string inputJsonWithCollectionElements =
+                @"{
+                    ""Array"":[""1""],
+                    ""List"":[""2""],
+                    ""ListWrapper"":[""3""],
+                    ""MyImmutableArray"":[""4""],
+                    ""MyImmutableList"":[""5""]
+                }";
+
+            ClassWithNonNullEnumerableGetters obj = JsonSerializer.Deserialize<ClassWithNonNullEnumerableGetters>(inputJsonWithCollectionElements);
+            Assert.Equal(1, obj.Array.Length);
+            Assert.Equal("1", obj.Array[0]);
+
+            Assert.Equal(1, obj.List.Count);
+            Assert.Equal("2", obj.List[0]);
+
+            Assert.Equal(1, obj.ListWrapper.Count);
+            Assert.Equal("3", obj.ListWrapper[0]);
+
+            Assert.Equal(1, obj.MyImmutableArray.Length);
+            Assert.Equal("4", obj.MyImmutableArray[0]);
+
+            Assert.Equal(1, obj.MyImmutableList.Count);
+            Assert.Equal("5", obj.MyImmutableList[0]);
+
+            TestRoundTrip(obj);
+
+            const string inputJsonWithoutCollectionElements =
+                @"{
+                    ""Array"":[],
+                    ""List"":[],
+                    ""ListWrapper"":[],
+                    ""MyImmutableArray"":[],
+                    ""MyImmutableList"":[]
+                }";
+
+            obj = JsonSerializer.Deserialize<ClassWithNonNullEnumerableGetters>(inputJsonWithoutCollectionElements);
+            Assert.Equal(0, obj.Array.Length);
+            Assert.Equal(0, obj.List.Count);
+            Assert.Equal(0, obj.ListWrapper.Count);
+            Assert.Equal(0, obj.MyImmutableArray.Length);
+            Assert.Equal(0, obj.MyImmutableList.Count);
+            TestRoundTrip(obj);
+
+            // Skip ImmutableArray due to https://github.com/dotnet/corefx/issues/42399.
+            const string inputJsonWithNullCollections =
+                @"{
+                    ""Array"":null,
+                    ""List"":null,
+                    ""ListWrapper"":null,
+                    ""MyImmutableList"":null
+                }";
+
+            obj = JsonSerializer.Deserialize<ClassWithNonNullEnumerableGetters>(inputJsonWithNullCollections);
+            TestRoundTrip(obj);
+        }
     }
 }