From b392875dc5aeb6da971fcf3da4b15179dd25d71c Mon Sep 17 00:00:00 2001 From: Sam Thanawalla <17936816+samthanawalla@users.noreply.github.com> Date: Wed, 6 Aug 2025 12:07:57 -0400 Subject: [PATCH] internal/util: move FieldJSONInfo into jsonschema/util (#251) This CL effectively removes any dependencies on mcp from jsonschema. This will enable us to move the repo. This CL also deletes unused marshalStructWithMap and unmarshalStructWithMap which had a dependency on FieldJSONInfo. Fixes: #250 --- internal/util/util.go | 39 ------------- internal/util/util_test.go | 38 ------------ jsonschema/infer.go | 12 ++-- jsonschema/schema.go | 8 +-- jsonschema/util.go | 46 +++++++++++++-- jsonschema/util_test.go | 28 +++++++++ jsonschema/validate.go | 10 ++-- mcp/util.go | 117 ------------------------------------- mcp/util_test.go | 48 --------------- 9 files changed, 82 insertions(+), 264 deletions(-) delete mode 100644 internal/util/util_test.go delete mode 100644 mcp/util_test.go diff --git a/internal/util/util.go b/internal/util/util.go index f8c5baf..4b5c325 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -8,9 +8,7 @@ import ( "cmp" "fmt" "iter" - "reflect" "slices" - "strings" ) // Helpers below are copied from gopls' moremaps package. @@ -38,43 +36,6 @@ func KeySlice[M ~map[K]V, K comparable, V any](m M) []K { return r } -type JSONInfo struct { - Omit bool // unexported or first tag element is "-" - Name string // Go field name or first tag element. Empty if Omit is true. - Settings map[string]bool // "omitempty", "omitzero", etc. -} - -// FieldJSONInfo reports information about how encoding/json -// handles the given struct field. -// If the field is unexported, JSONInfo.Omit is true and no other JSONInfo field -// is populated. -// If the field is exported and has no tag, then Name is the field's name and all -// other fields are false. -// Otherwise, the information is obtained from the tag. -func FieldJSONInfo(f reflect.StructField) JSONInfo { - if !f.IsExported() { - return JSONInfo{Omit: true} - } - info := JSONInfo{Name: f.Name} - if tag, ok := f.Tag.Lookup("json"); ok { - name, rest, found := strings.Cut(tag, ",") - // "-" means omit, but "-," means the name is "-" - if name == "-" && !found { - return JSONInfo{Omit: true} - } - if name != "" { - info.Name = name - } - if len(rest) > 0 { - info.Settings = map[string]bool{} - for _, s := range strings.Split(rest, ",") { - info.Settings[s] = true - } - } - } - return info -} - // Wrapf wraps *errp with the given formatted message if *errp is not nil. func Wrapf(errp *error, format string, args ...any) { if *errp != nil { diff --git a/internal/util/util_test.go b/internal/util/util_test.go deleted file mode 100644 index 6a2b867..0000000 --- a/internal/util/util_test.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2025 The Go MCP SDK Authors. All rights reserved. -// Use of this source code is governed by an MIT-style -// license that can be found in the LICENSE file. - -package util - -import ( - "reflect" - "testing" -) - -func TestJSONInfo(t *testing.T) { - type S struct { - A int - B int `json:","` - C int `json:"-"` - D int `json:"-,"` - E int `json:"echo"` - F int `json:"foxtrot,omitempty"` - g int `json:"golf"` - } - want := []JSONInfo{ - {Name: "A"}, - {Name: "B"}, - {Omit: true}, - {Name: "-"}, - {Name: "echo"}, - {Name: "foxtrot", Settings: map[string]bool{"omitempty": true}}, - {Omit: true}, - } - tt := reflect.TypeFor[S]() - for i := range tt.NumField() { - got := FieldJSONInfo(tt.Field(i)) - if !reflect.DeepEqual(got, want[i]) { - t.Errorf("got %+v, want %+v", got, want[i]) - } - } -} diff --git a/jsonschema/infer.go b/jsonschema/infer.go index 080d9d0..aa01fcd 100644 --- a/jsonschema/infer.go +++ b/jsonschema/infer.go @@ -14,8 +14,6 @@ import ( "reflect" "regexp" "time" - - "github.com/modelcontextprotocol/go-sdk/internal/util" ) // ForOptions are options for the [For] function. @@ -192,8 +190,8 @@ func forType(t reflect.Type, seen map[reflect.Type]bool, ignore bool, schemas ma for i := range t.NumField() { field := t.Field(i) - info := util.FieldJSONInfo(field) - if info.Omit { + info := fieldJSONInfo(field) + if info.omit { continue } if s.Properties == nil { @@ -216,9 +214,9 @@ func forType(t reflect.Type, seen map[reflect.Type]bool, ignore bool, schemas ma } fs.Description = tag } - s.Properties[info.Name] = fs - if !info.Settings["omitempty"] && !info.Settings["omitzero"] { - s.Required = append(s.Required, info.Name) + s.Properties[info.name] = fs + if !info.settings["omitempty"] && !info.settings["omitzero"] { + s.Required = append(s.Required, info.name) } } diff --git a/jsonschema/schema.go b/jsonschema/schema.go index 0b4764d..9a68cd5 100644 --- a/jsonschema/schema.go +++ b/jsonschema/schema.go @@ -15,8 +15,6 @@ import ( "math" "reflect" "slices" - - "github.com/modelcontextprotocol/go-sdk/internal/util" ) // A Schema is a JSON schema object. @@ -424,9 +422,9 @@ var ( func init() { for _, sf := range reflect.VisibleFields(reflect.TypeFor[Schema]()) { - info := util.FieldJSONInfo(sf) - if !info.Omit { - schemaFieldInfos = append(schemaFieldInfos, structFieldInfo{sf, info.Name}) + info := fieldJSONInfo(sf) + if !info.omit { + schemaFieldInfos = append(schemaFieldInfos, structFieldInfo{sf, info.name}) } } slices.SortFunc(schemaFieldInfos, func(i1, i2 structFieldInfo) int { diff --git a/jsonschema/util.go b/jsonschema/util.go index 71c3443..f529153 100644 --- a/jsonschema/util.go +++ b/jsonschema/util.go @@ -15,9 +15,8 @@ import ( "math/big" "reflect" "slices" + "strings" "sync" - - "github.com/modelcontextprotocol/go-sdk/internal/util" ) // Equal reports whether two Go values representing JSON values are equal according @@ -410,11 +409,48 @@ func jsonNames(t reflect.Type) map[string]bool { } continue } - info := util.FieldJSONInfo(field) - if !info.Omit { - m[info.Name] = true + info := fieldJSONInfo(field) + if !info.omit { + m[info.name] = true } } jsonNamesMap.Store(t, m) return m } + +type jsonInfo struct { + omit bool // unexported or first tag element is "-" + name string // Go field name or first tag element. Empty if omit is true. + settings map[string]bool // "omitempty", "omitzero", etc. +} + +// fieldJSONInfo reports information about how encoding/json +// handles the given struct field. +// If the field is unexported, jsonInfo.omit is true and no other jsonInfo field +// is populated. +// If the field is exported and has no tag, then name is the field's name and all +// other fields are false. +// Otherwise, the information is obtained from the tag. +func fieldJSONInfo(f reflect.StructField) jsonInfo { + if !f.IsExported() { + return jsonInfo{omit: true} + } + info := jsonInfo{name: f.Name} + if tag, ok := f.Tag.Lookup("json"); ok { + name, rest, found := strings.Cut(tag, ",") + // "-" means omit, but "-," means the name is "-" + if name == "-" && !found { + return jsonInfo{omit: true} + } + if name != "" { + info.name = name + } + if len(rest) > 0 { + info.settings = map[string]bool{} + for _, s := range strings.Split(rest, ",") { + info.settings[s] = true + } + } + } + return info +} diff --git a/jsonschema/util_test.go b/jsonschema/util_test.go index 03ccb4d..7934bff 100644 --- a/jsonschema/util_test.go +++ b/jsonschema/util_test.go @@ -184,3 +184,31 @@ func TestMarshalStructWithMap(t *testing.T) { } }) } + +func TestJSONInfo(t *testing.T) { + type S struct { + A int + B int `json:","` + C int `json:"-"` + D int `json:"-,"` + E int `json:"echo"` + F int `json:"foxtrot,omitempty"` + g int `json:"golf"` + } + want := []jsonInfo{ + {name: "A"}, + {name: "B"}, + {omit: true}, + {name: "-"}, + {name: "echo"}, + {name: "foxtrot", settings: map[string]bool{"omitempty": true}}, + {omit: true}, + } + tt := reflect.TypeFor[S]() + for i := range tt.NumField() { + got := fieldJSONInfo(tt.Field(i)) + if !reflect.DeepEqual(got, want[i]) { + t.Errorf("got %+v, want %+v", got, want[i]) + } + } +} diff --git a/jsonschema/validate.go b/jsonschema/validate.go index 3b86410..ca4f340 100644 --- a/jsonschema/validate.go +++ b/jsonschema/validate.go @@ -697,8 +697,8 @@ func properties(v reflect.Value) iter.Seq2[string, reflect.Value] { for name, sf := range structPropertiesOf(v.Type()) { val := v.FieldByIndex(sf.Index) if val.IsZero() { - info := util.FieldJSONInfo(sf) - if info.Settings["omitempty"] || info.Settings["omitzero"] { + info := fieldJSONInfo(sf) + if info.settings["omitempty"] || info.settings["omitzero"] { continue } } @@ -750,9 +750,9 @@ func structPropertiesOf(t reflect.Type) propertyMap { } props := map[string]reflect.StructField{} for _, sf := range reflect.VisibleFields(t) { - info := util.FieldJSONInfo(sf) - if !info.Omit { - props[info.Name] = sf + info := fieldJSONInfo(sf) + if !info.omit { + props[info.name] = sf } } structProperties.Store(t, props) diff --git a/mcp/util.go b/mcp/util.go index 82d8794..102c088 100644 --- a/mcp/util.go +++ b/mcp/util.go @@ -6,12 +6,6 @@ package mcp import ( "crypto/rand" - "encoding/json" - "fmt" - "reflect" - "sync" - - "github.com/modelcontextprotocol/go-sdk/internal/util" ) func assert(cond bool, msg string) { @@ -33,114 +27,3 @@ func randText() string { } return string(src) } - -// marshalStructWithMap marshals its first argument to JSON, treating the field named -// mapField as an embedded map. The first argument must be a pointer to -// a struct. The underlying type of mapField must be a map[string]any, and it must have -// an "omitempty" json tag. -// -// For example, given this struct: -// -// type S struct { -// A int -// Extra map[string] any `json:,omitempty` -// } -// -// and this value: -// -// s := S{A: 1, Extra: map[string]any{"B": 2}} -// -// the call marshalJSONWithMap(s, "Extra") would return -// -// {"A": 1, "B": 2} -// -// It is an error if the map contains the same key as another struct field's -// JSON name. -// -// marshalStructWithMap calls json.Marshal on a value of type T, so T must not -// have a MarshalJSON method that calls this function, on pain of infinite regress. -// -// TODO: avoid this restriction on T by forcing it to marshal in a default way. -// See https://go.dev/play/p/EgXKJHxEx_R. -func marshalStructWithMap[T any](s *T, mapField string) ([]byte, error) { - // Marshal the struct and the map separately, and concatenate the bytes. - // This strategy is dramatically less complicated than - // constructing a synthetic struct or map with the combined keys. - if s == nil { - return []byte("null"), nil - } - s2 := *s - vMapField := reflect.ValueOf(&s2).Elem().FieldByName(mapField) - mapVal := vMapField.Interface().(map[string]any) - - // Check for duplicates. - names := jsonNames(reflect.TypeFor[T]()) - for key := range mapVal { - if names[key] { - return nil, fmt.Errorf("map key %q duplicates struct field", key) - } - } - - // Clear the map field, relying on the omitempty tag to omit it. - vMapField.Set(reflect.Zero(vMapField.Type())) - structBytes, err := json.Marshal(s2) - if err != nil { - return nil, fmt.Errorf("marshalStructWithMap(%+v): %w", s, err) - } - if len(mapVal) == 0 { - return structBytes, nil - } - mapBytes, err := json.Marshal(mapVal) - if err != nil { - return nil, err - } - if len(structBytes) == 2 { // must be "{}" - return mapBytes, nil - } - // "{X}" + "{Y}" => "{X,Y}" - res := append(structBytes[:len(structBytes)-1], ',') - res = append(res, mapBytes[1:]...) - return res, nil -} - -// unmarshalStructWithMap is the inverse of marshalStructWithMap. -// T has the same restrictions as in that function. -func unmarshalStructWithMap[T any](data []byte, v *T, mapField string) error { - // Unmarshal into the struct, ignoring unknown fields. - if err := json.Unmarshal(data, v); err != nil { - return err - } - // Unmarshal into the map. - m := map[string]any{} - if err := json.Unmarshal(data, &m); err != nil { - return err - } - // Delete from the map the fields of the struct. - for n := range jsonNames(reflect.TypeFor[T]()) { - delete(m, n) - } - if len(m) != 0 { - reflect.ValueOf(v).Elem().FieldByName(mapField).Set(reflect.ValueOf(m)) - } - return nil -} - -var jsonNamesMap sync.Map // from reflect.Type to map[string]bool - -// jsonNames returns the set of JSON object keys that t will marshal into. -// t must be a struct type. -func jsonNames(t reflect.Type) map[string]bool { - // Lock not necessary: at worst we'll duplicate work. - if val, ok := jsonNamesMap.Load(t); ok { - return val.(map[string]bool) - } - m := map[string]bool{} - for i := range t.NumField() { - info := util.FieldJSONInfo(t.Field(i)) - if !info.Omit { - m[info.Name] = true - } - } - jsonNamesMap.Store(t, m) - return m -} diff --git a/mcp/util_test.go b/mcp/util_test.go deleted file mode 100644 index f2cb0f5..0000000 --- a/mcp/util_test.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2025 The Go MCP SDK Authors. All rights reserved. -// Use of this source code is governed by an MIT-style -// license that can be found in the LICENSE file. - -package mcp - -import ( - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" -) - -func TestMarshalStructWithMap(t *testing.T) { - type S struct { - A int - B string `json:"b,omitempty"` - u bool - M map[string]any `json:",omitempty"` - } - t.Run("basic", func(t *testing.T) { - s := S{A: 1, B: "two", M: map[string]any{"!@#": true}} - got, err := marshalStructWithMap(&s, "M") - if err != nil { - t.Fatal(err) - } - want := `{"A":1,"b":"two","!@#":true}` - if g := string(got); g != want { - t.Errorf("\ngot %s\nwant %s", g, want) - } - - var un S - if err := unmarshalStructWithMap(got, &un, "M"); err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(s, un, cmpopts.IgnoreUnexported(S{})); diff != "" { - t.Errorf("mismatch (-want, +got):\n%s", diff) - } - }) - t.Run("duplicate", func(t *testing.T) { - s := S{A: 1, B: "two", M: map[string]any{"b": "dup"}} - _, err := marshalStructWithMap(&s, "M") - if err == nil || !strings.Contains(err.Error(), "duplicate") { - t.Errorf("got %v, want error with 'duplicate'", err) - } - }) -}