From bb50766c0b72bdb0a3781c13ca86c5076de498b3 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 28 Oct 2026 15:03:00 +0100 Subject: [PATCH] structs: share per-type struct field info structdiff and structwalk re-parsed struct tags for every visited field, and each package kept its own copy of the field rules (flattened embeds, json:"-", EmbeddedSlice, ForceSendFields). structaccess.GetStructInfo now computes them once per type for structdiff, structwalk and structaccess. No behavior change. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 10 +- libs/structs/structaccess/structinfo.go | 102 +++++++++++++++++++ libs/structs/structaccess/structinfo_test.go | 73 +++++++++++++ libs/structs/structdiff/diff.go | 92 ++++------------- libs/structs/structdiff/equal.go | 50 +++------ libs/structs/structwalk/walk.go | 65 +++--------- 6 files changed, 223 insertions(+), 169 deletions(-) create mode 100644 libs/structs/structaccess/structinfo.go create mode 100644 libs/structs/structaccess/structinfo_test.go diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 9bd43bf4603..c2aa100a452 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -327,13 +327,11 @@ func forceSendFields(owner reflect.Value) reflect.Value { if !owner.IsValid() || owner.Kind() != reflect.Struct { return reflect.Value{} } - for i := range owner.Type().NumField() { - field := owner.Type().Field(i) - if field.Name == "ForceSendFields" && !field.Anonymous { - return owner.Field(i) - } + index := GetStructInfo(owner.Type()).ForceSendFieldsIndex + if index < 0 { + return reflect.Value{} } - return reflect.Value{} + return owner.Field(index) } // forceSendFieldsContains reports whether a struct forces the named field to be sent. diff --git a/libs/structs/structaccess/structinfo.go b/libs/structs/structaccess/structinfo.go new file mode 100644 index 00000000000..78a3fa0621a --- /dev/null +++ b/libs/structs/structaccess/structinfo.go @@ -0,0 +1,102 @@ +package structaccess + +import ( + "reflect" + "sync" + + "github.com/databricks/cli/libs/structs/structtag" +) + +const forceSendFieldsName = "ForceSendFields" + +// Field describes one struct field the way encoding/json and the structs packages see it. +type Field struct { + // Index is the field's index for reflect.Value.Field. + Index int + + StructField reflect.StructField + + // Name is the JSON key: the json tag name, or the Go field name when the tag has none. + Name string + + OmitEmpty bool + + // Flattened marks an anonymous struct that encoding/json promotes into the parent, so its + // fields appear at the parent's path. An anonymous field carrying a json name is not one of + // these: encoding/json serializes it as a nested object, so it is a named field. + Flattened bool + + // EmbeddedSlice marks the EmbeddedSliceFieldName field, whose elements appear at the parent's path. + EmbeddedSlice bool +} + +// StructInfo is the per-type view of a struct's fields, computed once per type. +type StructInfo struct { + // Fields lists the exported fields in declaration order, except ForceSendFields and + // fields tagged exactly `json:"-"`. + Fields []Field + + // ForceSendFieldsIndex is the index of the ForceSendFields []string the struct declares + // itself, or -1. A ForceSendFields promoted from an embedded struct only governs that + // struct's fields: the SDK marshaller rejects names of fields its struct does not have. + ForceSendFieldsIndex int +} + +// Keyed by reflect.Type; struct walkers visit the same types many times. +var structInfoCache sync.Map + +// GetStructInfo returns the StructInfo of struct type t. +func GetStructInfo(t reflect.Type) *StructInfo { + if cached, ok := structInfoCache.Load(t); ok { + return cached.(*StructInfo) + } + info := newStructInfo(t) + structInfoCache.Store(t, info) + return info +} + +func newStructInfo(t reflect.Type) *StructInfo { + info := &StructInfo{ForceSendFieldsIndex: -1} + for i := range t.NumField() { + sf := t.Field(i) + if sf.Name == forceSendFieldsName { + if !sf.Anonymous && sf.Type == reflect.TypeFor[[]string]() { + info.ForceSendFieldsIndex = i + } + continue + } + if !sf.IsExported() { + continue + } + if IsFlattenedEmbed(sf) { + info.Fields = append(info.Fields, Field{Index: i, StructField: sf, Flattened: true}) + continue + } + if IsSkippedField(sf) { + continue + } + + jsonTag := structtag.JSONTag(sf.Tag.Get("json")) + name := jsonTag.Name() + if name == "" { + name = sf.Name + } + info.Fields = append(info.Fields, Field{ + Index: i, + StructField: sf, + Name: name, + OmitEmpty: jsonTag.OmitEmpty(), + EmbeddedSlice: sf.Name == EmbeddedSliceFieldName, + }) + } + return info +} + +// ForceSendFields returns the ForceSendFields that struct v declares itself, or nil. +func (info *StructInfo) ForceSendFields(v reflect.Value) []string { + if info.ForceSendFieldsIndex < 0 { + return nil + } + result, _ := reflect.TypeAssert[[]string](v.Field(info.ForceSendFieldsIndex)) + return result +} diff --git a/libs/structs/structaccess/structinfo_test.go b/libs/structs/structaccess/structinfo_test.go new file mode 100644 index 00000000000..3844d61d7b9 --- /dev/null +++ b/libs/structs/structaccess/structinfo_test.go @@ -0,0 +1,73 @@ +package structaccess + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +type InfoInner struct { + A string `json:"a,omitempty"` + ForceSendFields []string `json:"-"` +} + +type InfoTagged struct { + B string `json:"b"` +} + +type infoOuter struct { + InfoInner + InfoTagged `json:"tagged"` + Named string `json:"named,omitempty"` + NoTag int + Skipped string `json:"-"` + Dash string `json:"-,omitempty"` //nolint:staticcheck // the field is named "-" + EmbeddedSlice []string `json:"items"` + unexported string //nolint:unused + ForceSendFields []string `json:"-"` +} + +type infoPromoted struct { + InfoInner + Own string `json:"own,omitempty"` +} + +func TestGetStructInfo(t *testing.T) { + info := GetStructInfo(reflect.TypeFor[infoOuter]()) + + type summary struct { + Index int + Name string + OmitEmpty bool + Flattened bool + EmbeddedSlice bool + } + var got []summary + for _, f := range info.Fields { + got = append(got, summary{f.Index, f.Name, f.OmitEmpty, f.Flattened, f.EmbeddedSlice}) + } + assert.Equal(t, []summary{ + {Index: 0, Flattened: true}, + {Index: 1, Name: "tagged"}, + {Index: 2, Name: "named", OmitEmpty: true}, + {Index: 3, Name: "NoTag"}, + {Index: 5, Name: "-", OmitEmpty: true}, + {Index: 6, Name: "items", EmbeddedSlice: true}, + }, got) + assert.Equal(t, 8, info.ForceSendFieldsIndex) + + v := reflect.ValueOf(infoOuter{ForceSendFields: []string{"Named"}}) + assert.Equal(t, []string{"Named"}, info.ForceSendFields(v)) + + assert.Same(t, info, GetStructInfo(reflect.TypeFor[infoOuter]())) +} + +// A ForceSendFields promoted from an embedded struct governs only that struct's fields. +func TestGetStructInfoPromotedForceSendFields(t *testing.T) { + info := GetStructInfo(reflect.TypeFor[infoPromoted]()) + assert.Equal(t, -1, info.ForceSendFieldsIndex) + + v := reflect.ValueOf(infoPromoted{ForceSendFields: []string{"Own"}}) + assert.Nil(t, info.ForceSendFields(v)) +} diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index 244321f3a66..423d55b14a1 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -9,7 +9,6 @@ import ( "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structpath" - "github.com/databricks/cli/libs/structs/structtag" ) type Change struct { @@ -218,69 +217,35 @@ func equalLeaf(v1, v2 reflect.Value) bool { } func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Value, changes *[]Change) error { - t := s1.Type() - forced1 := getForceSendFields(s1) - forced2 := getForceSendFields(s2) + info := structaccess.GetStructInfo(s1.Type()) + forced1 := info.ForceSendFields(s1) + forced2 := info.ForceSendFields(s2) - for i := range t.NumField() { - sf := t.Field(i) - if !sf.IsExported() || sf.Name == "ForceSendFields" { - continue - } + for _, f := range info.Fields { + v1Field := s1.Field(f.Index) + v2Field := s2.Field(f.Index) - // Continue traversing embedded structs. Do not add the key to the path though. An - // anonymous field carrying a json name is not one of these: encoding/json serializes it - // as a nested object, so it is handled as a named field below. - if structaccess.IsFlattenedEmbed(sf) { - if err := diffValues(ctx, path, s1.Field(i), s2.Field(i), changes); err != nil { + // Continue traversing embedded structs. Do not add the key to the path though. + if f.Flattened { + if err := diffValues(ctx, path, v1Field, v2Field, changes); err != nil { return err } continue } - // Only the exact tag json:"-" skips a field; json:"-," names it "-". - if structaccess.IsSkippedField(sf) { - continue - } - - jsonTag := structtag.JSONTag(sf.Tag.Get("json")) - - // Resolve field name from JSON tag or fall back to Go field name - fieldName := jsonTag.Name() - - isEmbed := sf.Name == structaccess.EmbeddedSliceFieldName - - if fieldName == "" || isEmbed { - fieldName = sf.Name - } - - v1Field := s1.Field(i) - v2Field := s2.Field(i) - - zero1 := v1Field.IsZero() - zero2 := v2Field.IsZero() - - if zero1 || zero2 { - if jsonTag.OmitEmpty() { - if zero1 { - if !slices.Contains(forced1, sf.Name) { - v1Field = reflect.ValueOf(nil) - } - } - if zero2 { - if !slices.Contains(forced2, sf.Name) { - v2Field = reflect.ValueOf(nil) - } - } + if f.OmitEmpty { + if v1Field.IsZero() && !slices.Contains(forced1, f.StructField.Name) { + v1Field = reflect.ValueOf(nil) + } + if v2Field.IsZero() && !slices.Contains(forced2, f.StructField.Name) { + v2Field = reflect.ValueOf(nil) } } // EmbeddedSlice: diff at parent path level without adding field name. - var node *structpath.PathNode - if isEmbed { - node = path - } else { - node = structpath.NewDotString(path, fieldName) + node := path + if !f.EmbeddedSlice { + node = structpath.NewDotString(path, f.Name) } if err := diffValues(ctx, node, v1Field, v2Field, changes); err != nil { @@ -352,27 +317,6 @@ func diffStringMaps(path *structpath.PathNode, m1, m2 map[string]string, changes } } -func getForceSendFields(v reflect.Value) []string { - if !v.IsValid() || v.Kind() != reflect.Struct { - return nil - } - // Only the struct's own ForceSendFields: one promoted from an embedded struct names that - // struct's fields, since the SDK marshaller rejects names its struct does not have. - sf, ok := v.Type().FieldByName("ForceSendFields") - if !ok || len(sf.Index) != 1 { - return nil - } - fsField := v.Field(sf.Index[0]) - if fsField.Kind() != reflect.Slice { - return nil - } - result, ok := reflect.TypeAssert[[]string](fsField) - if ok { - return result - } - return nil -} - // findKeyFunc returns the KeyFunc for the given path, or nil if none matches. // Path patterns support [*] to match any slice index. func (ctx *diffContext) findKeyFunc(path *structpath.PathNode) KeyFunc { diff --git a/libs/structs/structdiff/equal.go b/libs/structs/structdiff/equal.go index dd0589d7a7e..b5b99917c2c 100644 --- a/libs/structs/structdiff/equal.go +++ b/libs/structs/structdiff/equal.go @@ -6,7 +6,6 @@ import ( "slices" "github.com/databricks/cli/libs/structs/structaccess" - "github.com/databricks/cli/libs/structs/structtag" ) // IsEqual compares two Go structs and returns true if they are equal. @@ -99,50 +98,29 @@ func equalValues(v1, v2 reflect.Value) bool { } func equalStruct(s1, s2 reflect.Value) bool { - t := s1.Type() - forced1 := getForceSendFields(s1) - forced2 := getForceSendFields(s2) + info := structaccess.GetStructInfo(s1.Type()) + forced1 := info.ForceSendFields(s1) + forced2 := info.ForceSendFields(s2) - for i := range t.NumField() { - sf := t.Field(i) - if !sf.IsExported() || sf.Name == "ForceSendFields" { - continue - } + for _, f := range info.Fields { + v1Field := s1.Field(f.Index) + v2Field := s2.Field(f.Index) // Continue traversing embedded structs. A tagged anonymous field is a named field to // encoding/json, so it goes through the path below. - if structaccess.IsFlattenedEmbed(sf) { - if !equalValues(s1.Field(i), s2.Field(i)) { + if f.Flattened { + if !equalValues(v1Field, v2Field) { return false } continue } - jsonTag := structtag.JSONTag(sf.Tag.Get("json")) - - // Skip fields with json:"-" - if structaccess.IsSkippedField(sf) { - continue - } - - v1Field := s1.Field(i) - v2Field := s2.Field(i) - - zero1 := v1Field.IsZero() - zero2 := v2Field.IsZero() - - if zero1 || zero2 { - if jsonTag.OmitEmpty() { - if zero1 { - if !slices.Contains(forced1, sf.Name) { - v1Field = reflect.ValueOf(nil) - } - } - if zero2 { - if !slices.Contains(forced2, sf.Name) { - v2Field = reflect.ValueOf(nil) - } - } + if f.OmitEmpty { + if v1Field.IsZero() && !slices.Contains(forced1, f.StructField.Name) { + v1Field = reflect.ValueOf(nil) + } + if v2Field.IsZero() && !slices.Contains(forced2, f.StructField.Name) { + v2Field = reflect.ValueOf(nil) } } diff --git a/libs/structs/structwalk/walk.go b/libs/structs/structwalk/walk.go index 1bbb425d1a8..d91bb34b7d7 100644 --- a/libs/structs/structwalk/walk.go +++ b/libs/structs/structwalk/walk.go @@ -7,7 +7,6 @@ import ( "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structpath" - "github.com/databricks/cli/libs/structs/structtag" ) // VisitFunc is invoked for every scalar (int, uint, float, string, bool) field encountered while walking v. @@ -102,72 +101,32 @@ func walkValue(path *structpath.PathNode, val reflect.Value, field *reflect.Stru } func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) { - forced := getForceSendFields(s) + info := structaccess.GetStructInfo(s.Type()) + forced := info.ForceSendFields(s) - st := s.Type() - for i := range st.NumField() { - sf := st.Field(i) - if sf.PkgPath != "" { - continue // unexported - } - // Skip the ForceSendFields slice itself from traversal. - if sf.Name == "ForceSendFields" { - continue - } + for _, f := range info.Fields { + // A copy, so a visitor cannot modify the cached field. + sf := f.StructField + fieldVal := s.Field(f.Index) - // Directly walk into embedded structs without adding the key to the path. An - // anonymous field carrying a json name is not one of these: encoding/json serializes it - // as a nested object, so it is walked as a named field below. - if structaccess.IsFlattenedEmbed(sf) { - walkValue(path, s.Field(i), &sf, visit) + // Directly walk into embedded structs without adding the key to the path. + if f.Flattened { + walkValue(path, fieldVal, &sf, visit) continue } - jsonTag := structtag.JSONTag(sf.Tag.Get("json")) - if structaccess.IsSkippedField(sf) { - continue // encoding/json omits it entirely - } - - // Resolve field name from JSON tag or fall back to Go field name - fieldName := jsonTag.Name() - if fieldName == "" { - fieldName = sf.Name - } - - fieldVal := s.Field(i) // Skip zero values with omitempty unless field is explicitly forced. - if jsonTag.OmitEmpty() && fieldVal.IsZero() && !slices.Contains(forced, sf.Name) { + if f.OmitEmpty && fieldVal.IsZero() && !slices.Contains(forced, sf.Name) { continue } // EmbeddedSlice: walk directly without adding the field name to the path. - if sf.Name == structaccess.EmbeddedSliceFieldName { + if f.EmbeddedSlice { walkValue(path, fieldVal, &sf, visit) continue } - node := structpath.NewDotString(path, fieldName) + node := structpath.NewDotString(path, f.Name) walkValue(node, fieldVal, &sf, visit) } } - -func getForceSendFields(v reflect.Value) []string { - if !v.IsValid() || v.Kind() != reflect.Struct { - return nil - } - // Only the struct's own ForceSendFields: one promoted from an embedded struct names that - // struct's fields, since the SDK marshaller rejects names its struct does not have. - sf, ok := v.Type().FieldByName("ForceSendFields") - if !ok || len(sf.Index) != 1 { - return nil - } - fsField := v.Field(sf.Index[0]) - if fsField.Kind() != reflect.Slice { - return nil - } - result, ok := reflect.TypeAssert[[]string](fsField) - if ok { - return result - } - return nil -}