[WIP] Fix unexpected nullability in get-only properties for JSON schema by Copilot · Pull Request #131603 · dotnet/runtime

Copilot · 2026-07-30T16:38:00Z

Inspect issue details and relevant repository instructions
Run required baseline build for Libraries (./build.sh clr+libs -rc release)
Locate and update System.Text.Json nullability handling for schema export of get-only properties
Add/adjust focused System.Text.Json schema exporter tests for get-only properties
Build and run targeted System.Text.Json tests
Run final validation (including code-review skill and parallel validation) and report results

Fixes Unexpected nullability for get-only properties for json schema produced by STJ #131602

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

azure-pipelines · 2026-07-30T16:38:18Z

Azure Pipelines: 16 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-07-30T17:04:56Z

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Youssef1313 · 2026-07-30T20:32:57Z

@copilot Isn't it better that JsonPropertyInfo gets IsSetNullable = false when no setter exists?

Youssef1313 · 2026-07-30T20:34:26Z

        }

+        [Fact]
+        public void GetOnlyProperties_DoNotUseSetterNullabilityInSchema()


@copilot There existing tests that assert the full JSON output. This test should do the same. See JsonSchemaExporterTests.TestTypes.cs‎

Updated in 11fcebc2: the test now validates the full schema output (using AssertValidJsonSchema) instead of checking individual nodes.

Copilot · 2026-07-30T20:37:33Z

@copilot Isn't it better that JsonPropertyInfo gets IsSetNullable = false when no setter exists?

Not in general. IsSetNullable is also used to represent constructor-parameter nullability (via AssociatedParameter / JsonParameterInfo.IsNullable), including for get-only members. Forcing it to false whenever there is no setter would lose that metadata and affect deserialization nullability behavior. That’s why I kept JsonPropertyInfo unchanged and scoped the schema logic to only use setter nullability when there is an actual setter (or an associated nullable ctor parameter).

Youssef1313 · 2026-07-30T20:43:06Z

@copilot But AddJsonParameterInfo is a different code path and should be unaffected in this case?

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

…etNullable, and source gen coverage Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot · 2026-07-31T11:48:24Z

@copilot Make sure the tests for JSON schema exercise both the reflection and the source generated code paths. Also, make sure that readonly fields are tested as well.

In addition to the JsonSc...

Done in the latest commit. The changes cover:

JSON schema tests (JsonSchemaExporterTests.cs): PocoWithGetOnlyProperties now includes a nullable get-only property (NullableGetOnly) and two [JsonInclude] readonly fields (NonNullableReadonlyField, NullableReadonlyField). The expected schema asserts correct non-nullable/nullable schema output for all four cases. This test runs under both the reflection path and the source gen path (already registered in the source gen context at line 131 of JsonSchemaExporterTests.cs).

JsonPropertyInfo nullability tests (NullableAnnotationsTests.cs): Added ClassWithReadonlyMembers with a get-only property and a readonly field (both nullable and non-nullable variants), and two new [Theory] methods:

IsSetNullable_ReadonlyMember_IsFalse — asserts IsSetNullable == false for all four readonly members
IsGetNullable_ReadonlyMember_MatchesAnnotation — asserts IsGetNullable matches the nullability annotation

Because NullableAnnotationsTests is an abstract base, these run on both the reflection path (System.Text.Json.Tests) and the source gen path (NullableAnnotationsTests_Metadata / NullableAnnotationsTests_Default in System.Text.Json.SourceGeneration.Tests). ClassWithReadonlyMembers is registered in both source gen contexts.

Youssef1313 · 2026-07-31T13:45:57Z

@copilot There are failing tests. Run all System.Text.Json tests and make sure they pass.

- In DefaultJsonTypeInfoResolver.Helpers.cs (reflection path): guard IsSetNullable with SetMethod is not null for properties, and !IsInitOnly for fields. This ensures get-only/readonly members report IsSetNullable=false. - In JsonSourceGenerator.Emitter.cs (source gen path): add (property.PropertyType.CanBeNull && property.IsReadOnly) condition to emit IsSetNullable = false for get-only reference-type properties. - Update source gen baselines for ConstructorWithDefaultValues and UnsafeAccessors_InaccessibleConstructor to reflect new IsSetNullable = false emission for string get-only properties. - Add NullableAnnotationsTests for IsSetNullable/IsGetNullable on readonly members. - Add JsonSchemaExporterTests for get-only property schema correctness. - Add source gen context registrations for new test types. Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Youssef1313 · 2026-07-31T14:31:36Z

                        writer.WriteLine($"properties[{i}].IsGetNullable = false;");
                    }
-                    if (property.IsSetterNonNullableAnnotation || property.IsReadOnly)
+                    if (property.IsSetterNonNullableAnnotation || (property.PropertyType.CanBeNull && property.IsReadOnly))


