From 757673e069bfab7bbcbbebbdced12464f0c71d79 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 6 Aug 2026 02:51:10 +0800 Subject: [PATCH 1/2] fix: embedded schema's additionalProperties swallows the outer schema's declared fields A struct embedding a type with generated MarshalJSON/UnmarshalJSON inherited those methods by promotion, and the shadow trick did not stop it: a shadow still embeds the parent. Decoding a composed type therefore ran the parent's unmarshaler, which knows only the parent's fields and swept the composed type's declared properties into the parent's catch-all map. With an embedded union the promoted marshaler emitted only the union value, dropping the outer fields or producing invalid JSON for scalar variants, and with two catch-all parents the ambiguous selector promoted nothing, so behavior flipped on how many parents a schema composes. A struct with marshaler-bearing embeds now gets part-wise marshalers, even when it has no catch-all of its own: each such embed is encoded through its own marshaler and the resulting objects are merged with the struct's directly declared fields and extras, so no promoted method ever speaks for the whole object. Unmarshaling decodes each embed and the struct's own fields from the same object, then cleans the embedded catch-alls: cleared when the outer type collects extras itself, pruned of declared names otherwise. Structs without such embeds keep the shadow approach, now assembled through the same object merge helpers, which also retires the byte-splice and its assumption that the shadow marshal produced an object. dropShadowedCatchAlls is removed: it existed to keep a second catch-all from fighting the promoted one, and with promotion out of the picture a composed schema's own additionalProperties works again. Fixes #50 --- internal/analyzer/aliascycles.go | 41 --- internal/analyzer/analyzer.go | 1 - .../e2e_additional_properties_test.go | 268 ++++++++++++++++++ internal/generator/funcmap.go | 154 ++++++++++ internal/templates/types.go.tmpl | 223 +++++++++++---- 5 files changed, 593 insertions(+), 94 deletions(-) diff --git a/internal/analyzer/aliascycles.go b/internal/analyzer/aliascycles.go index 456ebbc..085adfd 100644 --- a/internal/analyzer/aliascycles.go +++ b/internal/analyzer/aliascycles.go @@ -1,7 +1,6 @@ package analyzer import ( - "slices" "strings" "github.com/parallelworks/openapi-client-generator/internal/ir" @@ -101,46 +100,6 @@ func breakStructCycles(types []*ir.TypeDef) { } } -// dropShadowedCatchAlls removes the catch-all from a struct that embeds one, -// whose promoted marshalers would otherwise win and emit only their own fields. -func dropShadowedCatchAlls(types []*ir.TypeDef) { - byName := ir.TypesByName(types) - - hasCatchAll := func(td *ir.TypeDef) bool { - return slices.ContainsFunc(td.Fields, func(f *ir.Field) bool { return f.CatchAll }) - } - - // Reports whether td or anything it embeds carries a catch-all. - var embedsCatchAll func(td *ir.TypeDef, depth int) bool - embedsCatchAll = func(td *ir.TypeDef, depth int) bool { - if td == nil || depth > len(types) { - return false - } - for _, f := range td.Fields { - if !f.Embedded { - continue - } - embedded := ir.StructNamed(byName, strings.TrimPrefix(f.Type, "*")) - if embedded == nil { - continue - } - if hasCatchAll(embedded) || embedsCatchAll(embedded, depth+1) { - return true - } - } - return false - } - - for _, td := range types { - if td == nil || td.Kind != ir.TypeKindStruct || !hasCatchAll(td) { - continue - } - if embedsCatchAll(td, 0) { - td.Fields = slices.DeleteFunc(td.Fields, func(f *ir.Field) bool { return f.CatchAll }) - } - } -} - // aliasTarget returns the named type an alias's Go type expression refers to, // peeling the slice, pointer, and map wrappers that do not stop a Go alias from // expanding. It returns "" for a builtin or a composite with no single referent. diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index d0eab7a..8e5680f 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -93,7 +93,6 @@ func (a *Analyzer) Analyze(packageName string) (*ir.Package, error) { // a struct may only do it through an indirection. breakAliasCycles(pkg.Types) breakStructCycles(pkg.Types) - dropShadowedCatchAlls(pkg.Types) // Detect paginated operations. a.detectPagination(pkg) diff --git a/internal/generator/e2e_additional_properties_test.go b/internal/generator/e2e_additional_properties_test.go index 4c02984..d665de9 100644 --- a/internal/generator/e2e_additional_properties_test.go +++ b/internal/generator/e2e_additional_properties_test.go @@ -58,6 +58,63 @@ components: properties: note: { type: string } additionalProperties: true + Parent: + type: object + properties: + id: { type: string } + additionalProperties: true + Child: + allOf: + - $ref: "#/components/schemas/Parent" + - type: object + properties: + name: { type: string } + additionalProperties: true + GrandChild: + allOf: + - $ref: "#/components/schemas/Child" + - type: object + properties: + depth: { type: integer } + additionalProperties: true + TwinA: + type: object + properties: + a: { type: string } + additionalProperties: true + TwinB: + type: object + properties: + b: { type: string } + additionalProperties: true + TwoParents: + allOf: + - $ref: "#/components/schemas/TwinA" + - $ref: "#/components/schemas/TwinB" + - type: object + properties: + own: { type: string } + Circle: + type: object + properties: + radius: { type: number } + Shape: + oneOf: + - $ref: "#/components/schemas/Circle" + - type: string + Tagged: + allOf: + - $ref: "#/components/schemas/Shape" + - type: object + properties: + label: { type: string } + additionalProperties: true + LabeledShape: + allOf: + - $ref: "#/components/schemas/Shape" + - type: object + properties: + title: { type: string } ` const additionalPropertiesRuntimeTest = `package petsapi @@ -237,6 +294,217 @@ func TestOffTypeExtraDoesNotFailTheDecode(t *testing.T) { } } +// When the embedded schema has additionalProperties of its own, its promoted +// unmarshaler used to run for the composed type and sweep the composed type's +// declared fields into the embedded catch-all. +func TestEmbeddedCatchAllDoesNotSwallowDeclaredFields(t *testing.T) { + var c Child + if err := json.Unmarshal([]byte(` + "`" + `{"id":"x","name":"n","extra":"e"}` + "`" + `), &c); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if c.ID == nil || *c.ID != "x" { + t.Errorf("inherited property lost: %v", c.ID) + } + if c.Name == nil || *c.Name != "n" { + t.Errorf("declared property swallowed by the embedded catch-all: %v", c.Name) + } + if got := c.AdditionalProperties["extra"]; got != "e" { + t.Errorf("AdditionalProperties[extra] = %v, want e", got) + } + if len(c.Parent.AdditionalProperties) != 0 { + t.Errorf("embedded catch-all should stay empty, got %v", c.Parent.AdditionalProperties) + } + + out, err := json.Marshal(c) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"id"` + "`" + `, ` + "`" + `"name"` + "`" + `, ` + "`" + `"extra"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } + var got map[string]any + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("re-unmarshal: %v", err) + } + want := map[string]any{"id": "x", "name": "n", "extra": "e"} + if len(got) != len(want) { + t.Fatalf("round trip = %s, want %v", out, want) + } + for k, v := range want { + if got[k] != v { + t.Errorf("round trip[%s] = %v, want %v", k, got[k], v) + } + } +} + +// The same swallowing, two levels deep: the grandparent's catch-all sits behind +// two promotions. +func TestEmbeddedCatchAllChainThroughGrandparent(t *testing.T) { + var g GrandChild + if err := json.Unmarshal([]byte(` + "`" + `{"id":"x","name":"n","depth":2,"extra":"e"}` + "`" + `), &g); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if g.ID == nil || *g.ID != "x" { + t.Errorf("grandparent property lost: %v", g.ID) + } + if g.Name == nil || *g.Name != "n" { + t.Errorf("parent property lost: %v", g.Name) + } + if g.Depth == nil || *g.Depth != 2 { + t.Errorf("declared property lost: %v", g.Depth) + } + if got := g.AdditionalProperties["extra"]; got != "e" { + t.Errorf("AdditionalProperties[extra] = %v, want e", got) + } + if len(g.Child.AdditionalProperties) != 0 || len(g.Child.Parent.AdditionalProperties) != 0 { + t.Errorf("embedded catch-alls should stay empty, got %v and %v", + g.Child.AdditionalProperties, g.Child.Parent.AdditionalProperties) + } + + out, err := json.Marshal(g) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"id"` + "`" + `, ` + "`" + `"name"` + "`" + `, ` + "`" + `"depth"` + "`" + `, ` + "`" + `"extra"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } +} + +// With two embedded catch-all parents Go promotes no marshalers at all (the +// selector is ambiguous), so without generated ones the behavior flips on how +// many parents a schema composes. The composed schema declares no +// additionalProperties of its own; the extras stay on the embedded parents. +func TestTwoEmbeddedCatchAllParents(t *testing.T) { + var p TwoParents + if err := json.Unmarshal([]byte(` + "`" + `{"a":"1","b":"2","own":"3","extra":"e"}` + "`" + `), &p); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if p.A == nil || *p.A != "1" { + t.Errorf("A = %v, want 1", p.A) + } + if p.B == nil || *p.B != "2" { + t.Errorf("B = %v, want 2", p.B) + } + if p.Own == nil || *p.Own != "3" { + t.Errorf("Own = %v, want 3", p.Own) + } + if got := p.TwinA.AdditionalProperties["extra"]; got != "e" { + t.Errorf("TwinA.AdditionalProperties[extra] = %v, want e", got) + } + for _, key := range []string{"a", "b", "own"} { + if _, ok := p.TwinA.AdditionalProperties[key]; ok { + t.Errorf("declared property %q landed in TwinA's catch-all", key) + } + if _, ok := p.TwinB.AdditionalProperties[key]; ok { + t.Errorf("declared property %q landed in TwinB's catch-all", key) + } + } + + out, err := json.Marshal(p) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"a"` + "`" + `, ` + "`" + `"b"` + "`" + `, ` + "`" + `"own"` + "`" + `, ` + "`" + `"extra"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } + var got map[string]any + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("re-unmarshal: %v", err) + } + want := map[string]any{"a": "1", "b": "2", "own": "3", "extra": "e"} + if len(got) != len(want) { + t.Fatalf("round trip = %s, want %v", out, want) + } + for k, v := range want { + if got[k] != v { + t.Errorf("round trip[%s] = %v, want %v", k, got[k], v) + } + } +} + +// A composed type embedding a union used to promote the union's marshaler, +// which emitted only the union value and dropped the composed type's fields. +func TestEmbeddedUnionObjectVariant(t *testing.T) { + var v Tagged + if err := json.Unmarshal([]byte(` + "`" + `{"radius":1.5,"label":"L","extra":"e"}` + "`" + `), &v); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if v.Label == nil || *v.Label != "L" { + t.Errorf("Label = %v, want L", v.Label) + } + circle, ok := v.Shape.Value.(Circle) + if !ok { + t.Fatalf("Shape.Value = %T, want Circle", v.Shape.Value) + } + if circle.Radius == nil || *circle.Radius != 1.5 { + t.Errorf("Radius = %v, want 1.5", circle.Radius) + } + + out, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"radius"` + "`" + `, ` + "`" + `"label"` + "`" + `, ` + "`" + `"extra"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } + var got map[string]any + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("re-unmarshal: %v", err) + } + for k, want := range map[string]any{"radius": 1.5, "label": "L", "extra": "e"} { + if got[k] != want { + t.Errorf("round trip[%s] = %v, want %v", k, got[k], want) + } + } +} + +// A schema with no additionalProperties of its own still needs marshalers when +// it embeds a type that has them, or the promotion swallows its fields anyway. +func TestEmbeddedUnionWithoutOwnCatchAll(t *testing.T) { + var s LabeledShape + if err := json.Unmarshal([]byte(` + "`" + `{"radius":2,"title":"T"}` + "`" + `), &s); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if s.Title == nil || *s.Title != "T" { + t.Errorf("Title = %v, want T", s.Title) + } + circle, ok := s.Shape.Value.(Circle) + if !ok { + t.Fatalf("Shape.Value = %T, want Circle", s.Shape.Value) + } + if circle.Radius == nil || *circle.Radius != 2 { + t.Errorf("Radius = %v, want 2", circle.Radius) + } + + out, err := json.Marshal(s) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"radius"` + "`" + `, ` + "`" + `"title"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } +} + +// A scalar union value cannot be part of a JSON object; marshaling used to fail +// with a bare syntax error from splicing after the promoted union marshaler. +func TestEmbeddedUnionScalarVariantFailsWithNamedError(t *testing.T) { + label := "L" + v := Tagged{Shape: Shape{Value: "not an object"}, Label: &label} + if _, err := json.Marshal(v); err == nil || !strings.Contains(err.Error(), "Shape") { + t.Errorf("marshal = %v, want an error naming the embedded Shape", err) + } +} + // The embedded schema is reached through an alias, so the catch-all still has to // recognize the properties that alias promotes as already declared. func TestPropertiesInheritedThroughAnAliasAreNotRecollected(t *testing.T) { diff --git a/internal/generator/funcmap.go b/internal/generator/funcmap.go index 58d81d8..7785476 100644 --- a/internal/generator/funcmap.go +++ b/internal/generator/funcmap.go @@ -33,6 +33,11 @@ func FuncMap() template.FuncMap { "catchAllValueType": catchAllValueType, "declaredJSONNames": declaredJSONNames, "hasCatchAllTypes": hasCatchAllTypes, + "marshalerEmbeds": marshalerEmbeds, + "plainFields": plainFields, + "fieldGoName": fieldGoName, + "embeddedCatchAlls": embeddedCatchAlls, + "hasMarshalerEmbeds": hasMarshalerEmbeds, "discriminatorFieldName": discriminatorFieldName, "hasPaginatedOps": hasPaginatedOps, "paginationItemType": paginationItemType, @@ -261,6 +266,155 @@ func hasCatchAllTypes(types []*ir.TypeDef) bool { }) } +// terminalTypeName resolves a Go type expression to the named type it denotes, +// following aliases, or "" for builtins and composites. +func terminalTypeName(byName map[string]*ir.TypeDef, goType string) string { + name := ir.NamedType(strings.TrimPrefix(goType, "*")) + for range len(byName) + 1 { + td := byName[name] + if td == nil || td.Kind != ir.TypeKindAlias { + return name + } + next := ir.NamedType(strings.TrimPrefix(td.GoType, "*")) + if next == "" { + return "" + } + name = next + } + return name +} + +// customMarshalerTypes returns the names of generated types whose method set +// carries MarshalJSON/UnmarshalJSON: unions, structs with a catch-all, and +// structs that embed such a type and so inherit the methods by promotion. +func customMarshalerTypes(pkg *ir.Package) map[string]bool { + byName := ir.TypesByName(pkg.Types) + bearing := make(map[string]bool) + for _, td := range pkg.Types { + if td == nil { + continue + } + if td.Kind == ir.TypeKindUnion || catchAllField(td) != nil { + bearing[td.Name] = true + } + } + for changed := true; changed; { + changed = false + for _, td := range pkg.Types { + if td == nil || td.Kind != ir.TypeKindStruct || bearing[td.Name] { + continue + } + for _, f := range td.Fields { + if f.Embedded && bearing[terminalTypeName(byName, f.Type)] { + bearing[td.Name] = true + changed = true + break + } + } + } + } + return bearing +} + +// marshalerEmbeds returns the embedded fields whose types carry custom JSON +// marshalers, which the enclosing struct must encode part by part: handing the +// whole struct to encoding/json would promote those methods and let one +// embedded type speak for the entire object. A struct whose single field is +// such an embed has nothing of its own to lose to promotion, so it gets none. +func marshalerEmbeds(pkg *ir.Package, td *ir.TypeDef) []*ir.Field { + if td.Kind != ir.TypeKindStruct || len(td.Fields) < 2 { + return nil + } + byName := ir.TypesByName(pkg.Types) + bearing := customMarshalerTypes(pkg) + var embeds []*ir.Field + for _, f := range td.Fields { + if f.Embedded && bearing[terminalTypeName(byName, f.Type)] { + embeds = append(embeds, f) + } + } + return embeds +} + +// plainFields returns the fields a part-wise marshaled struct encodes directly: +// everything except the catch-all and the marshaler-bearing embeds. +func plainFields(pkg *ir.Package, td *ir.TypeDef) []*ir.Field { + skip := make(map[*ir.Field]bool) + for _, f := range marshalerEmbeds(pkg, td) { + skip[f] = true + } + var fields []*ir.Field + for _, f := range td.Fields { + if !f.CatchAll && !skip[f] { + fields = append(fields, f) + } + } + return fields +} + +// fieldGoName returns the name a field is selected by: for an embedded field +// that is the type name, with any pointer indirection stripped. +func fieldGoName(f *ir.Field) string { + if f.Embedded { + return strings.TrimPrefix(f.Type, "*") + } + return f.Name +} + +// EmbeddedCatchAll locates a catch-all map inside an embedded type: the Go +// selector path from the enclosing struct, and the nil checks any pointers on +// that path require. +type EmbeddedCatchAll struct { + Guard string + Path string +} + +// embeddedCatchAlls returns the catch-all maps reachable through a struct's +// embedded types, which the enclosing type's UnmarshalJSON has to clean up +// after the embedded unmarshalers have run. +func embeddedCatchAlls(pkg *ir.Package, td *ir.TypeDef) []EmbeddedCatchAll { + byName := ir.TypesByName(pkg.Types) + + var found []EmbeddedCatchAll + seen := map[string]bool{td.Name: true} + var walk func(td *ir.TypeDef, path, guard string) + walk = func(td *ir.TypeDef, path, guard string) { + for _, f := range td.Fields { + if !f.Embedded { + continue + } + embedded := ir.StructNamed(byName, strings.TrimPrefix(f.Type, "*")) + if embedded == nil || seen[embedded.Name] { + continue + } + fieldPath := path + fieldGoName(f) + fieldGuard := guard + if strings.HasPrefix(f.Type, "*") { + if fieldGuard != "" { + fieldGuard += " && " + } + fieldGuard += "t." + fieldPath + " != nil" + } + if ca := catchAllField(embedded); ca != nil { + found = append(found, EmbeddedCatchAll{Guard: fieldGuard, Path: fieldPath + "." + ca.Name}) + } + seen[embedded.Name] = true + walk(embedded, fieldPath+".", fieldGuard) + delete(seen, embedded.Name) + } + } + walk(td, "", "") + return found +} + +// hasMarshalerEmbeds reports whether any struct needs part-wise marshalers, +// which is what pulls the JSON object merge helpers into the generated types. +func hasMarshalerEmbeds(pkg *ir.Package) bool { + return slices.ContainsFunc(pkg.Types, func(td *ir.TypeDef) bool { + return td != nil && len(marshalerEmbeds(pkg, td)) > 0 + }) +} + // hasOperations returns true if the package has any operations defined. func hasOperations(pkg *ir.Package) bool { return len(pkg.Operations) > 0 diff --git a/internal/templates/types.go.tmpl b/internal/templates/types.go.tmpl index a9f22fe..d4dba0f 100644 --- a/internal/templates/types.go.tmpl +++ b/internal/templates/types.go.tmpl @@ -3,10 +3,13 @@ package {{ .Name }} import ( -{{- if or (hasUnions .Types) (hasCatchAllTypes .Types) }} +{{- if or (hasUnions .Types) (hasCatchAllTypes .Types) (hasMarshalerEmbeds $) }} "encoding/json" "fmt" {{- end }} +{{- if or (hasCatchAllTypes .Types) (hasMarshalerEmbeds $) }} + "bytes" +{{- end }} {{- if hasCatchAllTypes .Types }} "strings" {{- end }} @@ -17,7 +20,7 @@ import ( // deleteDeclaredProperties drops the keys encoding/json already consumed into // struct fields. The match is case-insensitive because that is the fallback // encoding/json itself uses, so "Name" must not also land in the catch-all map. -func deleteDeclaredProperties(obj map[string]json.RawMessage, declared []string) { +func deleteDeclaredProperties[V any](obj map[string]V, declared []string) { for key := range obj { for _, name := range declared { if strings.EqualFold(key, name) { @@ -28,6 +31,77 @@ func deleteDeclaredProperties(obj map[string]json.RawMessage, declared []string) } } {{- end }} +{{- if or (hasCatchAllTypes .Types) (hasMarshalerEmbeds $) }} + +// jsonMember is one key/value pair of a JSON object under assembly. +type jsonMember struct { + key string + raw json.RawMessage +} + +// mergeObjectMembers appends src's members to dst in order, replacing the value +// of a key dst already holds. src must be a JSON object; null is a no-op, the +// way encoding/json treats a nil embedded pointer. +func mergeObjectMembers(dst []jsonMember, src []byte) ([]jsonMember, error) { + trimmed := bytes.TrimSpace(src) + if bytes.Equal(trimmed, []byte("null")) { + return dst, nil + } + if len(trimmed) == 0 || trimmed[0] != '{' { + return nil, fmt.Errorf("cannot merge non-object value %s into a JSON object", trimmed) + } + dec := json.NewDecoder(bytes.NewReader(trimmed)) + if _, err := dec.Token(); err != nil { + return nil, err + } + for dec.More() { + keyToken, err := dec.Token() + if err != nil { + return nil, err + } + key, ok := keyToken.(string) + if !ok { + return nil, fmt.Errorf("unexpected token %v in JSON object", keyToken) + } + var value json.RawMessage + if err := dec.Decode(&value); err != nil { + return nil, err + } + replaced := false + for i := range dst { + if dst[i].key == key { + dst[i].raw = value + replaced = true + break + } + } + if !replaced { + dst = append(dst, jsonMember{key: key, raw: value}) + } + } + return dst, nil +} + +// encodeObject renders members as a JSON object, preserving their order. +func encodeObject(members []jsonMember) ([]byte, error) { + var buf bytes.Buffer + buf.WriteByte('{') + for i, m := range members { + if i > 0 { + buf.WriteByte(',') + } + key, err := json.Marshal(m.key) + if err != nil { + return nil, err + } + buf.Write(key) + buf.WriteByte(':') + buf.Write(m.raw) + } + buf.WriteByte('}') + return buf.Bytes(), nil +} +{{- end }} {{ range .Types }} {{ $typeDoc := typeDocComment . }}{{ if eq .Kind 0 }}{{ $typeName := .Name }}{{ $td := . }}{{ if $typeDoc }}{{ $typeDoc }} @@ -36,78 +110,123 @@ func deleteDeclaredProperties(obj map[string]json.RawMessage, declared []string) {{ end }}{{ if .Embedded }} {{ .Type }} {{ else }} {{ .Name }} {{ .Type }} `{{ fieldTag . }}` {{ end }}{{ end }}} -{{ with catchAllField . }} -// MarshalJSON implements json.Marshaler for {{ $typeName }}, inlining -// {{ .Name }} alongside the schema's declared properties. -func (t {{ $typeName }}) MarshalJSON() ([]byte, error) { - type shadow {{ $typeName }} - data, err := json.Marshal(shadow(t)) +{{ $catchAll := catchAllField . }}{{ $embeds := marshalerEmbeds $ . }}{{ $plain := plainFields $ . }}{{ if or $catchAll $embeds }} +{{ if $embeds }}// MarshalJSON implements json.Marshaler for {{ $typeName }}. The embedded types +// carry marshalers of their own, so each part is encoded separately and merged; +// marshaling the whole struct would promote one embedded type's marshaler and +// let it speak for the entire object. +{{ else }}// MarshalJSON implements json.Marshaler for {{ $typeName }}, inlining +// {{ $catchAll.Name }} alongside the schema's declared properties. +{{ end }}func (t {{ $typeName }}) MarshalJSON() ([]byte, error) { + var members []jsonMember +{{ if $embeds }}{{ range $i, $f := $embeds }} part{{ $i }}, err := json.Marshal(t.{{ fieldGoName $f }}) if err != nil { return nil, err } - if len(t.{{ .Name }}) == 0 { - return data, nil + if members, err = mergeObjectMembers(members, part{{ $i }}); err != nil { + return nil, fmt.Errorf("marshaling {{ $typeName }}: embedded {{ fieldGoName $f }}: %w", err) } - - extra := make(map[string]json.RawMessage, len(t.{{ .Name }})) - for key, value := range t.{{ .Name }} { - raw, err := json.Marshal(value) - if err != nil { - return nil, fmt.Errorf("marshaling additional property %q: %w", key, err) - } - extra[key] = raw +{{ end }}{{ if $plain }} own, err := json.Marshal(struct { +{{ range $plain }}{{ if .Embedded }} {{ .Type }} +{{ else }} {{ .Name }} {{ .Type }} `{{ fieldTag . }}` +{{ end }}{{ end }} }{ +{{ range $plain }} {{ fieldGoName . }}: t.{{ fieldGoName . }}, +{{ end }} }) + if err != nil { + return nil, err } - deleteDeclaredProperties(extra, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) - if len(extra) == 0 { - return data, nil + if members, err = mergeObjectMembers(members, own); err != nil { + return nil, err } - encoded, err := json.Marshal(extra) +{{ end }}{{ else }} type shadow {{ $typeName }} + data, err := json.Marshal(shadow(t)) if err != nil { return nil, err } - - // Splice the two objects together so the declared properties keep their - // struct field order instead of being re-sorted through a map. - merged := make([]byte, 0, len(data)+len(encoded)) - merged = append(merged, data[:len(data)-1]...) - if len(data) > 2 { - merged = append(merged, ',') - } - return append(merged, encoded[1:]...), nil + if members, err = mergeObjectMembers(members, data); err != nil { + return nil, err + } +{{ end }}{{ with $catchAll }} if len(t.{{ .Name }}) > 0 { + extra := make(map[string]json.RawMessage, len(t.{{ .Name }})) + for key, value := range t.{{ .Name }} { + raw, err := json.Marshal(value) + if err != nil { + return nil, fmt.Errorf("marshaling additional property %q: %w", key, err) + } + extra[key] = raw + } + deleteDeclaredProperties(extra, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) + if len(extra) > 0 { + encoded, err := json.Marshal(extra) + if err != nil { + return nil, err + } + if members, err = mergeObjectMembers(members, encoded); err != nil { + return nil, err + } + } + } +{{ end }} return encodeObject(members) } -// UnmarshalJSON implements json.Unmarshaler for {{ $typeName }}, collecting -// properties the schema does not declare into {{ .Name }}. -func (t *{{ $typeName }}) UnmarshalJSON(data []byte) error { - type shadow {{ $typeName }} - t.{{ .Name }} = nil +{{ if $embeds }}// UnmarshalJSON implements json.Unmarshaler for {{ $typeName }}, decoding the +// embedded types and the struct's own fields from the same object{{ if $catchAll }}, then +// collecting properties the schema does not declare into {{ $catchAll.Name }}{{ end }}. +{{ else }}// UnmarshalJSON implements json.Unmarshaler for {{ $typeName }}, collecting +// properties the schema does not declare into {{ $catchAll.Name }}. +{{ end }}func (t *{{ $typeName }}) UnmarshalJSON(data []byte) error { +{{ if $embeds }}{{ range $embeds }} if err := json.Unmarshal(data, &t.{{ fieldGoName . }}); err != nil { + return err + } +{{ end }}{{ if $plain }} own := struct { +{{ range $plain }}{{ if .Embedded }} {{ .Type }} +{{ else }} {{ .Name }} {{ .Type }} `{{ fieldTag . }}` +{{ end }}{{ end }} }{ +{{ range $plain }} {{ fieldGoName . }}: t.{{ fieldGoName . }}, +{{ end }} } + if err := json.Unmarshal(data, &own); err != nil { + return err + } +{{ range $plain }} t.{{ fieldGoName . }} = own.{{ fieldGoName . }} +{{ end }}{{ end }}{{ else }} type shadow {{ $typeName }} if err := json.Unmarshal(data, (*shadow)(t)); err != nil { return err } - +{{ end }}{{ with $catchAll }} t.{{ .Name }} = nil var obj map[string]json.RawMessage if err := json.Unmarshal(data, &obj); err != nil { return err } deleteDeclaredProperties(obj, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) - if len(obj) == 0 { - return nil - } - t.{{ .Name }} = make({{ .Type }}, len(obj)) - for key, raw := range obj { - var value {{ catchAllValueType . }} - // A property that is both undeclared and not of the type the schema gives - // additionalProperties is the least useful thing in the payload, so it is - // dropped rather than failing the decode of everything alongside it. - if err := json.Unmarshal(raw, &value); err != nil { - continue + if len(obj) > 0 { + t.{{ .Name }} = make({{ .Type }}, len(obj)) + for key, raw := range obj { + var value {{ catchAllValueType . }} + // A property that is both undeclared and not of the type the schema gives + // additionalProperties is the least useful thing in the payload, so it is + // dropped rather than failing the decode of everything alongside it. + if err := json.Unmarshal(raw, &value); err != nil { + continue + } + t.{{ .Name }}[key] = value + } + if len(t.{{ .Name }}) == 0 { + t.{{ .Name }} = nil } - t.{{ .Name }}[key] = value } - if len(t.{{ .Name }}) == 0 { - t.{{ .Name }} = nil +{{ end }}{{ if $embeds }}{{ with embeddedCatchAlls $ $td }}{{ if $catchAll }} // The embedded unmarshalers ran on the full object, so their catch-alls hold + // this type's declared fields and the extras collected above; clear them. +{{ range . }}{{ if .Guard }} if {{ .Guard }} { + t.{{ .Path }} = nil + } +{{ else }} t.{{ .Path }} = nil +{{ end }}{{ end }}{{ else }} // The embedded unmarshalers ran on the full object, so their catch-alls also + // collected this type's declared fields; only true extras belong there. +{{ range . }}{{ if .Guard }} if {{ .Guard }} { + deleteDeclaredProperties(t.{{ .Path }}, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) } - return nil +{{ else }} deleteDeclaredProperties(t.{{ .Path }}, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) +{{ end }}{{ end }}{{ end }}{{ end }}{{ end }} return nil } {{ end }} {{ else if eq .Kind 1 }}{{ if $typeDoc }}{{ $typeDoc }} From a30052062eb1b21e66f7af49bc1ace1f27f8797b Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 6 Aug 2026 03:21:02 +0800 Subject: [PATCH 2/2] fix: harden part-wise marshalers against cycles, union key leaks, and lost extras Post-review fixes to the part-wise marshalers: - Cyclic allOf schemas made the generated unmarshalers re-enter each other with the same bytes until the stack overflowed. A pointer embed that closes a reference cycle is no longer decoded, matching encoding/json, which also stops its field walk where a type repeats. - An embedded union's variant keys landed in the composed type's catch-all and overrode an updated union value on re-marshal. The catch-all is now pruned with the keys the decoded union value actually marshals. - The embedded catch-all cleanup nilled the maps outright, dropping an extra that only a wider embedded map could hold when the outer catch-all is typed narrower. Cleanup now removes declared names and the keys the outer catch-all collected, leaving the rest with the embed that accepted them. - The catch-all reset ran after the embed decodes, so a failed re-unmarshal into a reused struct kept the previous payload's extras. The reset is back at the top of UnmarshalJSON. Also: the package-wide bearing fixed point is computed once per package behind a mutex instead of per helper call, the declared-names literal and alias walks are shared instead of repeated, and new e2e coverage exercises cyclic schemas, narrowly typed outer catch-alls, and union re-marshal. --- .../e2e_additional_properties_test.go | 103 ++++++++++++++- internal/generator/funcmap.go | 121 +++++++++++++++--- internal/templates/types.go.tmpl | 49 ++++--- 3 files changed, 233 insertions(+), 40 deletions(-) diff --git a/internal/generator/e2e_additional_properties_test.go b/internal/generator/e2e_additional_properties_test.go index d665de9..169064a 100644 --- a/internal/generator/e2e_additional_properties_test.go +++ b/internal/generator/e2e_additional_properties_test.go @@ -115,6 +115,25 @@ components: - type: object properties: title: { type: string } + StrictChild: + allOf: + - $ref: "#/components/schemas/Parent" + - type: object + properties: + name: { type: string } + additionalProperties: + type: string + Node: + allOf: + - $ref: "#/components/schemas/NodeBase" + - type: object + properties: + label: { type: string } + additionalProperties: true + NodeBase: + allOf: + - $ref: "#/components/schemas/Node" + additionalProperties: true ` const additionalPropertiesRuntimeTest = `package petsapi @@ -294,9 +313,9 @@ func TestOffTypeExtraDoesNotFailTheDecode(t *testing.T) { } } -// When the embedded schema has additionalProperties of its own, its promoted -// unmarshaler used to run for the composed type and sweep the composed type's -// declared fields into the embedded catch-all. +// An embedded schema with additionalProperties of its own must not let its +// promoted unmarshaler sweep the composed type's declared fields into the +// embedded catch-all. func TestEmbeddedCatchAllDoesNotSwallowDeclaredFields(t *testing.T) { var c Child if err := json.Unmarshal([]byte(` + "`" + `{"id":"x","name":"n","extra":"e"}` + "`" + `), &c); err != nil { @@ -428,8 +447,8 @@ func TestTwoEmbeddedCatchAllParents(t *testing.T) { } } -// A composed type embedding a union used to promote the union's marshaler, -// which emitted only the union value and dropped the composed type's fields. +// A composed type embedding a union keeps its own fields: promotion would let +// the union's marshaler emit only the union value. func TestEmbeddedUnionObjectVariant(t *testing.T) { var v Tagged if err := json.Unmarshal([]byte(` + "`" + `{"radius":1.5,"label":"L","extra":"e"}` + "`" + `), &v); err != nil { @@ -445,6 +464,9 @@ func TestEmbeddedUnionObjectVariant(t *testing.T) { if circle.Radius == nil || *circle.Radius != 1.5 { t.Errorf("Radius = %v, want 1.5", circle.Radius) } + if _, ok := v.AdditionalProperties["radius"]; ok { + t.Errorf("union variant property leaked into AdditionalProperties: %v", v.AdditionalProperties) + } out, err := json.Marshal(v) if err != nil { @@ -464,6 +486,20 @@ func TestEmbeddedUnionObjectVariant(t *testing.T) { t.Errorf("round trip[%s] = %v, want %v", k, got[k], want) } } + + two := 2.0 + v.Shape.Value = Circle{Radius: &two} + updated, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal updated: %v", err) + } + var re map[string]any + if err := json.Unmarshal(updated, &re); err != nil { + t.Fatalf("re-unmarshal updated: %v", err) + } + if re["radius"] != 2.0 { + t.Errorf("stale catch-all copy overrode the updated union value: %s", updated) + } } // A schema with no additionalProperties of its own still needs marshalers when @@ -495,8 +531,8 @@ func TestEmbeddedUnionWithoutOwnCatchAll(t *testing.T) { } } -// A scalar union value cannot be part of a JSON object; marshaling used to fail -// with a bare syntax error from splicing after the promoted union marshaler. +// A scalar union value cannot be part of a JSON object, so marshaling fails +// with an error that names the embedded field. func TestEmbeddedUnionScalarVariantFailsWithNamedError(t *testing.T) { label := "L" v := Tagged{Shape: Shape{Value: "not an object"}, Label: &label} @@ -505,6 +541,59 @@ func TestEmbeddedUnionScalarVariantFailsWithNamedError(t *testing.T) { } } +// The composed type's catch-all is narrower than the embedded one, so an extra +// only the wider embedded map can hold must survive there. +func TestOffTypeExtraSurvivesInTheWiderEmbeddedCatchAll(t *testing.T) { + var s StrictChild + if err := json.Unmarshal([]byte(` + "`" + `{"id":"x","name":"n","note":"ok","count":3}` + "`" + `), &s); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := s.AdditionalProperties["note"]; got != "ok" { + t.Errorf("AdditionalProperties[note] = %v, want ok", got) + } + if _, ok := s.AdditionalProperties["count"]; ok { + t.Errorf("off-type extra landed in the string catch-all: %v", s.AdditionalProperties) + } + if got := s.Parent.AdditionalProperties["count"]; got != float64(3) { + t.Errorf("Parent.AdditionalProperties[count] = %v, want 3", got) + } + + out, err := json.Marshal(s) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"id"` + "`" + `, ` + "`" + `"name"` + "`" + `, ` + "`" + `"note"` + "`" + `, ` + "`" + `"count"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } +} + +// Mutually recursive allOf schemas embed each other; decoding must terminate +// instead of re-entering the same unmarshaler until the stack overflows. +func TestCyclicSchemasDecodeWithoutOverflowingTheStack(t *testing.T) { + var node Node + if err := json.Unmarshal([]byte(` + "`" + `{"label":"a","extra":"e"}` + "`" + `), &node); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if node.Label == nil || *node.Label != "a" { + t.Errorf("Label = %v, want a", node.Label) + } + if got := node.AdditionalProperties["extra"]; got != "e" { + t.Errorf("AdditionalProperties[extra] = %v, want e", got) + } + + out, err := json.Marshal(node) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, key := range []string{` + "`" + `"label"` + "`" + `, ` + "`" + `"extra"` + "`" + `} { + if n := strings.Count(string(out), key); n != 1 { + t.Errorf("%s emitted %d times: %s", key, n, out) + } + } +} + // The embedded schema is reached through an alias, so the catch-all still has to // recognize the properties that alias promotes as already declared. func TestPropertiesInheritedThroughAnAliasAreNotRecollected(t *testing.T) { diff --git a/internal/generator/funcmap.go b/internal/generator/funcmap.go index 7785476..c7e910d 100644 --- a/internal/generator/funcmap.go +++ b/internal/generator/funcmap.go @@ -2,7 +2,9 @@ package generator import ( "slices" + "strconv" "strings" + "sync" "text/template" naming "github.com/giraffesyo/openapi-go-naming" @@ -31,13 +33,15 @@ func FuncMap() template.FuncMap { "hasUntypedVariant": hasUntypedVariant, "catchAllField": catchAllField, "catchAllValueType": catchAllValueType, - "declaredJSONNames": declaredJSONNames, "hasCatchAllTypes": hasCatchAllTypes, "marshalerEmbeds": marshalerEmbeds, "plainFields": plainFields, "fieldGoName": fieldGoName, "embeddedCatchAlls": embeddedCatchAlls, "hasMarshalerEmbeds": hasMarshalerEmbeds, + "decodedEmbeds": decodedEmbeds, + "opaqueEmbeds": opaqueEmbeds, + "declaredNamesLiteral": declaredNamesLiteral, "discriminatorFieldName": discriminatorFieldName, "hasPaginatedOps": hasPaginatedOps, "paginationItemType": paginationItemType, @@ -232,7 +236,7 @@ func catchAllValueType(f *ir.Field) string { // fields, an embedded type's included: those are promoted onto the struct, so a // catch-all that re-collected them would emit each one twice. func declaredJSONNames(pkg *ir.Package, td *ir.TypeDef) []string { - byName := ir.TypesByName(pkg.Types) + byName := typeIndex(pkg).byName var names []string visited := make(map[string]bool) @@ -260,6 +264,16 @@ func declaredJSONNames(pkg *ir.Package, td *ir.TypeDef) []string { return names } +// declaredNamesLiteral renders declaredJSONNames as a Go []string literal. +func declaredNamesLiteral(pkg *ir.Package, td *ir.TypeDef) string { + names := declaredJSONNames(pkg, td) + quoted := make([]string, len(names)) + for i, name := range names { + quoted[i] = strconv.Quote(name) + } + return "[]string{" + strings.Join(quoted, ", ") + "}" +} + func hasCatchAllTypes(types []*ir.TypeDef) bool { return slices.ContainsFunc(types, func(td *ir.TypeDef) bool { return catchAllField(td) != nil @@ -284,36 +298,56 @@ func terminalTypeName(byName map[string]*ir.TypeDef, goType string) string { return name } -// customMarshalerTypes returns the names of generated types whose method set -// carries MarshalJSON/UnmarshalJSON: unions, structs with a catch-all, and -// structs that embed such a type and so inherit the methods by promotion. -func customMarshalerTypes(pkg *ir.Package) map[string]bool { - byName := ir.TypesByName(pkg.Types) - bearing := make(map[string]bool) +// pkgTypeIndex carries the package-wide lookups the marshaler helpers share. +// bearing holds the names of generated types whose method set carries +// MarshalJSON/UnmarshalJSON: unions, structs with a catch-all, and structs that +// embed such a type and so inherit the methods by promotion. +type pkgTypeIndex struct { + byName map[string]*ir.TypeDef + bearing map[string]bool +} + +// The bearing fixed point is package-wide but templates ask per type, so the +// last package's index is cached. A single entry never outlives its package by +// more than one generate and stays safe under concurrent generates. +var typeIndexMu sync.Mutex +var lastTypeIndex struct { + pkg *ir.Package + idx *pkgTypeIndex +} + +func typeIndex(pkg *ir.Package) *pkgTypeIndex { + typeIndexMu.Lock() + defer typeIndexMu.Unlock() + if lastTypeIndex.pkg == pkg { + return lastTypeIndex.idx + } + idx := &pkgTypeIndex{byName: ir.TypesByName(pkg.Types), bearing: map[string]bool{}} for _, td := range pkg.Types { if td == nil { continue } if td.Kind == ir.TypeKindUnion || catchAllField(td) != nil { - bearing[td.Name] = true + idx.bearing[td.Name] = true } } for changed := true; changed; { changed = false for _, td := range pkg.Types { - if td == nil || td.Kind != ir.TypeKindStruct || bearing[td.Name] { + if td == nil || td.Kind != ir.TypeKindStruct || idx.bearing[td.Name] { continue } for _, f := range td.Fields { - if f.Embedded && bearing[terminalTypeName(byName, f.Type)] { - bearing[td.Name] = true + if f.Embedded && idx.bearing[terminalTypeName(idx.byName, f.Type)] { + idx.bearing[td.Name] = true changed = true break } } } } - return bearing + lastTypeIndex.pkg, lastTypeIndex.idx = pkg, idx + return idx } // marshalerEmbeds returns the embedded fields whose types carry custom JSON @@ -325,11 +359,62 @@ func marshalerEmbeds(pkg *ir.Package, td *ir.TypeDef) []*ir.Field { if td.Kind != ir.TypeKindStruct || len(td.Fields) < 2 { return nil } - byName := ir.TypesByName(pkg.Types) - bearing := customMarshalerTypes(pkg) + idx := typeIndex(pkg) var embeds []*ir.Field for _, f := range td.Fields { - if f.Embedded && bearing[terminalTypeName(byName, f.Type)] { + if f.Embedded && idx.bearing[terminalTypeName(idx.byName, f.Type)] { + embeds = append(embeds, f) + } + } + return embeds +} + +// decodedEmbeds returns the marshaler embeds UnmarshalJSON decodes in place. A +// pointer embed that closes a reference cycle stays nil: decoding it would +// re-enter this unmarshaler with the same bytes and recurse until the stack +// overflows, and encoding/json also stops its field walk where a type repeats. +func decodedEmbeds(pkg *ir.Package, td *ir.TypeDef) []*ir.Field { + byName := typeIndex(pkg).byName + var embeds []*ir.Field + for _, f := range marshalerEmbeds(pkg, td) { + if strings.HasPrefix(f.Type, "*") && embedsReach(byName, f.Type, td.Name) { + continue + } + embeds = append(embeds, f) + } + return embeds +} + +func embedsReach(byName map[string]*ir.TypeDef, goType, target string) bool { + seen := map[string]bool{} + var walk func(goType string) bool + walk = func(goType string) bool { + td := ir.StructNamed(byName, strings.TrimPrefix(goType, "*")) + if td == nil || seen[td.Name] { + return false + } + if td.Name == target { + return true + } + seen[td.Name] = true + for _, f := range td.Fields { + if f.Embedded && walk(f.Type) { + return true + } + } + return false + } + return walk(goType) +} + +// opaqueEmbeds returns the marshaler embeds whose wire keys cannot be listed +// statically (unions): the unmarshaler prunes the catch-all with the keys the +// decoded value actually marshals instead. +func opaqueEmbeds(pkg *ir.Package, td *ir.TypeDef) []*ir.Field { + byName := typeIndex(pkg).byName + var embeds []*ir.Field + for _, f := range marshalerEmbeds(pkg, td) { + if ir.StructNamed(byName, strings.TrimPrefix(f.Type, "*")) == nil { embeds = append(embeds, f) } } @@ -373,7 +458,7 @@ type EmbeddedCatchAll struct { // embedded types, which the enclosing type's UnmarshalJSON has to clean up // after the embedded unmarshalers have run. func embeddedCatchAlls(pkg *ir.Package, td *ir.TypeDef) []EmbeddedCatchAll { - byName := ir.TypesByName(pkg.Types) + byName := typeIndex(pkg).byName var found []EmbeddedCatchAll seen := map[string]bool{td.Name: true} @@ -383,7 +468,7 @@ func embeddedCatchAlls(pkg *ir.Package, td *ir.TypeDef) []EmbeddedCatchAll { if !f.Embedded { continue } - embedded := ir.StructNamed(byName, strings.TrimPrefix(f.Type, "*")) + embedded := ir.StructNamed(byName, fieldGoName(f)) if embedded == nil || seen[embedded.Name] { continue } diff --git a/internal/templates/types.go.tmpl b/internal/templates/types.go.tmpl index d4dba0f..6683bbf 100644 --- a/internal/templates/types.go.tmpl +++ b/internal/templates/types.go.tmpl @@ -155,7 +155,7 @@ func encodeObject(members []jsonMember) ([]byte, error) { } extra[key] = raw } - deleteDeclaredProperties(extra, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) + deleteDeclaredProperties(extra, {{ declaredNamesLiteral $ $td }}) if len(extra) > 0 { encoded, err := json.Marshal(extra) if err != nil { @@ -175,7 +175,8 @@ func encodeObject(members []jsonMember) ([]byte, error) { {{ else }}// UnmarshalJSON implements json.Unmarshaler for {{ $typeName }}, collecting // properties the schema does not declare into {{ $catchAll.Name }}. {{ end }}func (t *{{ $typeName }}) UnmarshalJSON(data []byte) error { -{{ if $embeds }}{{ range $embeds }} if err := json.Unmarshal(data, &t.{{ fieldGoName . }}); err != nil { +{{ with $catchAll }} t.{{ .Name }} = nil +{{ end }}{{ if $embeds }}{{ range decodedEmbeds $ $td }} if err := json.Unmarshal(data, &t.{{ fieldGoName . }}); err != nil { return err } {{ end }}{{ if $plain }} own := struct { @@ -192,13 +193,24 @@ func encodeObject(members []jsonMember) ([]byte, error) { if err := json.Unmarshal(data, (*shadow)(t)); err != nil { return err } -{{ end }}{{ with $catchAll }} t.{{ .Name }} = nil - var obj map[string]json.RawMessage +{{ end }}{{ with $catchAll }} var obj map[string]json.RawMessage if err := json.Unmarshal(data, &obj); err != nil { return err } - deleteDeclaredProperties(obj, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) - if len(obj) > 0 { + deleteDeclaredProperties(obj, {{ declaredNamesLiteral $ $td }}) +{{ range opaqueEmbeds $ $td }} // The embedded union's keys depend on its decoded variant, so the catch-all + // is pruned with the keys the value actually marshals. + if part, err := json.Marshal(t.{{ fieldGoName . }}); err == nil { + var partObj map[string]json.RawMessage + if json.Unmarshal(part, &partObj) == nil { + keys := make([]string, 0, len(partObj)) + for key := range partObj { + keys = append(keys, key) + } + deleteDeclaredProperties(obj, keys) + } + } +{{ end }} if len(obj) > 0 { t.{{ .Name }} = make({{ .Type }}, len(obj)) for key, raw := range obj { var value {{ catchAllValueType . }} @@ -214,18 +226,25 @@ func encodeObject(members []jsonMember) ([]byte, error) { t.{{ .Name }} = nil } } -{{ end }}{{ if $embeds }}{{ with embeddedCatchAlls $ $td }}{{ if $catchAll }} // The embedded unmarshalers ran on the full object, so their catch-alls hold - // this type's declared fields and the extras collected above; clear them. +{{ end }}{{ if $embeds }}{{ with embeddedCatchAlls $ $td }} // The embedded unmarshalers ran on the full object, so their catch-alls also + // collected this type's declared fields{{ if $catchAll }} and the extras gathered above; an + // off-type extra a narrower catch-all rejected stays with the embed that took it{{ end }}. {{ range . }}{{ if .Guard }} if {{ .Guard }} { - t.{{ .Path }} = nil + deleteDeclaredProperties(t.{{ .Path }}, {{ declaredNamesLiteral $ $td }}) +{{ if $catchAll }} for key := range t.{{ $catchAll.Name }} { + delete(t.{{ .Path }}, key) + } + if len(t.{{ .Path }}) == 0 { + t.{{ .Path }} = nil + } +{{ end }} } +{{ else }} deleteDeclaredProperties(t.{{ .Path }}, {{ declaredNamesLiteral $ $td }}) +{{ if $catchAll }} for key := range t.{{ $catchAll.Name }} { + delete(t.{{ .Path }}, key) } -{{ else }} t.{{ .Path }} = nil -{{ end }}{{ end }}{{ else }} // The embedded unmarshalers ran on the full object, so their catch-alls also - // collected this type's declared fields; only true extras belong there. -{{ range . }}{{ if .Guard }} if {{ .Guard }} { - deleteDeclaredProperties(t.{{ .Path }}, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) + if len(t.{{ .Path }}) == 0 { + t.{{ .Path }} = nil } -{{ else }} deleteDeclaredProperties(t.{{ .Path }}, []string{ {{ range $i, $n := declaredJSONNames $ $td }}{{ if $i }}, {{ end }}{{ printf "%q" $n }}{{ end }} }) {{ end }}{{ end }}{{ end }}{{ end }}{{ end }} return nil } {{ end }}