[STJ Source Gen] Propagate type parameter constraints to generic UnsafeAccessor wrapper classes#126506
[STJ Source Gen] Propagate type parameter constraints to generic UnsafeAccessor wrapper classes#126506steveisok wants to merge 1 commit intodotnet:mainfrom
Conversation
…feAccessor wrapper classes The source generator emits a generic wrapper class for UnsafeAccessor methods when the declaring type is generic. However, it only forwarded the type parameter names without their constraints. This causes CS0314 compilation errors when the declaring type has constrained type parameters (e.g., `where TResponse : AuthenticatorResponse`), because the wrapper class's unconstrained type parameters cannot satisfy the original type's constraints. Fix: collect the constraint clauses from each ITypeParameterSymbol in the parser and emit matching `where` clauses on the generated wrapper class. Fixes compilation of aspnetcore's IdentityJsonSerializerContext which uses PublicKeyCredential<TResponse> where TResponse : notnull, AuthenticatorResponse. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-system-text-json |
|
@eiriktsarpalis you beat me to it. Closing in favor of #126507 |
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the System.Text.Json source generator where generic [UnsafeAccessor] wrapper classes for members on generic declaring types were emitted without propagating the declaring type’s generic type-parameter constraints, leading to compilation errors (e.g., CS0314).
Changes:
- Extend
PropertyGenerationSpecto carry declaring type generic constraint clauses. - Parse
ITypeParameterSymbolconstraints intowhere ...clauses during spec creation. - Emit collected
where ...constraints on generated__GenericAccessors_*<...>wrapper classes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/libraries/System.Text.Json/gen/Model/PropertyGenerationSpec.cs |
Adds a model field to carry declaring-type generic constraint clauses to emission. |
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs |
Builds where clauses from Roslyn type-parameter constraint metadata and stores them on the spec. |
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs |
Emits the stored generic constraint clauses on the generated generic UnsafeAccessor wrapper class. |
| } | ||
| else if (tp.HasReferenceTypeConstraint) | ||
| { | ||
| (parts ??= new()).Add("class"); |
There was a problem hiding this comment.
GetTypeParameterConstraints emits "class" whenever HasReferenceTypeConstraint is true, but it doesn’t account for nullable reference type constraints ("class?") via ITypeParameterSymbol.ReferenceTypeConstraintNullableAnnotation. Emitting "class" for an original "class?" constraint is stricter and can cause compilation failures when the declaring type uses nullable reference type constraints. Consider emitting "class?" when the nullable annotation is Annotated (and keeping "class" otherwise).
| (parts ??= new()).Add("class"); | |
| (parts ??= new()).Add(tp.ReferenceTypeConstraintNullableAnnotation == NullableAnnotation.Annotated ? "class?" : "class"); |
| /// <summary> | ||
| /// Builds the set of "where T : ..." constraint clauses for the given type parameters. | ||
| /// Returns null when no type parameter has any constraint. | ||
| /// </summary> | ||
| private static ImmutableEquatableArray<string>? GetTypeParameterConstraints( | ||
| ImmutableArray<ITypeParameterSymbol> typeParameters) | ||
| { | ||
| List<string>? clauses = null; | ||
|
|
||
| foreach (ITypeParameterSymbol tp in typeParameters) | ||
| { |
There was a problem hiding this comment.
GetTypeParameterConstraints doesn’t propagate the C# 13 "allows ref struct" constraint (Roslyn ITypeParameterSymbol.AllowsRefLikeType). If a declaring type has this constraint, the generated wrapper class must repeat it; otherwise ref-like type arguments won’t be permitted and the generated code can fail to compile. Consider adding support for this constraint (likely via reflection like other generators in the repo do to keep compatibility across Roslyn versions).
| DeclaringTypeParameterNames = memberInfo.ContainingType is INamedTypeSymbol { IsGenericType: true } namedType && _knownSymbols.SupportsGenericUnsafeAccessors | ||
| ? namedType.OriginalDefinition.TypeParameters.Select(tp => tp.Name).ToImmutableEquatableArray() : null, | ||
| DeclaringTypeParameterConstraints = memberInfo.ContainingType is INamedTypeSymbol { IsGenericType: true } namedType2 && _knownSymbols.SupportsGenericUnsafeAccessors | ||
| ? GetTypeParameterConstraints(namedType2.OriginalDefinition.TypeParameters) : null, |
There was a problem hiding this comment.
DeclaringTypeParameterConstraints is built from namedType2.OriginalDefinition.TypeParameters, which only includes type parameters declared on the immediate type (and excludes containing type parameters for nested generic types). For nested generics, this will omit outer constraints and can still produce CS0314/constraint-mismatch errors in the generated _GenericAccessors* wrapper. Consider gathering type parameters (and constraints) from the full containing-type chain (similar to how GetTotalTypeParameterCount handles nested generics).
| writer.WriteLine($"private static class __GenericAccessors_{typeFriendlyName}<{typeParamList}>"); | ||
|
|
||
| if (firstProperty.DeclaringTypeParameterConstraints is { } constraints) | ||
| { | ||
| writer.Indentation++; | ||
| foreach (string constraint in constraints) | ||
| { | ||
| writer.WriteLine(constraint); | ||
| } | ||
| writer.Indentation--; | ||
| } |
There was a problem hiding this comment.
This change introduces new emitted syntax (generic wrapper type parameter constraint clauses) but there isn’t a unit/baseline test exercising a constrained generic declaring type (e.g., where T : notnull, SomeBase or class?). Adding a source-gen output test for constrained generic init-only properties would help prevent regressions like #124650 and validate the exact emitted where clauses.
Description
Fixes the regression introduced in #124650 where the STJ source generator emits generic wrapper classes for
[UnsafeAccessor]methods without propagating the type parameter constraints from the declaring type.The Problem
When a generic type has constrained type parameters (e.g.,
PublicKeyCredential<TResponse> where TResponse : notnull, AuthenticatorResponse) andinit-only properties, the source generator emits a wrapper class like:This causes
CS0314compilation errors. Reported by @akoeplinger in dotnet/dotnet#5880 (comment) — aspnetcore'sIdentityJsonSerializerContextusesPublicKeyCredential<TResponse> where TResponse : notnull, AuthenticatorResponsewithinit-only properties.The Fix
Three changes across the source generator:
Model (
PropertyGenerationSpec): AddedDeclaringTypeParameterConstraintsproperty to carry the constraint clauses (e.g.,["where TResponse : notnull, global::Microsoft.AspNetCore.Identity.AuthenticatorResponse"]).Parser: Added
GetTypeParameterConstraints()helper that readsITypeParameterSymbolproperties (HasNotNullConstraint,HasValueTypeConstraint,HasReferenceTypeConstraint,HasUnmanagedTypeConstraint,HasConstructorConstraint,ConstraintTypes) and builds the correspondingwhereclauses.Emitter: Emits the constraint clauses on the generated wrapper class:
cc @eiriktsarpalis @stephentoub