Skip to content

Commit 834a098

Browse files
authored
Merge branch 'master' into 12ya-patch
2 parents d99fbf4 + cc4b6fe commit 834a098

29 files changed

+1193
-464
lines changed

go/analysis/passes/printf/printf.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,9 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgIndex *int, firs
758758
pass.ReportRangef(call, "%s format %s has arg %s of wrong type %s%s", name, operation.Text, analysisinternal.Format(pass.Fset, arg), typeString, details)
759759
return false
760760
}
761-
if v.typ&argString != 0 && v.verb != 'T' && !strings.Contains(operation.Flags, "#") {
761+
// Detect recursive formatting via value's String/Error methods.
762+
// The '#' flag suppresses the methods, except with %x, %X, and %q.
763+
if v.typ&argString != 0 && v.verb != 'T' && (!strings.Contains(operation.Flags, "#") || strings.ContainsRune("qxX", v.verb)) {
762764
if methodName, ok := recursiveStringer(pass, arg); ok {
763765
pass.ReportRangef(call, "%s format %s with arg %s causes recursive %s method call", name, operation.Text, analysisinternal.Format(pass.Fset, arg), methodName)
764766
return false

go/analysis/passes/printf/testdata/src/a/a.go

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -567,10 +567,16 @@ type recursiveStringer int
567567
func (s recursiveStringer) String() string {
568568
_ = fmt.Sprintf("%d", s)
569569
_ = fmt.Sprintf("%#v", s)
570-
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveStringer\).String method call`
571-
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveStringer\).String method call`
572-
_ = fmt.Sprintf("%T", s) // ok; does not recursively call String
573-
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveStringer\).String method`
570+
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveStringer\).String method call`
571+
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveStringer\).String method call`
572+
_ = fmt.Sprintf("%#x", s) // want `fmt.Sprintf format %#x with arg s causes recursive \(a.recursiveStringer\).String method call`
573+
_ = fmt.Sprintf("%#x", &s) // want `fmt.Sprintf format %#x with arg &s causes recursive \(a.recursiveStringer\).String method call`
574+
_ = fmt.Sprintf("%#X", s) // want `fmt.Sprintf format %#X with arg s causes recursive \(a.recursiveStringer\).String method call`
575+
_ = fmt.Sprintf("%#X", &s) // want `fmt.Sprintf format %#X with arg &s causes recursive \(a.recursiveStringer\).String method call`
576+
_ = fmt.Sprintf("%#q", s) // want `fmt.Sprintf format %#q with arg s causes recursive \(a.recursiveStringer\).String method call`
577+
_ = fmt.Sprintf("%#q", &s) // want `fmt.Sprintf format %#q with arg &s causes recursive \(a.recursiveStringer\).String method call`
578+
_ = fmt.Sprintf("%T", s) // ok; does not recursively call String
579+
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveStringer\).String method`
574580
}
575581

576582
type recursivePtrStringer int
@@ -586,10 +592,16 @@ type recursiveError int
586592
func (s recursiveError) Error() string {
587593
_ = fmt.Sprintf("%d", s)
588594
_ = fmt.Sprintf("%#v", s)
589-
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveError\).Error method call`
590-
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveError\).Error method call`
591-
_ = fmt.Sprintf("%T", s) // ok; does not recursively call Error
592-
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveError\).Error method`
595+
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveError\).Error method call`
596+
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveError\).Error method call`
597+
_ = fmt.Sprintf("%#x", s) // want `fmt.Sprintf format %#x with arg s causes recursive \(a.recursiveError\).Error method call`
598+
_ = fmt.Sprintf("%#x", &s) // want `fmt.Sprintf format %#x with arg &s causes recursive \(a.recursiveError\).Error method call`
599+
_ = fmt.Sprintf("%#X", s) // want `fmt.Sprintf format %#X with arg s causes recursive \(a.recursiveError\).Error method call`
600+
_ = fmt.Sprintf("%#X", &s) // want `fmt.Sprintf format %#X with arg &s causes recursive \(a.recursiveError\).Error method call`
601+
_ = fmt.Sprintf("%#q", s) // want `fmt.Sprintf format %#q with arg s causes recursive \(a.recursiveError\).Error method call`
602+
_ = fmt.Sprintf("%#q", &s) // want `fmt.Sprintf format %#q with arg &s causes recursive \(a.recursiveError\).Error method call`
603+
_ = fmt.Sprintf("%T", s) // ok; does not recursively call Error
604+
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveError\).Error method`
593605
}
594606

595607
type recursivePtrError int
@@ -605,19 +617,31 @@ type recursiveStringerAndError int
605617
func (s recursiveStringerAndError) String() string {
606618
_ = fmt.Sprintf("%d", s)
607619
_ = fmt.Sprintf("%#v", s)
608-
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveStringerAndError\).String method call`
609-
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveStringerAndError\).String method call`
610-
_ = fmt.Sprintf("%T", s) // ok; does not recursively call String
611-
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveStringerAndError\).String method`
620+
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveStringerAndError\).String method call`
621+
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveStringerAndError\).String method call`
622+
_ = fmt.Sprintf("%#x", s) // want `fmt.Sprintf format %#x with arg s causes recursive \(a.recursiveStringerAndError\).String method call`
623+
_ = fmt.Sprintf("%#x", &s) // want `fmt.Sprintf format %#x with arg &s causes recursive \(a.recursiveStringerAndError\).String method call`
624+
_ = fmt.Sprintf("%#X", s) // want `fmt.Sprintf format %#X with arg s causes recursive \(a.recursiveStringerAndError\).String method call`
625+
_ = fmt.Sprintf("%#X", &s) // want `fmt.Sprintf format %#X with arg &s causes recursive \(a.recursiveStringerAndError\).String method call`
626+
_ = fmt.Sprintf("%#q", s) // want `fmt.Sprintf format %#q with arg s causes recursive \(a.recursiveStringerAndError\).String method call`
627+
_ = fmt.Sprintf("%#q", &s) // want `fmt.Sprintf format %#q with arg &s causes recursive \(a.recursiveStringerAndError\).String method call`
628+
_ = fmt.Sprintf("%T", s) // ok; does not recursively call String
629+
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveStringerAndError\).String method`
612630
}
613631

614632
func (s recursiveStringerAndError) Error() string {
615633
_ = fmt.Sprintf("%d", s)
616634
_ = fmt.Sprintf("%#v", s)
617-
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveStringerAndError\).Error method call`
618-
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveStringerAndError\).Error method call`
619-
_ = fmt.Sprintf("%T", s) // ok; does not recursively call Error
620-
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveStringerAndError\).Error method`
635+
_ = fmt.Sprintf("%v", s) // want `fmt.Sprintf format %v with arg s causes recursive \(a.recursiveStringerAndError\).Error method call`
636+
_ = fmt.Sprintf("%v", &s) // want `fmt.Sprintf format %v with arg &s causes recursive \(a.recursiveStringerAndError\).Error method call`
637+
_ = fmt.Sprintf("%#x", s) // want `fmt.Sprintf format %#x with arg s causes recursive \(a.recursiveStringerAndError\).Error method call`
638+
_ = fmt.Sprintf("%#x", &s) // want `fmt.Sprintf format %#x with arg &s causes recursive \(a.recursiveStringerAndError\).Error method call`
639+
_ = fmt.Sprintf("%#X", s) // want `fmt.Sprintf format %#X with arg s causes recursive \(a.recursiveStringerAndError\).Error method call`
640+
_ = fmt.Sprintf("%#X", &s) // want `fmt.Sprintf format %#X with arg &s causes recursive \(a.recursiveStringerAndError\).Error method call`
641+
_ = fmt.Sprintf("%#q", s) // want `fmt.Sprintf format %#q with arg s causes recursive \(a.recursiveStringerAndError\).Error method call`
642+
_ = fmt.Sprintf("%#q", &s) // want `fmt.Sprintf format %#q with arg &s causes recursive \(a.recursiveStringerAndError\).Error method call`
643+
_ = fmt.Sprintf("%T", s) // ok; does not recursively call Error
644+
return fmt.Sprintln(s) // want `fmt.Sprintln arg s causes recursive call to \(a.recursiveStringerAndError\).Error method`
621645
}
622646

