Skip to content

Optional: infer required fields from Go types#112

Merged
seanogdev merged 1 commit into
masterfrom
infer-required
May 14, 2026
Merged

Optional: infer required fields from Go types#112
seanogdev merged 1 commit into
masterfrom
infer-required

Conversation

@seanogdev
Copy link
Copy Markdown
Contributor

Summary

Adds an opt-in infer-required config flag that marks response/body struct fields as required in the generated schema when the Go type implies presence:

  • non-pointer
  • no omitempty in the struct tag
  • no explicit {optional} doc tag

Path, query, and form parameters keep their existing required handling (untouched).

Default is off — fully backwards compatible. Existing {required} doc tags continue to work and stack with inference.

Motivation

In projectsapigo/swagger.yaml today, every field on every response view is optional, because kommentaar only emits required from explicit {required} doc tags. For a typical view struct like:

type Currency struct {
    ID            int64   `json:"id"`
    Name          string  `json:"name"`
    Code          string  `json:"code"`
    Symbol        *string `json:"symbol"`
    DecimalPoints *int64  `json:"decimalPoints"`
}

…all five fields show up optional in the spec, even though id, name, and code are always populated. Consumers can't trust the spec, and we'd have to hand-annotate thousands of fields to fix it.

This flag lets us flip that on per-project (or per-section, eventually) and have the Go type system do the work.

Behaviour

With infer-required true:

Field Inferred required?
F string ✅ yes
F string \json:"f"`` ✅ yes
F *string ❌ no (pointer)
F string \json:"f,omitempty"`` ❌ no (omitempty)
F string with {optional} doc ❌ no (explicit)
F *string with {required} doc ✅ yes (explicit)
embedded field ❌ no
query/path/form param unchanged

Tests

  • testdata/openapi2/src/infer-required/ — end-to-end fixture covering every branch (pointer, omitempty, {optional}, {required} on a pointer, plain field, query-param struct that must not be affected).
  • TestIsInferredRequired — unit test for the helper across 9 cases.
  • Full suite (go test ./...) green.

Discussion points for the API team

  • Default — should this stay opt-in, or become the default once projectsapigo (and other repos) have been audited?
  • Scope — currently triggers in response/body context only; should it also apply to request bodies (it does today via the same code path; mention in case anyone wants to exclude that)?
  • Naming — infer-required vs required-by-default vs something else.

Marked as draft pending that discussion.

When enabled, struct fields used in response/body schemas are added to
the parent's `required` array unless the field is a pointer, has
`omitempty` in its struct tag, or carries an explicit `{optional}` doc
tag. Path/query/form parameters are unaffected — they have their own
required handling.

Off by default; opt in via `infer-required true` in the config.
@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 25836839371

Coverage increased (+0.4%) to 53.429%

Details

  • Coverage increased (+0.4%) from the base build.
  • Patch coverage: 4 uncovered changes across 1 file (24 of 28 lines covered, 85.71%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
docparse/jsonschema.go 28 24 85.71%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2231
Covered Lines: 1192
Line Coverage: 53.43%
Coverage Strength: 41.68 hits per line

💛 - Coveralls

@seanogdev seanogdev marked this pull request as ready for review May 14, 2026 01:50
Copy link
Copy Markdown
Member

@marcizhu marcizhu left a comment

Choose a reason for hiding this comment

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

LGTM!

@seanogdev seanogdev merged commit 9df27d6 into master May 14, 2026
3 checks passed
@seanogdev seanogdev deleted the infer-required branch May 14, 2026 10:05
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.

4 participants