Skip to content

fix(analyzer): allOf entries that redeclare a property lost it on the wire - #121

Merged
giraffesyo merged 1 commit into
canaryfrom
allof-duplicate-properties
Aug 21, 2026
Merged

fix(analyzer): allOf entries that redeclare a property lost it on the wire#121
giraffesyo merged 1 commit into
canaryfrom
allof-duplicate-properties

Conversation

@giraffesyo

Copy link
Copy Markdown
Member

Closes #120. Found by the corpus test from #119, which vets the generated package rather than only building it.

allOf composes one value, so entries may declare the same property. Each declaration became a field:

type WebhookForkForkee struct {
	AllowForking  *bool  `json:"allow_forking,omitempty"`
	...
	AllowForking2 *bool  `json:"allow_forking,omitempty"`
types.go:24880:2: struct field AllowForking2 repeats json tag "allow_forking" also at types.go:24776
... 247 in that client

The package compiles. encoding/json resolves two fields at one depth carrying one tag by ignoring both, so allow_forking stopped decoding and stopped marshaling while the struct still looked right. A build-only gate cannot see it, which is why #119 added go vet to the corpus test and why this turned up the same day.

GitHub's webhook-fork.forkee is the case: two inline objects, the second refining 78 of the first's 90 properties.

Fix

  • One field per property. The first declaration wins the type, which matters little in practice since entries that disagree on type describe a value nothing can satisfy.
  • Requirements are read from every entry first. A property required by a later entry is now required by the struct, rather than depending on which entry declared it first, which is what allOf means.
  • A property beside an embedded type is untouched. Those live at different depths, Go shadows one with the other, and the tags do not collide.

Worth noting the history: before #96 fixed field-name collisions, this shape failed to compile. That was louder, and in one narrow sense better than a struct that builds and drops data.

Verification

  • GitHub: 247 vet failures, now 0, still 1898 types and 242 iterators.
  • ACTIVATE, Mealie, Stripe, ledger: build and vet clean, unchanged counts.

Tests

  • internal/analyzer/schemas_allof_property_test.go: a property two entries declare is one field, required because one entry requires it, and a property beside an embed is kept.
  • testdata/combinations.yaml grows the shape, so the corpus covers it.

gofmt, golangci-lint, go vet ./..., and go test ./... pass.

… wire

allOf composes one value, so entries may declare the same property, and each
declaration became a field. Two fields at one depth carrying one JSON tag is
not untidy, it is silent: encoding/json ignores both, so the property stops
decoding and stops marshaling while the struct still looks right.

GitHub's webhook-fork.forkee composes two inline objects, the second
refining 78 of the first's 90 properties. Its client compiled and go vet
reported 247 duplicate tags.

A property is one field now, and requirements are read from every entry
before any field is built, so one required by a later entry is required by
the struct rather than depending on which entry declared it first.

A property beside an embedded type is untouched: those live at different
depths, where Go shadows one with the other and the tags do not collide.
@giraffesyo
giraffesyo merged commit 1499cc8 into canary Aug 21, 2026
7 checks passed
@giraffesyo
giraffesyo deleted the allof-duplicate-properties branch August 21, 2026 19:58
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.

allOf entries that redeclare a property produce duplicate JSON tags, and encoding/json then ignores both

1 participant