Skip to content

Rename SupportsNullableProperty to DoesNotSupportNullValue#67386

Merged
Youssef1313 merged 1 commit into
mainfrom
dev/ygerges/fix-local-method-name
Jun 23, 2026
Merged

Rename SupportsNullableProperty to DoesNotSupportNullValue#67386
Youssef1313 merged 1 commit into
mainfrom
dev/ygerges/fix-local-method-name

Conversation

@Youssef1313

Copy link
Copy Markdown
Member

Following a discussion with @mikekistler, the method name is wrong. It's used actually in a code path that removes "null" flag.

Copilot AI review requested due to automatic review settings June 23, 2026 14:54
@Youssef1313
Youssef1313 requested a review from a team as a code owner June 23, 2026 14:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR corrects the naming of a local helper in JsonNodeSchemaExtensions.ApplyParameterInfo, aligning it with the actual behavior: removing null from the generated OpenAPI schema types for binding sources where model binding doesn’t support nullable values.

Changes:

  • Renames SupportsNullableProperty to DoesNotSupportNullValue to reflect that the code path removes null from the schema.
  • Updates the call site to use the renamed helper.
Comments suppressed due to low confidence (1)

src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs:420

  • The binding source list in DoesNotSupportNullValue duplicates IsNonBodyBindingSource directly below. Keeping two independent copies makes it easy for the lists to drift and produce inconsistent behavior if new binding sources are added/removed later.
        static bool DoesNotSupportNullValue(BindingSource bindingSource) => bindingSource == BindingSource.Header
            || bindingSource == BindingSource.Query
            || bindingSource == BindingSource.Path
            || bindingSource == BindingSource.Form
            || bindingSource == BindingSource.FormFile;

@mikekistler mikekistler left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good! 👍

@Youssef1313
Youssef1313 enabled auto-merge (squash) June 23, 2026 14:59
@Youssef1313
Youssef1313 merged commit 01c1e4f into main Jun 23, 2026
28 checks passed
@Youssef1313
Youssef1313 deleted the dev/ygerges/fix-local-method-name branch June 23, 2026 16:17
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone Jun 24, 2026
@Youssef1313 Youssef1313 added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi labels Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants