From 3379aa8a3274791753a8f009e373fd3e3fa0f45a Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 12:36:05 -0400 Subject: [PATCH 1/3] test: specs for walking instance properties only A static property holds data of the type, not of the model, and Validator, MVC and System.Text.Json ignore it. The walk read it because Type.GetProperties() with no arguments returns static properties too, so a shared invalid static object failed every model of that type. Co-Authored-By: Claude Opus 5.5 --- .../OddShapeTests.cs | 38 ++++++++++++++++--- .../UpgradeCompatibilityTests.cs | 5 ++- .../ValidatorHardeningTests.cs | 6 ++- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs index edf0873..0e6fa11 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs @@ -874,8 +874,11 @@ public void Attribute_with_the_property_target_is_validated() /// /// Members that the validator reads, with effects a model author may not expect. - /// - A static property is walked, like an instance one. A shared static object that is - /// invalid fails every model of that type. Type.GetProperties includes static members. + /// - A static property is not walked. It holds data of the type, not of the model, and + /// Validator, MVC model validation and System.Text.Json all ignore static properties. + /// Up to 2.3.3 it was walked, because Type.GetProperties() with no arguments returns + /// public static properties too, so a shared static object that was invalid failed + /// every model of that type. Since 3.0 the walk asks for public instance properties only. /// - A Lazy<T> property is walked through Value, so validation runs the factory. /// - A Task<T> property is walked through Result. A completed task is fine. /// See the class summary for a task that has not completed. @@ -903,13 +906,38 @@ public class Circle : IShape public int Radius { get; set; } } + public class WithStaticList + { + public static List All { get; } = new List { new Leaf() }; + } + [Fact] - public void Invalid_static_property_fails_every_instance() + public void Invalid_static_property_is_not_walked() { var (valid, errors) = Run(new WithStatic()); - Assert.False(valid); - Assert.Equal(ResultText.Expect("Shared.Name" + NameRequired), errors); + Assert.True(valid); + Assert.Empty(errors); + } + + [Fact] + public void Static_collection_property_is_not_enumerated() + { + var (valid, errors) = Run(new WithStaticList()); + + Assert.True(valid); + Assert.Empty(errors); + } + + // The same for an item. A type whose only object property is static has nothing to + // walk, so its items are not even enumerated. + [Fact] + public void Static_property_of_an_item_is_not_walked() + { + var (valid, errors) = Run(new Holder> { Value = new List { new WithStatic() } }); + + Assert.True(valid); + Assert.Empty(errors); } [Fact] diff --git a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs index 02a46e4..1640923 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs @@ -810,8 +810,9 @@ public void Init_only_and_private_setter_properties_are_walked() } /// - /// Only public instance and static properties are walked. Non-public properties, - /// write-only properties and indexers are not. + /// Only public instance properties are walked. Non-public properties, write-only + /// properties and indexers are not. Up to 2.3.3, public static properties were walked + /// too (see OddShapeTests.MembersWithSideEffects). /// See: https://learn.microsoft.com/dotnet/api/system.type.getproperties /// [Fact] diff --git a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs index 17dfe5a..36ea2a4 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs @@ -552,8 +552,10 @@ public record OriginPoint(int X, int Y) public OriginPoint Origin => new OriginPoint(0, 0); } - // The same pattern as a static property. Type.GetProperties() returns public static - // properties as well as instance properties, so the validator walks this one too. + // The same pattern as a static property. Type.GetProperties() with no arguments returns + // public static properties as well as instance properties, so up to 2.3.3 the validator + // walked this one too. Since 3.0 it walks instance properties only, so this guard + // passes without the Equals check. // See: https://learn.microsoft.com/dotnet/api/system.type.getproperties public record StaticOriginPoint(int X, int Y) { From b3618f2bec1cc43d4abf007f8858eaaed264c195 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 12:52:19 -0400 Subject: [PATCH 2/3] fix: walk public instance properties only Both the walk and the leaf type check now ask for public instance properties. A static property is no longer walked, so a shared invalid static object no longer fails every model of its type, and a static property that builds a new object on each read no longer needs the Equals check to stop. Co-Authored-By: Claude Opus 5.5 --- .../Extensions/TypeExtensions.cs | 13 ++++++++++++- .../RecursiveDataAnnotationValidator.cs | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs index 788eca9..6f179a3 100644 --- a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs +++ b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs @@ -136,6 +136,17 @@ public static bool IsWalked(this PropertyInfo property) && !property.DeclaringType.IsUnsafeToWalk(); } + /// + /// The public instance properties of the type that the validator walks (see + /// ). A static property is left out: it holds data of the type, not + /// of the object, and Validator ignores it too. Type.GetProperties() with no arguments + /// would return public static properties as well. + /// + public static List GetWalkedProperties(this Type type) + { + return type.GetProperties(BindingFlags.Public | BindingFlags.Instance).Where(IsWalked).ToList(); + } + /// /// True when the type's Equals(object) is not object.Equals, so two different objects /// of the type can be equal. Records, structs (through ValueType.Equals) and classes @@ -216,7 +227,7 @@ private static bool FindIsLeafType(Type type) return (type.IsValueType || type.IsSealed) && !HasValidationAttributes(type) && !typeof(IValidatableObject).IsAssignableFrom(type) - && !type.GetProperties().Any(IsWalked) + && type.GetWalkedProperties().Count == 0 && (!typeof(IEnumerable).IsAssignableFrom(type) || type.IsCollectionOfLeafType()); } diff --git a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs index c5a4859..147201c 100644 --- a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs +++ b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -309,7 +309,7 @@ private bool Walk(WorkItem item) //IsWalked leaves out properties declared by framework types that throw or never end when //read, such as Uri.Segments on a relative Uri, DirectoryInfo.Root, or the properties of //a Thread or Process read from the wrong thread or process (see IsUnsafeToWalk) - var properties = type.GetProperties().Where(prop => prop.IsWalked()).ToList(); + var properties = type.GetWalkedProperties(); foreach (var property in properties) { From be883a34c47a33ca38ceac16dbe999d0705130ed Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 12:52:28 -0400 Subject: [PATCH 3/3] docs: describe that static properties are not walked Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25a0ea3..c58c8d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Breaking changes are marked **BREAKING**. - 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** Static properties are no longer walked. Only public instance properties are, as in `Validator`, MVC model validation and System.Text.Json. Before, a public static property was walked like an instance one, so a shared static object that was invalid failed every model of that type, with an error such as `Shared.Name`. A model that failed only because of a static property passes now. Attributes on a static property never ran, and still do not. - **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`. diff --git a/README.md b/README.md index 4237a74..5566197 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ The [`[SkipRecursiveValidation]`](https://github.com/tgharold/RecursiveDataAnnot - Each object is validated once, even when several properties point to it. This also stops cycles, such as a child that points back to its parent. The error names the shortest path to the object (see "Order of results" below). - A struct is the exception to the first rule. The validator cannot tell that two copies of a struct are the same value, so it reports an invalid struct once for each route that reaches it. For example, two properties that hold the same array of structs report each struct twice, and so does a `List` that holds an `ArraySegment` of structs, because the segment returns its items by enumeration and through its `Array` property. The error is not lost, but the result list has a duplicate with a different member name. For a collection type of your own, mark the property that repeats the items with `[SkipRecursiveValidation]`. - Objects are compared by reference. Two separate objects that are `Equals` to each other, such as records with the same values or entities with the same `Id`, are each validated. -- Public static properties are walked, as well as instance properties. +- Only public instance properties are walked. A static property holds data of the type, not of your object, so it is not walked, as `Validator` ignores it too. - A property that builds a new object on each read, such as `public Money Zero => new Money(0)`, could make the walk go on forever. So when an object's type overrides `Equals`, and the object equals an object of the same type, a base type or a derived type on its own path from the root, the validator checks that object's own attributes but does not walk into its properties from there. The check uses the shortest path to the object. A longer path that does not pass an equal object walks into it. An invalid object that every path reaches through an equal ancestor is not reached. This can happen when a shared object, such as a customer, is reached first through an equal ancestor. - For a type that does not override `Equals`, mark such a property with `[SkipRecursiveValidation]`. Otherwise the walk goes on until it reaches the maximum depth (see below), and the validation fails. - Two records that reference each other can also overflow the stack. A record's generated `Equals` compares properties in declaration order, so it follows the reference forever if it reaches it before a property that differs. This happens when the records have equal values, or when the reference is declared first. On .NET Framework it always happens, because the framework's `Validator` calls the record's generated `GetHashCode`, which follows the reference too. Marking the reference with `[SkipRecursiveValidation]` avoids the walk, but not the `GetHashCode` call on .NET Framework. There, override `GetHashCode` so it does not include the reference.