Skip to content

Commit 2b705fa

Browse files
Compute union switch-arm pattern type from the case symbol in the parser
The emitter previously derived the C# `value switch` pattern type by stripping a trailing `?` character from the case's fully-qualified name — a stringly-typed shortcut that relied on the `SymbolDisplayFormat` spelling of `Nullable<T>` always ending in `?` and would silently produce a malformed identifier for any FQN that did not. Reviewer feedback flagged this as unsafe. Move the decision to the parser, where the actual `ITypeSymbol` is in scope. `UnionCaseSpec` gains a `PatternType : TypeRef` property which is * `Nullable<T>.TypeArguments[0]` (the underlying `T`) for value-type `Nullable<T>` cases, and * `CaseType` for everything else. The emitter just reads `caseSpec.PatternType.FullyQualifiedName`; the `GetPatternTypeFullyQualifiedName` helper is deleted. Snapshot output is byte-identical (`StringOrIntUnion` baseline still matches), all four test suites stay at the same counts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2ea38bb commit 2b705fa

3 files changed

Lines changed: 26 additions & 23 deletions

File tree

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ private static SourceText GenerateForUnion(ContextGenerationSpec contextSpec, Ty
677677
continue;
678678
}
679679

680-
string patternTypeFQN = GetPatternTypeFullyQualifiedName(caseSpec);
680+
string patternTypeFQN = caseSpec.PatternType.FullyQualifiedName;
681681
writer.WriteLine($"{patternTypeFQN} caseValue{ctorArmIndex} => new {genericArg}(caseValue{ctorArmIndex}),");
682682
ctorArmIndex++;
683683
}
@@ -751,7 +751,7 @@ private static SourceText GenerateForUnion(ContextGenerationSpec contextSpec, Ty
751751
continue;
752752
}
753753

754-
string patternTypeFQN = GetPatternTypeFullyQualifiedName(caseSpec);
754+
string patternTypeFQN = caseSpec.PatternType.FullyQualifiedName;
755755
writer.WriteLine($"{patternTypeFQN} caseValue{deconArmIndex} => (typeof({caseSpec.CaseType.FullyQualifiedName}), (object?)caseValue{deconArmIndex}),");
756756
deconArmIndex++;
757757
}
@@ -784,26 +784,6 @@ private static SourceText GenerateForUnion(ContextGenerationSpec contextSpec, Ty
784784
return CompleteSourceFileAndReturnText(writer);
785785
}
786786

787-
/// <summary>
788-
/// Returns the type name to use in a <c>value switch</c> pattern. C# rejects
789-
/// <c>Nullable&lt;T&gt;</c> (in any spelling — <c>T?</c>, <c>Nullable&lt;T&gt;</c>,
790-
/// <c>System.Nullable&lt;T&gt;</c>) in a pattern with CS8116 and directs the user
791-
/// to the underlying <c>T</c>. At runtime a boxed <c>Nullable&lt;T&gt;</c> with
792-
/// HasValue=true is bit-identical to a boxed <c>T</c>, so the resulting arm
793-
/// naturally covers the non-null payloads of both <c>Foo(T)</c> and
794-
/// <c>Foo(Nullable&lt;T&gt;)</c> ctors.
795-
/// </summary>
796-
private static string GetPatternTypeFullyQualifiedName(UnionCaseSpec caseSpec)
797-
{
798-
string fqn = caseSpec.CaseType.FullyQualifiedName;
799-
if (caseSpec.CaseType.SpecialType is SpecialType.System_Nullable_T && fqn.Length > 0 && fqn[fqn.Length - 1] == '?')
800-
{
801-
return fqn.Substring(0, fqn.Length - 1);
802-
}
803-
804-
return fqn;
805-
}
806-
807787
/// <summary>
808788
/// Formats the cast prefix used by the <c>null =&gt;</c> arm of the constructor
809789
/// switch (e.g. <c>(int?)</c>). For a Nullable&lt;T&gt; case the FQN already ends

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,9 +749,21 @@ private TypeGenerationSpec ParseTypeGenerationSpec(in TypeToGenerate typeToGener
749749
break;
750750
}
751751

752+
TypeRef caseTypeRef = EnqueueType(caseType, typeToGenerate.Mode);
753+
754+
// C# rejects Nullable<T> in `value switch` patterns (CS8116). The CLR layer
755+
// boxes a Nullable<T> with HasValue=true bit-identically to a boxed T, so the
756+
// generated pattern arm uses the underlying T symbol — never the source
757+
// Nullable<T> string spelling. Compute this from the symbol here so the
758+
// emitter never has to manipulate FQN strings.
759+
TypeRef patternTypeRef = caseType is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } nullableCaseType
760+
? new TypeRef(nullableCaseType.TypeArguments[0])
761+
: caseTypeRef;
762+
752763
resolvedUnionCaseSpecs.Add(new UnionCaseSpec
753764
{
754-
CaseType = EnqueueType(caseType, typeToGenerate.Mode),
765+
CaseType = caseTypeRef,
766+
PatternType = patternTypeRef,
755767
IsNullable = acceptsNull,
756768
IsSwitchArm = switchArmRoles[i],
757769
});

‎src/libraries/System.Text.Json/gen/Model/UnionCaseSpec.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,17 @@ public sealed record UnionCaseSpec
1818
{
1919
public required TypeRef CaseType { get; init; }
2020

21+
/// <summary>
22+
/// Type symbol used in the generated <c>value switch</c> arm pattern.
23+
/// For a value-type <c>Nullable&lt;T&gt;</c> case this is the underlying
24+
/// <c>T</c> (C# rejects <c>Nullable&lt;T&gt;</c> in a pattern with CS8116
25+
/// — at the CLR layer a boxed <c>Nullable&lt;T&gt;</c> with HasValue=true
26+
/// is bit-identical to a boxed <c>T</c>, so the underlying-type arm covers
27+
/// both <c>Foo(T)</c> and <c>Foo(Nullable&lt;T&gt;)</c> non-null payloads).
28+
/// For every other shape this equals <see cref="CaseType"/>.
29+
/// </summary>
30+
public required TypeRef PatternType { get; init; }
31+
2132
public required bool IsNullable { get; init; }
2233

2334
/// <summary>

0 commit comments

Comments
 (0)