Skip indexer properties in validation source generator#65432
Open
adityamandaleeka wants to merge 2 commits intodotnet:mainfrom
Open
Skip indexer properties in validation source generator#65432adityamandaleeka wants to merge 2 commits intodotnet:mainfrom
adityamandaleeka wants to merge 2 commits intodotnet:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a runtime crash in the minimal API validation source generator by ensuring C# indexer properties (e.g., this[int]) are not treated as validatable members, preventing invalid generated ValidatablePropertyInfo entries for types like JsonElement and Dictionary<TKey,TValue>.
Changes:
- Skip indexer properties during validatable member discovery (
IPropertySymbol.IsIndexer). - Add a generator test covering
JsonElementas a parameter and as a property on another type. - Update generator snapshot baselines to reflect the new exclusion behavior (notably removing the previously-generated
Dictionary<,>entry driven by its indexer).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Validation/gen/Parsers/ValidationsGenerator.TypesParser.cs | Filters out indexer properties during member extraction to avoid generating invalid property metadata. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.ComplexType.cs | Adds a regression test validating behavior with JsonElement and ensuring other properties still validate. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.SkipsIndexerPropertiesOnTypes#ValidatableInfoResolver.g.verified.cs | New snapshot verifying generated output excludes indexer-driven members and only includes meaningful properties. |
| src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.CanValidateParameters#ValidatableInfoResolver.g.verified.cs | Updates snapshot to remove previously-generated Dictionary<string, TestService> validatable info that came solely from an indexer. |
…on source generator.
Member
Author
|
http2 CI failure is unrelated. Opened #65454 to fix that flakiness. |
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.
Fixes #65424
The validation source generator crashes at runtime when encountering types with indexers (e.g.
JsonElement,Dictionary<K,V>) used as minimal API endpoint parameters withAddValidation()enabled.The source generator discovers validatable members by iterating
typeSymbol.GetMembers().OfType<IPropertySymbol>(), but Roslyn exposes C# indexers (this[int index]) asIPropertySymbolwithName = "this[]". The generator emits these asValidatablePropertyInfoentries. At runtime,ValidatablePropertyInfo.ValidateAsynccallsDeclaringType.GetProperty("this[]"), which returns null because reflection names indexers"Item", causing:Confirmed this by decompiling the generated assembly, which showed:
Fix
Added
member.IsIndexerto the existing skip checks inExtractValidatableMembers. Indexers represent parameterized access patterns, not data properties — they aren't meaningful targets for data annotation validation.Testing
SkipsIndexerPropertiesOnTypestest verifyingJsonElementworks as both a direct parameter and a property on another type, and that validation still runs on non-indexer properties of the containing type.CanValidateParameterssnapshot —Dictionary<string, TestService>was previously emitted solely due to its indexer, which would crash at runtime. It's now correctly excluded.