Decide dominance on the fields encoding/json sees - #129
Merged
Merged
Conversation
Three struct shapes documented a property name the wire never carried. Each is quiet: the document built, validated and rendered, and only a consumer diffing it against real traffic would have noticed. An unexported struct embedded under json:"-" grew a property literally named - holding the nested object. inspectField routed unexported fields to unexportedEmbed before the tag check an exported field would have hit, so the skip never ran. That check moves above the branch, and stays limited to the bare tag: json:"-," names the key -, and encoding/json writes the embedded object under it whether or not the embedding is exported. A chan, func or unsafe.Pointer field sharing a json: tag with an ordinary field left that rival documented on its own. encoding/json keeps the unmarshalable field while it resolves names, so the two conflict and neither is written; stdocs removed it in inspectField, before dominance ran, and documented the key the conflict takes away. Such a field now comes back as an excluded candidate: it competes for the name and is filtered out once dominance has settled. The openapi: tag panic for a field with no JSON form moves along with it. The same held one level up, which the issue does not cover. A struct embedded under openapi:"-" was cut before its fields could collide with a name outside it, so a rival it hides came out documented unopposed. The walk now queues each type with an excluded flag, so a hidden subtree is still walked for names and its candidates travel excluded. TestDroppedFieldsMatchWire pins all three against a live marshal. Two of its types are assembled with reflect.StructOf, because go vet rejects a literal struct that promotes one json tag twice and no nolint directive quiets it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #127.
Three struct shapes documented a property name the wire never carried. Each is quiet: the document built, validated and rendered, and only a consumer diffing it against real traffic would have noticed.
An unexported struct embedded under
json:"-"grew a property literally named-holding the nested object.inspectFieldrouted unexported fields tounexportedEmbedbefore the tag check an exported field would have hit, so the skip never ran. That check moves above the branch, and stays limited to the bare tag:json:"-,"names the key-, andencoding/jsonwrites the embedded object under it whether or not the embedding is exported.A
chan,funcorunsafe.Pointerfield sharing ajson:tag with an ordinary field left that rival documented on its own.encoding/jsonkeeps the unmarshalable field while it resolves names, so the two conflict and neither is written; stdocs removed it ininspectField, before dominance ran, and documented the key the conflict takes away. Such a field now comes back as an excluded candidate: it competes for the name and is filtered out once dominance has settled. Theopenapi:tag panic for a field with no JSON form moves along with it.A third shape, not in the issue. The same held one level up: a struct embedded under
openapi:"-"was cut before its fields could collide with a name outside it, so a rival it hides came out documented unopposed. The walk now queues each type with an excluded flag, so a hidden subtree is still walked for names and its candidates travel excluded.Testing
TestDroppedFieldsMatchWirepins all three against a livejson.Marshal, in the differential style the issue suggested. Two of its types are assembled withreflect.StructOf, becausego vetrejects a literal struct that promotes one json tag twice and no nolint directive quiets it.Each fix was negative-controlled: reverting it individually makes its case fail (the third with
schema documents "dup" but the wire never carries it) and leaves the others passing.Full suite green under
-raceon both go1.26.6 and go1.27.0 (15 packages plus the YAML round-trip module),gofmt/go vetclean,golangci-lint0 issues, and 25s ofFuzzReflectSchemaon 1.27 with no new failures.