C#: Adds version 8 and expands nullability to reference types (#1632)#2694
Merged
Conversation
|
Hey all.... We've just bumped into this issue due to using the models generated from the schema in a .NET API due to lack of support of nullable reference types the binder fails if the null, this looks like the fix we need. Is there anything we can do to support this PR? Thanks |
Fixes on top of PR glideapps#2694: - Gate the System.Text.Json [JsonRequired] attribute behind the existing check-required option instead of emitting it unconditionally. It now matches the spirit of Newtonsoft's [JsonProperty(Required = ...)]: only emitted for non-optional properties when check-required is on. - Un-skip required.schema, strict-optional.schema and intersection.schema for the System.Text.Json fixture: with [JsonRequired] the renderer now implements check-required, so the expected-failure samples fail as expected. - Emit "#nullable enable" (plus the same CS8618/CS8601/CS8603 pragma suppressions the System.Text.Json renderer already uses) in the Newtonsoft renderer when csharp-version >= 8, so the new "?" reference type annotations compile without warnings. Versions 5 and 6 are unchanged. - Bump the C# test fixtures to net8.0 (and CI to the .NET 8 SDK): [JsonRequired] requires System.Text.Json 7+. - Add { "csharp-version": "8" } quick-test combinations for both the Newtonsoft and System.Text.Json fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The generated Newtonsoft JsonConverter overrides declare their parameters as non-nullable "object" while Newtonsoft.Json 13 annotates them as "object?", which produces CS8765 warnings under "#nullable enable". "dotnet run" prints warnings to stdout, which would break the fixture round-trips. Verified with the .NET 8 SDK: with this suppression every priority/samples JSON input compiles with zero warnings at csharp-version 8 for both the Newtonsoft and System.Text.Json renderers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
schani
added a commit
that referenced
this pull request
Jul 20, 2026
With #2694 merged, C# 8 output with nullable reference types becomes the out-of-box experience, pairing with the SystemTextJson default. Both csharp fixtures now pin csharp-version=5 and =6 in their quicktests (replacing the now-redundant =8 rows), so all three language-version code paths stay covered. Promoting v8 to the base schema-fixture runs put Newtonsoft v8 over the full schema corpus for the first time (quicktests only cover the JSON fixtures) and exposed a gap in #2694's NRT pragma set: the emitted constraint-check and string-transformer helpers produce CS8602, CS8604, and CS8625 warnings under '#nullable enable', which 'dotnet run' prints to stdout ahead of the JSON, breaking the fixture comparison. Those three codes are now suppressed alongside the existing CS8618/CS8601/CS8603(/CS8765) pragmas; a corpus-wide compile scan of every schema under both frameworks confirms no other warning codes remain. Suppressing CS8602 also fixes the exact issue behind three schema-csharp-SystemTextJson skips (minmaxlength, optional-constraints, optional-const-ref), so those schemas are un-skipped. Verified locally with dotnet SDK 8.0.423: QUICKTEST runs of csharp, csharp-SystemTextJson, schema-csharp, and schema-csharp-SystemTextJson all pass end to end (272 tests), with every quicktest row confirmed executing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Relaxes nullability check for C# 8+ targeting. C# 8 introduced nullable reference types, thereby removing the restriction that only value types can be made nullable.
Related Issue
(#1632)
Motivation and Context
Specifically when using System.Text.Json, the new nullable ignore attribute does not trigger Roslyn warning CS8601, leading to potential runtime null reference exceptions.
Previous Behaviour / Output
Previously, reference types such as
stringwould not include the?nullability indicator.New Behaviour / Output
Now, all C# types (reference and value) will include
?when optional, since the serialized data may contain null values for those properties per the specification.How Has This Been Tested?
Ran before and after examples in a Docker environment.
Screenshots (if appropriate):