Enumerate a collection that is passed as the root object

CHANGELOG.md

@@ -17,6 +17,7 @@ Breaking changes are marked **BREAKING**.
 - A struct collection that a property holds, such as `ImmutableArray<T>` or a struct of your own that implements `IEnumerable<T>`, is now validated. Before, the validator skipped every property whose type is a struct, so an invalid object inside passed, although the same struct was validated as an item of a list. The error is reported like one for a class collection, for example `Lines[0].Sku`. A model that passed before can fail now. This includes an item of a list, or the value of a dictionary, that holds such a struct. It was skipped before as having nothing to walk. A struct property that is not a collection, such as a `Money`, is still not walked.
 - Objects inside an item that is itself a collection are now validated. Before, a `List<List<T>>`, a `List<HashSet<T>>`, an `object[]` that holds a list, and a list of a sealed or struct collection such as `ImmutableList<T>` passed validation even when an object inside was invalid. The error is reported with the index of each level, for example `Value[0][0].Name`. A model that passed before can fail now.
 - An item that is a collection is still validated as an object first, so its own attributes and `IValidatableObject.Validate` run, as before. Its items are then validated as well.
+- A collection that you pass to the validator as the root object, such as a `List<T>`, is now enumerated. Before, its items were not validated, so the list passed even when an item was invalid. The error starts with the index of the item, for example `[1].Name`. The collection is still validated as an object first, so its own attributes run. A model that passed before can fail now.
 - A struct item, or a property declared as a struct, that is its default value, such as an `ImmutableArray<T>` or an `ArraySegment<T>` that nobody set, is skipped, because it holds nothing and enumerating it throws. The validator compares the memory of the struct with its default, and does not call the `Equals` of your struct, which can say "equal" for a struct that holds objects, or throw.
 
 ### Changed
@@ -27,7 +28,8 @@ Breaking changes are marked **BREAKING**.
   - `LinkedList<T>`: `Value[0].First.List[1].Name` is now `Value[0][1].Name`.
   - `SortedSet<T>`: `Value[0].Min.Name` is now `Value[0][0].Name`.
   - A collection of your own with a property that returns its items, such as `View`: `Value[0].View[0].Name` is now `Value[0][0].Name`.
-- **BREAKING** An item that is a collection is now enumerated, so a lazy sequence in an item runs, as it does when a property holds it. An item that throws when enumerated now throws from validation, and an item that never ends makes validation hang. The same holds for a struct collection that a property holds, such as `ImmutableArray<T>`: one that throws when enumerated now throws from validation, and one that builds new objects on each read stops at the maximum depth and fails the validation. To avoid it, mark the property that holds the collection with `[SkipRecursiveValidation]`.
+  - An array or `ArrayList` passed as the root object: `SyncRoot[0].Name` is now `[0].Name`.
+- **BREAKING** An item that is a collection is now enumerated, so a lazy sequence in an item runs, as it does when a property holds it. An item that throws when enumerated now throws from validation, and an item that never ends makes validation hang. The same holds for a struct collection that a property holds, such as `ImmutableArray<T>`: one that throws when enumerated now throws from validation, and one that builds new objects on each read stops at the maximum depth and fails the validation. To avoid it, mark the property that holds the collection with `[SkipRecursiveValidation]`. A collection passed as the root object is now enumerated too, so a lazy sequence passed in runs during validation.
 - **BREAKING** The validator walks the graph breadth first, not depth first. Change any code that depends on the order of the results or on the member name of an object that two paths reach:
   - The results come shallowest first. The errors of one object stay together and in the same order. An item of a collection that a property holds is two levels below the object, so for a model with `First`, `Items` and `Last`, the order was `First`, `Items[0]`, `Items[1]`, `Last` and is now `First`, `Last`, `Items[0]`, `Items[1]`. Sort the list if you need a fixed order.
   - An object that two paths reach is reported with the shortest path. Before, it was the first path in property order. If two paths are equally short, the first property still wins. In a graph with links back to parents, such as the entities of an ORM, the member names are now much shorter. For example, in a store where each order shares a product with the next order, the second order in `Store.Orders` was reported as `Orders[0].Lines[1].Product.Lines[1].Order.Code` when a longer path reached it first, and is now `Orders[1].Code`.

