Support generic converters on generic types with JsonConverterAttribute by Copilot · Pull Request #123209 · dotnet/runtime

Copilot · 2026-01-15T09:52:54Z

Description

JsonConverterAttribute now supports open generic converter types on generic types when the type parameter arity matches. This works for both the reflection-based provider and source generator.

Example Usage

[JsonConverter(typeof(OptionConverter<>))]
public readonly struct Option<T>
{
    public bool HasValue { get; }
    public T Value { get; }

    public Option(T value)
    {
        HasValue = true;
        Value = value;
    }
}

public sealed class OptionConverter<T> : JsonConverter<Option<T>>
{
    // ... implementation
}

// Now works - previously threw "Type.ContainsGenericParameters is true"
JsonSerializer.Serialize(new Option<int>(42));

Changes

DefaultJsonTypeInfoResolver.Converters.cs: When converter type is an open generic with matching arity to typeToConvert, automatically construct the closed generic converter type using MakeGenericType. The reflection APIs (Type.GetGenericArguments() and Type.MakeGenericType()) handle nested generic types implicitly.
JsonSourceGenerator.Parser.cs:
- Added GetTotalTypeParameterCount() to count all type parameters including those from containing types for nested generics
- Added ConstructNestedGenericType() to properly construct closed generic types for nested generic converters (e.g., Container<>.NestedConverter<>)
- Propagate typeToConvert through converter attribute resolution to enable open generic detection
- The source generator requires manual traversal of the nesting hierarchy because Roslyn's INamedTypeSymbol only exposes TypeParameters for the immediate type, unlike reflection's Type.GetGenericArguments() which returns all type parameters across the hierarchy
Tests: Comprehensive coverage across reflection, source generator integration, and source generator unit tests including:
- Basic single type parameter (Option<T> with OptionConverter<T>)
- Multiple type parameters (Result<TValue, TError> with ResultConverter<TValue, TError>)
- Property-level open generic converters
- Nested generic converters (Container<>.NestedConverter<>)
- Deeply nested converters with non-generic intermediate types (OuterGeneric<>.MiddleNonGeneric.InnerConverter<>)
- Converters nested in non-generic outer classes (NonGenericOuter.SingleLevelConverter<>)
- Asymmetric nesting with many type parameters distributed unevenly across levels (Level1<,,>.Level2<>.Level3Converter<> with 5 params split 3+1+1)
- Constrained generic converters (where T : class)
- Error scenarios: arity mismatch, constraint violations, converter type mismatch
- Source generator diagnostic tests for arity mismatch warnings
- Tests use [Theory] with [InlineData] where applicable per codebase conventions

Supported Scenarios

Simple generic converters: [JsonConverter(typeof(OptionConverter<>))] on Option<T>
Multiple type parameters: [JsonConverter(typeof(ResultConverter<,>))] on Result<TValue, TError>
Nested generic converters: [JsonConverter(typeof(Container<>.NestedConverter<>))] on TypeWithNestedConverter<T1, T2> — type parameters matched in order of definition regardless of nestedness
Deeply nested converters: [JsonConverter(typeof(OuterGeneric<>.MiddleNonGeneric.InnerConverter<>))] — generic within non-generic within generic
Asymmetric nesting: [JsonConverter(typeof(Level1<,,>.Level2<>.Level3Converter<>))] — each nesting level consumes a different number of type parameters (3+1+1)
Non-generic outer converters: [JsonConverter(typeof(NonGenericOuter.SingleLevelConverter<>))]
Constrained generic converters: ConverterWithClassConstraint<T> where T : class

Error Handling

