Repository navigation
Walk struct properties that have something to validate - #90
Merged
Merged
Conversation
A struct property is to be walked when the struct has something to validate, as a struct item already is. Specs: an attribute inside a struct, a nullable and a record struct, a default struct, a KeyValuePair property and a struct that holds an object. The linked-list item gains a route through LinkedListNode.Value. Guards: a leaf struct such as DateTime is not walked, a property of its own struct type is not walked, value tuple items (fields) are not. Specs for the deny list: a null SqlString, SqlDecimal or SqlBytes, a GCHandle that is not allocated and ValueTask<T>.Result are not read, in an object property, a list item or a typed property. The upgrade test no longer holds a struct property, because 3.0 differs from 2.2 there. Co-Authored-By: Claude Opus 5.5 <[email protected]>
A struct property is now walked when the struct is not a leaf type: it has a validation attribute, implements IValidatableObject, has a walked property such as KeyValuePair.Value, or is a collection of objects. Before, only a struct collection was walked, so an invalid struct in a property passed while the same struct as an item failed. A property of a struct that returns the struct's own type, such as DateTime.Date, is not walked. Each read is a new copy, and without this DateTime would stop being a leaf type. Deny GCHandle, INullable (the SqlTypes) and ValueTask<T>, whose getters throw or wait. ValueTask<T> is matched by name, because netstandard2.0 cannot name it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Add a Structs section to the README: which struct properties are walked, that a default struct is validated, that a property of the struct's own type is not walked, and that value tuple items are fields. Add GCHandle, the SqlTypes and ValueTask<T> to the framework types that are not walked. Changelog: a Fixed entry for struct properties, a BREAKING entry for the getters that are now read and the deny list, and the new duplicate route in the struct known limit. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
A struct in a property was never walked unless it was a collection. So
[Range]on theAmountof aMoneyproperty, a positionalrecord structwith[property: Range], an invalid object in aKeyValuePair<string, T>property, and a struct that wraps a class all passed validation. The same struct as an item of a list was validated. This closes those false passes for 3.0, which already carries breaking changes. Walking struct properties means reading getters the validator never read before, so the PR also denies the framework structs whose getters throw or wait.Summary
IValidatableObject, has a walked property such asKeyValuePair.Value, or is a collection of objects. A struct with nothing to validate, such asint,GuidorDateTime, is still skipped.DateTime.Date, is not walked. Each read is a new copy that the walk can't match by reference, and without the ruleDateTimewould stop being a leaf type.GCHandle,INullable(all SqlTypes) andValueTask<T>, matched by name because netstandard2.0 can't name it. A nullSqlStringor an unallocatedGCHandlein anobjectproperty or aList<object>no longer throws.ValueTask<T>.Resultis no longer read, even in anobjectproperty.Probe of all 510 public runtime structs: under this rule 138 are newly walkable. 20 still throw on a default value: struct enumerators, async method builders, metadata-reader structs,
DependentHandle,GCHandle<T>andPinnedGCHandle<T>. They are not model types and were already walked in anobjectproperty, so they are left off the deny list.Test plan
-p:IncludeNetFramework=trueUpgradeCompatibilityTestsstill holds only shapes where 3.0 matches 2.2. The struct property moved toOddShapeTests.StructProperties