-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Add SkipValidationAttribute to Microsoft.Extensions.Validation #63103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
5b29848
to
c30a1b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds an experimental [SkipValidation]
attribute to Microsoft.Extensions.Validation that allows developers to opt out of validation for specific properties, method parameters, or entire types. This provides fine-grained control over validation behavior while maintaining the default validation functionality.
Key changes:
- Introduces the
SkipValidationAttribute
that can be applied to classes, properties, or parameters - Updates the source generator to skip validation code generation for annotated elements
- Fixes test infrastructure to properly await
VerifyValidatableType
calls to prevent lost assertion exceptions
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
SkipValidationAttribute.cs | New experimental attribute class with proper target specification |
ValidationsGenerator.TypesParser.cs | Updates source generator to check for and skip validation-skipped elements |
ITypeSymbolExtensions.cs | Helper methods to determine if symbols have skip validation attributes |
RuntimeValidatableParameterInfoResolver.cs | Runtime resolver updated to respect skip validation attributes |
ValidationsGeneratorTestBase.cs | Test infrastructure fix to properly await validation type verification |
Multiple test snapshots | Generated code snapshots showing proper validation skipping behavior |
...lidation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGeneratorTestBase.cs
Show resolved
Hide resolved
...lidation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGeneratorTestBase.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far.
I'll take a more detailed look at the tests tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Let's have @captainsafia take a look too before we merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good on overall structure and implementation!
I left some non-blocking feedback inline about test coverage and source location.
...n/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.SkipValidation.cs
Show resolved
Hide resolved
Thanks, I made the suggested changes 👍 |
Adds an experimental attribute
[SkipValidation]
that can be used to opt out of validation for a specific property, method parameter, or type.Also fixes issue with
GeneratorTests
that were usingVerifyValidatableType
- now the tests can actually fail because the call is awaited and the assertion exceptions are not lost. (Tests usingVerifyEndpoint
were already awaited.)Fixes #62862