README.md

@@ -56,7 +56,7 @@ An object that several paths reach is validated once, and its errors name the sh
 - An item that is itself a collection is validated too, at each level: `Matrix[0][2].Name`. This holds for lists, arrays, sets, dictionaries and your own collection types. A dictionary is enumerated as `KeyValuePair` items, so a value that is an object is reported as `Map[0].Value.Name`. A value that is a struct collection is validated, as it is when a property holds it (see below). A value that is any other struct, such as a `Money`, is not walked, because the validator reads only properties of reference types.
 - An item that is a collection is validated as an object first, so its own attributes and `IValidatableObject.Validate` run, then its items.
 - A collection of simple values, such as `List<int>` or `string[]`, is not enumerated, because it cannot hold an invalid object.
-- A collection that you pass to the validator as the root object is not enumerated. Wrap it in an object with a property.
+- A collection that you pass to the validator as the root object is validated as an object, then its items are validated. The error starts with the index of the item: `[1].Name`.
 - A collection that is a struct, such as `ImmutableArray<T>`, is validated when a property holds it and when it is an item of another collection: `Lines[0].Sku`. The validator walks a property whose type is a class, or a struct that is a collection of objects. A struct that is not a collection, such as a `Money`, a tuple or a `KeyValuePair`, is not walked, because the validator only reads properties of reference types.
 - The validator runs each collection it enumerates, so a lazy sequence, such as a LINQ query or an iterator, runs during validation. A sequence that never ends makes validation hang. The validator does not catch exceptions, so an exception that a collection throws when it is enumerated reaches your code. A struct collection that is its default value, such as an `ImmutableArray<T>` or an `ArraySegment<T>` that nobody set, is skipped when it is an item or when the property is declared as that struct, because it holds nothing and enumerating it throws. The validator compares the memory of the struct with its default. It does not call the `Equals` of your struct. A property declared as an interface or as `object` is always enumerated.
 

src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs

@@ -171,7 +171,7 @@ public bool Run(object root)
                 }
 
                 queuedObjects.Add(root);
-                queue.Enqueue(new WorkItem(root, null, 0, null, false, false, false));
+                queue.Enqueue(new WorkItem(root, null, 0, null, false, false));
 
                 var valid = true;
                 while (queue.Count > 0)
@@ -199,8 +199,7 @@ private void Enqueue(
                 string propertyName,
                 int index,
                 int depth,
-                EqualsAncestor ancestors,
-                bool enumerateItems
+                EqualsAncestor ancestors
                 )
             {
                 //an object of a leaf type can never produce a result, such as a boxed int in an object[] (see IsLeafType)
@@ -224,7 +223,6 @@ bool enumerateItems
                     new PathStep(parent, propertyName, index),
                     depth,
                     ancestors,
-                    enumerateItems,
                     false,
                     stopHere
                     ));
@@ -247,7 +245,7 @@ EqualsAncestor ancestors
                     //NOTE: Possibly should have a separate case for Dictionary which reports on the key
 
                     if (item == null) continue;
-                    Enqueue(item, collectionPath, null, arrayIndex, itemDepth, ancestors, true);
+                    Enqueue(item, collectionPath, null, arrayIndex, itemDepth, ancestors);
                 }
             }
 
@@ -291,15 +289,16 @@ private bool Walk(WorkItem item)
                 //an object that a shorter path stopped at was validated there, so only walk it now
                 var result = (stoppedObjects.Count > 0 && stoppedObjects.Contains(obj)) || Validate(item);
 
-                //An item that is a collection is validated as an object above, so its own attributes run.
-                //Then its items are queued, before its properties. An object that the collection also
-                //returns from a property, such as Array.SyncRoot, is then reported at its index
-                //(Value[0][0]) and not through the property (Value[0].SyncRoot[0]).
+                //A collection that gets here is the root object or an item of another collection. It is
+                //validated as an object above, so its own attributes run. Then its items are queued,
+                //before its properties. An object that the collection also returns from a property,
+                //such as Array.SyncRoot, is then reported at its index (Value[0][0], or [0] for the root)
+                //and not through the property (Value[0].SyncRoot[0]). A collection that a property
+                //holds never gets here: it waits in the queue as a step of its own (see below).
                 //A collection of leaf types is skipped, like a collection that a property holds. A default
                 //struct, such as an ImmutableArray nobody set, is skipped: it holds nothing and enumerating it throws.
                 var enumeratedItems = false;
-                if (item.EnumerateItems
-                    && obj is IEnumerable items
+                if (obj is IEnumerable items
                     && !type.IsCollectionOfLeafType()
                     && !obj.IsDefaultStruct())
                 {
@@ -343,14 +342,13 @@ private bool Walk(WorkItem item)
                                 new PathStep(item.Path, property.Name, -1),
                                 item.Depth + 1,
                                 ancestors,
-                                false,
                                 true,
                                 false
                                 ));
                             break;
 
                         default:
-                            Enqueue(value, item.Path, property.Name, -1, item.Depth + 1, ancestors, false);
+                            Enqueue(value, item.Path, property.Name, -1, item.Depth + 1, ancestors);
                             break;
                     }
                 }
@@ -425,17 +423,13 @@ private static bool EqualsAnAncestor(object obj, Type type, EqualsAncestor ances
         //IsCollectionOfProperty is true for the collection that a property holds. It is not validated
         //as an object. Its items are queued when it comes out of the queue.
         //Ancestors are the objects on the path whose type overrides Equals, the nearest one first.
-        //EnumerateItems is true for an item of a collection. If the item is itself a collection, its
-        //items are walked too. A collection that a property holds is enumerated by the object that
-        //holds it, and the root object is never enumerated.
         private sealed class WorkItem
         {
             public WorkItem(
                 object value,
                 PathStep path,
                 int depth,
                 EqualsAncestor ancestors,
-                bool enumerateItems,
                 bool isCollectionOfProperty,
                 bool equalsAnAncestor
                 )
@@ -445,15 +439,13 @@ bool equalsAnAncestor
                 Path = path;
                 Depth = depth;
                 Ancestors = ancestors;
-                EnumerateItems = enumerateItems;
                 IsCollectionOfProperty = isCollectionOfProperty;
             }
 
             public object Value { get; }
             public PathStep Path { get; }
             public int Depth { get; }
             public EqualsAncestor Ancestors { get; }
-            public bool EnumerateItems { get; }
             public bool IsCollectionOfProperty { get; }
             public bool EqualsAnAncestor { get; }
         }

test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs

@@ -29,7 +29,7 @@ namespace RecursiveDataAnnotationsValidation.Tests
     /// - A spec. It is not skipped and fails until the fix lands. It states the behavior the fix gives.
     /// Every result below is the same on release 2.2.0 and on the current code, except the
     /// framework types in MembersThatThrow, which are no longer walked, and the collections in
-    /// CollectionsInsideCollections, which 3.0 enumerates.
+    /// CollectionsInsideCollections and CollectionAsRootObject, which 3.0 enumerates.
     /// Not covered here, because it stops the test run: a Task that has not completed makes the
     /// walk read Task.Result, which waits forever. The open decision, with skipped specs, is in
     /// TaskPropertyTests.
@@ -580,31 +580,98 @@ public void Member_name_that_starts_with_a_bracket_keeps_the_dot()
         }
 
         /// <summary>
