Simplify the generated code from ValidationsGenerator#67630
Simplify the generated code from ValidationsGenerator#67630Youssef1313 wants to merge 4 commits into
Conversation
ca9a987 to
3308ec8
Compare
There was a problem hiding this comment.
Pull request overview
This PR simplifies source-generated validation output by removing generated overrides/caches for type-level validation attributes and shifting responsibility to the runtime ValidatableTypeInfo base implementation. It also updates the generator test snapshots to reflect the new generated output.
Changes:
- Changed
ValidatableTypeInfo.GetValidationAttributes()fromabstracttovirtualand added a default reflection-based implementation. - Simplified generated output by removing the generated
GetValidationAttributes()override and the generated type-attribute cache helper. - Updated many generator snapshot baselines accordingly.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Validation/src/ValidatableTypeInfo.cs | Makes GetValidationAttributes() virtual and provides a default reflection-based implementation. |
| src/Validation/src/PublicAPI.Unshipped.txt | Adds the new virtual ValidatableTypeInfo.GetValidationAttributes() API entry. |
| src/Validation/gen/Emitters/ValidationsGenerator.Emitter.cs | Stops emitting the type-level GetValidationAttributes() override and removes the generated type-attribute cache method. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.ValidatesPropertiesWithJsonIgnoreWhenWritingConditions#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.ValidatesInternalTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.SkipsNonReadableAndStaticProperties#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.SkipsIndexerPropertiesOnTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.SkipsFileLocalTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.SkipsClassesWithNonAccessibleTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.DoesNotEmitForExemptTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.DoesNotEmit_ForSkipValidationAttribute_OnRecordProperties#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.DoesNotEmit_ForSkipValidationAttribute_OnEndpointParameters#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.DoesNotEmit_ForSkipValidationAttribute_OnClassProperties#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateValidationAttributesOnClasses#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateTypeWithParsableProperties#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateRecursiveTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateRecordTypesWithAttribute#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateRecordTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateRecordStructTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidatePolymorphicTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateParameters#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateMultipleNamespaces#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateIValidatableObject#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateIValidatableObject_WithoutPropertyValidations#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateComplexTypesWithJsonIgnore#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateComplexTypes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateClassTypesWithAttribute#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanUseBothFrameworkAndGeneratedValidatableTypeAttributes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanGenerateWhenAddValidationCalledMultipleTimes#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanDiscoverGeneratedValidatableTypeAttribute#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.TypeDisplayName_WithResourceType#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.TypeDisplayName_WithNameOnly#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.TypeDisplayName_WithDisplayNameAttribute#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.RecordPropertyDisplayName_WithResourceType#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.RecordPropertyDisplayName_WithNameOnly#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.RecordPropertyDisplayName_LiteralOnConstructorParameter#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.PropertyDisplayName_WithResourceType#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.PropertyDisplayName_WithoutDisplayAttribute_UsesPropertyName#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.PropertyDisplayName_WithNameOnly#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.PropertyDisplayName_WithDisplayNameAttribute#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.PropertyDisplayName_WithControlCharacters_EmitsValidLiteral#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.ParameterDisplayName_WithResourceType#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorDisplayNameTests.ParameterDisplayName_WithNameOnly#ValidatableInfoResolver.g.verified.cs | Snapshot update for new generated resolver/type info output. |
| @@ -1,495 +1,500 @@ | |||
| // Licensed to the .NET Foundation under one or more agreements. | |||
There was a problem hiding this comment.
There seems to be a line-ending switch in this file that create a long spurious diff.
| } | ||
|
|
||
| {{GeneratedCodeAttribute}} | ||
| file static class DisplayAttributeCache |
There was a problem hiding this comment.
Lets also remove the DisplayAttributeCache and simply store the DisplayAttribute instance in PropertyResourceDisplayName/TypeResourceDisplayName. These are created at most once per cached type info.
| /// <returns>An array of validation attributes to apply to this property.</returns> | ||
| protected abstract ValidationAttribute[] GetValidationAttributes(); | ||
| protected virtual ValidationAttribute[] GetValidationAttributes() | ||
| => _validationAttributes; |
There was a problem hiding this comment.
I know I said to remove the use of Lazy<T>, but maybe we could have a simple
protected virtual ValidationAttribute[] GetValidationAttributes()
=> _validationAttributes ?? = GetValidationAttributesCore();This way a derived implementation that overrides GetValidationAttributes can avoid calling the reflection-based GetValidationAttributesCore completely.
| var results = new List<ValidationAttribute>(); | ||
|
|
||
| // Get attributes from the property | ||
| var property = Property; |
There was a problem hiding this comment.
The following test needs to be fixed to conform with the new throwing DeclaredOnly property access:
Microsoft.Extensions.Validation.Tests.ValidatableTypeInfoTests.TryFindProperty_LocalDeclarationShadowsInheritedProperty
{PR title}
Summary of the changes (Less than 80 chars)
Description
{Detail}
Fixes #{bug number} (in this specific format)