-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fix Validation SG for Blazor Wasm SDK projects #63687
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
8708102
to
1bee48c
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 PR fixes validation source generator support for Blazor WebAssembly SDK projects by ensuring the generator is packaged with the NuGet package and handling missing well-known types gracefully. The changes allow the validation functionality to work in environments where certain ASP.NET Core assemblies are not available.
- Package the validation source generator in the NuGet package for Blazor WASM compatibility
- Replace exception throwing with dummy type symbols for missing well-known types
- Introduce a fallback mechanism using
MissingType
when assemblies are not present
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Validation/src/Microsoft.Extensions.Validation.csproj | Adds project reference and packaging configuration to include the validation source generator in the NuGet package |
src/Shared/RoslynUtils/WellKnownTypes.cs | Replaces exception throwing with dummy type symbol fallback when well-known types cannot be resolved |
{ | ||
_lazyWellKnownTypes = new INamedTypeSymbol?[WellKnownTypeData.WellKnownTypeNames.Length]; | ||
_compilation = compilation; | ||
_missingTypeSymbol = compilation.GetTypeByMetadataName(typeof(MissingType).FullName!)!; |
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.
I wonder if we should consider reusing SpecialType.None as the fallback in this case as Roslyn does in their WellKnownType implementation (ref). That let's us reuse an existing concept instead of defining out own marker type.
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.
Compilation.GetSpecialType
throws out-of-range exception if you try to resolve INamedTypeSymbol
for SpecialType.None
. The way CodeAnalysis.WellKnownTypes
is set up in Roslyn, there is no actual type symbol associated with an unknown type. So if I am not overlooking something, we can't use it as a marker type for our purposes.
/backport to release/10.0 |
Started backporting to release/10.0: https://github.com/dotnet/aspnetcore/actions/runs/17827846351 |
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.
LGTM! The WellKnownTypes are shared across all our analyzers.
I imagine the material impact here is that scenarios that previously threw exceptions/gold-barred will now fail silently assuming the logic where the symbol comparisons happen is sane.
Some smoke testing on our framework analyzers should give us confidence here.
Based on offline discussion with @captainsafia I updated the PR so that other analyzers are not affected by the change in type symbol retrieval logic. I added the The existing |
/backport to release/10.0 |
Started backporting to release/10.0: https://github.com/dotnet/aspnetcore/actions/runs/17852955682 |
@oroztocil backporting to "release/10.0" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Proof-of-concept fix for getting validation SG to work in Blazor Wasm SDK projects
Using index info to reconstruct a base tree...
M src/Shared/RoslynUtils/SymbolExtensions.cs
M src/Shared/RoslynUtils/WellKnownTypes.cs
M src/Validation/gen/Extensions/ITypeSymbolExtensions.cs
M src/Validation/src/Microsoft.Extensions.Validation.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/Shared/RoslynUtils/SymbolExtensions.cs
Auto-merging src/Shared/RoslynUtils/WellKnownTypes.cs
CONFLICT (content): Merge conflict in src/Shared/RoslynUtils/WellKnownTypes.cs
Auto-merging src/Validation/gen/Extensions/ITypeSymbolExtensions.cs
Auto-merging src/Validation/src/Microsoft.Extensions.Validation.csproj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Proof-of-concept fix for getting validation SG to work in Blazor Wasm SDK projects
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
The PR solves two issues that prevent the new validation from working in Blazor Wasm SDK projects:
Microsoft.Extensions.Validation
NuGet package needs to include the source generator, so that it is available in Blazor Wasm SDK projects (which are not based on the ASP.NET Core runtime pack, unlike the Web SDK projects).Microsoft.AspNetCore.Http.Abstractions
is not present in Blazor Wasm SDK projects). To avoid the need to refactor larger portion of the existing code, the PR solves this by returning a type symbol for a dummy type instead of throwing. This type symbol is then used in place of the missing types and comparisons against it always return false, thus providing the same outer behavior.Fixes #63670