Updates based on review feedback

src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs → src/EFCore/ChangeTracking/ListOfNullableValueTypesComparer.cs

@@ -11,7 +11,7 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking;
 /// </summary>
 /// <remarks>
 ///     <para>
-///         This comparer should be used for nullable value types. Use <see cref="NullableValueTypeListComparer{TConcreteCollection,TElement}" /> for reference
+///         This comparer should be used for nullable value types. Use <see cref="ListOfNullableValueTypesComparer{TConcreteCollection,TElement}" /> for reference
 ///         types and non-nullable value types.
 ///     </para>
 ///     <para>
@@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking;
 /// </remarks>
 /// <typeparam name="TConcreteCollection">The collection type to create an index of, if needed.</typeparam>
 /// <typeparam name="TElement">The element type.</typeparam>
-public sealed class NullableValueTypeListComparer<TConcreteCollection, TElement> : ValueComparer<IEnumerable<TElement?>>
+public sealed class ListOfNullableValueTypesComparer<TConcreteCollection, TElement> : ValueComparer<IEnumerable<TElement?>>
     where TElement : struct
 {
     private static readonly bool IsArray = typeof(TConcreteCollection).IsArray;
@@ -33,7 +33,7 @@ public sealed class NullableValueTypeListComparer<TConcreteCollection, TElement>
     ///     Creates a new instance of the list comparer.
     /// </summary>
     /// <param name="elementComparer">The comparer to use for comparing elements.</param>
-    public NullableValueTypeListComparer(ValueComparer elementComparer)
+    public ListOfNullableValueTypesComparer(ValueComparer elementComparer)
         : base(
             (a, b) => Compare(a, b, (ValueComparer<TElement?>)elementComparer),
             o => GetHashCode(o, (ValueComparer<TElement?>)elementComparer),

src/EFCore/ChangeTracking/ObjectListComparer.cs → src/EFCore/ChangeTracking/ListOfReferenceTypesComparer.cs

@@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking;
 /// </remarks>
 /// <typeparam name="TConcreteCollection">The collection type to create an index of, if needed.</typeparam>
 /// <typeparam name="TElement">The element type.</typeparam>
-public sealed class ObjectListComparer<TConcreteCollection, TElement> : ValueComparer<object>
+public sealed class ListOfReferenceTypesComparer<TConcreteCollection, TElement> : ValueComparer<object>
     where TElement : class
 {
     private static readonly bool IsArray = typeof(TConcreteCollection).IsArray;
@@ -33,7 +33,7 @@ public sealed class ObjectListComparer<TConcreteCollection, TElement> : ValueCom
     ///     Creates a new instance of the list comparer.
     /// </summary>
     /// <param name="elementComparer">The comparer to use for comparing elements.</param>
-    public ObjectListComparer(ValueComparer elementComparer)
+    public ListOfReferenceTypesComparer(ValueComparer elementComparer)
         : base(
             (a, b) => Compare(a, b, elementComparer),
             o => GetHashCode((IEnumerable)o, elementComparer),

src/EFCore/ChangeTracking/ListComparer.cs → src/EFCore/ChangeTracking/ListOfValueTypesComparer.cs

@@ -12,15 +12,15 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking;
 /// <remarks>
 ///     <para>
 ///         This comparer should be used for reference types and non-nullable value types. Use
-///         <see cref="NullableValueTypeListComparer{TConcreteCollection, TElement}" /> for nullable value types.
+///         <see cref="ListOfNullableValueTypesComparer{TConcreteCollection,TElement}" /> for nullable value types.
 ///     </para>
 ///     <para>
 ///         See <see href="https://aka.ms/efcore-docs-value-comparers">EF Core value comparers</see> for more information and examples.
 ///     </para>
 /// </remarks>
 /// <typeparam name="TConcreteCollection">The collection type to create an index of, if needed.</typeparam>
 /// <typeparam name="TElement">The element type.</typeparam>
-public sealed class ListComparer<TConcreteCollection, TElement> : ValueComparer<IEnumerable<TElement>>
+public sealed class ListOfValueTypesComparer<TConcreteCollection, TElement> : ValueComparer<IEnumerable<TElement>>
     where TElement : struct
 {
     private static readonly bool IsArray = typeof(TConcreteCollection).IsArray;
@@ -33,7 +33,7 @@ public sealed class ListComparer<TConcreteCollection, TElement> : ValueComparer<
     ///     Creates a new instance of the list comparer.
     /// </summary>
     /// <param name="elementComparer">The comparer to use for comparing elements.</param>
-    public ListComparer(ValueComparer elementComparer)
+    public ListOfValueTypesComparer(ValueComparer elementComparer)
         : base(
             (a, b) => Compare(a, b, (ValueComparer<TElement>)elementComparer),
             o => GetHashCode(o, (ValueComparer<TElement>)elementComparer),

src/EFCore/Design/Internal/CSharpRuntimeAnnotationCodeGenerator.cs

@@ -418,7 +418,7 @@ public static void Create(
         var mainBuilder = parameters.MainBuilder;
 
         var constructor = comparer.GetType().GetDeclaredConstructor([typeof(ValueComparer)]);
-        var elementComparerProperty = comparer.GetType().GetProperty(nameof(ListComparer<object, int>.ElementComparer));
+        var elementComparerProperty = comparer.GetType().GetProperty(nameof(ListOfValueTypesComparer<object, int>.ElementComparer));
         if (constructor == null
             || elementComparerProperty == null)
         {

src/EFCore/Storage/Json/JsonCollectionOfReferencesReaderWriter.cs

@@ -13,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Storage.Json;
 /// <typeparam name="TConcreteCollection">The collection type to create an index of, if needed.</typeparam>
 /// <typeparam name="TElement">The element type.</typeparam>
 public class JsonCollectionOfReferencesReaderWriter<TConcreteCollection, TElement> :
-    JsonValueReaderWriter<object?>,
+    JsonValueReaderWriter<object>,
     ICompositeJsonValueReaderWriter
     where TElement : class?
 {
@@ -70,6 +70,7 @@ public override object FromJsonTyped(ref Utf8JsonReaderManager manager, object?
                 case JsonTokenType.Number:
                 case JsonTokenType.True:
                 case JsonTokenType.False:
+                case JsonTokenType.StartArray:
                     collection.Add((TElement)_elementReaderWriter.FromJson(ref manager));
                     break;
                 case JsonTokenType.Null:
@@ -78,9 +79,6 @@ public override object FromJsonTyped(ref Utf8JsonReaderManager manager, object?
                 case JsonTokenType.Comment:
                 case JsonTokenType.EndArray:
                     break;
-                case JsonTokenType.StartArray:
-                    collection.Add((TElement)_elementReaderWriter.FromJson(ref manager));
-                    break;
                 case JsonTokenType.None: // Explicitly listing all states that we throw for
                 case JsonTokenType.StartObject:
                 case JsonTokenType.EndObject:

src/EFCore/Storage/TypeMappingSourceBase.cs

@@ -177,10 +177,10 @@ protected virtual bool TryFindJsonCollectionMapping(
 
                 elementComparer = (ValueComparer?)Activator.CreateInstance(
                     elementType.IsNullableValueType()
-                        ? typeof(NullableValueTypeListComparer<,>).MakeGenericType(typeToInstantiate, elementType.UnwrapNullableType())
+                        ? typeof(ListOfNullableValueTypesComparer<,>).MakeGenericType(typeToInstantiate, elementType.UnwrapNullableType())
                         : elementType.IsValueType
-                            ? typeof(ListComparer<,>).MakeGenericType(typeToInstantiate, elementType)
-                            : typeof(ObjectListComparer<,>).MakeGenericType(typeToInstantiate, elementType),
+                            ? typeof(ListOfValueTypesComparer<,>).MakeGenericType(typeToInstantiate, elementType)
+                            : typeof(ListOfReferenceTypesComparer<,>).MakeGenericType(typeToInstantiate, elementType),
                     elementMapping.Comparer.ToNullableComparer(elementType)!);
 
                 return true;

test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs

@@ -870,6 +870,7 @@ await Can_add_update_delete_with_collection(
             },
             new List<byte?[]> { new byte?[] { 3, null }, null });
 
+        // TODO: Dictionary mapping Issue #29825
         // await Can_add_update_delete_with_collection<IReadOnlyList<Dictionary<string, string>>>(
         //     new Dictionary<string, string>[] { new() { { "1", null } } },
         //     c =>
@@ -895,6 +896,7 @@ await Can_add_update_delete_with_collection(
             },
             new[] { new decimal?[] { 1, 3 } });
 
+        // TODO: Dictionary mapping Issue #29825
         // await Can_add_update_delete_with_collection(
         //     new Dictionary<string, List<int>> { { "1", [1] } },
         //     c =>
@@ -903,6 +905,7 @@ await Can_add_update_delete_with_collection(
         //     },
         //     new Dictionary<string, List<int>> { { "1", [1] }, { "2", [3] } });
 
+        // TODO: Dictionary mapping Issue #29825
         // await Can_add_update_delete_with_collection<IDictionary<string, long?[]>>(
         //     new SortedDictionary<string, long?[]> { { "2", [2] }, { "1", [1] } },
         //     c =>

test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs

@@ -1035,6 +1035,32 @@ public class ManyTypes
         public EnumU32?[] NullableEnumU32AsStringArray { get; set; } = null!;
         public EnumU64?[] NullableEnumU64AsStringArray { get; set; } = null!;
 
+        public bool[][] BoolNestedCollection { get; set; } = null!;
+        public List<byte[]> UInt8NestedCollection { get; set; } = null!;
+        public sbyte[][][] Int8NestedCollection { get; set; } = null!;
+        public int[][] Int32NestedCollection { get; set; } = null!;
+        public IList<long[]>[] Int64NestedCollection { get; set; } = null!;
+        public char[][] CharNestedCollection { get; set; } = null!;
+        public ICollection<Guid[][]> GuidNestedCollection { get; set; } = null!;
+        public string[][] StringNestedCollection { get; set; } = null!;
+        public byte[][][] BytesNestedCollection { get; set; } = null!;
+
+        public byte?[][] NullableUInt8NestedCollection { get; set; } = null!;
+        public int?[][] NullableInt32NestedCollection { get; set; } = null!;
+        public List<long?[][]> NullableInt64NestedCollection { get; set; } = null!;
+        public Guid?[][] NullableGuidNestedCollection { get; set; } = null!;
+        public string?[][] NullableStringNestedCollection { get; set; } = null!;
+        public byte[]?[][] NullableBytesNestedCollection { get; set; } = null!;
+        public IEnumerable<PhysicalAddress?[][]> NullablePhysicalAddressNestedCollection { get; set; } = null!;
+
+        public Enum8[][] Enum8NestedCollection { get; set; } = null!;
+        public List<Enum32>[][] Enum32NestedCollection { get; set; } = null!;
+        public EnumU64[][] EnumU64NestedCollection { get; set; } = null!;
+
+        public Enum8?[][] NullableEnum8NestedCollection { get; set; } = null!;
+        public Enum32?[][][] NullableEnum32NestedCollection { get; set; } = null!;
+        public EnumU64?[][] NullableEnumU64NestedCollection { get; set; } = null!;
+
         public bool BoolToStringConverterProperty { get; set; }
         public bool BoolToTwoValuesConverterProperty { get; set; }
         public bool BoolToZeroOneConverterProperty { get; set; }