Fix JSON schema nullability for readonly properties

[Youssef1313]

This a simpler fix for #131602 that is specific in JsonSchemaExporter.

Long-term, I think we would need to fix the underlying values of IsGetNullable and IsSetNullable. So this is more of a workaround to take in RC1, and hence keeping the linked issue open.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 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]

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

[Copilot]

Pull request overview

Adjusts System.Text.Json’s JsonSchemaExporter nullability inference so schema nullability reflects available accessors (getter/setter) rather than treating missing accessors as nullable, and adds regression coverage for get-only/readonly members.

Changes:

• Update schema nullability logic to only consult IsGetNullable when a getter exists, and IsSetNullable when a setter exists.
• Add a regression test covering get-only properties and readonly fields in schema generation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs	Refines nullability decision to ignore setter nullability when no setter delegate exists (and vice versa).
src/libraries/System.Text.Json/tests/Common/JsonSchemaExporterTests.cs	Adds regression test + new POCO to validate non-nullable schema output for get-only properties and readonly fields.

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs:452

• IsNullableSchema now correctly ignores IsSetNullable when propertyInfo.Set is null, but the polymorphic path still computes parentPolymorphicTypeIsNonNullable using propertyInfo is { IsGetNullable: false, IsSetNullable: false } (around line ~134). For get-only/read-only members, IsSetNullable is typically true (WriteState == Unknown) even though there is no setter, which means derived schemas can still be made nullable via the !parentPolymorphicTypeIsNonNullable check.

Consider updating the parentPolymorphicTypeIsNonNullable computation to use the same accessor-existence gating as this change (i.e., treat a missing setter as non-nullable rather than consulting IsSetNullable).

                        if (propertyInfo is not null)
                        {
                            return (propertyInfo.Get is not null && propertyInfo.IsGetNullable) ||
                                (propertyInfo.Set is not null && propertyInfo.IsSetNullable);
                        }

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs:451

• The comment above IsNullableSchema still implies setter nullability is considered even when a property has no setter. Since the implementation now gates IsGetNullable/IsSetNullable on accessor presence, please update the comment to match the new semantics (this avoids confusion for get-only properties, which is the scenario this change fixes).

                        if (propertyInfo is not null)
                        {
                            return (propertyInfo.Get is not null && propertyInfo.IsGetNullable) ||
                                (propertyInfo.Set is not null && propertyInfo.IsSetNullable);

[Youssef1313]

@eiriktsarpalis I'm thinking more. Is this whole idea even correct? A property isn't necessarily writeable only via a setter. It could be writeable via a constructor.

public class C
{
    public C(string? s) => S = s ?? string.Empty;

    // get-only property. Non-nullable.
    // But a null is okay to get deserialized.
    // It's not produced by serialization, though.
    public string S { get; }
}

[Youssef1313]

I switched to an approach to prefer the nullability from AssociatedParameter.

[jeffhandley]

@Youssef1313 FYI I don't expect @eiriktsarpalis will be able to review this for porting into RC1 so we will target a backport to RC2.

[eiriktsarpalis]

This is fixing an old bug which therefore does not meet the bar for servicing in RC2. This should be targeting .NET 12 only.

[Youssef1313]

This is fixing an old bug which therefore does not meet the bar for servicing in RC2. This should be targeting .NET 12 only.

I agree. I was hoping to get it in before RC1 snap but that didn't happen. It's fine to get it for .NET 12.

[eiriktsarpalis]

Let's revisit reviewing this PR after RC2 work is completed. Feel free to ping me at that time if I forget.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs:452

• The comment describing the property-level nullability rules is now out of date: the implementation also considers constructor-parameter nullability (AssociatedParameter), and only considers getter/setter nullability when that accessor exists. Updating the comment will help keep the logic maintainable and avoid future regressions.

                        if (propertyInfo is not null)
                        {
                            if (propertyInfo.Get is not null && propertyInfo.IsGetNullable)
                            {
                                return true;

src/libraries/System.Text.Json/tests/Common/JsonSchemaExporterTests.cs:86

• This test asserts behavior for both get-only properties and readonly fields, but the method name only mentions properties. Renaming it makes the intent clearer (and also removes the trailing whitespace on the signature line).

        [Fact]
        public void GetOnlyProperties_DoNotUseSetterNullabilityInSchema() 

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs:450

• The nullability rules comment is now slightly out of sync with the implementation: IsNullableSchema also considers propertyInfo.AssociatedParameter.IsNullable (and it only considers IsSetNullable when a setter exists). Updating the comment will prevent future readers from missing that behavior.

                        // 3. We have a schema for a reference type, unless we're explicitly treating null-oblivious types as non-nullable.

                        if (propertyInfo is not null)
                        {
                            if (propertyInfo.Get is not null && propertyInfo.IsGetNullable)

[Youssef1313]

@eiriktsarpalis This is green now and ready for review.