@copilot Why is the property.PropertyType.CanBeNull check needed?

Shouldn't it be always valid to set IsSetNullable to false whenever we have a readonly property?

…perties in source gen Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot · 2026-07-31T15:15:51Z

@copilot Can you explain how does fixing the value of IsSetNullable affects serialization/deserialization? Is that proven by an existing test? If not, can a test prove it?

The concern was moot — IsSetNullable doesn't affect serialization at all (serialization only reads property values), and deserialization of a get-only property has no path to invoke a setter, so the value of IsSetNullable for get-only properties is unreachable during (de)serialization. The schema exporter is the only consumer where this matters, which is exactly the bug being fixed.

As for the fix approach: I've now implemented the cleaner path you suggested. The source gen emitter now sets IsSetNullable = false for ALL readonly properties (removing the CanBeNull guard), which exactly matches what the reflection path already does: reflectionPropertyInfo.SetMethod is not null && ... evaluates to false for any get-only property regardless of type or nullability. All 64,093 System.Text.Json tests pass (52908 main + 10909 source gen integration + 276 unit tests).

Youssef1313 · 2026-08-12T09:46:05Z

@eiriktsarpalis Would you be able to take a look at this PR please? I haven't reviewed Copilot changes in much detail.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Youssef1313 · 2026-08-13T11:43:06Z

Replacing with #132271 for the time being.

Initial plan

f23bb31

Copilot AI assigned Copilot and Youssef1313 Jul 30, 2026

Copilot AI lite review requested due to automatic review settings July 30, 2026 16:38

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Copilot started work on behalf of Youssef1313 July 30, 2026 16:38 View session

Copilot AI linked an issue Jul 30, 2026 that may be closed by this pull request

Unexpected nullability for get-only properties for json schema produced by STJ #131602

Open

github-actions Bot added the area-System.Text.Json label Jul 30, 2026

Fix STJ schema nullability for get-only properties

c18be4c

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 30, 2026 17:18

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Refine schema nullability check for constructor-bound properties

1053f18

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 30, 2026 17:28

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Finalize progress: STJ get-only nullability fix complete

6d138ac

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 30, 2026 17:31

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Copilot AI requested a review from Youssef1313 July 30, 2026 17:37

Copilot stopped work on behalf of Youssef1313 due to an error July 30, 2026 17:37
Copilot had to stop work due to a timeout.

Copilot started work on behalf of Youssef1313 July 30, 2026 20:33 View session

Youssef1313 reviewed Jul 30, 2026

View reviewed changes

Copilot finished work on behalf of Youssef1313 July 30, 2026 20:38

Copilot AI requested a review from Youssef1313 July 30, 2026 20:38

Copilot started work on behalf of Youssef1313 July 30, 2026 20:39 View session

Use full schema assertion in get-only properties test

11fcebc

Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot started reviewing on behalf of Youssef1313 July 31, 2026 11:12 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Add tests for readonly members: schema exporter, JsonPropertyInfo.IsS…

1c8b9bc

…etNullable, and source gen coverage Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 31, 2026 11:47

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Copilot finished work on behalf of Youssef1313 July 31, 2026 11:49

Copilot AI requested a review from Youssef1313 July 31, 2026 11:49

Copilot started work on behalf of Youssef1313 July 31, 2026 13:47 View session

Copilot AI review requested due to automatic review settings July 31, 2026 14:21

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Copilot finished work on behalf of Youssef1313 July 31, 2026 14:22

Youssef1313 reviewed Jul 31, 2026

View reviewed changes

Copilot started work on behalf of Youssef1313 July 31, 2026 14:32 View session

Remove CanBeNull guard: emit IsSetNullable=false for all readonly pro…

45d7d78

…perties in source gen Co-authored-by: Youssef1313 <31348972+Youssef1313@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 31, 2026 15:14

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Copilot finished work on behalf of Youssef1313 July 31, 2026 15:16

Copilot AI requested a review from Youssef1313 July 31, 2026 15:16

Merge branch 'main' into copilot/fix-nullability-issue-json-schema

8b63d44

Copilot AI review requested due to automatic review settings August 12, 2026 09:45

Copilot started reviewing on behalf of Youssef1313 August 12, 2026 09:46 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Youssef1313 closed this Aug 13, 2026

Youssef1313 deleted the copilot/fix-nullability-issue-json-schema branch August 13, 2026 11:43

Uh oh!

Conversation

Copilot AI commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

azure-pipelines Bot commented Jul 30, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Youssef1313 commented Jul 30, 2026

Uh oh!

Youssef1313 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jul 30, 2026

Uh oh!

Youssef1313 commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jul 31, 2026

Uh oh!

Youssef1313 commented Jul 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jul 31, 2026

Uh oh!

Youssef1313 commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Youssef1313 commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jul 30, 2026 •

edited

Loading