Skip to content

Fix #3217: resolve enum contracts against the underlying type under source generation - #4131

Merged
martincostello merged 3 commits into
domaindrivendev:masterfrom
RaphaelFakhri:fix/3217-sourcegen-nullable-enum
Oct 2, 2026
Merged

martincostello merged 3 commits into
domaindrivendev:masterfrom
RaphaelFakhri:fix/3217-sourcegen-nullable-enum

Conversation

@RaphaelFakhri

Copy link
Copy Markdown
Contributor

Fixes #3217.

The bug as filed no longer reproduces. The mechanism identified in the thread, OpenApiAnyFactory.CreateFromJson deserializing to a JsonElement and failing without reflection, went away when that path was replaced by JsonModelFactory using JsonNode.Parse, which needs no metadata. With a strict source-generated TypeInfoResolver, master emits enum values correctly today, both integer and string forms. Nothing guards that, which is part of why the issue stayed open.

What is still live is the nullable case, and it is fatal rather than null:

System.NotSupportedException : JsonTypeInfo metadata for type 'System.Nullable`1[TimeRange]'
was not provided by TypeInfoResolver of type 'SourceGenerationContext'.
   at JsonSerializerDataContractResolver.JsonConverterFunc(Object value, Type type)
   at JsonSerializerDataContractResolver.GetDataContractForType(Type type)

The enum branch of GetDataContractForType unwraps Nullable<T> into effectiveType for every purpose except the two calls that serialize a sample value, which still receive the original type. Under a reflection-based resolver that is invisible, because System.Text.Json synthesizes Nullable<T> metadata on demand. Under a source-generated context the metadata exists only if some registered type happens to have a TEnum? member, and an optional enum route or query parameter, which is the shape in the issue, gives it no reason to. So document generation throws.

The fix is to pass effectiveType to both.

Four tests in a new JsonSourceGenerationSchemaGeneratorTests, driven by a real JsonSerializerContext set as the sole TypeInfoResolver: an int-backed enum, a nullable int-backed enum, a string enum through JsonStringEnumConverter<T>, and an enum reached as a property. Against master's source three pass and the nullable one throws with the exception above. With the change, Swashbuckle.AspNetCore.SwaggerGen.Test is 764 passed, 0 failed on net8.0.

One thing to say before it gets asked: the identical mismatch exists in the adjacent primitive branch, so int? and Guid? under source generation have the same latent crash. I tried the same swap there and it broke twelve GenerateSchema_SetsDefault_IfPropertyHasDefaultValueAttribute cases, which serialize a null default through the nullable type on purpose. That needs the null-default path handled separately, so this PR is deliberately scoped to the enum branch.

@codecov

codecov Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.20%. Comparing base (d0b1e26) to head (4c18b15).
⚠️ Report is 16 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4131      +/-   ##
==========================================
- Coverage   95.24%   95.20%   -0.04%     
==========================================
  Files         111      111              
  Lines        4142     4174      +32     
  Branches      848      853       +5     
==========================================
+ Hits         3945     3974      +29     
- Misses        197      200       +3     
Flag Coverage Δ
Linux 95.20% <100.00%> (-0.04%) ⬇️
Windows 95.20% <100.00%> (-0.04%) ⬇️
macOS 95.20% <100.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The fix bypasses nullable-enum-specific converters even when nullable metadata is available.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes nullable enum schema generation with strict source-generated JSON metadata.

Changes:

  • Resolves enum serialization against the underlying enum type.
  • Adds source-generation coverage for integer, nullable, string, and property enums.
File summaries
File Description
JsonSerializerDataContractResolver.cs Uses the effective enum type during serialization.
JsonSourceGenerationSchemaGeneratorTests.cs Adds source-generated enum regression tests.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ng type under source generation

The enum branch of GetDataContractForType unwraps Nullable<T> into effectiveType
for everything except the two calls that serialize a sample value, which were
still passed the original type. Under a reflection-based resolver that is
harmless, because System.Text.Json synthesizes Nullable<T> metadata on demand.
Under a source-generated TypeInfoResolver the metadata only exists if some
registered type happens to have a TEnum? member, so an optional enum parameter
threw NotSupportedException while the document was being generated.
@RaphaelFakhri

Copy link
Copy Markdown
Contributor Author

The review point is valid and is addressed. The enum contract now serializes against Nullable<T> when the serializer has metadata for it, and falls back to the underlying enum type only when it does not (for example a source-generated context that registers only the enum). A converter registered for Nullable<T> is therefore still used.

A new test registers a JsonConverter<TEnum?> together with a context that includes TEnum?, and asserts that the schema values come from the converter. It fails against the previous version of the change and passes now. The original source-generation tests still pass on net8.0, net9.0 and net10.0.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The metadata probe bypasses nullable enum converters under normal reflection-based serializer options.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The targeted fix preserves converter behavior and is covered across the relevant source-generation scenarios.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@martincostello martincostello added this to the v10.3.0 milestone Oct 2, 2026
@martincostello
martincostello merged commit d0ade60 into domaindrivendev:master Oct 2, 2026
14 checks passed
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution @RaphaelFakhri - the changes from this pull request have been published as part of version 10.3.0 📦, which is now available from NuGet.org 🚀

This was referenced Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: All enum values show up as "null" when using JSON Source Generation

3 participants