Skip to content

Annotate SchemaRepository for nullable reference types#3958

Open
KitKeen wants to merge 1 commit into
domaindrivendev:masterfrom
KitKeen:fix/schemarepository-nullable
Open

Annotate SchemaRepository for nullable reference types#3958
KitKeen wants to merge 1 commit into
domaindrivendev:masterfrom
KitKeen:fix/schemarepository-nullable

Conversation

@KitKeen

@KitKeen KitKeen commented May 21, 2026

Copy link
Copy Markdown
Contributor

Motivation

SchemaRepository.TryLookupByType already carries [NotNullWhen(true)], but because the file is not #nullable enabled the out parameter is declared as the oblivious type OpenApiSchemaReference. In a nullable-enabled caller the compiler treats the out value as non-null regardless of the return, so the NotNullWhen attribute can't actually drive null-state flow — hidden null-deref bugs slip past the compiler.

Calling code currently needs a wrapper to recover the contract:

#nullable enable
public static bool TryLookupByTypeSafe(this SchemaRepository repo,
    Type type, [NotNullWhen(true)] out OpenApiSchemaReference? schema)
{
    bool ok = repo.TryLookupByType(type, out OpenApiSchemaReference? oblivious);
    schema = ok ? oblivious : null;
    return ok;
}
#nullable restore

Change

  • Add #nullable enable to SchemaRepository.cs.
  • Annotate the constructor parameter / DocumentName property / TryLookupByType out parameter / local TryGet outs.
  • Suppress RS0037 locally — project-wide PublicAPI nullability tracking has not yet been adopted (see TODO in Directory.Build.props), so this single-file opt-in mustn't force assembly-wide rollout. PublicAPI tracker still records the surface (no nullability info), the analyzer is satisfied.

Adds three unit tests in SchemaRepositoryTests:

  • TryLookupByType_KnownType_ReturnsTrueAndNonNullReference
  • TryLookupByType_UnknownType_ReturnsFalseAndNullReference
  • TryLookupByType_NotNullWhenAttributeIsAppliedToOutParameter (reflection guard so the contract metadata can't be removed accidentally)

Verification

dotnet test test/Swashbuckle.AspNetCore.SwaggerGen.Test/Swashbuckle.AspNetCore.SwaggerGen.Test.csproj -c Release -f net10.0
# Passed: 721 / Failed: 0

Full test suite (slnx) green: 996 tests, 0 failures across the four test assemblies that run on net10.0.

Fixes #3659.

…endev#3659)

Add `#nullable enable` to `SchemaRepository.cs` so that consumers in
nullable-enabled contexts get proper flow analysis around
`TryLookupByType`: the `[NotNullWhen(true)]` attribute can now
participate in null-state tracking because the `out` parameter is
declared as `OpenApiSchemaReference?` rather than the implicitly
non-null oblivious type.

Project-wide nullability tracking has not yet been adopted (tracked
by the TODO in Directory.Build.props), so RS0037 is suppressed
locally for this file to avoid forcing assembly-wide adoption from
a single-file change.

Adds three tests:

- `TryLookupByType` returns true and a non-null reference for a
  known type.
- `TryLookupByType` returns false and a null reference for an
  unknown type.
- The out parameter carries `[NotNullWhen(true)]`, ensuring the
  metadata that downstream callers rely on for null-state tracking
  isn't accidentally removed.

Fixes domaindrivendev#3659.
if (result)
{
referenceSchema = new OpenApiSchemaReference(schemaId);
referenceSchema = new OpenApiSchemaReference(schemaId!);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To avoid this mismatch, _reservedIds could be changed to be of type Dictionary<Type, string?>. Otherwise the value should be verifiably non-null so we don't need to use ! here.

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (134e1c4) to head (4b04f95).
⚠️ Report is 44 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3958   +/-   ##
=======================================
  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.

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.

Please annotate SchemaRepository.TryLookupByType

2 participants