Give a nested error of a whole object the path of the object

CHANGELOG.md

@@ -30,6 +30,10 @@ Breaking changes are marked **BREAKING**.
   - 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`.
   - 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** An error of a whole nested object now has the path of the object as its member name. This covers a class-level attribute and an `IValidatableObject.Validate` result with no member names, or with a null or empty one. Before, such an error had no member names once nested, so a caller could not tell which object failed, and a null or empty name gave the path with a dot at the end. Change any code that matches these names:
+  - No member names: none is now `Range`, or `Items[1]` for an item.
+  - A null or empty member name: `Value[0].` is now `Value[0]`.
+  - An error of the root object keeps its member names, as before.
 - **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

@@ -22,6 +22,10 @@ Usage of the recursive validation is nearly identical to using the standard vali
 
 There are more examples in the [example](https://github.com/tgharold/RecursiveDataAnnotationsValidation/tree/master/examples) and [test](https://github.com/tgharold/RecursiveDataAnnotationsValidation/tree/master/test) projects.
 
+### Member names
+
+The member name of a nested error is the path from the root object: `Customer.Address.Zip`, `Lines[1].Quantity`. The message is the one that the nested object produced, so it names only its own property. An error of a whole nested object, such as one from a class-level attribute or from `IValidatableObject.Validate` with no member names, gets the path of the object as its member name: `Lines[1]`. An error of the root object keeps the member names it has, so a class-level error of the root has none.
+
 ### SkipRecursiveValidationAttribute
 
 The [`[SkipRecursiveValidation]`](https://github.com/tgharold/RecursiveDataAnnotationsValidation/blob/master/src/RecursiveDataAnnotationsValidation/Attributes/SkipRecursiveValidation.cs) attribute can be used on properties where you do not want to recursively validate.  An example of this can be seen in [SkippedChildrenExample.cs](https://github.com/tgharold/RecursiveDataAnnotationsValidation/blob/master/test/RecursiveDataAnnotationsValidation.Tests/TestModels/SkippedChildrenExample.cs).

src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs

@@ -360,7 +360,8 @@ private bool Walk(WorkItem item)
             //object goes to the caller's list as it is. A result of any other object gets its member
             //names prefixed by the path of the object, so the names are the full path from the root.
             //The root object has no path, so its results keep their member names. A result that has no
-            //member names, such as an error of the whole object, stays without names.
+            //member names, or a null or empty one, is an error of the whole object, such as one from a
+            //class-level attribute, so it gets the path of the object itself (see MemberNames).
             private bool Validate(WorkItem item)
             {
                 if (item.Path == null)
@@ -377,13 +378,29 @@ private bool Validate(WorkItem item)
                 var path = item.Path.ToString();
                 foreach (var validationResult in results)
                 {
-                    var memberNames = validationResult.MemberNames.Select(x => path + "." + x).ToList();
-                    validationResults.Add(new ValidationResult(validationResult.ErrorMessage, memberNames));
+                    validationResults.Add(new ValidationResult(validationResult.ErrorMessage, MemberNames(validationResult, path)));
                 }
 
                 return false;
             }
 
+            //The member names of a nested result, as paths from the root. A null or empty name means
+            //the object itself, so it becomes the path alone ("Lines[1]"), and not the path with a dot
+            //and nothing after it. A result with no names at all gets the path as its only name.
+            private static List<string> MemberNames(ValidationResult validationResult, string path)
+            {
+                var memberNames = validationResult.MemberNames
+                    .Select(x => string.IsNullOrEmpty(x) ? path : path + "." + x)
+                    .ToList();
+
+                if (memberNames.Count == 0)
+                {
+                    memberNames.Add(path);
+                }
+
+                return memberNames;
+            }
+
             private bool TryValidateObject(object obj, ICollection<ValidationResult> results)
             {
                 return Validator.TryValidateObject(

test/RecursiveDataAnnotationsValidation.Tests/MemberNameFormatTests.cs

@@ -12,6 +12,10 @@ namespace RecursiveDataAnnotationsValidation.Tests
     /// - The error message is the one the attribute produced on the nested object, unchanged,
     ///   so it names the nested property only ("The Text field is required."), not the path.
     /// - Each error appears once, with one member name.
+    /// - A nested result that names no member is an error of the whole object, such as one from
+    ///   a class-level attribute or from IValidatableObject.Validate. Since 3.0 its member name is
+    ///   the path of the object: "Lines[1]". Before 3.0 it had no member names, so a caller could
+    ///   not tell which object failed. See ObjectLevelResults.
     /// The test compares the complete set of results, so an extra, missing or reworded result
     /// fails it. It sorts both sides first, because reflection does not promise a property order.
     /// See: https://learn.microsoft.com/dotnet/api/system.type.getproperties
@@ -134,9 +138,8 @@ public class Container
         // - A dictionary item is a KeyValuePair, so its value is reached through ".Value", and the
         //   index is the position in enumeration order, not the key.
         // - A result with two member names gets the prefix on each of them.
-        // - A result with no member names has no path at all once nested. This is a known gap
-        //   (see ValidatorHardeningTests.PrimitiveCollections). If it is fixed, this guard fails
-        //   on purpose, because the format callers see changes.
+        // - A result with no member names gets the path of the item as its member name.
+        //   Before 3.0 it had no path at all.
         // See: https://learn.microsoft.com/dotnet/api/system.collections.generic.keyvaluepair-2
         [Fact]
         public void Collection_items_keep_their_member_names()
@@ -159,10 +162,106 @@ public void Collection_items_keep_their_member_names()
                 "NoteList[1].Text | The Text field is required.",
                 "NoteMap[0].Value.Text | The Text field is required.",
                 "Ranges[0].Low,Ranges[0].High | Low must not exceed High.",
-                " | The range is negative.",
+                "Ranges[0] | The range is negative.",
             };
             var actual = results.Select(r => $"{string.Join(",", r.MemberNames)} | {r.ErrorMessage}");
             Assert.Equal(expected.OrderBy(x => x), actual.OrderBy(x => x));
         }
+
+        /// <summary>
+        /// Results that belong to a whole object and not to one of its members. Validator gives
+        /// such a result no member names when it comes from a class-level ValidationAttribute.
+        /// IValidatableObject.Validate gives none when it calls the ValidationResult constructor
+        /// with only a message. A class-level attribute that passes ValidationContext.MemberName
+        /// gives a null name, because no member is being validated, and some code passes "".
+        /// All of these mean "this object". On a nested object, each of them is reported with the
+        /// path of the object as its only member name, as MVC keys a model-level error by the
+        /// prefix of the model. A result of the root object keeps its member names, because the
+        /// root has no path.
+        /// Before 3.0 a nested result with no member names had none, and a null or empty name gave
+        /// the path with a dot and nothing after it, such as "Value[0].".
+        /// See: https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.validationresult.-ctor
+        /// See: https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.validationcontext.membername
+        /// </summary>
+        public class ObjectLevelResults
+        {
+            public class SelfValidating : IValidatableObject
+            {
+                public string[] MemberNames { get; set; }
+
+                public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
+                {
+                    yield return MemberNames == null
+                        ? new ValidationResult("The object is not valid.")
+                        : new ValidationResult("The object is not valid.", MemberNames);
+                }
+            }
+
+            public class Holder
+            {
+                public SelfValidating Inner { get; set; }
+
+                public List<SelfValidating> Items { get; set; }
+            }
+
+            private static List<string> Run(object model)
+            {
+                var results = new List<ValidationResult>();
+                Assert.False(new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(model, results));
+                return ResultText.Describe(results);
+            }
+
+            [Fact]
+            public void Result_with_no_member_names_gets_the_path_of_the_object()
+            {
+                var errors = Run(new Holder { Inner = new SelfValidating() });
+
+                Assert.Equal(ResultText.Expect("Inner | The object is not valid."), errors);
+            }
+
+            [Fact]
+            public void Result_with_no_member_names_on_an_item_gets_the_path_of_the_item()
+            {
+                var errors = Run(new Holder { Items = new List<SelfValidating> { null, new SelfValidating() } });
+
+                Assert.Equal(ResultText.Expect("Items[1] | The object is not valid."), errors);
+            }
+
+            [Fact]
+            public void Result_with_no_member_names_on_an_item_of_a_root_collection_gets_its_index()
+            {
+                var errors = Run(new List<SelfValidating> { new SelfValidating() });
+
+                Assert.Equal(ResultText.Expect("[0] | The object is not valid."), errors);
+            }
+
+            [Theory]
+            [InlineData(null)]
+            [InlineData("")]
+            public void Null_or_empty_member_name_gets_the_path_of_the_object(string name)
+            {
+                var errors = Run(new Holder { Inner = new SelfValidating { MemberNames = new[] { name } } });
+
+                Assert.Equal(ResultText.Expect("Inner | The object is not valid."), errors);
+            }
+
+            // A result that names a member and the object at once keeps both, each with the path.
+            [Fact]
+            public void Named_and_unnamed_members_in_one_result_each_get_the_path()
+            {
+                var errors = Run(new Holder { Inner = new SelfValidating { MemberNames = new[] { "Low", null } } });
+
+                Assert.Equal(ResultText.Expect("Inner.Low,Inner | The object is not valid."), errors);
+            }
+
+            // Guard. The root object has no path, so its results keep their member names.
+            [Fact]
+            public void Result_of_the_root_object_keeps_no_member_names()
+            {
+                var errors = Run(new SelfValidating());
+
+                Assert.Equal(ResultText.Expect(" | The object is not valid."), errors);
+            }
+        }
     }
 }

test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs

@@ -485,12 +485,14 @@ public void Jagged_array_is_validated()
         /// - A class-level attribute that passes ValidationContext.MemberName gives a null member
         ///   name, because no member is being validated. This is the usual way to write one.
         ///   The same holds for an IValidatableObject that yields a null member name.
-        ///   The validator prefixes the path to each name, and a null name gives "Value[0]." with
-        ///   nothing after the dot. Code that builds the path must not throw on null.
+        ///   A null name means the item itself, so since 3.0 the member name is the path of the
+        ///   item: "Value[0]". Up to 2.3.3 the validator prefixed the path to each name, and a null
+        ///   name gave "Value[0]." with nothing after the dot. Code that builds the path must not
+        ///   throw on null. See also MemberNameFormatTests.ObjectLevelResults.
         /// - A member name that starts with "[", such as "[Totals]", is a name the item chose. It
         ///   is not the index of a nested collection, so the path keeps the dot: "Value[0].[Totals]".
-        /// The tests also run on 2.3.3, which gives the same paths. Release 3.0 first read the
-        /// name to decide whether it was an index, and threw NullReferenceException on null.
+        /// The bracket tests also run on 2.3.3, which gives the same paths. An early 3.0 build read
+        /// the name to decide whether it was an index, and threw NullReferenceException on null.
         /// See: https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.validationcontext.membername
         /// </summary>
         public class UnusualMemberNames
@@ -523,7 +525,7 @@ public void Null_member_name_from_a_class_level_attribute_on_an_item()
                 var (valid, errors) = Run(new Holder<List<ClassLevelItem>> { Value = new List<ClassLevelItem> { new ClassLevelItem() } });
 
                 Assert.False(valid);
-                Assert.Equal(ResultText.Expect("Value[0]. | The item is not valid."), errors);
+                Assert.Equal(ResultText.Expect("Value[0] | The item is not valid."), errors);
             }
 
             [Fact]
@@ -532,10 +534,10 @@ public void Null_member_name_from_Validate_on_an_item()
                 var (valid, errors) = Run(new Holder<List<SelfValidatingItem>> { Value = new List<SelfValidatingItem> { new SelfValidatingItem() } });
 
                 Assert.False(valid);
-                Assert.Equal(ResultText.Expect("Value[0]. | The item is not valid."), errors);
+                Assert.Equal(ResultText.Expect("Value[0] | The item is not valid."), errors);
             }
 
-            // In a nested collection the path has the index of each level, then the empty name.
+            // In a nested collection the path has the index of each level.
             // Nested collections are enumerated from 3.0, so 2.3.3 reports nothing here.
             [Fact]
             public void Null_member_name_on_an_item_of_a_nested_collection()
@@ -546,7 +548,7 @@ public void Null_member_name_on_an_item_of_a_nested_collection()
                 });
 
                 Assert.False(valid);
-                Assert.Equal(ResultText.Expect("Value[0][0]. | The item is not valid."), errors);
+                Assert.Equal(ResultText.Expect("Value[0][0] | The item is not valid."), errors);
             }
 
             // A name that starts with a bracket on an item of a nested collection is still a name.

test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs

@@ -160,15 +160,16 @@ public void Nested_objects_are_validated_when_the_root_fails()
 
             /// <summary>
             /// A ValidationAttribute on a class runs with no member name, so its result has none.
-            /// Once nested, the result still has no member names, so it carries no path.
+            /// Once nested, the result gets the path of the object as its member name.
+            /// Changed in 3.0: up to 2.3.3 the result had no member names, so it carried no path.
             /// </summary>
             [Fact]
-            public void Class_level_attribute_on_a_nested_object_has_no_member_name()
+            public void Class_level_attribute_on_a_nested_object_is_reported_at_its_path()
             {
                 var holder = new Holder { Range = new DateRange { Start = 5, End = 1 } };
 
                 Assert.False(Validate(holder, out var results));
-                Assert.Equal(ResultText.Expect(" | Start must not be after End."), results);
+                Assert.Equal(ResultText.Expect("Range | Start must not be after End."), results);
             }
 
             [AttributeUsage(AttributeTargets.Class)]

test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs

@@ -902,9 +902,9 @@ public void Stream_property_does_not_throw()
         ///
         /// Accepted gap: a type whose non-generic enumerator yields different items than its
         /// IEnumerable<T> breaks the IEnumerable<T> contract. It is skipped by its declared type.
-        /// Known gap, not changed here: a type-level error on an item, such as the enum attribute
-        /// above, has no member names. The validator builds each path by prefixing the item's
-        /// member names, so that error is reported with no path at all.
+        /// A type-level error on an item, such as the enum attribute above, has no member names.
+        /// Since 3.0 the validator reports it with the path of the item as its member name, such
+        /// as "Items[1]". Before, it had no path at all.
         /// Not solved here: lazy or infinite sequences of objects, and lazy queryables of objects
         /// that hit a database. Those need a separate decision.
         /// </summary>
@@ -1065,15 +1065,15 @@ public void Collections_of_objects_are_still_enumerated()
             }
 
             // Guard. An enum is a leaf type, but this one has a type-level validation attribute
-            // in its source. Each item must still be validated. The error has no member names,
-            // so this test checks the count only (see the class summary).
+            // in its source. Each item must still be validated. The error belongs to the item
+            // itself, so its member name is the path of the item (see the class summary).
             [Fact]
             public void Collections_of_enums_with_a_validation_attribute_are_still_validated()
             {
                 var (valid, results, enumerationCount) = Validate(CheckedColor.Red, (CheckedColor)99);
 
                 Assert.False(valid);
-                Assert.Single(results);
+                Assert.Equal(new[] { "Items[1]" }, Assert.Single(results).MemberNames);
                 Assert.Equal(1, enumerationCount);
             }