Fix JsonSerializer eagerly resolving JsonTypeInfo for types with custom converters

[Copilot]

Fix JsonSerializer eagerly resolving JsonTypeInfo for types with custom converters

Progress

• Explore repository structure and understand the issue
• Reproduce the issue with a test case
• Identify root cause in JsonPropertyInfo.Configure() and JsonTypeInfo.Configure()
• Modify JsonPropertyInfo.Configure() to avoid resolving JsonTypeInfo when CustomConverter is present
• Modify JsonTypeInfo.Configure() to skip element/key type resolution for custom converters
• Add comprehensive tests for reflection-based serialization
• Fix JsonPropertyInfo methods to handle null _jsonTypeInfo
• Add source generation tests and enable them in build
• Address code review feedback
• Build and test all changes - ALL TESTS PASSING
• Run code review
• Run CodeQL security scan

Changes Made

1. JsonPropertyInfo.Configure() (JsonPropertyInfo.cs)

• Check if CustomConverter is set before attempting to get JsonTypeInfo
• Only expand the converter if it exists
• This prevents eager resolution of JsonTypeInfo for properties with custom converters
• Improved comment clarity about when JsonTypeInfo resolution is needed

2. JsonPropertyInfoOfT.DetermineEffectiveConverter() (JsonPropertyInfoOfT.cs)

• Updated signature to accept nullable JsonTypeInfo and expanded converter
• Removed redundant JsonTypeInfo assignment - caller now handles this
• Added Debug.Assert to ensure contract is met

3. JsonTypeInfo.Configure() (JsonTypeInfo.cs)

• Added check to skip element/key type resolution for custom converters
• Only resolve element and key types when using built-in converters (Converter.IsInternalConverter)
• This prevents trying to create JsonTypeInfo for element types that may not be serializable

4. JsonPropertyInfo methods (JsonPropertyInfo.cs)

• Updated DetermineNumberHandlingForProperty() to handle null _jsonTypeInfo
• Updated DetermineEffectiveObjectCreationHandlingForProperty() to handle null _jsonTypeInfo
• These changes allow properties with custom converters to skip TypeInfo resolution

5. Tests

Reflection Tests (CustomConverterTests.InvalidTypes.cs) - 5 tests covering:

• Properties with invalid element types (collections) with converters applied to the property
• Properties with invalid types with converters applied to the property
• Properties with invalid types with converters applied to the type via [JsonConverter] attribute
• Dictionary properties with invalid value types with converters
• Validation that without converters, the expected exception is still thrown

Source Generation Tests (InvalidTypesWithConvertersTests.cs) - 4 tests covering:

• Source generated serialization with custom converters on properties with invalid element types
• Source generated serialization with custom converters on properties with invalid types
• Source generated serialization with custom converters on types with ref properties
• Source generated serialization with custom converters on dictionaries with invalid value types

Test Results

✅ All 12 reflection-based tests passing
✅ All 4 source generation tests passing (Roslyn 3.11)
✅ All 4 source generation tests passing (Roslyn 4.4)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>JsonSerializer fails on types it shouldn't be looking at in the first place</issue_title>
<issue_description>### Description

A property has a non-serializable type, but is attributed with a [JsonConverter] that can successfully handle that type. However, JsonSerializer is still trying to create a JsonTypeInfo for the non-serializable type, and fails because it's, well, non-serializable. In this case, that type has a ref property.

Reproduction Steps

using System.Text.Json;
using System.Text.Json.Serialization;

JsonSerializer.Serialize(new MyCollection());

class MyCollection
{
    [JsonConverter(typeof(MyTestsConverter))]
    public IList<MyTest> Tests { get; set; } = [];
}

class MyTest
{
    public int Value1 { get; set; }

    private int _value2;
    public ref int Value2 => ref _value2;
}

class MyTestsConverter : JsonConverter<IList<MyTest>>
{
    public override IList<MyTest>? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return [];
    }

    public override void Write(Utf8JsonWriter writer, IList<MyTest> value, JsonSerializerOptions options)
    {
        writer.WriteStartArray();
        writer.WriteEndArray();
    }
}

Expected behavior

Runs successfully.

Actual behavior

Unhandled exception. System.InvalidOperationException: The type 'System.Int32&' of property 'Value2' on type 'MyTest' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(Type typeToConvert, Type declaringType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreatePropertyInfo(JsonTypeInfo typeInfo, Type typeToConvert, MemberInfo memberInfo, NullabilityInfoContext nullabilityCtx, JsonSerializerOptions options, Boolean shouldCheckForRequiredKeyword, Boolean hasJsonIncludeAttribute)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.AddMembersDeclaredBySuperType(JsonTypeInfo typeInfo, Type currentType, NullabilityInfoContext nullabilityCtx, Boolean constructorHasSetsRequiredMembersAttribute, PropertyHierarchyResolutionState& state)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.PopulateProperties(JsonTypeInfo typeInfo, NullabilityInfoContext nullabilityCtx)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreateTypeInfoCore(Type type, JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoNoCaching(Type type)
   at System.Text.Json.JsonSerializerOptions.CachingContext.CreateCacheEntry(Type type, CachingContext context)
--- End of stack trace from previous location ---
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Nullable`1 ensureNotNull, Boolean resolveIfMutable, Boolean fallBackToNearestAncestorType)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureSynchronized|174_0()
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Nullable`1 ensureNotNull, Boolean resolveIfMutable, Boolean fallBackToNearestAncestorType)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.ConfigureProperties()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureSynchronized|174_0()
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Nullable`1 ensureNotNull, Boolean resolveIfMutable, Boolean fallBackToNearestAncestorType)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoForRootType(Type type, Boolean fallBackToNearestAncestorType)
   at System.Text.Json.JsonSerializer.GetTypeInfo[T](JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at Program.<Main>$(String[] args)

Regression?

No

Known Workarounds

None

Configuration

.NET 10 (also repros on .NET 9 and .NET 8)

Other information

No response</issue_description>

<agent_instructions>The root cause of the is likely due to the fact that the serializer is eagerly resolving the JsonTypeInfo<MyTest> even though it shouldn't need to do so because the IList<MyTest> employs a custom converter.

Make sure that the test covers both the source generated and reflection-...

• Fixes JsonSerializer fails on types it shouldn't be looking at in the first place #121320

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[stephentoub]

@copilot, please add tests that exercise this. They should exercise cases where properties are of invalid types with and without converters applied to the property or applied to the type.

[Copilot]

Pull Request Overview

This PR optimizes the JSON serialization configuration process to avoid unnecessarily resolving JsonTypeInfo when custom converters are present. The key improvement is deferring type info resolution until it's confirmed that no custom converter will handle the serialization.

• Defers JsonTypeInfo resolution for properties with custom converters until after converter expansion
• Skips element/key type resolution for types using custom converters in JsonTypeInfo.Configure()
• Updates the DetermineEffectiveConverter signature to accept both nullable JsonTypeInfo and an expanded custom converter

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
JsonTypeInfo.cs	Guards element and key type resolution to only occur for built-in converters, preventing unnecessary resolution when custom converters handle serialization
JsonPropertyInfo.cs	Expands custom converters before eagerly resolving JsonTypeInfo to avoid failures for unsupported property types with custom converters
JsonPropertyInfoOfT.cs	Updates implementation to handle nullable JsonTypeInfo and pre-expanded converters, with fallback logic to resolve JsonTypeInfo when needed

[Copilot]

The assignment to JsonTypeInfo property (line 164) duplicates the logic already present in JsonPropertyInfo.Configure() (line 443-444). This creates redundancy where _jsonTypeInfo is set in two different places. Consider whether this assignment is necessary here, or if it should only be set in the calling code path in JsonPropertyInfo.Configure() to maintain a single source of truth for this field.

Suggested change

JsonTypeInfo = jsonTypeInfo;

[Copilot]

Fixed in commit a1ee595. Removed the redundant assignment and added a Debug.Assert to ensure the caller provides JsonTypeInfo when no custom converter is available. The caller in JsonPropertyInfo.Configure() now handles setting _jsonTypeInfo before calling DetermineEffectiveConverter.

[Copilot]

The comment on line 442 doesn't fully explain the logical flow. It should clarify that when expandedCustomConverter is null, it could mean either (1) no custom converter was specified, OR (2) a custom converter was specified but the factory returned null after expansion. The current comment could be misread as only covering case 1.

Suggested change

	// No custom converter or the factory returned null, so we need to get the JsonTypeInfo
	// If expandedCustomConverter is null, it means either:
	// (1) no custom converter was specified, or
	// (2) a custom converter was specified but the factory returned null after expansion.
	// In either case, we need to get the JsonTypeInfo.

[Copilot]

Fixed in commit a1ee595. Expanded the comment to clarify both cases when expandedCustomConverter is null: (1) no custom converter was specified, or (2) a custom converter was specified but the factory returned null after expansion.

[Copilot]

@copilot, please add tests that exercise this. They should exercise cases where properties are of invalid types with and without converters applied to the property or applied to the type.

Added comprehensive tests in commit f12b99f. Tests cover:

• Properties with invalid element types (collections) with converters applied to the property
• Properties with invalid types with converters applied to the property
• Properties with invalid types with converters applied to the type
• Dictionary properties with invalid value types with converters
• Validation that without converters, the expected exception is thrown

All 5 tests pass for reflection-based serialization. Source generation tests were added but are in a separate test project structure that requires additional integration work.

[stephentoub]

Source generation tests were added but are in a separate test project structure that requires additional integration work.

@copilot, does that mean they work or not? Please do whatever additional integration work is necessary to enable this.

[Copilot]

Source generation tests were added but are in a separate test project structure that requires additional integration work.

@copilot, does that mean they work or not? Please do whatever additional...

The source generation tests now work. Fixed in commit a1ee595 by adding InvalidTypesWithConvertersTests.cs to the source generation test targets file. All 4 tests pass for both Roslyn 3.11 and Roslyn 4.4 versions.