Fix conflicting required+nullable schema when only NonNullableReferen…

[KitKeen]

Fix conflicting required + nullable schema when only NonNullableReferenceTypesAsRequired is enabled

Fixes #3297

Problem

When NonNullableReferenceTypesAsRequired() is enabled without SupportNonNullableReferenceTypes(), non-nullable reference-type properties end up with both "required": true and "nullable": true in the generated schema.

Root cause

IsNullable only checks SupportNonNullableReferenceTypes:

if (_generatorOptions.SupportNonNullableReferenceTypes)
{
    nullable &= !memberInfo.IsNonNullableReferenceType();
}

So when only -AsRequired is set, the property gets added to required (via GenerateSchemaForMembers) but nullable is never cleared.

Fix

if (_generatorOptions.SupportNonNullableReferenceTypes || _generatorOptions.NonNullableReferenceTypesAsRequired)
{
    nullable &= !memberInfo.IsNonNullableReferenceType();
}

Before / after

builder.Services.AddSwaggerGen(c => c.NonNullableReferenceTypesAsRequired());

public class WeatherForecast
{
    public string Summary { get; set; }
}

Before:

"summary": { "type": "string", "nullable": true }

After:

"summary": { "type": "string" }

Tests

Added GenerateSchema_NonNullableReferenceTypesAsRequired_DoesNotMarkPropertyAsNullable for both TypeWithNullableContextAnnotated and TypeWithNullableContextNotAnnotated. Fails on main, passes after the fix. Full suite 720/720 on net10.0, clean build on net8.0/net9.0/net10.0.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (d22870e) to head (78f5b52).
⚠️ Report is 62 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3912   +/-   ##
=======================================
  Coverage   95.04%   95.04%           
=======================================
  Files         111      111           
  Lines        3958     3958           
  Branches      801      801           
=======================================
  Hits         3762     3762           
  Misses        196      196           

Flag	Coverage Δ
Linux	95.04% <100.00%> (ø)
Windows	95.04% <100.00%> (ø)
macOS	95.04% <100.00%> (ø)

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.

[martincostello]

I need to think about this more before doing a proper review on this PR.

Members can still be required yet allow nulls (i.e. property must be present, may not have a value).

For example:

public class SomeType
{
    public required string? A { get; set; }

    public required string? C { get; set; }
}

// Bad payload, c is missing
{
  "a": "b"
}

// Good payload, c is specified as explicitly null
{
  "a": "b",
  "c": null
}

// Good payload, c is specified with a value
{
  "a": "b",
  "c": "d"
}

Any changes here need to support different scenarios people may have when nullable reference types are and aren't enabled and when members are required or not.

The defaults should where possible match expectations, but at the same type we don't want to introduce any breaking changes or make different opinions users may have on how to treat required and/or nullable reference types together.

[KitKeen]

Martin, I appreciate your review!

You're right that required + nullable: true is a valid OpenAPI combination
and should be preserved. The fix handles this correctly because the guard
memberInfo.IsNonNullableReferenceType() distinguishes between the two cases:

public class SomeType
{
public string A { get; set; } // non-nullable → IsNonNullableReferenceType() = true
public string? B { get; set; } // nullable → IsNonNullableReferenceType() = false
}

With NonNullableReferenceTypesAsRequired enabled:

property required nullable correctness
string A true false ✅ non-nullable, must be present and have a value
string? B false true ✅ nullable, not required

Your required string? example is also covered:

public class SomeType
{
public required string? A { get; set; }
}

Here IsNonNullableReferenceType() returns false for string?, so nullable
is not cleared- the schema correctly reflects required: true, nullable: true.

The fix only clears nullable when both conditions are true:
а) NonNullableReferenceTypesAsRequired is enabled
b) The member is genuinely non-nullable (IsNonNullableReferenceType() = true)

I've also added a test
GenerateSchema_NonNullableReferenceTypesAsRequired_PreservesNullable_IfPropertyHasRequiredKeywordAndIsNullable covering exactly this scenario required string? stays required + nullable, required string stays required + not nullable.

[KitKeen]

take your time reviewing it, I'd rather listen to all of the conserns before merging.

[KitKeen]

Hi @martincostello- CI hasn't run on this PR yet (the "first-time contributor needs workflow approval" gate). Could you kick it off when you have a moment? Happy to address whatever the checks surface.

[KitKeen]

@martincostello just a kind reminder!

[martincostello]

Test are failing.

[KitKeen]

@martincostello pushed 78f5b52 - added the missing Assert.NotNull(schema.Required); before the .ToArray() call so the new test compiles under #nullable enable on all three target frameworks. Could you re-approve the CI workflow when you have a moment? Thanks!

[github-actions]

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