From 60dac4f19b7df407911e73bfa47d8c2152ce8282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20K=C3=B6plinger?= Date: Tue, 4 Aug 2026 15:08:36 +0200 Subject: [PATCH] Always match the union payload with a property pattern in the STJ generator Follow-up to the review feedback on #131569. The generated union deconstructor only used the `{ Value: T x }` form for the case whose type is the union type itself and kept a bare type pattern for every other case. Emitting the property pattern unconditionally is more robust: it does not depend on the compiler's union-unwrapping rule for type patterns, it always verifies the payload rather than the union instance, and it removes the special case from the emitter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8591ab4e-b1d7-40df-80f0-9a476a7bfd9d --- .../gen/JsonSourceGenerator.Emitter.cs | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 6f5654c1e7fbd4..aaae2f3312996a 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -782,23 +782,14 @@ private static SourceText GenerateForUnion(ContextGenerationSpec contextSpec, Ty continue; } - string patternTypeFQN = caseSpec.PatternType.FullyQualifiedName; - - if (patternTypeFQN == typeMetadata.TypeRef.FullyQualifiedName) - { - // Recursive case: the case type is the union type itself. A type pattern `T` - // applied to a union is equivalent to `T or { Value: T }`, so a bare type - // pattern here is also satisfied by the union instance itself. That both binds - // the union rather than its payload -- making the converter recurse on the same - // value forever -- and renders any later arm unreachable. Match the payload - // explicitly so that only the unwrapped value is bound. - writer.WriteLine($"{{ Value: {patternTypeFQN} caseValue{deconArmIndex} }} => (typeof({caseSpec.CaseType.FullyQualifiedName}), (object?)caseValue{deconArmIndex}),"); - } - else - { - writer.WriteLine($"{patternTypeFQN} caseValue{deconArmIndex} => (typeof({caseSpec.CaseType.FullyQualifiedName}), (object?)caseValue{deconArmIndex}),"); - } - + // Match the payload through a property pattern rather than applying a type + // pattern to the union itself. A type pattern `T` applied to a union is + // equivalent to `T or { Value: T }`, so for a case whose type is the union + // type itself the union instance also matches: that would bind the union + // rather than its payload -- making the converter recurse on the same value + // forever -- and would render every later arm unreachable. The explicit form + // binds only the unwrapped value and behaves the same for all other cases. + writer.WriteLine($"{{ Value: {caseSpec.PatternType.FullyQualifiedName} caseValue{deconArmIndex} }} => (typeof({caseSpec.CaseType.FullyQualifiedName}), (object?)caseValue{deconArmIndex}),"); deconArmIndex++; }