internal/mcp: unify json tag parsing

Do a thorough job of parsing json struct tags.

Move the code to a place where both mcp and jsonschema can use it.

Change-Id: I6c86308834706b12a1e22cd96ae120590f49d3f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/678757
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: jba

internal/mcp/internal/util/util.go

@@ -7,7 +7,9 @@ package util
 import (
 	"cmp"
 	"iter"
+	"reflect"
 	"slices"
+	"strings"
 )
 
 // Helpers below are copied from gopls' moremaps package.
@@ -34,3 +36,40 @@ 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
+}

internal/mcp/internal/util/util_test.go

@@ -0,0 +1,38 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-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])
+		}
+	}
+}

internal/mcp/jsonschema/infer.go

@@ -9,8 +9,8 @@ package jsonschema
 import (
 	"fmt"
 	"reflect"
-	"slices"
-	"strings"
+
+	"golang.org/x/tools/internal/mcp/internal/util"
 )
 
 // For constructs a JSON schema object for the given type argument.
@@ -108,19 +108,19 @@ func typeSchema(t reflect.Type) (*Schema, error) {
 
 		for i := range t.NumField() {
 			field := t.Field(i)
-			name, required, include := parseField(field)
-			if !include {
+			info := util.FieldJSONInfo(field)
+			if info.Omit {
 				continue
 			}
 			if s.Properties == nil {
 				s.Properties = make(map[string]*Schema)
 			}
-			s.Properties[name], err = typeSchema(field.Type)
+			s.Properties[info.Name], err = typeSchema(field.Type)
 			if err != nil {
 				return nil, err
 			}
-			if required {
-				s.Required = append(s.Required, name)
+			if !info.Settings["omitempty"] && !info.Settings["omitzero"] {
+				s.Required = append(s.Required, info.Name)
 			}
 		}
 
@@ -133,23 +133,3 @@ func typeSchema(t reflect.Type) (*Schema, error) {
 	}
 	return s, nil
 }
-
-func parseField(f reflect.StructField) (name string, required, include bool) {
-	if !f.IsExported() {
-		return "", false, false
-	}
-	name = f.Name
-	required = true
-	if tag, ok := f.Tag.Lookup("json"); ok {
-		props := strings.Split(tag, ",")
-		if props[0] != "" {
-			if props[0] == "-" {
-				return "", false, false
-			}
-			name = props[0]
-		}
-		// TODO: support 'omitzero' as well.
-		required = !slices.Contains(props[1:], "omitempty")
-	}
-	return name, required, true
-}

internal/mcp/jsonschema/schema.go

@@ -17,6 +17,8 @@ import (
 	"reflect"
 	"regexp"
 	"slices"
+
+	"golang.org/x/tools/internal/mcp/internal/util"
 )
 
 // A Schema is a JSON schema object.
@@ -426,8 +428,9 @@ var (
 
 func init() {
 	for _, sf := range reflect.VisibleFields(reflect.TypeFor[Schema]()) {
-		if name, ok := jsonName(sf); ok {
-			schemaFieldInfos = append(schemaFieldInfos, structFieldInfo{sf, name})
+		info := util.FieldJSONInfo(sf)
+		if !info.Omit {
+			schemaFieldInfos = append(schemaFieldInfos, structFieldInfo{sf, info.Name})
 		}
 	}
 	slices.SortFunc(schemaFieldInfos, func(i1, i2 structFieldInfo) int {

internal/mcp/jsonschema/validate.go

@@ -16,6 +16,8 @@ import (
 	"strings"
 	"sync"
 	"unicode/utf8"
+
+	"golang.org/x/tools/internal/mcp/internal/util"
 )
 
 // The value of the "$schema" keyword for the version that we can validate.
@@ -688,7 +690,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() {
-					if tag, ok := sf.Tag.Lookup("json"); ok && (strings.Contains(tag, "omitempty") || strings.Contains(tag, "omitzero")) {
+					info := util.FieldJSONInfo(sf)
+					if info.Settings["omitempty"] || info.Settings["omitzero"] {
 						continue
 					}
 				}
@@ -740,30 +743,11 @@ func structPropertiesOf(t reflect.Type) propertyMap {
 	}
 	props := map[string]reflect.StructField{}
 	for _, sf := range reflect.VisibleFields(t) {
-		if name, ok := jsonName(sf); ok {
-			props[name] = sf
+		info := util.FieldJSONInfo(sf)
+		if !info.Omit {
+			props[info.Name] = sf
 		}
 	}
 	structProperties.Store(t, props)
 	return props
 }
-
-// jsonName returns the name for f as would be used by [json.Marshal].
-// That is the name in the json struct tag, or the field name if there is no tag.
-// If f is not exported or the tag is "-", jsonName returns "", false.
-func jsonName(f reflect.StructField) (string, bool) {
-	if !f.IsExported() {
-		return "", false
-	}
-	if tag, ok := f.Tag.Lookup("json"); ok {
-		name, _, found := strings.Cut(tag, ",")
-		// "-" means omit, but "-," means the name is "-"
-		if name == "-" && !found {
-			return "", false
-		}
-		if name != "" {
-			return name, true
-		}
-	}
-	return f.Name, true
-}

internal/mcp/jsonschema/validate_test.go

@@ -259,26 +259,6 @@ func TestStructInstance(t *testing.T) {
 	}
 }
 
-func TestJSONName(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 := []string{"A", "B", "", "-", "echo", "foxtrot", ""}
-	tt := reflect.TypeFor[S]()
-	for i := range tt.NumField() {
-		got, _ := jsonName(tt.Field(i))
-		if got != want[i] {
-			t.Errorf("got %q, want %q", got, want[i])
-		}
-	}
-}
-
 func mustMarshal(x any) json.RawMessage {
 	data, err := json.Marshal(x)
 	if err != nil {

internal/mcp/util.go

@@ -9,8 +9,9 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
-	"strings"
 	"sync"
+
+	"golang.org/x/tools/internal/mcp/internal/util"
 )
 
 func assert(cond bool, msg string) {
@@ -135,32 +136,11 @@ func jsonNames(t reflect.Type) map[string]bool {
 	}
 	m := map[string]bool{}
 	for i := range t.NumField() {
-		if n, ok := jsonName(t.Field(i)); ok {
-			m[n] = true
+		info := util.FieldJSONInfo(t.Field(i))
+		if !info.Omit {
+			m[info.Name] = true
 		}
 	}
 	jsonNamesMap.Store(t, m)
 	return m
 }
-
-// jsonName returns the name for f as would be used by [json.Marshal].
-// That is the name in the json struct tag, or the field name if there is no tag.
-// If f is not exported or the tag is "-", jsonName returns "", false.
-//
-// Copied from jsonschema/validate.go.
-func jsonName(f reflect.StructField) (string, bool) {
-	if !f.IsExported() {
-		return "", false
-	}
-	if tag, ok := f.Tag.Lookup("json"); ok {
-		name, _, found := strings.Cut(tag, ",")
-		// "-" means omit, but "-," means the name is "-"
-		if name == "-" && !found {
-			return "", false
-		}
-		if name != "" {
-			return name, true
-		}
-	}
-	return f.Name, true
-}