Skip records with no validatable members in the validations generator - #68831
Open
Arul1998 wants to merge 2 commits into
Open
Skip records with no validatable members in the validations generator#68831Arul1998 wants to merge 2 commits into
Arul1998 wants to merge 2 commits into
Conversation
The record primary-constructor path in ExtractValidatableMembers added members unconditionally, unlike the property path which skips members that have no validation attributes and whose type is not itself validatable. As a result any record with a primary constructor was emitted as a validatable type, which also made every property of that record type a validatable member of its containing types. This inflated generated code, added per-request traversal for members that can never produce an error, consumed the MaxDepth budget for nested record graphs, and made records behave differently from equivalently-shaped classes. Apply the same gate to the record path, capturing the previously-discarded TryExtractValidatableType result and checking the parameter as well as the corresponding property. Attributes on record primary-constructor parameters bind to the parameter rather than the property by default, so both are checked, matching how the runtime already resolves them. Fixes dotnet#68805
dotnet-policy-service
Bot
added
the
community-contribution
Contributor
|
Thanks for your PR, @Arul1998. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
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
1 participant
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.
Skip records with no validatable members in the validations generator
Description
The record primary-constructor path in
ValidationsGenerator.TypesParser.ExtractValidatableMembersadded members unconditionally, unlike the regular property path, which skips members that have no validation attributes and whose type is not itself validatable.As a result, any record with a primary constructor was emitted as a validatable type, and every property of that record type became a validatable member of its containing types. This inflated generated code, added per-request traversal for members that can never produce an error, consumed the
MaxDepthbudget for nested record graphs, and made records behave differently from equivalently-shaped classes (e.g.Holder.PlainRecwas a validatable member whileHolder.PlainClsof identical shape was not).This change applies the same gate to the record path: it captures the previously-discarded
TryExtractValidatableTyperesult and skips the member when neither the parameter nor the corresponding property has a validation attribute and the property type is not itself validatable. Both the parameter and the property are checked because attributes on record primary-constructor parameters bind to the parameter rather than the property by default — matching how the runtime already resolves them.Added a test (
DoesNotEmit_ForRecordWithNoValidatableMembers) asserting that plain records/classes and a mixed record are not emitted or pulled in as validatable members, while a record with[Required]on a primary-constructor parameter still is.Fixes #68805