Arity mismatch: Throws ArgumentException at runtime; source generator emits warning diagnostic
Constraint violation: Throws ArgumentException at runtime
Converter type mismatch: Throws InvalidOperationException at runtime (converter doesn't convert the target type)

Original prompt

This section details on the original issue you should resolve

<issue_title>JsonConverterAttribute should support generic converters on generic types</issue_title>
<issue_description>The following code:
using System.Text.Json;
using System.Text.Json.Serialization;

JsonSerializer.Serialize(new Option<int>(42));

[JsonConverter(typeof(OptionConverter<>))]
public readonly struct Option<T>
{
    public bool HasValue { get; }
    public T Value { get; }

    public Option(T value)
    {
        HasValue = true;
        Value = value;
    }

    public static implicit operator Option<T>(T value) => new(value);
}

public sealed class OptionConverter<T> : JsonConverter<Option<T>>
{
    public override bool HandleNull => true;

    public override Option<T> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return new(JsonSerializer.Deserialize<T>(ref reader, options)!);
    }

    public override void Write(Utf8JsonWriter writer, Option<T> value, JsonSerializerOptions options)
    {
        if (value.HasValue)
        {
            writer.WriteNullValue();
            return;
        }

        JsonSerializer.Serialize(writer, value.Value, options);
    }
}
fails today with the error:
Unhandled exception. System.ArgumentException: Cannot create an instance of OptionConverter`1[T] because Type.ContainsGenericParameters is true.
We should make such use cases work provided that type parameters of the converter precisely match the arity of the targeted type. Such a convention would also work well in the context of the source generator and would provide a good solution for the problem as posited in #73124</issue_description>

<agent_instructions>See if you can make this work for both the reflection based provider and source generator. As a reference you can use https://github.com/eiriktsarpalis/PolyType which already supports this pattern for marshallers in surrogate types.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes JsonConverterAttribute should support generic converters on generic types #123208

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

eiriktsarpalis · 2026-01-15T11:51:26Z

@copilot there appear to be build failures related to the source generator. Please fix those.

Copilot · 2026-01-15T12:29:58Z

@copilot there appear to be build failures related to the source generator. Please fix those.

Fixed in e5944c5. The issue was that the code tried to assign to namedConverterType from a pattern-matched variable that wasn't in scope when entering the if block via certain conditions. I restructured the code to first check for unbound generic types and construct them, then validate the resulting type.

eiriktsarpalis

@copilot you should add a few tests that check what happens if there is a mismatch in type parameters or type constraints between the two types, or if a the generic instantiation implements a converter not matching the original type. Add tests both at runtime and source generator unit tests checking for relevant diagnostic messages.

...stem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.CustomConverters.cs

Copilot · 2026-01-15T17:01:55Z

@copilot you should add a few tests that check what happens if there is a mismatch in type parameters or type constraints between the two types, or if a the generic instantiation implements a converte...

Added tests in 92d1a10:

Arity mismatch: GenericConverterAttribute_ArityMismatch_ThrowsArgumentException verifies that converters with mismatched type parameter counts throw ArgumentException
Constraint violation: GenericConverterAttribute_ConstraintViolation_ThrowsArgumentException verifies that constraint violations throw ArgumentException
Type mismatch: GenericConverterAttribute_ConverterTypeMismatch_ThrowsInvalidOperationException verifies that converters for different types throw InvalidOperationException
Source generator unit tests: GenericConverterArityMismatch_WarnsAsExpected and GenericConverterTypeMismatch_NoSourceGeneratorWarning_FailsAtRuntime (source generator cannot detect type mismatch at compile time)

eiriktsarpalis · 2026-01-26T09:57:27Z

@stephentoub @PranavSenthilnathan before we get this merged, I would appreciate feedback on the merits of this new convention.

Copilot

Pull request overview

This PR extends JsonConverterAttribute to accept open generic converter types when applied to generic target types with matching arity, supporting both the reflection-based resolver and the source generator (including nested generic converter scenarios).

Changes:

Runtime: construct a closed converter type from an open generic converter type when the target type is a closed generic with matching arity.
Source generator: resolve JsonConverterAttribute for open generic (including nested) converter types by constructing the appropriate closed converter type.
Tests: add reflection and source-generation test coverage for supported scenarios and key failure modes (e.g., arity mismatch diagnostics).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs	Constructs closed generic converter types at runtime when attribute specifies an open generic converter with matching arity.
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs	Adds logic to close unbound generic (including nested) converter types during generator parsing.
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Attribute.cs	Adds reflection-based tests covering open generic converters, nested converters, constraints, and mismatch scenarios.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs	Adds generator diagnostic tests for arity mismatch and verifies generator behavior for other scenarios.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/CompilationHelper.cs	Adds compilation helpers to generate test sources for the new diagnostic and nested/constrained scenarios.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.CustomConverters.cs	Adds source-gen test model types/converters for open generic and nested generic converter cases.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs	Adds integration tests validating source-gen contexts can serialize/deserialize with open generic converters.

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

stephentoub · 2026-01-26T16:40:54Z

@stephentoub @pranavkm before we get this merged, I would appreciate feedback on the merits of this new convention.

@eiriktsarpalis what's the downside?

eiriktsarpalis · 2026-01-26T16:46:18Z

@stephentoub @pranavkm before we get this merged, I would appreciate feedback on the merits of this new convention.

@eiriktsarpalis what's the downside?

None that I'm aware of. With the exception of debugger proxies I'm not aware of any serializers/source generators applying this convention, so it might be difficult for users to discover.

stephentoub · 2026-01-26T16:47:46Z

It seems fine to me. Hard to imagine what else it could mean.

eiriktsarpalis · 2026-01-26T17:45:45Z

@copilot please file a PR against https://github.com/dotnet/docs extending the https://github.com/dotnet/docs/blob/main/docs/standard/serialization/system-text-json/converters-how-to.md article with instructions and examples of how to use this new convention. Explain how in certain cases it provides an AOT compatible alternative JsonConverterFactory.

eiriktsarpalis

@stephentoub I would appreciate your review here.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

stephentoub · 2026-02-25T13:35:20Z

@copilot please invoke the code-review skill and post the analysis/results as a comment on this PR

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

...t.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs

...System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs

Copilot · 2026-02-25T13:49:03Z

@copilot please invoke the code-review skill and post the analysis/results as a comment on this PR

🤖 Copilot Code Review — PR #113609

Holistic Assessment

Motivation: The PR addresses a clear gap — [JsonConverter(typeof(OptionConverter<>))] on Option<T> would throw Type.ContainsGenericParameters is true. This is a natural extension that users would expect to work.

Approach: The reflection path is minimal and correct — a single MakeGenericType call, because Type.GetGenericArguments() and MakeGenericType() inherently handle nested generics. The source generator path is necessarily more complex because Roslyn's INamedTypeSymbol only exposes TypeParameters for the immediate type, requiring manual traversal of the nesting hierarchy.

Summary: ⚠️ Needs Human Review. The core logic is correct and well-tested. I have a few concerns about edge cases and code structure that a maintainer should evaluate.

Detailed Findings

✅ Reflection path — Clean and correct

The reflection change (Converters.cs:195-199) is minimal:

Type.GetGenericArguments() returns all type parameters across the nesting hierarchy
Type.MakeGenericType() distributes them correctly
Constraint violations and arity mismatches naturally throw ArgumentException from MakeGenericType
The subsequent CanConvert check catches type mismatches (converter for wrong type)

No issues found here.

⚠️ Source generator — `ConstructNestedGenericType` uses LINQ in a potentially hot path

ConstructNestedGenericType (line 1764) uses typeArguments.Skip(typeArgIndex).Take(typeParamCount).ToArray() per nesting level. This allocates per level. For a source generator that runs during compilation, this is unlikely to be a real perf issue, but it's inconsistent with the rest of the source generator code which generally avoids LINQ. Consider using typeArguments.AsSpan().Slice(typeArgIndex, typeParamCount).ToArray() or ImmutableArray.CreateRange.

⚠️ Source generator — `GetConverterTypeFromAttribute` line 372 call doesn't pass `typeToConvert`

At line 372, GetConverterTypeFromAttribute is called for converters in JsonSourceGenerationOptionsAttribute.Converters without a typeToConvert. This is correct since those converters are global (not bound to a specific type), but it means if someone puts an open generic converter in the global list, it will fall through to the diagnostic warning. This is probably the correct behavior, but worth documenting with a comment.

✅ Type-level attribute path handles `typeToConvert` correctly

At line 783, when [JsonConverter] is on the type itself, typeToConvert defaults to declaringSymbol (the type) via the ??= fallback at line 1676. This is correct.

✅ Property-level attribute path passes `memberType`

At line 1360, ProcessMemberCustomAttributes passes memberType to GetConverterTypeFromJsonConverterAttribute. This correctly handles [JsonConverter(typeof(WrapperConverter<>))] on a property of type Wrapper<int>.

⚠️ Test coverage — Missing negative test for unsatisfied constraint in source generator

The source generator unit tests include GenericConverterArityMismatch_WarnsAsExpected and GenericConverterTypeMismatch_NoSourceGeneratorWarning_FailsAtRuntime, but there's no source gen unit test for an unsatisfied constraint (e.g., [JsonConverter(typeof(ConverterWithClassConstraint<>))] on a type instantiated with a value type like int). While the Roslyn Construct() call may handle this gracefully, it would be good to verify the diagnostic behavior.

💡 Test style — Many individual `[Fact]` methods for closely related scenarios

The runtime tests add 10+ individual [Fact] methods (e.g., GenericConverterAttributeOnGenericType_Serialize, _Deserialize, _ComplexType, _NestedInClass, etc.). Per codebase convention, [Theory] with [InlineData] is preferred over multiple duplicative [Fact] methods. Some of these could be consolidated. This is a style nit and not blocking.

✅ Error handling — Failure modes are well-covered

Arity mismatch: MakeGenericType throws ArgumentException (runtime), source gen warns
Constraint violation: MakeGenericType throws ArgumentException (runtime)
Type mismatch: CanConvert returns false → InvalidOperationException (runtime)
Source gen can't detect type mismatch at compile time (correctly documented in test comments)

…converter tests Co-authored-by: stephentoub <[email protected]>

...stem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.CustomConverters.cs

… levels (3+1+1) Co-authored-by: stephentoub <[email protected]>

Copilot AI assigned Copilot and eiriktsarpalis Jan 15, 2026

Copilot started work on behalf of eiriktsarpalis January 15, 2026 09:53 View session

Copilot AI changed the title ~~[WIP] Add support for generic converters on generic types~~ Support generic converters on generic types with JsonConverterAttribute Jan 15, 2026

Copilot AI requested a review from eiriktsarpalis January 15, 2026 10:47

Copilot finished work on behalf of eiriktsarpalis January 15, 2026 10:47

Copilot started work on behalf of eiriktsarpalis January 15, 2026 11:52 View session

Copilot finished work on behalf of eiriktsarpalis January 15, 2026 12:31

This was referenced Jan 15, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

MsQuic fails with QUIC_STATUS_OUT_OF_MEMORY on AzureLinux #123216

Open

eiriktsarpalis reviewed Jan 15, 2026

View reviewed changes

...stem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.CustomConverters.cs Show resolved Hide resolved

Copilot started work on behalf of eiriktsarpalis January 15, 2026 16:18 View session

Copilot finished work on behalf of eiriktsarpalis January 15, 2026 17:10

Copilot AI requested a review from eiriktsarpalis January 15, 2026 17:10

stephentoub added the area-System.Text.Json label Jan 26, 2026

eiriktsarpalis marked this pull request as ready for review January 26, 2026 09:54

eiriktsarpalis requested review from PranavSenthilnathan, Copilot and stephentoub January 26, 2026 09:54

Copilot started reviewing on behalf of eiriktsarpalis January 26, 2026 09:54 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs Show resolved Hide resolved

eiriktsarpalis approved these changes Jan 26, 2026

View reviewed changes

eiriktsarpalis force-pushed the copilot/support-generic-converters branch from f77e83d to 3ee8f69 Compare February 20, 2026 15:10

eiriktsarpalis requested a review from Copilot February 20, 2026 15:11

eiriktsarpalis approved these changes Feb 20, 2026

View reviewed changes

Copilot started reviewing on behalf of eiriktsarpalis February 20, 2026 15:12 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

This was referenced Feb 20, 2026

Sometimes the helix SDK uses GetWorkItemsAsync when workitems aren't done processing. dotnet/dnceng#6011

Open

[android][clr] No peer certificates when executing System.Net.Http.Functional.Tests on Android emulator #124526

Open

Merge branch 'main' into copilot/support-generic-converters

5c5afd0

Copilot started work on behalf of stephentoub February 25, 2026 13:35 View session

stephentoub reviewed Feb 25, 2026

View reviewed changes

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs Show resolved Hide resolved

stephentoub reviewed Feb 25, 2026

View reviewed changes

...t.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs Show resolved Hide resolved

Copilot AI requested a review from stephentoub February 25, 2026 13:42

stephentoub reviewed Feb 25, 2026

View reviewed changes

...System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs Outdated Show resolved Hide resolved

Copilot started work on behalf of stephentoub February 25, 2026 13:43 View session

stephentoub reviewed Feb 25, 2026

View reviewed changes

...System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs Show resolved Hide resolved

stephentoub approved these changes Feb 25, 2026

View reviewed changes

Copilot finished work on behalf of stephentoub February 25, 2026 13:50

Copilot AI requested a review from stephentoub February 25, 2026 13:50

Copilot started work on behalf of stephentoub February 25, 2026 13:50 View session

Combine source gen tests with [Theory] and add deeply nested generic …

837d090

…converter tests Co-authored-by: stephentoub <[email protected]>

Copilot finished work on behalf of stephentoub February 25, 2026 14:21

stephentoub reviewed Feb 25, 2026

View reviewed changes

...stem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.CustomConverters.cs Show resolved Hide resolved

Copilot started work on behalf of stephentoub February 25, 2026 14:24 View session

Add test with 5 type params asymmetrically distributed across nesting…

270b4e1

… levels (3+1+1) Co-authored-by: stephentoub <[email protected]>

Copilot AI requested a review from stephentoub February 25, 2026 14:57

Copilot finished work on behalf of stephentoub February 25, 2026 14:57

build-analysis bot mentioned this pull request Feb 25, 2026

[FAIL] System.Net.Http.Functional.Tests.SocketsHttpHandler_HttpClientHandlerTest.LargeUriAndHeaders_Works #124487

Open

Conversation

Copilot AI commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Example Usage

Changes

Supported Scenarios

Error Handling

Comments on the Issue (you are @copilot in this section)

Uh oh!

eiriktsarpalis commented Jan 15, 2026

Uh oh!

Copilot AI commented Jan 15, 2026

Uh oh!

eiriktsarpalis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Jan 15, 2026

Uh oh!

eiriktsarpalis commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

stephentoub commented Jan 26, 2026

Uh oh!

eiriktsarpalis commented Jan 26, 2026

Uh oh!

stephentoub commented Jan 26, 2026

Uh oh!

eiriktsarpalis commented Jan 26, 2026

Uh oh!

eiriktsarpalis left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

stephentoub commented Feb 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Feb 25, 2026

🤖 Copilot Code Review — PR #113609

Holistic Assessment

Detailed Findings

✅ Reflection path — Clean and correct

⚠️ Source generator — ConstructNestedGenericType uses LINQ in a potentially hot path

⚠️ Source generator — GetConverterTypeFromAttribute line 372 call doesn't pass typeToConvert

✅ Type-level attribute path handles typeToConvert correctly

✅ Property-level attribute path passes memberType

⚠️ Test coverage — Missing negative test for unsatisfied constraint in source generator

💡 Test style — Many individual [Fact] methods for closely related scenarios

✅ Error handling — Failure modes are well-covered

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Jan 15, 2026 •

edited

Loading

eiriktsarpalis commented Jan 26, 2026 •

edited

Loading

⚠️ Source generator — `ConstructNestedGenericType` uses LINQ in a potentially hot path

⚠️ Source generator — `GetConverterTypeFromAttribute` line 372 call doesn't pass `typeToConvert`

✅ Type-level attribute path handles `typeToConvert` correctly

✅ Property-level attribute path passes `memberType`

💡 Test style — Many individual `[Fact]` methods for closely related scenarios