From 4aad5b3bb767e4e66b7b6cf392b6275b257096ec Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:14:57 -0400 Subject: [PATCH 1/7] test: unskip the specs for struct collection properties Unskip the 8 specs, and add specs for an object shared by two struct collections and for [SkipRecursiveValidation]. They fail until the fix is added. The guards that pin the old gap still pass. Co-Authored-By: Claude Sonnet 5.5 --- .../StructCollectionPropertyTests.cs | 74 +++++++++++++++---- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs index 0c2e889..1ed4077 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs @@ -5,13 +5,14 @@ using System.Collections.Immutable; #endif using System.ComponentModel.DataAnnotations; +using RecursiveDataAnnotationsValidation.Attributes; using Xunit; namespace RecursiveDataAnnotationsValidation.Tests { /// - /// A collection that is a struct, held in a property. Not fixed in 2.4.0. The specs here are - /// skipped and describe the behavior planned for the next release. + /// A collection that is a struct, held in a property. The specs here describe the planned + /// behavior. The guards that pin today's gap pass until the fix lands, and the specs fail until then. /// /// The inconsistency. The validator walks only the properties of a reference type (see /// IsWalked), so a property whose type is a struct is never read. That includes a struct @@ -58,8 +59,6 @@ namespace RecursiveDataAnnotationsValidation.Tests /// public class StructCollectionPropertyTests { - private const string Reason = "Next release. A struct collection held in a property is not enumerated yet."; - public class Leaf { [Required] @@ -121,8 +120,17 @@ public class Counter public int Enumerations { get; set; } } + // The field makes a value that is not default(ThrowingStructBag). A struct with no fields + // always equals its default, and the validator skips a default struct (see IsDefaultStruct). public readonly struct ThrowingStructBag : IEnumerable { + private readonly int _marker; + + public ThrowingStructBag(int marker) + { + _marker = marker; + } + public IEnumerator GetEnumerator() => throw new InvalidOperationException("enumeration failed"); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); @@ -144,7 +152,7 @@ public void Struct_collection_is_validated_as_an_item_but_not_as_a_property() Assert.Empty(asProperty.Errors); } - [Fact(Skip = Reason)] + [Fact] public void Struct_collection_property_is_validated() { var (valid, errors) = Run(new Holder { Value = new LeafBag(new Leaf()) }); @@ -155,7 +163,7 @@ public void Struct_collection_property_is_validated() // A nullable struct is boxed as the struct itself when it has a value. // See: https://learn.microsoft.com/dotnet/csharp/language-reference/builtin-types/nullable-value-types#boxing-and-unboxing - [Fact(Skip = Reason)] + [Fact] public void Nullable_struct_collection_property_is_validated() { var (valid, errors) = Run(new Holder { Value = new LeafBag(new Leaf()) }); @@ -173,7 +181,7 @@ public void Nullable_struct_collection_property_without_a_value_is_valid() Assert.Empty(errors); } - [Fact(Skip = Reason)] + [Fact] public void Struct_collection_property_below_a_class_property_is_validated() { var (valid, errors) = Run(new Holder> @@ -185,7 +193,7 @@ public void Struct_collection_property_below_a_class_property_is_validated() Assert.Equal(ResultText.Expect("Value.Value[0].Name" + NameRequired), errors); } - [Fact(Skip = Reason)] + [Fact] public void Struct_collection_property_in_an_item_is_validated() { var (valid, errors) = Run(new Holder>> @@ -198,7 +206,7 @@ public void Struct_collection_property_in_an_item_is_validated() } // The error of each object is reported once, and the valid ones are not reported. - [Fact(Skip = Reason)] + [Fact] public void Only_the_invalid_items_of_a_struct_collection_property_are_reported() { var (valid, errors) = Run(new Holder @@ -210,6 +218,44 @@ public void Only_the_invalid_items_of_a_struct_collection_property_are_reported( Assert.Equal(ResultText.Expect("Value[2].Name" + NameRequired), errors); } + public class TwoBags + { + public LeafBag First { get; set; } + + public LeafBag Second { get; set; } + } + + public class SkippedBag + { + [SkipRecursiveValidation] + public LeafBag Value { get; set; } + } + + // The items of a struct collection are objects of a class, so the reference check applies to + // them as to any other object: one that two properties share is validated, and reported, once. + // Only an item that is itself a struct is reported once for each route (see + // StructItemsReportedTwiceTests). + [Fact] + public void Object_shared_by_two_struct_collection_properties_is_reported_once() + { + var shared = new Leaf(); + + var (valid, errors) = Run(new TwoBags { First = new LeafBag(shared), Second = new LeafBag(shared) }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("First[0].Name" + NameRequired), errors); + } + + // The opt-out works for a struct collection as for any other property. + [Fact] + public void Struct_collection_property_marked_to_skip_is_not_enumerated() + { + var (valid, errors) = Run(new SkippedBag { Value = new LeafBag(new Leaf()) }); + + Assert.True(valid); + Assert.Empty(errors); + } + // Guard. A struct collection of leaf types has nothing to validate, like a List of int, so // it is not enumerated. The count shows it, because the result is the same either way. [Fact] @@ -226,10 +272,10 @@ public void Struct_collection_of_values_is_not_enumerated() // A struct collection that throws when it is enumerated throws from a property, like an // item (NestedCollectionEdgeCaseTests, Case 1). It passes today, because it is not read. - [Fact(Skip = Reason)] + [Fact] public void Struct_collection_property_that_throws_when_enumerated_propagates() { - var ex = Record.Exception(() => Run(new Holder { Value = new ThrowingStructBag() })); + var ex = Record.Exception(() => Run(new Holder { Value = new ThrowingStructBag(1) })); Assert.IsType(ex); } @@ -237,14 +283,14 @@ public void Struct_collection_property_that_throws_when_enumerated_propagates() [Fact] public void Struct_collection_that_throws_when_enumerated_passes_today() { - var (valid, errors) = Run(new Holder { Value = new ThrowingStructBag() }); + var (valid, errors) = Run(new Holder { Value = new ThrowingStructBag(1) }); Assert.True(valid); Assert.Empty(errors); } #if NET8_0_OR_GREATER - [Fact(Skip = Reason)] + [Fact] public void Immutable_array_property_is_validated() { var (valid, errors) = Run(new Holder> { Value = ImmutableArray.Create(new Leaf()) }); @@ -253,7 +299,7 @@ public void Immutable_array_property_is_validated() Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), errors); } - [Fact(Skip = Reason)] + [Fact] public void Immutable_array_of_immutable_arrays_property_is_validated() { var (valid, errors) = Run(new Holder>> From 0e09005e305bccddc74ce81d82708e32003c1f8c Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:14:57 -0400 Subject: [PATCH 2/7] fix: validate a struct collection that a property holds The validator skipped every property of a struct type, so an invalid object inside an ImmutableArray property passed, while the same struct was validated as an item of a list. A property of a struct that is a collection of items that can have attributes is now enumerated. A default struct is skipped, as an item is. Delete the two guards that pinned the gap. Co-Authored-By: Claude Sonnet 5.5 --- .../Extensions/TypeExtensions.cs | 20 +++++- .../RecursiveDataAnnotationValidator.cs | 5 ++ .../NestedCollectionEdgeCaseTests.cs | 5 +- .../OddShapeTests.cs | 20 ++---- .../StructCollectionPropertyTests.cs | 70 +++++++------------ 5 files changed, 57 insertions(+), 63 deletions(-) diff --git a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs index 86376cb..4445104 100644 --- a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs +++ b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs @@ -121,13 +121,14 @@ public static bool IsLeafType(this Type type) /// /// True for a property the validator walks into: readable, not an indexer, not marked - /// [SkipRecursiveValidation], of a reference type other than string, and not declared by a - /// framework type whose properties are unsafe to read (see ). + /// [SkipRecursiveValidation], of a reference type other than string or of a struct that is a + /// collection of items that can have attributes, and not declared by a framework type whose + /// properties are unsafe to read (see ). /// public static bool IsWalked(this PropertyInfo property) { return property.PropertyType != typeof(string) - && !property.PropertyType.IsValueType + && IsWalkedType(property.PropertyType) && property.CanRead && property.GetIndexParameters().Length == 0 && !property.IsDefined(typeof(SkipRecursiveValidationAttribute), false) @@ -174,6 +175,19 @@ public static bool IsDefaultStruct(this object obj) return type.IsValueType && obj.Equals(DefaultValues.GetOrAdd(type, FormatterServices.GetUninitializedObject)); } + // A property of a reference type is walked. A property of a struct is not, because a struct such + // as a Point or a Money has nothing to walk into. The exception is a struct that is a collection + // of items that can have attributes, such as ImmutableArray of a class, because the same + // struct is enumerated when it is an item of another collection, and a model must not pass + // because the collection sits one level higher. A Nullable is checked as T. + private static bool IsWalkedType(Type type) + { + type = Nullable.GetUnderlyingType(type) ?? type; + + return !type.IsValueType + || (typeof(IEnumerable).IsAssignableFrom(type) && !type.IsCollectionOfLeafType()); + } + private static bool IsInSystemNamespace(Type type) { var ns = type.Namespace; diff --git a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs index 06b71dd..5ba9431 100644 --- a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs +++ b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -261,6 +261,11 @@ private bool TryValidateObjectRecursive( case IEnumerable _ when value.GetType().IsCollectionOfLeafType(): continue; + //a struct collection nobody set, such as a default ImmutableArray, holds nothing + //and enumerating it throws, so skip it as an item is skipped + case IEnumerable _ when value.IsDefaultStruct(): + continue; + case IEnumerable asEnumerable: //an item that was enumerated above can return itself from a property, such as //Array.SyncRoot, and enumerating it a second time would find nothing new diff --git a/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs index 65d8d37..184696a 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs @@ -109,8 +109,9 @@ public void Item_that_throws_when_enumerated_propagates() /// - A default struct holds no objects, so "valid" is the correct answer, and the answer on /// 2.3.3. A fix that makes it throw turns a passing model into a crash for a reason the /// caller did not cause and cannot see in the model. - /// - The same struct held in a property is not walked at all (StructsAreNotWalked), so it - /// passes today and after the fix. Only an item would throw. That difference is a trap. + /// - The same struct held in a property is enumerated too, since 3.0, and a default one is + /// skipped the same way (StructCollectionPropertyTests). Before that it was not walked at + /// all, so only an item would throw, and that difference was a trap. /// Options that were weighed: /// - A. Skip a default ImmutableArray only. It leaves ArraySegment and the caller's own /// structs to throw. diff --git a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs index 6149e53..82ed901 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs @@ -330,8 +330,8 @@ public void Object_array_holding_a_struct_collection_is_validated() } #if NET8_0_OR_GREATER - // ImmutableArray is a struct. See the limitation guards in StructsAreNotWalked for the - // same type held in a property. + // ImmutableArray is a struct. See StructCollectionPropertyTests for the same type held + // in a property. // See: https://learn.microsoft.com/dotnet/api/system.collections.immutable.immutablearray-1 [Fact] public void List_of_immutable_arrays_is_validated() @@ -701,10 +701,9 @@ public void Validate_on_a_collection_item_runs() /// Structs. The validator walks into properties of reference types only, so a struct /// property is checked for its own validation attributes by Validator when the parent is /// validated, but nothing inside the struct is walked. A property of the struct that - /// carries an attribute is never checked, and neither are the items of a collection that is - /// itself a struct, such as ImmutableArray<T>, when a property holds it. A struct - /// collection that is an item of another collection is enumerated since 2.4.0. The planned - /// fix for the property case is in StructCollectionPropertyTests. + /// carries an attribute is never checked. The exception is a struct that is a collection, + /// such as ImmutableArray<T>: its items are validated, as an item of another collection + /// and as a property (see StructCollectionPropertyTests). /// These are limitation guards. Record structs with positional `[property: ...]` /// attributes are a modern way to model a value, so this one may surprise callers. /// See: https://learn.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct @@ -760,15 +759,6 @@ public void Record_struct_property_with_an_attribute_is_not_validated() Assert.Empty(errors); } - [Fact] - public void Struct_collection_items_are_not_validated() - { - var (valid, errors) = Run(new Holder { Value = new LeafBag(new Leaf()) }); - - Assert.True(valid); - Assert.Empty(errors); - } - [Fact] public void Struct_that_is_the_root_object_is_validated() { diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs index 1ed4077..252c6a3 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs @@ -11,51 +11,36 @@ namespace RecursiveDataAnnotationsValidation.Tests { /// - /// A collection that is a struct, held in a property. The specs here describe the planned - /// behavior. The guards that pin today's gap pass until the fix lands, and the specs fail until then. + /// A collection that is a struct, held in a property, such as Holder<ImmutableArray<T>>. /// - /// The inconsistency. The validator walks only the properties of a reference type (see - /// IsWalked), so a property whose type is a struct is never read. That includes a struct - /// that is a collection, such as ImmutableArray<T>. Since 2.4.0 the validator enumerates an - /// item that is a collection, and it does so for a struct too, because an item is validated as - /// an object whatever its type is. So the same ImmutableArray of invalid objects is: + /// Why it matters. The validator walks only the properties of a reference type (see IsWalked), + /// so a property whose type is a struct used to be skipped. That included a struct that is a + /// collection, such as ImmutableArray<T>. The validator enumerates an item that is a + /// collection, and it does so for a struct too, because an item is validated as an object + /// whatever its type is. So the same ImmutableArray of invalid objects was: /// - validated when it is an item, such as List<ImmutableArray<T>>, and /// - not validated, silently, when it is the value of a property. - /// A model can pass validation because the collection sits one level higher. - /// Both behaviors are pinned below, in a guard that passes today and fails on purpose when the - /// property case is fixed. + /// A model could pass validation because the collection sat one level higher. /// - /// Why a struct property is skipped. The rule suits a Point or a Money, where the validator - /// has nothing to walk into. It covers a struct that is a collection as a side effect, and - /// nothing in the code or the tests records a decision about collections. A struct property with - /// an attribute on the struct's own members is still not walked (see - /// OddShapeTests.StructsAreNotWalked), and this fix does not change that. A property that holds - /// a struct is only enumerated when the struct is a collection, so that a model does not pass - /// because the collection sits one level higher. - /// See: https://learn.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct - /// - /// The plan for the fix: - /// - IsWalked admits a property of a struct type when the struct is a collection that is not a - /// collection of leaf types, and a Nullable of such a struct. A struct that is not a - /// collection, such as a Money, is still skipped. - /// - The walk reads the value, which is boxed, and enumerates it like any other collection. - /// The path is the same as for a collection that is a class: Value[0].Name. + /// The rule now. IsWalked admits a property of a struct type when the struct is a collection + /// that is not a collection of leaf types, and a Nullable of such a struct. The walk reads the + /// value, which is boxed, and enumerates it like any other collection. The path is the same + /// as for a collection that is a class: Value[0].Name. + /// - A struct that is not a collection, such as a Money, is still skipped. An attribute on + /// its members is still not checked (see OddShapeTests.StructsAreNotWalked). /// - A struct in a property that equals its default value, such as a default ImmutableArray or /// ArraySegment, is skipped, as an item is (see NestedCollectionEdgeCaseTests, Case 2). It - /// passes today, so enumerating it would add a crash. + /// passed before, and enumerating it would add a crash. A struct with no fields always + /// equals its default, so it is skipped too. /// - A struct collection that throws when enumerated throws from the property too, like an /// item and like a class. This is a change for the few models that hold one. /// - Check 4 of IsLeafType uses IsWalked, so a struct with such a property is no longer a /// leaf type. That is the right answer, because the validator now looks inside it. + /// - [SkipRecursiveValidation] on the property still skips it. + /// See: https://learn.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct /// - /// When the fix lands: - /// - Unskip the specs below. - /// - Delete Struct_collection_is_validated_as_an_item_but_not_as_a_property, which pins the gap. - /// - Replace OddShapeTests.StructsAreNotWalked.Struct_collection_items_are_not_validated, which - /// pins the same gap, and update the comment of that class, which mentions ImmutableArray. - /// - Add a Fixed entry to the changelog. A model that passes today can fail. List the changed - /// member name format as BREAKING only if one exists. A property that was never walked has no - /// path to change. + /// A model that passed before can fail now. The path of an error is new, so no member name + /// that callers matched has changed. /// public class StructCollectionPropertyTests { @@ -136,20 +121,18 @@ public ThrowingStructBag(int marker) IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } - /// - /// Guard that pins the gap. The same struct is validated as an item, and passes as a - /// property. It fails on purpose when the property case is fixed. - /// + // The same struct is validated as an item and as a property, with the same member names + // except for the extra index of the list. [Fact] - public void Struct_collection_is_validated_as_an_item_but_not_as_a_property() + public void Struct_collection_is_validated_as_an_item_and_as_a_property() { var asItem = Run(new Holder> { Value = new List { new LeafBag(new Leaf()) } }); var asProperty = Run(new Holder { Value = new LeafBag(new Leaf()) }); Assert.False(asItem.Valid); Assert.Equal(ResultText.Expect("Value[0][0].Name" + NameRequired), asItem.Errors); - Assert.True(asProperty.Valid); - Assert.Empty(asProperty.Errors); + Assert.False(asProperty.Valid); + Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), asProperty.Errors); } [Fact] @@ -280,10 +263,11 @@ public void Struct_collection_property_that_throws_when_enumerated_propagates() Assert.IsType(ex); } + // A default struct is skipped, as an item is, so one that would throw passes. [Fact] - public void Struct_collection_that_throws_when_enumerated_passes_today() + public void Default_struct_collection_that_throws_when_enumerated_is_skipped() { - var (valid, errors) = Run(new Holder { Value = new ThrowingStructBag(1) }); + var (valid, errors) = Run(new Holder { Value = default }); Assert.True(valid); Assert.Empty(errors); From 482698371b2b975ccd617e2294ee04045188e3a0 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:14:57 -0400 Subject: [PATCH 3/7] docs: describe struct collections in properties Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 4 ++-- README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe5fa49..0e1211d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,10 @@ Breaking changes are marked **BREAKING**. ### Fixed +- A struct collection that a property holds, such as `ImmutableArray` or a struct of your own that implements `IEnumerable`, 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. A struct property that is not a collection, such as a `Money`, is still not walked. A struct collection that throws when enumerated now throws from the property too. - Objects inside an item that is itself a collection are now validated. Before, a `List>`, a `List>`, an `object[]` that holds a list, and a list of a sealed or struct collection such as `ImmutableList` 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 struct item that equals its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped, because it holds nothing and enumerating it throws. +- A struct item or struct property that equals its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped, because it holds nothing and enumerating it throws. ### Changed @@ -25,7 +26,6 @@ Breaking changes are marked **BREAKING**. - **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. To avoid it, mark the property that holds the collection with `[SkipRecursiveValidation]`. - A deeply nested collection uses more of the stack. On a 1 MB stack, a chain of `List` that holds the next list passes at 1,100 levels and overflows at 1,200, which ends the process (net8.0, macOS, Release). Before, such a list passed without being enumerated. The validator still has no maximum depth. - The NuGet package title now reads "Recursive DataAnnotations Validation". It was misspelled "Recurisive". -- Known limit: a struct collection such as `ImmutableArray` is validated when it is an item, but not yet when a property holds it. - Known limit: an invalid struct that an item collection returns by enumeration and also through one of its properties is reported twice, with two member names, for example `Value[0][0].Text` and `Value[0].Array[0].Text` for an `ArraySegment`. The validator cannot match two copies of a struct by reference. A struct that two properties hold was already reported twice. Objects of a class are reported once. ## 2.3.3 - 2026-10-01 diff --git a/README.md b/README.md index 9523f84..7c2524c 100644 --- a/README.md +++ b/README.md @@ -43,8 +43,8 @@ The [`[SkipRecursiveValidation]`](https://github.com/tgharold/RecursiveDataAnnot - 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` 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 is a struct, such as `ImmutableArray`, is validated when it is an item of another collection, but not when a property holds it. 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 item that equals its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped, because it holds nothing and enumerating it throws. +- A collection that is a struct, such as `ImmutableArray`, is validated when a property holds it and when it is an item of another collection: `Lines[0].Sku`. A struct that is not a collection, such as a `Money`, 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, as an item or as a property, that equals its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped, because it holds nothing and enumerating it throws. ### Framework types that are not walked From a947134164211a490b42929d5af3c00c0548cee2 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:44:57 -0400 Subject: [PATCH 4/7] test: pin the default struct check and struct collection depth Specs for a struct collection whose Equals says "equal to default" while it holds invalid objects, or throws. The first group fails until IsDefaultStruct stops calling the Equals of the caller's type. Guards for a struct collection property against the maximum depth, for a dictionary value that is a struct collection, and for IsWalked and IsLeafType. Co-Authored-By: Claude Sonnet 5.5 --- .../Extensions/TypeExtensionsTests.cs | 84 ++++++++ .../MaxDepthTests.cs | 73 +++++++ .../StructCollectionPropertyTests.cs | 190 ++++++++++++++++++ 3 files changed, 347 insertions(+) diff --git a/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs index b292b9a..9838512 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs @@ -198,6 +198,72 @@ public void Sealed_collection_of_values_is_a_leaf_type() Assert.True(typeof(string).IsLeafType()); } + public class StructBagHolder + { + public ChildBagStruct Bag { get; set; } + + public ChildBagStruct? MaybeBag { get; set; } + + public PlainPoint Point { get; set; } + + public KeyValuePair Pair { get; set; } + } + + // A property of a struct is walked only when the struct is a collection of items that can have + // attributes. A struct that is not a collection is not, and neither is a KeyValuePair, which + // is not an IEnumerable. + [Fact] + public void Struct_collection_property_is_walked() + { + Assert.True(typeof(StructBagHolder).GetProperty(nameof(StructBagHolder.Bag)).IsWalked()); + Assert.True(typeof(StructBagHolder).GetProperty(nameof(StructBagHolder.MaybeBag)).IsWalked()); + } + + [Fact] + public void Struct_property_that_is_not_a_collection_is_not_walked() + { + Assert.False(typeof(StructBagHolder).GetProperty(nameof(StructBagHolder.Point)).IsWalked()); + Assert.False(typeof(StructBagHolder).GetProperty(nameof(StructBagHolder.Pair)).IsWalked()); + } + + // Check 4 of IsLeafType uses IsWalked. A type whose only walked member is a struct collection + // was a leaf type before struct collection properties were walked, and it is not now. + [Fact] + public void Type_with_a_struct_collection_property_is_not_a_leaf_type() + { + Assert.False(typeof(SealedStructBagHolder).IsLeafType()); + } + + public sealed class SealedStructBagHolder + { + public ChildBagStruct Bag { get; set; } + } + +#if NET8_0_OR_GREATER + public class ImmutableArrayHolder + { + public System.Collections.Immutable.ImmutableArray Children { get; set; } + + public System.Collections.Immutable.ImmutableArray? MaybeChildren { get; set; } + + public System.Collections.Immutable.ImmutableArray Numbers { get; set; } + } + + [Fact] + public void Immutable_array_property_of_objects_is_walked() + { + Assert.True(typeof(ImmutableArrayHolder).GetProperty(nameof(ImmutableArrayHolder.Children)).IsWalked()); + Assert.True(typeof(ImmutableArrayHolder).GetProperty(nameof(ImmutableArrayHolder.MaybeChildren)).IsWalked()); + } + + // A collection of leaf types has nothing to validate, so its property is not walked. + [Fact] + public void Immutable_array_property_of_values_is_not_walked() + { + Assert.False(typeof(ImmutableArrayHolder).GetProperty(nameof(ImmutableArrayHolder.Numbers)).IsWalked()); + } +#endif + // A default struct collection holds nothing, and enumerating one throws, so the validator skips it. // The check is the struct's own Equals against default(T), so it needs no list of types. [Fact] @@ -233,6 +299,24 @@ public void Struct_that_has_a_value_is_not_detected() Assert.False(((object)5).IsDefaultStruct()); } + // The check compares the memory of the struct with default(T). It does not call the Equals of + // the caller's type, which can say "equal" for a struct that holds objects, or throw. + [Fact] + public void Struct_whose_Equals_ignores_its_items_is_not_a_default_struct() + { + var bag = new StructCollectionPropertyTests.StructsWithOddEquals.IdBag(0, new StructCollectionPropertyTests.Leaf()); + + Assert.True(bag.Equals(default(StructCollectionPropertyTests.StructsWithOddEquals.IdBag))); + Assert.False(((object)bag).IsDefaultStruct()); + } + + [Fact] + public void Struct_whose_Equals_throws_is_checked_without_calling_it() + { + Assert.True(((object)default(StructCollectionPropertyTests.StructsWithOddEquals.SequenceBag)).IsDefaultStruct()); + Assert.False(((object)new StructCollectionPropertyTests.StructsWithOddEquals.SequenceBag(new StructCollectionPropertyTests.Leaf())).IsDefaultStruct()); + } + [Fact] public void Object_of_a_class_is_not_a_default_struct() { diff --git a/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs index 3b0188a..ef9d007 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs @@ -371,5 +371,78 @@ public void Three_hundred_nested_lists_fail_the_validation() Assert.EndsWith($" | {TooDeep}", errors[0]); } } + +#if NET8_0_OR_GREATER + /// + /// A struct collection that a property holds, such as ImmutableArray. It counts like any + /// other collection: one level for the property and one for the index. System.Collections.Immutable + /// is not part of net481. + /// + public class StructCollectionProperties + { + public class ImmutableNode + { + public System.Collections.Immutable.ImmutableArray Children { get; set; } + + [Required(ErrorMessage = "Name is required")] + public string Name { get; set; } + } + + // Builds from the bottom: a chain of `levels` nodes below the root. + private static ImmutableNode ImmutableTree(int levels, string bottomName) + { + var node = new ImmutableNode { Name = bottomName }; + for (var i = 0; i < levels; i++) + { + node = new ImmutableNode + { + Name = "ok", + Children = System.Collections.Immutable.ImmutableArray.Create(node), + }; + } + + return node; + } + + /// + /// A property that builds a new struct collection of new objects on each read. Before the + /// limit, this overflowed the stack, which ends the process. + /// + public class Spawner + { + public System.Collections.Immutable.ImmutableArray Next => + System.Collections.Immutable.ImmutableArray.Create(new Spawner()); + } + + [Fact] + public void Tree_64_levels_deep_is_validated_to_the_bottom() + { + var (valid, errors) = Run(ImmutableTree(64, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{ChildrenPath(64)}.Name | Name is required"), errors); + } + + [Fact] + public void Tree_65_levels_deep_fails_at_the_65th_child() + { + var (valid, errors) = Run(ImmutableTree(65, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{ChildrenPath(65)} | {TooDeep}"), errors); + } + + [Fact] + public void Property_that_builds_a_new_struct_collection_on_each_read_fails_at_the_limit() + { + var (valid, errors) = Run(new Spawner()); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect($"{string.Join(".", Enumerable.Repeat("Next[0]", 65))} | {TooDeep}"), + errors); + } + } +#endif } } diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs index 252c6a3..201650a 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs @@ -315,5 +315,195 @@ public void Immutable_array_of_values_property_is_valid() Assert.Empty(errors); } #endif + + /// + /// Structs whose own Equals is not a reliable test for "nothing was set". The validator skips a + /// default struct, because enumerating one can throw, and it must decide that by the memory + /// of the struct, not by calling the Equals of the caller's type. A model that Equals its + /// default by accident would otherwise pass while it holds invalid objects, and an Equals + /// that throws would end validation. The types here are unusual, but a model that passed + /// only because of such an Equals is a false pass. + /// See: https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.runtimehelpers.equals + /// + public class StructsWithOddEquals + { + /// Equals compares the Id only, so a bag with Id 0 equals default(IdBag) whatever it holds. + public readonly struct IdBag : IEnumerable + { + private readonly Leaf[] _items; + + public IdBag(int id, params Leaf[] items) + { + Id = id; + _items = items; + } + + public int Id { get; } + + public override bool Equals(object obj) => obj is IdBag other && other.Id == Id; + + public override int GetHashCode() => Id; + + public IEnumerator GetEnumerator() => ((IEnumerable)_items).GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + /// + /// Equals compares the items, and throws when it is compared with default(SequenceBag), + /// because the default value has no array. + /// + public readonly struct SequenceBag : IEnumerable + { + private readonly Leaf[] _items; + + public SequenceBag(params Leaf[] items) + { + _items = items; + } + + public override bool Equals(object obj) => obj is SequenceBag other && System.Linq.Enumerable.SequenceEqual(_items, other._items); + + public override int GetHashCode() => 0; + + public IEnumerator GetEnumerator() => ((IEnumerable)_items).GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + /// A struct with no fields. It always equals default, and it enumerates a shared list. + public readonly struct RegistryView : IEnumerable + { + public static readonly List Registry = new List { new Leaf() }; + + public IEnumerator GetEnumerator() => Registry.GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + // Guard. On 2.3.3 an interface-typed property was walked and enumerated whatever the + // value was. A bag that Equals default by accident must not be skipped there. + [Fact] + public void Interface_typed_property_holding_a_bag_that_equals_default_is_validated() + { + var (valid, errors) = Run(new Holder> { Value = new IdBag(0, new Leaf()) }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), errors); + } + + [Fact] + public void Object_typed_property_holding_a_bag_that_equals_default_is_validated() + { + var (valid, errors) = Run(new Holder { Value = new IdBag(0, new Leaf()) }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), errors); + } + + // A field-less struct is a default struct by memory too, and a property of that struct type is + // skipped, as a default ImmutableArray is. Declared as an interface it is still enumerated. + [Fact] + public void Interface_typed_property_holding_a_field_less_struct_collection_is_validated() + { + var (valid, errors) = Run(new Holder> { Value = new RegistryView() }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), errors); + } + + [Fact] + public void Object_typed_property_holding_a_field_less_struct_collection_is_validated() + { + var (valid, errors) = Run(new Holder { Value = new RegistryView() }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), errors); + } + + [Fact] + public void Struct_typed_property_with_a_bag_that_equals_default_is_validated() + { + var (valid, errors) = Run(new Holder { Value = new IdBag(0, new Leaf()) }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0].Name" + NameRequired), errors); + } + + [Fact] + public void Struct_item_that_equals_default_is_validated() + { + var (valid, errors) = Run(new Holder> { Value = new List { new IdBag(0, new Leaf()) } }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0][0].Name" + NameRequired), errors); + } + + // The Equals of the struct is not called, so one that throws cannot end validation. + // Before, a model with such a bag passed on 2.3.3 and threw from 3.0 on. + [Fact] + public void Struct_typed_property_with_an_Equals_that_throws_is_validated() + { + var (valid, errors) = Run(new Holder { Value = new SequenceBag(new Leaf { Name = "a" }) }); + + Assert.True(valid); + Assert.Empty(errors); + } + + [Fact] + public void Interface_typed_property_with_an_Equals_that_throws_is_validated() + { + var (valid, errors) = Run(new Holder> { Value = new SequenceBag(new Leaf { Name = "a" }) }); + + Assert.True(valid); + Assert.Empty(errors); + } + + [Fact] + public void Struct_item_with_an_Equals_that_throws_is_validated() + { + var (valid, errors) = Run(new Holder> { Value = new List { new SequenceBag(new Leaf { Name = "a" }) } }); + + Assert.True(valid); + Assert.Empty(errors); + } + + [Fact] + public void Default_struct_with_an_Equals_that_throws_is_skipped() + { + var (valid, errors) = Run(new Holder { Value = default }); + + Assert.True(valid); + Assert.Empty(errors); + } + } + +#if NET8_0_OR_GREATER + // A dictionary value that is a struct collection. The KeyValuePair item of the dictionary is + // walked, and so is the struct collection that its Value property holds. + [Fact] + public void Dictionary_value_that_is_a_struct_collection_is_validated() + { + var (valid, errors) = Run(new Holder>> + { + Value = new Dictionary> { ["a"] = ImmutableArray.Create(new Leaf()) }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Value[0].Value[0].Name" + NameRequired), errors); + } + + [Fact] + public void Dictionary_value_that_is_a_default_struct_collection_is_valid() + { + var (valid, errors) = Run(new Holder>> + { + Value = new Dictionary> { ["a"] = default }, + }); + + Assert.True(valid); + Assert.Empty(errors); + } +#endif } } From 65846067ad19df9fad9df59d0cf09e8485dc32ec Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:45:22 -0400 Subject: [PATCH 5/7] fix: compare a default struct by memory, not by its Equals IsDefaultStruct called the Equals of the caller's struct. A struct that compares only an Id, or has no fields, then counted as default while it held invalid objects, so the validation passed. An Equals that throws ended the validation. Compare the memory with RuntimeHelpers.Equals instead. Skip a default struct in a property only when the property is declared as a struct. A property declared as an interface or object was always enumerated, and a boxed struct in it is enumerated again. Co-Authored-By: Claude Sonnet 5.5 --- .../Extensions/TypeExtensions.cs | 11 +++++++++-- .../RecursiveDataAnnotationValidator.cs | 6 ++++-- .../Extensions/TypeExtensionsTests.cs | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs index 4445104..ed018c0 100644 --- a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs +++ b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs @@ -8,6 +8,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.Serialization; using System.Threading; using RecursiveDataAnnotationsValidation.Attributes; @@ -166,13 +167,19 @@ public static bool IsUnsafeToWalk(this Type type) /// A default struct collection usually holds nothing to validate, and enumerating one throws: /// InvalidOperationException for these two framework types, NullReferenceException for a /// struct that wraps an array. The validator skips it, so a model that has an unset struct - /// field does not start to throw. Equality is the struct's own Equals. + /// field does not start to throw. + /// The struct is compared with default(T) by its memory. The Equals of the caller's type is + /// not called: it can say "equal" for a struct that holds objects, such as one that compares + /// only an Id, which would let invalid objects pass, and it can throw. A struct with no + /// fields is always default. + /// See: https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.runtimehelpers.equals /// public static bool IsDefaultStruct(this object obj) { var type = obj.GetType(); - return type.IsValueType && obj.Equals(DefaultValues.GetOrAdd(type, FormatterServices.GetUninitializedObject)); + return type.IsValueType + && RuntimeHelpers.Equals(obj, DefaultValues.GetOrAdd(type, FormatterServices.GetUninitializedObject)); } // A property of a reference type is walked. A property of a struct is not, because a struct such diff --git a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs index 999687e..eda7f25 100644 --- a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs +++ b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -282,8 +282,10 @@ private bool TryValidateObjectRecursive( continue; //a struct collection nobody set, such as a default ImmutableArray, holds nothing - //and enumerating it throws, so skip it as an item is skipped - case IEnumerable _ when value.IsDefaultStruct(): + //and enumerating it throws, so skip it as an item is skipped. Only for a property + //declared as a struct: a property declared as an interface or object was always + //enumerated, and a boxed struct in one must not start to be skipped. + case IEnumerable _ when property.PropertyType.IsValueType && value.IsDefaultStruct(): continue; case IEnumerable asEnumerable: diff --git a/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs index 9838512..0c5653c 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs @@ -265,7 +265,7 @@ public void Immutable_array_property_of_values_is_not_walked() #endif // A default struct collection holds nothing, and enumerating one throws, so the validator skips it. - // The check is the struct's own Equals against default(T), so it needs no list of types. + // The check compares the memory of the struct with default(T), so it needs no list of types. [Fact] public void Default_struct_is_detected() { From 55a3cf2b3311abcb7ea6112a136cba4e93595057 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:46:13 -0400 Subject: [PATCH 6/7] docs: describe the struct collection and default struct rules Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 6 +++--- README.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc2f4a..0fa72bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,10 @@ Breaking changes are marked **BREAKING**. ### Fixed -- A struct collection that a property holds, such as `ImmutableArray` or a struct of your own that implements `IEnumerable`, 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. A struct property that is not a collection, such as a `Money`, is still not walked. A struct collection that throws when enumerated now throws from the property too. +- A struct collection that a property holds, such as `ImmutableArray` or a struct of your own that implements `IEnumerable`, 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>`, a `List>`, an `object[]` that holds a list, and a list of a sealed or struct collection such as `ImmutableList` 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 struct item or struct property that equals its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped, because it holds nothing and enumerating it throws. +- A struct item, or a property declared as a struct, that is its default value, such as an `ImmutableArray` or an `ArraySegment` 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 +27,7 @@ Breaking changes are marked **BREAKING**. - `LinkedList`: `Value[0].First.List[1].Name` is now `Value[0][1].Name`. - `SortedSet`: `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. To avoid it, mark the property that holds the collection with `[SkipRecursiveValidation]`. +- **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`: 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 deeply nested collection uses more of the stack. On a 1 MB stack, a chain of `List` that holds the next list passes at 1,100 levels and overflows at 1,200, which ends the process (net8.0, macOS, Release). Before, such a list passed without being enumerated. The maximum depth of 128 now stops the walk long before the stack is full. - The NuGet package title now reads "Recursive DataAnnotations Validation". It was misspelled "Recurisive". - Known limit: an invalid struct that an item collection returns by enumeration and also through one of its properties is reported twice, with two member names, for example `Value[0][0].Text` and `Value[0].Array[0].Text` for an `ArraySegment`. The validator cannot match two copies of a struct by reference. A struct that two properties hold was already reported twice. Objects of a class are reported once. diff --git a/README.md b/README.md index acd1ad6..34ac433 100644 --- a/README.md +++ b/README.md @@ -47,12 +47,12 @@ A tree that holds its children in a `List` uses two levels for each tree leve ### Collections - The items of a collection that a property holds are validated. The error names the property and the index of the item: `Items[1].Name`. -- 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 is not walked, because the validator reads only properties of reference types. +- 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` 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 is a struct, such as `ImmutableArray`, is validated when a property holds it and when it is an item of another collection: `Lines[0].Sku`. A struct that is not a collection, such as a `Money`, 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, as an item or as a property, that equals its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped, because it holds nothing and enumerating it throws. +- A collection that is a struct, such as `ImmutableArray`, 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` or an `ArraySegment` 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. ### Framework types that are not walked From 73d2cb126f0867979b979598f82e547e22c47564 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 08:46:13 -0400 Subject: [PATCH 7/7] docs: fix stale comments and version labels Say "five checks" where five are listed, use 3.0 for the next release, and update the comments that said a struct collection property is not read. Co-Authored-By: Claude Sonnet 5.5 --- .../Extensions/TypeExtensions.cs | 2 +- .../Extensions/TypeExtensionsTests.cs | 4 ++-- .../NestedCollectionEdgeCaseTests.cs | 10 +++++----- .../OddShapeTests.cs | 8 ++++---- .../StructCollectionPropertyTests.cs | 13 ++++++++----- .../StructItemsReportedTwiceTests.cs | 4 ++-- .../UpgradeCompatibilityTests.cs | 2 +- .../ValidatorHardeningTests.cs | 2 +- 8 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs index ed018c0..788eca9 100644 --- a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs +++ b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs @@ -81,7 +81,7 @@ public static bool IsCollectionOfLeafType(this Type collectionType) /// /// True when validating an item of this type can never produce a result. A Nullable<T> /// is checked as T. The type must be a value type or a sealed class, so an item cannot be - /// a derived type with its own attributes. It must also pass all four checks: + /// a derived type with its own attributes. It must also pass all five checks: /// 1. No validation attribute on the type. /// 2. No validation attribute on any of its properties. /// 3. It does not implement IValidatableObject. diff --git a/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs index 0c5653c..c6c77a2 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/Extensions/TypeExtensionsTests.cs @@ -10,13 +10,13 @@ namespace RecursiveDataAnnotationsValidation.Tests.Extensions /// /// A leaf type is one where validating an item of that type can never produce a result. /// The validator skips a collection whose element types are all leaf types. - /// An item can produce a result in only four ways, so a leaf type must pass all four checks: + /// An item can produce a result in only five ways, so a leaf type must pass all five checks: /// 1. No validation attribute on the type. /// 2. No validation attribute on any of its properties. /// 3. It does not implement IValidatableObject. Validator calls Validate() on any item /// that implements it. /// 4. No property the validator would walk into: a readable, non-indexer property of a - /// reference type other than string. + /// reference type other than string, or of a struct that is a collection of objects. /// 5. If it is a collection, it is a collection of leaf types. The validator enumerates an /// item that is a collection, so a sealed class or a struct that yields objects must not /// be skipped. A collection that yields an unknown type, such as a non-generic one, counts diff --git a/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs index 184696a..dc3e706 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/NestedCollectionEdgeCaseTests.cs @@ -12,7 +12,7 @@ namespace RecursiveDataAnnotationsValidation.Tests /// /// Edge cases of the fix that enumerates an item that is itself a collection (see /// OddShapeTests.CollectionsInsideCollections). In 2.3.3 and earlier an item that is a - /// collection is validated as an object but never enumerated. Since 2.4.0 it is also + /// collection is validated as an object but never enumerated. Since 3.0 it is also /// enumerated, and enumerating can do two things that validating the object never did: /// it can throw, and it can run for a long time. Each case has the reasoning for the decision. /// @@ -26,7 +26,7 @@ namespace RecursiveDataAnnotationsValidation.Tests /// no spec for it here. A collection item behaves the same as a property after the fix. /// /// A fix that enumerates items therefore changes what a model can do to the caller. A model - /// that passes on 2.3.3 can throw on 2.4, if one of its items is a collection that throws + /// that passes on 2.3.3 can throw on 3.0, if one of its items is a collection that throws /// when it is enumerated. Only the cases below are realistic. A collection that throws on /// enumeration is a broken collection, but a default struct is a common way to get one. /// @@ -69,9 +69,9 @@ public class ThrowingBag : IEnumerable /// rule covers both and the caller handles one kind of failure. /// - Catching would turn a broken model into "valid" or "invalid", and neither is true. The /// caller could not tell a bad model from a bad collection. - /// - The cost is that a model which passes on 2.3.3 can throw on 2.4, because 2.3.3 never + /// - The cost is that a model which passes on 2.3.3 can throw on 3.0, because 2.3.3 never /// enumerates the item. This is a rare shape: a collection that throws on enumeration. - /// Decision: let it propagate. The changelog entry for 2.4.0 says so. + /// Decision: let it propagate. The changelog says so. /// public class ItemThatThrowsWhenEnumerated { @@ -165,7 +165,7 @@ public void Default_immutable_array_in_an_object_array_is_treated_as_empty() } // Guard. The item that is not default is validated by the spec in OddShapeTests, and a - // default array held in a property passes today and must keep passing. + // default array held in a property passes and must keep passing. // See StructCollectionPropertyTests for a non-default array held in a property. [Fact] public void Default_immutable_array_in_a_property_is_not_walked() diff --git a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs index 82ed901..a9b55c8 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs +++ b/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 2.4.0 enumerates. + /// CollectionsInsideCollections, 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. @@ -65,7 +65,7 @@ private static (bool Valid, List Errors) Run(object model) /// that returns the collection itself. The validator walked that property and enumerated it, /// so a jagged array reported the path "Value[0].SyncRoot[0].Name". A List of Dictionary /// worked through the public Values property, with the path "Value[0].Values[0].Name". - /// Release 2.4.0 enumerates an item that is a collection, and reports the index of each level: + /// Release 3.0 enumerates an item that is a collection, and reports the index of each level: /// "Value[0][0].Name". The tests in this class state that behavior. Most fail on 2.3.3 and /// earlier: the shapes that passed silently report nothing, and the shapes that worked by /// accident report a different path. The guards for null items, empty collections and @@ -489,7 +489,7 @@ public void Jagged_array_is_validated() /// nothing after the dot. Code that builds the path must not throw on null. /// - 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 2.4.0 first read the + /// 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. /// See: https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.validationcontext.membername /// @@ -536,7 +536,7 @@ public void Null_member_name_from_Validate_on_an_item() } // In a nested collection the path has the index of each level, then the empty name. - // Nested collections are enumerated from 2.4.0, so 2.3.3 reports nothing here. + // 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() { diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs index 201650a..71728a5 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs @@ -28,10 +28,13 @@ namespace RecursiveDataAnnotationsValidation.Tests /// as for a collection that is a class: Value[0].Name. /// - A struct that is not a collection, such as a Money, is still skipped. An attribute on /// its members is still not checked (see OddShapeTests.StructsAreNotWalked). - /// - A struct in a property that equals its default value, such as a default ImmutableArray or + /// - A struct in a property that is its default value, such as a default ImmutableArray or /// ArraySegment, is skipped, as an item is (see NestedCollectionEdgeCaseTests, Case 2). It - /// passed before, and enumerating it would add a crash. A struct with no fields always - /// equals its default, so it is skipped too. + /// passed before, and enumerating it would add a crash. The check compares memory, and does + /// not call the struct's Equals (see StructsWithOddEquals). A struct with no fields is + /// always default, so a property of that struct type is skipped too. Only a property declared + /// as a struct is skipped this way. A property declared as an interface or object was always + /// enumerated, and still is. /// - A struct collection that throws when enumerated throws from the property too, like an /// item and like a class. This is a change for the few models that hold one. /// - Check 4 of IsLeafType uses IsWalked, so a struct with such a property is no longer a @@ -254,7 +257,7 @@ public void Struct_collection_of_values_is_not_enumerated() } // A struct collection that throws when it is enumerated throws from a property, like an - // item (NestedCollectionEdgeCaseTests, Case 1). It passes today, because it is not read. + // item (NestedCollectionEdgeCaseTests, Case 1). It passed before 3.0, because the property was not read. [Fact] public void Struct_collection_property_that_throws_when_enumerated_propagates() { @@ -295,7 +298,7 @@ public void Immutable_array_of_immutable_arrays_property_is_validated() Assert.Equal(ResultText.Expect("Value[0][0].Name" + NameRequired), errors); } - // Guard. A default ImmutableArray in a property passes today and must pass after the fix. + // Guard. A default ImmutableArray in a property passed before the fix and still passes. // Enumerating it throws, so the fix has to skip it, as it does for an item. [Fact] public void Default_immutable_array_property_is_valid() diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructItemsReportedTwiceTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructItemsReportedTwiceTests.cs index b687ed3..838a645 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructItemsReportedTwiceTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructItemsReportedTwiceTests.cs @@ -20,7 +20,7 @@ namespace RecursiveDataAnnotationsValidation.Tests /// Where it happens. /// - Two properties hold the same array of structs. This is so in 2.3.3 too. /// - An item that is a collection returns its structs by enumeration and also through a public - /// property, such as ArraySegment<T>.Array. Since 2.4.0 the validator enumerates an item that + /// property, such as ArraySegment<T>.Array. Since 3.0 the validator enumerates an item that /// is a collection and also walks its properties, so such an item reports each struct twice. /// Before, only the property route found them. A LinkedList<T> item reports each struct three /// times, because its First node also reaches the list through two properties. @@ -102,7 +102,7 @@ public void Struct_reached_by_two_properties_is_reported_twice() Assert.Equal(ResultText.Expect("First[0].Text" + TextRequired, "Second[0].Text" + TextRequired), errors); } - // New in 2.4.0. 2.3.3 reports only the first path, through the Array property. + // New in 3.0. 2.3.3 reports only the first path, through the Array property. [Fact] public void Struct_in_an_array_segment_item_is_reported_twice() { diff --git a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs index b89e334..152ae5b 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs @@ -1080,7 +1080,7 @@ public void Collections_of_types_with_nothing_to_validate_pass() /// /// A struct in a collection is boxed and validated like any object, so its property - /// attributes run. A struct held directly in a property is not walked. + /// attributes run. A struct that is not a collection, held directly in a property, is not walked. /// See: https://learn.microsoft.com/dotnet/csharp/programming-guide/types/boxing-and-unboxing /// public struct CheckedPoint diff --git a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs index 14baf3e..ea5b795 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs @@ -874,7 +874,7 @@ public void Stream_property_does_not_throw() /// Fix: skip a collection when every element type it declares is a leaf type: a type /// where validating an item can never produce a result. The rule is a set of checks on /// the type, not a list of type names, so it also covers types the library cannot name, - /// such as DateOnly. TypeExtensionsTests lists the four checks. + /// such as DateOnly. TypeExtensionsTests lists the five checks. /// The element type comes from the array element type, or from each IEnumerable<T> the /// collection implements. A Nullable<T> element is checked as T, because a boxed /// Nullable<T> is either null or a boxed T. Each Dictionary item is a KeyValuePair, which