Skip to content

Preserve optional Swagger parameters as option types - #546

Closed
Smaug123 wants to merge 1 commit into
mainfrom
optional-query-params
Closed

Preserve optional Swagger parameters as option types#546
Smaug123 wants to merge 1 commit into
mainfrom
optional-query-params

Conversation

@Smaug123

@Smaug123 Smaug123 commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Fifth fix split out of #540; stacked on #545.

SwaggerParameter.Required was parsed but discarded, so every generated parameter was unconditionally mandatory, even though Swagger 2 defaults non-path parameters to optional. Query parameters without required: true are now option-typed (path parameters stay required per the spec; optional bodies are not modelled), e.g. AdminCronList(page : int option, limit : int option).

To support that, the HTTP client generator now understands option-typed [<Query>] parameters: the query string is computed at runtime as a List.choose/String.concat over per-parameter string option components, and appended (with separator) only when nonempty — so None is omitted from the URL and an all-None query leaves the URL bare. Required-only query strings are byte-identical to before. (The joining lambda is deliberately not id, which Gitea methods shadow with a parameter of that name.)

Tests were written first and observed failing (page=Some%283%29 in the URL): TestOptionalQueryParam.fs covers mixed present/missing/escaped/bare cases through a hand-written interface, and TestGiteaClient.fs gains end-to-end AdminCronList coverage for both None and Some.

Note: #541 rewrites this same query-string block off main to fix list-typed parameters; whichever merges second conflicts there. Reconciliation: unify on string list components, mapping optional scalars via Option.toList.

🤖 Generated with Claude Code

SwaggerParameter.Required was parsed but discarded, so every generated
parameter was unconditionally mandatory even though Swagger 2 defaults
non-path parameters to optional. Query parameters without
required: true are now option-typed (path parameters stay required per
the spec; optional bodies are not modelled), e.g.
AdminCronList(page : int option, limit : int option).

To support that, the HTTP client generator now understands option-typed
[<Query>] parameters: the query string is computed at runtime as a
List.choose/String.concat over per-parameter string option components
and appended (with separator) only when nonempty, so None is omitted
from the URL and an all-None query leaves the URL bare. Required-only
query strings are byte-identical to before. The joining lambda is
deliberately not `id`, which Gitea methods shadow with a parameter of
that name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Smaug123
Smaug123 force-pushed the optional-query-params branch from cd8a3bc to abd3925 Compare July 10, 2026 17:24
@Smaug123

Copy link
Copy Markdown
Owner Author

At the very least this should probably come with a major version bump

@Smaug123

Copy link
Copy Markdown
Owner Author

Hmm. I really don't like this change and I think it probably shouldn't go in. People are presumably not diligently annotating all their parameters as "required", so this will cause so much noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant