diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 9a10f45aa5..0f3d3cc1f4 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -228,96 +228,38 @@ func accessKeyValue(v reflect.Value, key, value string, path *structpath.PathNod return reflect.Value{}, &NotFoundError{fmt.Sprintf("%s: no element found with %s=%q", path.String(), key, value)} } -// findFieldInStruct searches for a field by JSON key in a single struct (no embedding). -// Returns: fieldValue, structField, found -func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.StructField, bool) { - t := v.Type() - for i := range t.NumField() { - sf := t.Field(i) - if sf.PkgPath != "" { // unexported - continue - } - if sf.Anonymous { // skip embedded fields - continue - } - - // Read JSON tag using structtag helper - name := structtag.JSONTag(sf.Tag.Get("json")).Name() - if name == "-" { - name = "" - } - - if sf.Name == EmbeddedSliceFieldName { - continue // EmbeddedSlice fields are not accessible by name - } - if name != "" && name == key { - // Skip fields marked as internal or readonly via bundle tag - btag := structtag.BundleTag(sf.Tag.Get("bundle")) - if btag.Internal() || btag.ReadOnly() { - continue - } - return v.Field(i), sf, true - } - } - return reflect.Value{}, reflect.StructField{}, false -} - -// findStructFieldByKey searches exported fields of struct v for a field matching key. -// It matches json tag name (when present and not "-") only. -// It also searches embedded anonymous structs recursively (flattening semantics), which -// FindStructFieldByKeyType does too: a bundle resource embeds a config struct that embeds -// the SDK request struct, so its fields sit two levels down. +// findStructFieldByKey resolves key against the type of v and then navigates v along the +// index chain the resolution produced. +// +// Resolving on the type is what keeps Get, Set and ValidatePattern agreeing with each other +// and with encoding/json: the type decides which of two same-named fields wins, whether the +// name is ambiguous, and by which path the winner is reached. Navigating the value afterwards +// means a nil pointer on that path reads as an absent field, rather than the search falling +// through to a deeper field of the same name that the wire format never carries. +// // Returns: fieldValue, structField, owner (the struct value declaring the field), found func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.StructField, reflect.Value, bool) { - // First pass: direct fields - if fv, sf, found := findFieldInStruct(v, key); found { - return fv, sf, v, true + index, sf, ok := findFieldIndexByKeyType(v.Type(), key) + if !ok { + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } - // Second pass: search embedded anonymous structs (flattening semantics) breadth-first, one - // level of embedding at a time. Not depth-first: encoding/json resolves a name declared at - // two embedding depths in favour of the shallower one, so descending fully into the first - // embed could pick a field three levels down over the same name two levels down in a - // later one -- and then reading or writing the field would not be the field serialized - // under that name. - level := embeddedStructs(v) - for len(level) > 0 { - var next []reflect.Value - for _, fv := range level { - if out, sf, found := findFieldInStruct(fv, key); found { - return out, sf, fv, true + cur := v + var owner reflect.Value + for _, i := range index { + for cur.Kind() == reflect.Pointer { + if cur.IsNil() { + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } - next = append(next, embeddedStructs(fv)...) + cur = cur.Elem() } - level = next - } - - return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false -} - -// embeddedStructs returns the anonymous struct fields of v, dereferenced, skipping any that -// cannot be descended into. -func embeddedStructs(v reflect.Value) []reflect.Value { - var out []reflect.Value - t := v.Type() - for i := range t.NumField() { - if !t.Field(i).Anonymous { - continue - } - fv := v.Field(i) - for fv.Kind() == reflect.Pointer { - if fv.IsNil() { - // Not initialized; can't descend. - break - } - fv = fv.Elem() - } - if fv.Kind() != reflect.Struct { - continue + if cur.Kind() != reflect.Struct { + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } - out = append(out, fv) + owner = cur + cur = cur.Field(i) } - return out + return cur, sf, owner, true } // forceSendFields returns the ForceSendFields slice a struct declares itself. A struct that diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index e736daf877..3fbfc05184 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -857,6 +857,10 @@ func TestSet_ShallowerEmbedWinsAcrossMembers(t *testing.T) { assert.Equal(t, "set", target.Value) assert.Empty(t, target.deeperEmbed.Value) + got, err := structaccess.GetByString(target, "value") + require.NoError(t, err) + assert.Equal(t, "set", got) + require.NoError(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) blob, err := json.Marshal(target) @@ -880,3 +884,394 @@ func TestSet_ShallowerEmbedWins(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{"value":"set"}`, string(blob)) } + +// Two embedded structs declaring one name at the same depth: encoding/json calls that +// ambiguous and omits the field, so there is nothing to read or write either. +type ambiguousA struct { + Value string `json:"value"` +} + +type ambiguousB struct { + Value string `json:"value"` +} + +type ambiguousEmbeds struct { + ambiguousA + ambiguousB //nolint:govet // the repeated json tag is the point: both embeds declare "value" +} + +func TestSet_AmbiguousEmbedIsNotFound(t *testing.T) { + target := &ambiguousEmbeds{} + + require.Error(t, structaccess.SetByString(target, "value", "set")) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + // Which is what json does with it: the name resolves to no field at all. + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) +} + +// A struct embedding a pointer to itself: the search must not walk the same type twice, or a +// key it never finds sends it round forever. +type cyclicEmbed struct { + *cyclicEmbed + Name string `json:"name"` +} + +func TestGet_CyclicEmbedTerminates(t *testing.T) { + target := &cyclicEmbed{Name: "n"} //exhaustruct:ignore + target.cyclicEmbed = target + + got, err := structaccess.GetByString(target, "name") + require.NoError(t, err) + assert.Equal(t, "n", got) + + _, err = structaccess.GetByString(target, "nope") + require.Error(t, err) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "nope")) +} + +// A diamond: two embeds reaching one type, so the name sits at the same depth twice. +// encoding/json omits it, and the search has to see both paths to notice. +type diamondLeaf struct { + Value string `json:"value"` +} + +type diamondLeft struct { + diamondLeaf +} + +type diamondRight struct { + diamondLeaf +} + +type diamondEmbeds struct { + diamondLeft + diamondRight +} + +func TestSet_DiamondEmbedIsAmbiguous(t *testing.T) { + target := &diamondEmbeds{} + + require.Error(t, structaccess.SetByString(target, "value", "set")) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) +} + +// Whether a name is ambiguous is a property of the type: two embedded pointers declaring it at +// the same depth make it one encoding/json omits, and that must not change with whether one of +// them happens to be nil right now. +type ambiguousPtrEmbeds struct { + *ambiguousA + *ambiguousB //nolint:govet // the repeated json tag is the point: both embeds declare "value" +} + +func TestSet_AmbiguousPointerEmbedsIgnoreNilness(t *testing.T) { + // One embed present, the other nil: the name is still ambiguous. + target := &ambiguousPtrEmbeds{ambiguousA: &ambiguousA{}} //exhaustruct:ignore + require.Error(t, structaccess.SetByString(target, "value", "set")) + _, err := structaccess.GetByString(target, "value") + require.Error(t, err) + + // And with both present, unchanged. + target = &ambiguousPtrEmbeds{ambiguousA: &ambiguousA{}, ambiguousB: &ambiguousB{}} + require.Error(t, structaccess.SetByString(target, "value", "set")) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) +} + +// A name declared behind a nil embedded pointer and again deeper down. encoding/json resolves +// it to the shallower declaration and then serializes nothing, because the pointer is nil -- +// so the deeper field, which the wire format never carries, is not the answer either. +type shallowLeaf struct { + Value string `json:"value,omitempty"` +} + +type deepHolder struct { + shallowLeaf +} + +type shallowBehindNil struct { + *shallowLeaf + deepHolder +} + +func TestGet_ShallowFieldBehindNilPointerIsAbsent(t *testing.T) { + target := &shallowBehindNil{deepHolder: deepHolder{shallowLeaf: shallowLeaf{Value: "deep"}}} //exhaustruct:ignore + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) + + _, err = structaccess.GetByString(target, "value") + require.Error(t, err, "the field encoding/json resolves to is absent, so there is nothing to read") +} + +// An anonymous field carrying a json name is a named field to encoding/json: it serializes as +// a nested object under that name rather than being flattened into the outer one. +type TaggedEmbedLeaf struct { + Value string `json:"value,omitempty"` +} + +type taggedEmbed struct { + TaggedEmbedLeaf `json:"leaf"` + + Own string `json:"own,omitempty"` +} + +func TestGetSet_TaggedEmbedIsANamedField(t *testing.T) { + target := &taggedEmbed{TaggedEmbedLeaf: TaggedEmbedLeaf{Value: "v"}, Own: "o"} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"v"},"own":"o"}`, string(blob)) + + // Not flattened: the outer object has no "value" member. + _, err = structaccess.GetByString(target, "value") + require.Error(t, err) + + value, err := structaccess.GetByString(target, "leaf.value") + require.NoError(t, err) + assert.Equal(t, "v", value) + + require.NoError(t, structaccess.SetByString(target, "leaf.value", "set")) + blob, err = json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"set"},"own":"o"}`, string(blob)) +} + +// At one depth, encoding/json prefers a field whose json tag names it over one that only has +// the matching Go field name, instead of calling the pair ambiguous. +type untaggedX struct { + X string +} + +type taggedAsX struct { + Y string `json:"X"` +} + +type taggedBeatsUntagged struct { + untaggedX + taggedAsX +} + +func TestGet_TaggedNameBeatsUntaggedAtTheSameDepth(t *testing.T) { + target := &taggedBeatsUntagged{untaggedX: untaggedX{X: "untagged"}, taggedAsX: taggedAsX{Y: "tagged"}} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"X":"tagged"}`, string(blob)) + + value, err := structaccess.GetByString(target, "X") + require.NoError(t, err) + assert.Equal(t, "tagged", value, "must resolve to the field encoding/json serializes") +} + +// A field whose tag sets only an option has no json name, so encoding/json serializes it under +// its Go field name and that is the name it has to be reachable by. +type optionOnlyTag struct { + Count int `json:"count,omitempty"` + Total int `json:",omitempty"` +} + +func TestGetSet_FieldWithoutATagNameUsesItsGoName(t *testing.T) { + target := &optionOnlyTag{Count: 1, Total: 2} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"count":1,"Total":2}`, string(blob)) + + value, err := structaccess.GetByString(target, "Total") + require.NoError(t, err) + assert.Equal(t, 2, value) + + require.NoError(t, structaccess.SetByString(target, "Total", 7)) + assert.Equal(t, 7, target.Total) +} + +// An anonymous field that is not a struct is not promoted: encoding/json serializes it as a +// member named after its type. +type EmbeddedName string + +type embeddedScalar struct { + EmbeddedName + + Own string `json:"own,omitempty"` +} + +func TestGet_AnonymousNonStructIsANamedMember(t *testing.T) { + target := &embeddedScalar{EmbeddedName: "n", Own: "o"} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"EmbeddedName":"n","own":"o"}`, string(blob)) + + value, err := structaccess.GetByString(target, "EmbeddedName") + require.NoError(t, err) + assert.Equal(t, EmbeddedName("n"), value) +} + +// The same embedded type reached by two routes: encoding/json descends into it once, so a name +// declared *below* it is not ambiguous, while a name the duplicated type declares itself is. +type repeatedLeaf struct { + Value string `json:"value,omitempty"` +} + +type repeatedMiddle struct { + repeatedLeaf +} + +type repeatedLeft struct { + repeatedMiddle +} + +type repeatedRight struct { + repeatedMiddle +} + +type repeatedEmbed struct { + repeatedLeft + repeatedRight +} + +func TestGet_TypeReachedTwiceIsNotAmbiguousBelowIt(t *testing.T) { + target := &repeatedEmbed{ + repeatedLeft: repeatedLeft{repeatedMiddle: repeatedMiddle{repeatedLeaf: repeatedLeaf{Value: "left"}}}, + repeatedRight: repeatedRight{repeatedMiddle: repeatedMiddle{repeatedLeaf: repeatedLeaf{Value: "right"}}}, + } + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"value":"left"}`, string(blob), "encoding/json takes the first route") + + value, err := structaccess.GetByString(target, "value") + require.NoError(t, err) + assert.Equal(t, "left", value) +} + +// Combinations of a repeated embedded type with tagged and untagged declarations of one name. +// encoding/json is the oracle for each: the assertion compares what it serializes under the +// name with what Get resolves, so the pair cannot drift. +type matrixTagged struct { + Y string `json:"x"` +} + +type matrixUntagged struct { + X string +} + +type ( + matrixLeftTagged struct{ matrixTagged } + matrixRightTagged struct{ matrixTagged } + matrixLeftUntagged struct{ matrixUntagged } + matrixRightUntagged struct{ matrixUntagged } +) + +// The repeated type declares the name itself: two routes, so encoding/json annihilates it. +type matrixRepeatDeclares struct { + matrixLeftTagged + matrixRightTagged +} + +// The repeated type's tagged name is annihilated, leaving a sibling's untagged X under "X". +type matrixRepeatTaggedPlusUntagged struct { + matrixLeftTagged + matrixRightTagged + matrixUntagged +} + +// The repeated type's untagged name never collides with "x"; the sibling's tagged one wins. +type matrixRepeatUntaggedPlusTagged struct { + matrixLeftUntagged + matrixRightUntagged + matrixTagged +} + +type ( + matrixDeepHolder struct{ matrixTagged } + matrixDeeperRoute struct{ matrixDeepHolder } +) + +// One route reaches the declaring type a level earlier than the other: the shallower wins. +type matrixMixedDepth struct { + matrixTagged + matrixDeeperRoute +} + +func TestGet_RepeatedEmbedMatrixMatchesEncodingJSON(t *testing.T) { + repeatDeclares := &matrixRepeatDeclares{} + repeatDeclares.matrixLeftTagged.Y = "L" + repeatDeclares.matrixRightTagged.Y = "R" + + taggedPlusUntagged := &matrixRepeatTaggedPlusUntagged{} + taggedPlusUntagged.matrixLeftTagged.Y = "L" + taggedPlusUntagged.matrixRightTagged.Y = "R" + taggedPlusUntagged.X = "U" + + untaggedPlusTagged := &matrixRepeatUntaggedPlusTagged{} + untaggedPlusTagged.matrixLeftUntagged.X = "L" + untaggedPlusTagged.matrixRightUntagged.X = "R" + untaggedPlusTagged.Y = "T" + + mixedDepth := &matrixMixedDepth{} + mixedDepth.Y = "shallow" + mixedDepth.Y = "deep" + + for _, tc := range []struct { + name string + value any + }{ + {"repeated type declares the name", repeatDeclares}, + {"repeated tagged plus untagged sibling", taggedPlusUntagged}, + {"repeated untagged plus tagged sibling", untaggedPlusTagged}, + {"one route shallower than the other", mixedDepth}, + } { + t.Run(tc.name, func(t *testing.T) { + blob, err := json.Marshal(tc.value) + require.NoError(t, err) + var emitted map[string]any + require.NoError(t, json.Unmarshal(blob, &emitted)) + + want, onTheWire := emitted["x"] + got, err := structaccess.GetByString(tc.value, "x") + + if !onTheWire { + require.Error(t, err, "encoding/json emitted %s, so there is no x to read", blob) + return + } + require.NoError(t, err) + assert.Equal(t, want, got, "encoding/json emitted %s", blob) + }) + } +} + +// Only the exact tag json:"-" omits a field. A tag whose name part is "-" followed by options +// names the field "-", which encoding/json serializes like any other name. +type dashNamed struct { + Skipped string `json:"-"` + Named string `json:"-,omitempty"` //nolint:staticcheck // the odd tag is the point + Kept string `json:"kept,omitempty"` +} + +func TestGetSet_DashIsAFieldNameWhenTheTagHasOptions(t *testing.T) { + target := &dashNamed{Skipped: "s", Named: "n", Kept: "k"} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"-":"n","kept":"k"}`, string(blob)) + + value, err := structaccess.GetByString(target, "-") + require.NoError(t, err) + assert.Equal(t, "n", value) + + require.NoError(t, structaccess.SetByString(target, "-", "set")) + assert.Equal(t, "set", target.Named) + assert.Equal(t, "s", target.Skipped, "the json:\"-\" field stays out of reach") +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index b55fa4c136..eb74e4c792 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -142,72 +142,238 @@ func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { // It also searches embedded anonymous structs (pointer or value) recursively. // Returns the StructField, the declaring owner type, and whether it was found. func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, reflect.Type, bool) { - if t.Kind() != reflect.Struct { + index, sf, ok := findFieldIndexByKeyType(t, key) + if !ok { return reflect.StructField{}, reflect.TypeOf(nil), false } + return sf, ownerTypeAt(t, index), true +} - // First pass: direct fields - if sf, ok := findDirectFieldByKeyType(t, key); ok { - return sf, t, true +// findFieldIndexByKeyType resolves key to a field of t and returns the chain of field indices +// leading to it, the way reflect.Type.FieldByName does. +// +// Embedded structs are searched breadth-first, mirroring encoding/json: a name declared at +// two embedding depths resolves to the shallower one, so a depth-first search could pick a +// field the wire format does not use. A name declared twice at one depth is ambiguous, which +// encoding/json resolves by serializing neither, so it resolves to nothing here too. +// +// Returning the index chain rather than a type matters: the same struct type can be reachable +// by more than one path, so a caller navigating a value needs the path json would take, not +// merely the type at the end of it. +func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructField, bool) { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return nil, reflect.StructField{}, false } - // Second pass: search embedded anonymous structs breadth-first, mirroring findStructFieldByKey - // (get.go) so a path validates against the same field Get and Set resolve it to, which is - // the one encoding/json serializes: the shallower of two same-named fields. - level := embeddedStructTypes(t) + if c, ok := pickCandidate(directCandidates(t, key, nil)); ok { + return c.index, c.field, true + } + + // A cycle must not be walked twice, or a key the type never declares sends the search + // round forever. + seen := map[reflect.Type]bool{t: true} + level := dedupeByType(embeddedIndexPaths(t, nil)) for len(level) > 0 { - var next []reflect.Type - for _, ft := range level { - if sf, ok := findDirectFieldByKeyType(ft, key); ok { - return sf, ft, true + var next []embeddedPath + var found []candidate + for _, embed := range level { + matches := directCandidates(embed.typ, key, embed.index) + if len(matches) > 0 { + found = append(found, matches...) + if embed.reached > 1 { + // Several members of the previous level reach this type, so encoding/json sees + // the names it declares once per route and annihilates them. One extra match is + // enough to make the name ambiguous below. + found = append(found, matches[0]) + } + continue + } + for _, deeper := range embeddedIndexPaths(embed.typ, embed.index) { + if seen[deeper.typ] { + continue + } + next = append(next, deeper) } - next = append(next, embeddedStructTypes(ft)...) } - level = next + level = dedupeByType(next) + for _, embed := range level { + seen[embed.typ] = true + } + if len(found) > 0 { + if c, ok := pickCandidate(found); ok { + return c.index, c.field, true + } + return nil, reflect.StructField{}, false + } + } + + return nil, reflect.StructField{}, false +} + +// embeddedPath is an embedded struct type together with the index chain that reaches it, and +// how many members of the previous level reach it. +type embeddedPath struct { + typ reflect.Type + index []int + reached int +} + +// dedupeByType collapses repeated embeds of one type into a single entry, counting how many +// routes reached it. encoding/json descends into a type once per level however many members +// embed it, so a name declared *below* a type reached twice is not ambiguous; a name the +// duplicated type declares itself is, and the count records that. +func dedupeByType(paths []embeddedPath) []embeddedPath { + var out []embeddedPath + index := map[reflect.Type]int{} + for _, path := range paths { + if at, ok := index[path.typ]; ok { + out[at].reached++ + continue + } + index[path.typ] = len(out) + path.reached = 1 + out = append(out, path) + } + return out +} + +// embeddedIndexPaths returns the embeds of t that encoding/json flattens, each with the index +// chain from the root that reaches it. +func embeddedIndexPaths(t reflect.Type, prefix []int) []embeddedPath { + var out []embeddedPath + for i := range t.NumField() { + sf := t.Field(i) + if !isFlattenedEmbed(sf) { + continue + } + ft := sf.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + if ft.Kind() != reflect.Struct { + continue + } + out = append(out, embeddedPath{typ: ft, index: append(append([]int{}, prefix...), i)}) + } + return out +} + +// ownerTypeAt returns the struct type that declares the field the index chain ends at. +func ownerTypeAt(t reflect.Type, index []int) reflect.Type { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + for _, i := range index[:len(index)-1] { + t = t.Field(i).Type + for t.Kind() == reflect.Pointer { + t = t.Elem() + } } + return t +} - return reflect.StructField{}, reflect.TypeOf(nil), false +// candidate is a field that matches a json name, with the index chain reaching it and whether +// the name came from a json tag. encoding/json prefers a tagged name over an untagged one at +// the same depth, so the distinction has to survive the search. +type candidate struct { + index []int + field reflect.StructField + tagged bool } -// findDirectFieldByKeyType matches key against the struct's own fields, by json tag name. -func findDirectFieldByKeyType(t reflect.Type, key string) (reflect.StructField, bool) { - for sf := range t.Fields() { +// directCandidates returns the struct's own fields that key can name. A field with a json tag +// name is matched on that; a field without one is matched on its Go field name, which is what +// encoding/json serializes it under. An embed that encoding/json flattens is not addressable by +// name at all, so it is not a candidate. +func directCandidates(t reflect.Type, key string, prefix []int) []candidate { + var out []candidate + for i := range t.NumField() { + sf := t.Field(i) if sf.PkgPath != "" { // unexported continue } - name := structtag.JSONTag(sf.Tag.Get("json")).Name() - if name == "-" || sf.Name == EmbeddedSliceFieldName { + if sf.Name == EmbeddedSliceFieldName || IsFlattenedEmbed(sf) { + continue + } + if IsSkippedField(sf) { continue } + name := structtag.JSONTag(sf.Tag.Get("json")).Name() + tagged := name != "" + if !tagged { + name = sf.Name + } if name != key { continue } - // Skip fields marked as internal/readonly + // Skip fields marked as internal/readonly. + // + // Known divergence from encoding/json: such a field still shadows a same-named field + // further down, so dropping it here lets the deeper one win and a caller can reach a + // field the wire format does not carry. resources.App is the live example -- its + // BaseResource.URL is internal and shadows the SDK's url, which json serializes as the + // internal one. Rejecting the name outright instead would make + // ${resources.apps.*.url} unresolvable, so which of the two is right is a decision + // about what internal means, not a detail of the search. btag := structtag.BundleTag(sf.Tag.Get("bundle")) if btag.Internal() || btag.ReadOnly() { continue } - return sf, true + out = append(out, candidate{ + index: append(append([]int{}, prefix...), i), + field: sf, + tagged: tagged, + }) } - return reflect.StructField{}, false + return out } -// embeddedStructTypes returns the anonymous struct fields of t, dereferenced. -func embeddedStructTypes(t reflect.Type) []reflect.Type { - var out []reflect.Type - for sf := range t.Fields() { - if !sf.Anonymous { - continue - } - ft := sf.Type - for ft.Kind() == reflect.Pointer { - ft = ft.Elem() - } - if ft.Kind() == reflect.Struct { - out = append(out, ft) +// pickCandidate applies encoding/json's precedence among fields that share a name at one +// depth: a single tagged name wins over untagged ones, a single match of either kind wins, and +// anything else is ambiguous and serialized as nothing. +func pickCandidate(candidates []candidate) (candidate, bool) { + if len(candidates) == 1 { + return candidates[0], true + } + var tagged []candidate + for _, c := range candidates { + if c.tagged { + tagged = append(tagged, c) } } - return out + if len(tagged) == 1 { + return tagged[0], true + } + return candidate{}, false +} + +// IsSkippedField reports whether encoding/json omits the field entirely. Only the exact tag +// `json:"-"` does that: `json:"-,"` and `json:"-,omitempty"` name the field "-", which is a +// distinction structtag's parsed name alone cannot carry, since it reports "-" for both. +func IsSkippedField(sf reflect.StructField) bool { + return sf.Tag.Get("json") == "-" +} + +// isFlattenedEmbed reports whether the field is an embed encoding/json flattens into the +// outer object. An anonymous field that carries a json name is a named field instead: it +// serializes as a nested object under that name. +func isFlattenedEmbed(sf reflect.StructField) bool { + if !sf.Anonymous { + return false + } + if structtag.JSONTag(sf.Tag.Get("json")).Name() != "" { + return false + } + // Only an anonymous *struct* is promoted. An embedded scalar, slice or interface is a member + // named after its type, so it belongs at its own path rather than the parent's. + ft := sf.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + return ft.Kind() == reflect.Struct } // IsFlattenedEmbed reports whether the field is an embed encoding/json flattens into the diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index 5e7a00cac4..7a93acc80b 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -223,7 +223,7 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu // Resolve field name from JSON tag or fall back to Go field name fieldName := jsonTag.Name() - if fieldName == "-" { + if structaccess.IsSkippedField(sf) { continue } diff --git a/libs/structs/structdiff/equal.go b/libs/structs/structdiff/equal.go index 50e703a97c..014f6f7af3 100644 --- a/libs/structs/structdiff/equal.go +++ b/libs/structs/structdiff/equal.go @@ -120,7 +120,7 @@ func equalStruct(s1, s2 reflect.Value) bool { jsonTag := structtag.JSONTag(sf.Tag.Get("json")) // Skip fields with json:"-" - if jsonTag.Name() == "-" { + if structaccess.IsSkippedField(sf) { continue } diff --git a/libs/structs/structwalk/walk.go b/libs/structs/structwalk/walk.go index 57d07c5138..0a1186a4e8 100644 --- a/libs/structs/structwalk/walk.go +++ b/libs/structs/structwalk/walk.go @@ -124,8 +124,8 @@ func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) { } jsonTag := structtag.JSONTag(sf.Tag.Get("json")) - if jsonTag.Name() == "-" { - continue // skip fields without json name + if structaccess.IsSkippedField(sf) { + continue // encoding/json omits it entirely } // Resolve field name from JSON tag or fall back to Go field name diff --git a/libs/structs/structwalk/walktype.go b/libs/structs/structwalk/walktype.go index bdb58cd164..4a485f2c1e 100644 --- a/libs/structs/structwalk/walktype.go +++ b/libs/structs/structwalk/walktype.go @@ -120,7 +120,7 @@ func walkTypeStruct(path *structpath.PatternNode, st reflect.Type, visit VisitTy // Skip fields marked as "-" in json tag jsonTagName := structtag.JSONTag(jsonTag).Name() - if jsonTagName == "-" { + if structaccess.IsSkippedField(sf) { continue } diff --git a/libs/structs/structwalk/walktype_test.go b/libs/structs/structwalk/walktype_test.go index b3c6b5f877..dc1783cb4c 100644 --- a/libs/structs/structwalk/walktype_test.go +++ b/libs/structs/structwalk/walktype_test.go @@ -55,6 +55,10 @@ func TestTypeScalar(t *testing.T) { func TestTypes(t *testing.T) { assert.Equal(t, map[string]any{ + // IgnoredFieldOdd and IgnoredFieldOddPtr are tagged `json:"-,omitempty"`, which names + // them "-" rather than omitting them: only the exact tag `json:"-"` is a skip. The two + // collide on that one name, so the walk reports it once. + "-": "", "ArrayString[*]": "", "Array[*].X": 0, "BoolField": false,