-        /// A collection as the object passed to the validator. The validator validates that object
-        /// and walks its properties, but never enumerates it, because an item is only reached
-        /// through a property. So a List that is the root hides its items, and a root array reports
-        /// its items through SyncRoot. This is not changed by the fix for nested collections.
-        /// These are limitation guards. If the validator learns to enumerate a root collection,
-        /// the tests fail on purpose, so the change is deliberate. The path format for it is open.
+        /// A collection as the object passed to the validator. Up to 3.0, the validator validated
+        /// that object and walked its properties, but never enumerated it, because an item was
+        /// only reached through a property. So a List that is the root hid its items and passed,
+        /// and a root array reported its items through SyncRoot: "SyncRoot[0].Name".
+        /// Release 3.0 enumerates a root collection like an item that is a collection. The path of
+        /// an item starts with its index, and has no name in front: "[0].Name". MVC model binding
+        /// uses the same format for a collection that is bound as the top-level object.
+        /// The root collection is still validated as an object first, so its own attributes run,
+        /// and their member names have no path, as for any root object.
+        /// See: https://learn.microsoft.com/aspnet/core/mvc/models/model-binding#collections
         /// </summary>
         public class CollectionAsRootObject
         {
+            public class PagedList<T> : List<T>
+            {
+                [Required]
+                public string Cursor { get; set; }
+            }
+
+            [Fact]
+            public void Items_of_a_root_list_are_validated()
+            {
+                var (valid, errors) = Run(new List<Leaf> { new Leaf { Name = "n" }, new Leaf() });
+
+                Assert.False(valid);
+                Assert.Equal(ResultText.Expect("[1].Name" + NameRequired), errors);
+            }
+
+            // An array returns itself from SyncRoot. The items are enumerated first, so the path
+            // no longer goes through SyncRoot.
             [Fact]
-            public void Items_of_a_root_list_are_not_validated()
+            public void Items_of_a_root_array_are_validated_at_their_index()
             {
-                var (valid, errors) = Run(new List<Leaf> { new Leaf() });
+                var (valid, errors) = Run(new[] { new Leaf() });
+
+                Assert.False(valid);
+                Assert.Equal(ResultText.Expect("[0].Name" + NameRequired), errors);
+            }
+
+            // A dictionary is enumerated as KeyValuePair items, as when a property holds it.
+            [Fact]
+            public void Values_of_a_root_dictionary_are_validated()
+            {
+                var (valid, errors) = Run(new Dictionary<string, Leaf> { ["a"] = new Leaf() });
+
+                Assert.False(valid);
+                Assert.Equal(ResultText.Expect("[0].Value.Name" + NameRequired), errors);
+            }
+
+            [Fact]
+            public void Items_of_a_root_list_of_lists_are_validated()
+            {
+                var (valid, errors) = Run(new List<List<Leaf>> { new List<Leaf> { new Leaf() } });
+
+                Assert.False(valid);
+                Assert.Equal(ResultText.Expect("[0][0].Name" + NameRequired), errors);
+            }
+
+            // The root collection's own attribute keeps its member name with no path, like any
+            // root object. Its items come one level later.
+            [Fact]
+            public void Root_collection_with_members_of_its_own_reports_both()
+            {
+                var page = new PagedList<Leaf> { Cursor = null };
+                page.Add(new Leaf());
+
+                var (valid, errors) = Run(page);
+
+                Assert.False(valid);
+                Assert.Equal(
+                    ResultText.Expect("Cursor | The Cursor field is required.", "[0].Name" + NameRequired),
+                    errors);
+            }
+
+            // Guard. A root collection of simple values has nothing to validate.
+            [Fact]
+            public void Root_list_of_strings_passes()
+            {
+                var (valid, errors) = Run(new List<string> { null, "a" });
 
                 Assert.True(valid);
                 Assert.Empty(errors);
             }
 
+            // Guard. A null item is skipped but still uses up its index, as in a property.
             [Fact]
-            public void Items_of_a_root_array_are_validated_through_SyncRoot()
+            public void Null_item_of_a_root_list_keeps_its_index()
             {
-                var (valid, errors) = Run(new[] { new Leaf() });
+                var (valid, errors) = Run(new List<Leaf> { null, new Leaf() });
 
                 Assert.False(valid);
-                Assert.Equal(ResultText.Expect("SyncRoot[0].Name" + NameRequired), errors);
+                Assert.Equal(ResultText.Expect("[1].Name" + NameRequired), errors);
             }
         }