internal/mcp/jsonschema: remove shared structure from schema inference
Schemas must form a tree. This is important for Schema.Resolve, and also for modifying parts of the schema before resolution, as with mcp SchemaOptions. Remove the `seen` argument from inference, creating all schemas afresh. Change-Id: Ief8295e8470f3f575ef51cc2ba697901250bbe6f Reviewed-on: https://go-review.googlesource.com/c/tools/+/676937 Auto-Submit: Jonathan Amsterdam <jba@google.com> Reviewed-by: Hongxiang Jiang <hxjiang@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
parent
d45330dd8f
commit
a9a69a03e4
2 changed files with 45 additions and 28 deletions
|
|
@ -44,21 +44,17 @@ func For[T any]() (*Schema, error) {
|
||||||
//
|
//
|
||||||
// TODO(rfindley): we could perhaps just skip these incompatible fields.
|
// TODO(rfindley): we could perhaps just skip these incompatible fields.
|
||||||
func ForType(t reflect.Type) (*Schema, error) {
|
func ForType(t reflect.Type) (*Schema, error) {
|
||||||
return typeSchema(t, make(map[reflect.Type]*Schema))
|
return typeSchema(t)
|
||||||
}
|
}
|
||||||
|
|
||||||
func typeSchema(t reflect.Type, seen map[reflect.Type]*Schema) (*Schema, error) {
|
func typeSchema(t reflect.Type) (*Schema, error) {
|
||||||
if t.Kind() == reflect.Pointer {
|
if t.Kind() == reflect.Pointer {
|
||||||
t = t.Elem()
|
t = t.Elem()
|
||||||
}
|
}
|
||||||
if s := seen[t]; s != nil {
|
|
||||||
return s, nil
|
|
||||||
}
|
|
||||||
var (
|
var (
|
||||||
s = new(Schema)
|
s = new(Schema)
|
||||||
err error
|
err error
|
||||||
)
|
)
|
||||||
seen[t] = s
|
|
||||||
|
|
||||||
switch t.Kind() {
|
switch t.Kind() {
|
||||||
case reflect.Bool:
|
case reflect.Bool:
|
||||||
|
|
@ -80,14 +76,14 @@ func typeSchema(t reflect.Type, seen map[reflect.Type]*Schema) (*Schema, error)
|
||||||
return nil, fmt.Errorf("unsupported map key type %v", t.Key().Kind())
|
return nil, fmt.Errorf("unsupported map key type %v", t.Key().Kind())
|
||||||
}
|
}
|
||||||
s.Type = "object"
|
s.Type = "object"
|
||||||
s.AdditionalProperties, err = typeSchema(t.Elem(), seen)
|
s.AdditionalProperties, err = typeSchema(t.Elem())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("computing map value schema: %v", err)
|
return nil, fmt.Errorf("computing map value schema: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
case reflect.Slice, reflect.Array:
|
case reflect.Slice, reflect.Array:
|
||||||
s.Type = "array"
|
s.Type = "array"
|
||||||
s.Items, err = typeSchema(t.Elem(), seen)
|
s.Items, err = typeSchema(t.Elem())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("computing element schema: %v", err)
|
return nil, fmt.Errorf("computing element schema: %v", err)
|
||||||
}
|
}
|
||||||
|
|
@ -113,7 +109,7 @@ func typeSchema(t reflect.Type, seen map[reflect.Type]*Schema) (*Schema, error)
|
||||||
if s.Properties == nil {
|
if s.Properties == nil {
|
||||||
s.Properties = make(map[string]*Schema)
|
s.Properties = make(map[string]*Schema)
|
||||||
}
|
}
|
||||||
s.Properties[name], err = typeSchema(field.Type, seen)
|
s.Properties[name], err = typeSchema(field.Type)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -41,31 +41,52 @@ func TestForType(t *testing.T) {
|
||||||
Type: "object",
|
Type: "object",
|
||||||
AdditionalProperties: &schema{},
|
AdditionalProperties: &schema{},
|
||||||
}},
|
}},
|
||||||
{"struct", forType[struct {
|
{
|
||||||
F int `json:"f"`
|
"struct",
|
||||||
G []float64
|
forType[struct {
|
||||||
P *bool
|
F int `json:"f"`
|
||||||
Skip string `json:"-"`
|
G []float64
|
||||||
NoSkip string `json:",omitempty"`
|
P *bool
|
||||||
unexported float64
|
Skip string `json:"-"`
|
||||||
unexported2 int `json:"No"`
|
NoSkip string `json:",omitempty"`
|
||||||
}](), &schema{
|
unexported float64
|
||||||
Type: "object",
|
unexported2 int `json:"No"`
|
||||||
Properties: map[string]*schema{
|
}](),
|
||||||
"f": {Type: "integer"},
|
&schema{
|
||||||
"G": {Type: "array", Items: &schema{Type: "number"}},
|
Type: "object",
|
||||||
"P": {Type: "boolean"},
|
Properties: map[string]*schema{
|
||||||
"NoSkip": {Type: "string"},
|
"f": {Type: "integer"},
|
||||||
|
"G": {Type: "array", Items: &schema{Type: "number"}},
|
||||||
|
"P": {Type: "boolean"},
|
||||||
|
"NoSkip": {Type: "string"},
|
||||||
|
},
|
||||||
|
Required: []string{"f", "G", "P"},
|
||||||
|
AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}},
|
||||||
},
|
},
|
||||||
Required: []string{"f", "G", "P"},
|
},
|
||||||
AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}},
|
{
|
||||||
}},
|
"no sharing",
|
||||||
|
forType[struct{ X, Y int }](),
|
||||||
|
&schema{
|
||||||
|
Type: "object",
|
||||||
|
Properties: map[string]*schema{
|
||||||
|
"X": {Type: "integer"},
|
||||||
|
"Y": {Type: "integer"},
|
||||||
|
},
|
||||||
|
Required: []string{"X", "Y"},
|
||||||
|
AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}},
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, test := range tests {
|
for _, test := range tests {
|
||||||
t.Run(test.name, func(t *testing.T) {
|
t.Run(test.name, func(t *testing.T) {
|
||||||
if diff := cmp.Diff(test.want, test.got, cmpopts.IgnoreUnexported(jsonschema.Schema{})); diff != "" {
|
if diff := cmp.Diff(test.want, test.got, cmpopts.IgnoreUnexported(jsonschema.Schema{})); diff != "" {
|
||||||
t.Errorf("ForType mismatch (-want +got):\n%s", diff)
|
t.Fatalf("ForType mismatch (-want +got):\n%s", diff)
|
||||||
|
}
|
||||||
|
// These schemas should all resolve.
|
||||||
|
if _, err := test.got.Resolve(nil); err != nil {
|
||||||
|
t.Fatalf("Resolving: %v", err)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue