diff --git a/internal/jsonrpc2/jsonrpc2.go b/internal/jsonrpc2/jsonrpc2.go index 1f2fd4cfe7..c37cd58940 100644 --- a/internal/jsonrpc2/jsonrpc2.go +++ b/internal/jsonrpc2/jsonrpc2.go @@ -103,7 +103,7 @@ func (c *Conn) Notify(ctx context.Context, method string, params interface{}) (e ctx, done := event.StartSpan(ctx, request.Method, tag.Method.Of(request.Method), tag.RPCDirection.Of(tag.Outbound), - tag.RPCID.Of(request.ID.String()), + tag.RPCID.Of(fmt.Sprintf("%q", request.ID)), ) defer func() { recordStatus(ctx, err) @@ -139,7 +139,7 @@ func (c *Conn) Call(ctx context.Context, method string, params, result interface ctx, done := event.StartSpan(ctx, request.Method, tag.Method.Of(request.Method), tag.RPCDirection.Of(tag.Outbound), - tag.RPCID.Of(request.ID.String()), + tag.RPCID.Of(fmt.Sprintf("%q", request.ID)), ) defer func() { recordStatus(ctx, err) @@ -298,7 +298,7 @@ func (c *Conn) Run(runCtx context.Context, handler Handler) error { reqCtx, spanDone := event.StartSpan(runCtx, msg.Method, tag.Method.Of(msg.Method), tag.RPCDirection.Of(tag.Inbound), - tag.RPCID.Of(msg.ID.String()), + tag.RPCID.Of(fmt.Sprintf("%q", msg.ID)), ) event.Record(reqCtx, tag.Started.Of(1), diff --git a/internal/jsonrpc2/serve_test.go b/internal/jsonrpc2/serve_test.go index 2cbfe8f97b..1b95610e0f 100644 --- a/internal/jsonrpc2/serve_test.go +++ b/internal/jsonrpc2/serve_test.go @@ -16,7 +16,7 @@ func TestIdleTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - ln, err := net.Listen("tcp", ":0") + ln, err := net.Listen("tcp", "localhost:0") if err != nil { t.Fatal(err) } diff --git a/internal/jsonrpc2/wire.go b/internal/jsonrpc2/wire.go index 127feb17e4..bfbf981799 100644 --- a/internal/jsonrpc2/wire.go +++ b/internal/jsonrpc2/wire.go @@ -7,7 +7,7 @@ package jsonrpc2 import ( "encoding/json" "fmt" - "strconv" + "math" ) // this file contains the go forms of the wire specification @@ -109,17 +109,24 @@ func (VersionTag) UnmarshalJSON(data []byte) error { return nil } -// String returns a string representation of the ID. -// The representation is non ambiguous, string forms are quoted, number forms -// are preceded by a # -func (id *ID) String() string { - if id == nil { - return "" +const invalidID int64 = math.MaxInt64 + +// Format writes the ID to the formatter. +// If the rune is q the representation is non ambiguous, +// string forms are quoted, number forms are preceded by a # +func (id *ID) Format(f fmt.State, r rune) { + numF, strF := `%d`, `%s` + if r == 'q' { + numF, strF = `#%d`, `%q` } - if id.Name != "" { - return strconv.Quote(id.Name) + switch { + case id == nil: + fmt.Fprintf(f, numF, invalidID) + case id.Name != "": + fmt.Fprintf(f, strF, id.Name) + default: + fmt.Fprintf(f, numF, id.Number) } - return "#" + strconv.FormatInt(id.Number, 10) } func (id *ID) MarshalJSON() ([]byte, error) { diff --git a/internal/jsonrpc2/wire_test.go b/internal/jsonrpc2/wire_test.go new file mode 100644 index 0000000000..12c45d2fcc --- /dev/null +++ b/internal/jsonrpc2/wire_test.go @@ -0,0 +1,109 @@ +// Copyright 2020 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 jsonrpc2_test + +import ( + "bytes" + "encoding/json" + "fmt" + "math" + "testing" + + "golang.org/x/tools/internal/jsonrpc2" +) + +var wireIDTestData = []struct { + name string + id *jsonrpc2.ID + encoded []byte + plain string + quoted string +}{ + { + name: `nil`, + id: nil, + encoded: []byte(`null`), + plain: fmt.Sprintf(`%d`, int64(math.MaxInt64)), + quoted: fmt.Sprintf(`#%d`, int64(math.MaxInt64)), + }, { + name: `empty`, + id: &jsonrpc2.ID{}, + encoded: []byte(`0`), + plain: `0`, + quoted: `#0`, + }, { + name: `number`, + id: &jsonrpc2.ID{Number: 43}, + encoded: []byte(`43`), + plain: `43`, + quoted: `#43`, + }, { + name: `string`, + id: &jsonrpc2.ID{Name: "life"}, + encoded: []byte(`"life"`), + plain: `life`, + quoted: `"life"`, + }, +} + +func TestIDFormat(t *testing.T) { + for _, test := range wireIDTestData { + t.Run(test.name, func(t *testing.T) { + if got := fmt.Sprint(test.id); got != test.plain { + t.Errorf("got %s expected %s", got, test.plain) + } + if got := fmt.Sprintf("%q", test.id); got != test.quoted { + t.Errorf("got %s want %s", got, test.quoted) + } + }) + } +} + +func TestIDEncode(t *testing.T) { + for _, test := range wireIDTestData { + t.Run(test.name, func(t *testing.T) { + data, err := json.Marshal(test.id) + if err != nil { + t.Fatal(err) + } + checkJSON(t, data, test.encoded) + }) + } +} + +func TestIDDecode(t *testing.T) { + for _, test := range wireIDTestData { + t.Run(test.name, func(t *testing.T) { + var got *jsonrpc2.ID + if err := json.Unmarshal(test.encoded, &got); err != nil { + t.Fatal(err) + } + if got == nil { + if test.id != nil { + t.Errorf("got nil want %s", test.id) + } + } else if test.id == nil { + t.Errorf("got %s want nil", got) + } else if *got != *test.id { + t.Errorf("got %s want %s", got, test.id) + } + }) + } +} + +func checkJSON(t *testing.T, got, want []byte) { + // compare the compact form, to allow for formatting differences + g := &bytes.Buffer{} + if err := json.Compact(g, []byte(got)); err != nil { + t.Fatal(err) + } + w := &bytes.Buffer{} + if err := json.Compact(w, []byte(want)); err != nil { + t.Fatal(err) + } + if g.String() != w.String() { + t.Fatalf("Got:\n%s\nWant:\n%s", g, w) + } +} diff --git a/internal/lsp/protocol/log.go b/internal/lsp/protocol/log.go index 30fa549539..d073fd2ae6 100644 --- a/internal/lsp/protocol/log.go +++ b/internal/lsp/protocol/log.go @@ -106,17 +106,6 @@ func (m *mapped) setServer(id string, r req) { const eor = "\r\n\r\n\r\n" -func strID(x *jsonrpc2.ID) string { - if x == nil { - // should never happen, but we need a number - return "999999999" - } - if x.Name != "" { - return x.Name - } - return fmt.Sprintf("%d", x.Number) -} - func logCommon(outfd io.Writer, data []byte) (*Combined, time.Time, string) { if outfd == nil { return nil, time.Time{}, "" @@ -141,13 +130,12 @@ func logOut(outfd io.Writer, data []byte) { if v == nil { return } + id := fmt.Sprint(v.ID) if v.Error != nil { - id := strID(v.ID) fmt.Fprintf(outfd, "[Error - %s] Received #%s %s%s", tmfmt, id, v.Error, eor) return } buf := strings.Builder{} - id := strID(v.ID) fmt.Fprintf(&buf, "[Trace - %s] ", tmfmt) // common beginning if v.ID != nil && v.Method != "" && v.Params != nil { fmt.Fprintf(&buf, "Received request '%s - (%s)'.\n", v.Method, id) @@ -194,16 +182,15 @@ func logIn(outfd io.Writer, data []byte) { if v == nil { return } + id := fmt.Sprint(v.ID) // ID Method Params => Sending request // ID !Method Result(might be null, but !Params) => Sending response (could we get an Error?) // !ID Method Params => Sending notification if v.Error != nil { // does this ever happen? - id := strID(v.ID) fmt.Fprintf(outfd, "[Error - %s] Sent #%s %s%s", tmfmt, id, v.Error, eor) return } buf := strings.Builder{} - id := strID(v.ID) fmt.Fprintf(&buf, "[Trace - %s] ", tmfmt) // common beginning if v.ID != nil && v.Method != "" && (v.Params != nil || v.Method == "shutdown") { fmt.Fprintf(&buf, "Sending request '%s - (%s)'.\n", v.Method, id)