623647
type recursivePtrStringerAndError int

gopls/internal/test/integration/bench/didchange_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
var editID int64 = time.Now().UnixNano()
2222

2323
type changeTest struct {
24-
repo string
24+
repo string // repo identifier + optional disambiguating ".foo" suffix
2525
file string
2626
canSave bool
2727
}
@@ -30,6 +30,7 @@ var didChangeTests = []changeTest{
3030
{"google-cloud-go", "internal/annotate.go", true},
3131
{"istio", "pkg/fuzz/util.go", true},
3232
{"kubernetes", "pkg/controller/lookup_cache.go", true},
33+
{"kubernetes.types", "staging/src/k8s.io/api/core/v1/types.go", true}, // results in 25K file batch!
3334
{"kuma", "api/generic/insights.go", true},
3435
{"oracle", "dataintegration/data_type.go", false}, // diagnoseSave fails because this package is generated
3536
{"pkgsite", "internal/frontend/server.go", true},

gopls/internal/test/integration/bench/repo_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"log"
1414
"os"
1515
"path/filepath"
16+
"strings"
1617
"sync"
1718
"testing"
1819
"time"
@@ -109,8 +110,13 @@ var repos = map[string]*repo{
109110

110111
// getRepo gets the requested repo, and skips the test if -short is set and
111112
// repo is not configured as a short repo.
113+
//
114+
// The name may include an optional ".foo" suffix after the repo
115+
// identifier. This allows several tests to use the same repo but have
116+
// distinct test names and associated file names.
112117
func getRepo(tb testing.TB, name string) *repo {
113118
tb.Helper()
119+
name, _, _ = strings.Cut(name, ".") // remove ".foo" suffix
114120
repo := repos[name]
115121
if repo == nil {
116122
tb.Fatalf("repo %s does not exist", name)

internal/jsonrpc2_v2/net.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,14 @@ func (l *netPiper) Accept(context.Context) (io.ReadWriteCloser, error) {
100100
// preferring the latter if already closed at the start of Accept.
101101
select {
102102
case <-l.done:
103-
return nil, errClosed
103+
return nil, net.ErrClosed
104104
default:
105105
}
106106
select {
107107
case rwc := <-l.dialed:
108108
return rwc, nil
109109
case <-l.done:
110-
return nil, errClosed
110+
return nil, net.ErrClosed
111111
}
112112
}
113113

@@ -133,6 +133,6 @@ func (l *netPiper) Dial(ctx context.Context) (io.ReadWriteCloser, error) {
133133
case <-l.done:
134134
client.Close()
135135
server.Close()
136-
return nil, errClosed
136+
return nil, net.ErrClosed
137137
}
138138
}

internal/jsonrpc2_v2/serve_go116.go

Lines changed: 0 additions & 13 deletions
This file was deleted.

internal/jsonrpc2_v2/serve_pre116.go

Lines changed: 0 additions & 15 deletions
This file was deleted.

internal/mcp/client.go

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func (c *Client) RemoveRoots(uris ...string) {
162162
// changeAndNotify is called when a feature is added or removed.
163163
// It calls change, which should do the work and report whether a change actually occurred.
164164
// If there was a change, it notifies a snapshot of the sessions.
165-
func (c *Client) changeAndNotify(notification string, params any, change func() bool) {
165+
func (c *Client) changeAndNotify(notification string, params Params, change func() bool) {
166166
var sessions []*ClientSession
167167
// Lock for the change, but not for the notification.
168168
c.mu.Lock()
@@ -231,8 +231,8 @@ func (cs *ClientSession) methodHandler() MethodHandler[ClientSession] {
231231
// getConn implements [session.getConn].
232232
func (cs *ClientSession) getConn() *jsonrpc2.Connection { return cs.conn }
233233

234-
func (c *ClientSession) ping(ct context.Context, params *PingParams) (struct{}, error) {
235-
return struct{}{}, nil
234+
func (c *ClientSession) ping(ct context.Context, params *PingParams) (Result, error) {
235+
return emptyResult{}, nil
236236
}
237237

238238
// Ping makes an MCP "ping" request to the server.
@@ -296,29 +296,21 @@ func (c *ClientSession) ReadResource(ctx context.Context, params *ReadResourcePa
296296
return standardCall[ReadResourceResult](ctx, c.conn, methodReadResource, params)
297297
}
298298

299-
func (c *Client) callToolChangedHandler(ctx context.Context, s *ClientSession, params *ToolListChangedParams) (any, error) {
299+
func (c *Client) callToolChangedHandler(ctx context.Context, s *ClientSession, params *ToolListChangedParams) (Result, error) {
300300
return callNotificationHandler(ctx, c.opts.ToolListChangedHandler, s, params)
301301
}
302302

303-
func (c *Client) callPromptChangedHandler(ctx context.Context, s *ClientSession, params *PromptListChangedParams) (any, error) {
303+
func (c *Client) callPromptChangedHandler(ctx context.Context, s *ClientSession, params *PromptListChangedParams) (Result, error) {
304304
return callNotificationHandler(ctx, c.opts.PromptListChangedHandler, s, params)
305305
}
306306

307-
func (c *Client) callResourceChangedHandler(ctx context.Context, s *ClientSession, params *ResourceListChangedParams) (any, error) {
307+
func (c *Client) callResourceChangedHandler(ctx context.Context, s *ClientSession, params *ResourceListChangedParams) (Result, error) {
308308
return callNotificationHandler(ctx, c.opts.ResourceListChangedHandler, s, params)
309309
}
310310

311-
func (c *Client) callLoggingHandler(ctx context.Context, cs *ClientSession, params *LoggingMessageParams) (any, error) {
311+
func (c *Client) callLoggingHandler(ctx context.Context, cs *ClientSession, params *LoggingMessageParams) (Result, error) {
312312
if h := c.opts.LoggingMessageHandler; h != nil {
313313
h(ctx, cs, params)
314314
}
315315
return nil, nil
316316
}
317-
318-
func standardCall[TRes, TParams any](ctx context.Context, conn *jsonrpc2.Connection, method string, params TParams) (*TRes, error) {
319-
var result TRes
320-
if err := call(ctx, conn, method, params, &result); err != nil {
321-
return nil, err
322-
}
323-
return &result, nil
324-
}

internal/mcp/design/design.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ Types needed for the protocol are generated from the [JSON schema of the MCP spe
243243
244244
These types will be included in the `mcp` package, but will be unexported unless they are needed for the user-facing API. Notably, JSON-RPC request types are elided, since they are handled by the `jsonrpc2` package and should not be observed by the user.
245245
246-
For user-provided data, we use `json.RawMessage`, so that marshalling/unmarshalling can be delegated to the business logic of the client or server.
246+
For user-provided data, we use `json.RawMessage` or `map[string]any`, depending on the use case.
247247
248248
For union types, which can't be represented in Go (specifically `Content` and `ResourceContents`), we prefer distinguished unions: struct types with fields corresponding to the union of all properties for union elements.
249249
@@ -255,9 +255,9 @@ type ReadResourceParams struct {
255255
}
256256

257257
type CallToolResult struct {
258-
Meta map[string]json.RawMessage `json:"_meta,omitempty"`
259-
Content []Content `json:"content"`
260-
IsError bool `json:"isError,omitempty"`
258+
Meta Meta `json:"_meta,omitempty"`
259+
Content []Content `json:"content"`
260+
IsError bool `json:"isError,omitempty"`
261261
}
262262

263263
// Content is the wire format for content.
@@ -276,6 +276,7 @@ type Content struct {
276276
func NewTextContent(text string) *Content
277277
// etc.
278278
```
279+
The `Meta` type includes a `map[string]any` for arbitrary data, and a `ProgressToken` field.
279280
280281
**Differences from mcp-go**: these types are largely similar, but our type generator flattens types rather than using struct embedding.
281282
@@ -480,15 +481,21 @@ The server observes a client cancellation as a cancelled context.
480481
481482
### Progress handling
482483
483-
A caller can request progress notifications by setting the `ProgressToken` field on any request.
484+
A caller can request progress notifications by setting the `Meta.ProgressToken` field on any request.
484485
485486
```go
486487
type XXXParams struct { // where XXX is each type of call
488+
Meta Meta
487489
...
490+
}
491+
492+
type Meta struct {
493+
Data map[string]any
488494
ProgressToken any // string or int
489495
}
490496
```
491497
498+
492499
Handlers can notify their peer about progress by calling the `NotifyProgress` method. The notification is only sent if the peer requested it by providing a progress token.
493500
494501
```go

internal/mcp/generate.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -299,13 +299,21 @@ func loadSchema(schemaFile string) (data []byte, err error) {
299299

300300
func writeDecl(configName string, config typeConfig, def *jsonschema.Schema, named map[string]*bytes.Buffer) error {
301301
var w io.Writer = io.Discard
302-
if typeName := config.Name; typeName != "-" {
302+
var typeName string
303+
if typeName = config.Name; typeName != "-" {
303304
if typeName == "" {
304305
typeName = configName
305306
}
306307
if _, ok := named[typeName]; ok {
307308
return nil
308309
}
310+
// The JSON schema does not accurately represent the source of truth, which is typescript.
311+
// Every Params and Result type should have a _meta property.
312+
// Also, those with a progress token will turn into a struct; we want the progress token to
313+
// be a map item. So replace all metas.
314+
if strings.HasSuffix(typeName, "Params") || strings.HasSuffix(typeName, "Result") {
315+
def.Properties["_meta"] = metaSchema
316+
}
309317
buf := new(bytes.Buffer)
310318
w = buf
311319
named[typeName] = buf
@@ -318,6 +326,12 @@ func writeDecl(configName string, config typeConfig, def *jsonschema.Schema, nam
318326
return err // Better error here?
319327
}
320328
fmt.Fprintf(w, "\n")
329+
330+
// Any decl with a _meta field gets a GetMeta method.
331+
if _, ok := def.Properties["_meta"]; ok {
332+
fmt.Fprintf(w, "\nfunc (x *%s) GetMeta() *Meta { return &x.Meta }", typeName)
333+
}
334+
321335
return nil
322336
}
323337

@@ -354,11 +368,7 @@ func writeType(w io.Writer, config *typeConfig, def *jsonschema.Schema, named ma
354368

355369
// For types that explicitly allow additional properties, we can either
356370
// unmarshal them into a map[string]any, or delay unmarshalling with
357-
// json.RawMessage. For now, use json.RawMessage as it defers the choice.
358-
//
359-
// TODO(jba): further refine this classification of object schemas.
360-
// For example, the typescript "object" type, which should map to a Go "any",
361-
// is represented in schema.json by `{type: object, properties: {}, additionalProperties: true}`.
371+
// json.RawMessage. We use any.
362372
if def.Type == "object" && canHaveAdditionalProperties(def) && def.Properties == nil {
363373
w.Write([]byte("map[string]"))
364374
return writeType(w, nil, def.AdditionalProperties, named)
@@ -372,7 +382,7 @@ func writeType(w io.Writer, config *typeConfig, def *jsonschema.Schema, named ma
372382
fmt.Fprintf(w, "*Content")
373383
} else {
374384
// E.g. union types.
375-
fmt.Fprintf(w, "json.RawMessage")
385+
fmt.Fprintf(w, "any")
376386
}
377387
} else {
378388
switch def.Type {
@@ -398,6 +408,11 @@ func writeType(w io.Writer, config *typeConfig, def *jsonschema.Schema, named ma
398408
if fieldDef.Description != "" {
399409
fmt.Fprintf(w, "%s\n", toComment(fieldDef.Description))
400410
}
411+
if name == "_meta" {
412+
fmt.Fprintln(w, "\tMeta Meta `json:\"_meta,omitempty\"`")
413+
continue
414+
}
415+
401416
export := exportName(name)
402417
fmt.Fprintf(w, "\t%s ", export)
403418

@@ -551,6 +566,12 @@ func isStruct(s *jsonschema.Schema) bool {
551566
return s.Type == "object" && s.Properties != nil && !canHaveAdditionalProperties(s)
552567
}
553568

569+
// The schema for "_meta".
570+
// We only need the description: the rest is a special case.
571+
var metaSchema = &jsonschema.Schema{
572+
Description: "This property is reserved by the protocol to allow clients and servers to attach additional metadata to their responses.",
573+
}
574+
554575
// schemaJSON returns the JSON for s.
555576
// For debugging.
556577
func schemaJSON(s *jsonschema.Schema) string {

0 commit comments

Comments
 (0)