internal/lsp: fix broken lsp logs
ID's are now by value not pointer, which caused it to not use the Format
method, resulting in broken id strings
The id maps need to be crossover (set and get go to different maps for a given direction of message)
Change-Id: Ide2b63ec1b009ae3587ee10e8bce018732ea342c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229244
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
diff --git a/gopls/integration/replay/main.go b/gopls/integration/replay/main.go
index 49486ff..cea66f2 100644
--- a/gopls/integration/replay/main.go
+++ b/gopls/integration/replay/main.go
@@ -145,7 +145,8 @@
if err != nil {
n = 0
}
- id = jsonrpc2.NewIntID(int64(n))
+ nid := jsonrpc2.NewIntID(int64(n))
+ id = &nid
}
var msg jsonrpc2.Message
var err error
diff --git a/internal/jsonrpc2/wire.go b/internal/jsonrpc2/wire.go
index 632c531..cfef6c2 100644
--- a/internal/jsonrpc2/wire.go
+++ b/internal/jsonrpc2/wire.go
@@ -125,22 +125,20 @@
const invalidID int64 = math.MaxInt64
// NewIntID returns a new numerical request ID.
-func NewIntID(v int64) *ID { return &ID{number: v} }
+func NewIntID(v int64) ID { return ID{number: v} }
// NewStringID returns a new string request ID.
-func NewStringID(v string) *ID { return &ID{name: v} }
+func NewStringID(v string) ID { return ID{name: v} }
// 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) {
+func (id ID) Format(f fmt.State, r rune) {
numF, strF := `%d`, `%s`
if r == 'q' {
numF, strF = `#%d`, `%q`
}
switch {
- case id == nil:
- fmt.Fprintf(f, numF, invalidID)
case id.name != "":
fmt.Fprintf(f, strF, id.name)
default:
diff --git a/internal/jsonrpc2/wire_test.go b/internal/jsonrpc2/wire_test.go
index 758b0bb..7cddb65 100644
--- a/internal/jsonrpc2/wire_test.go
+++ b/internal/jsonrpc2/wire_test.go
@@ -8,7 +8,6 @@
"bytes"
"encoding/json"
"fmt"
- "math"
"testing"
"golang.org/x/tools/internal/jsonrpc2"
@@ -16,20 +15,13 @@
var wireIDTestData = []struct {
name string
- id *jsonrpc2.ID
+ 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`,
@@ -64,7 +56,7 @@
func TestIDEncode(t *testing.T) {
for _, test := range wireIDTestData {
t.Run(test.name, func(t *testing.T) {
- data, err := json.Marshal(test.id)
+ data, err := json.Marshal(&test.id)
if err != nil {
t.Fatal(err)
}
@@ -81,12 +73,8 @@
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 nil want %s", test.id)
+ } else if *got != test.id {
t.Errorf("got %s want %s", got, test.id)
}
})
diff --git a/internal/lsp/protocol/log.go b/internal/lsp/protocol/log.go
index 1f97134..328336f 100644
--- a/internal/lsp/protocol/log.go
+++ b/internal/lsp/protocol/log.go
@@ -27,7 +27,7 @@
if err == nil {
s.logMu.Lock()
defer s.logMu.Unlock()
- logIn(s.log, msg)
+ logCommon(s.log, msg, true)
}
return msg, count, err
}
@@ -35,7 +35,7 @@
func (s *loggingStream) Write(ctx context.Context, msg jsonrpc2.Message) (int64, error) {
s.logMu.Lock()
defer s.logMu.Unlock()
- logOut(s.log, msg)
+ logCommon(s.log, msg, false)
count, err := s.stream.Write(ctx, msg)
return count, err
}
@@ -60,23 +60,19 @@
// these 4 methods are each used exactly once, but it seemed
// better to have the encapsulation rather than ad hoc mutex
// code in 4 places
-func (m *mapped) client(id string, del bool) req {
+func (m *mapped) client(id string) req {
m.mu.Lock()
defer m.mu.Unlock()
v := m.clientCalls[id]
- if del {
- delete(m.clientCalls, id)
- }
+ delete(m.clientCalls, id)
return v
}
-func (m *mapped) server(id string, del bool) req {
+func (m *mapped) server(id string) req {
m.mu.Lock()
defer m.mu.Unlock()
v := m.serverCalls[id]
- if del {
- delete(m.serverCalls, id)
- }
+ delete(m.serverCalls, id)
return v
}
@@ -94,7 +90,13 @@
const eor = "\r\n\r\n\r\n"
-func logCommon(outfd io.Writer, msg jsonrpc2.Message, direction, pastTense string) {
+func logCommon(outfd io.Writer, msg jsonrpc2.Message, isRead bool) {
+ direction, pastTense := "Received", "Received"
+ get, set := maps.client, maps.setServer
+ if isRead {
+ direction, pastTense = "Sending", "Sent"
+ get, set = maps.server, maps.setClient
+ }
if msg == nil || outfd == nil {
return
}
@@ -108,7 +110,7 @@
id := fmt.Sprint(msg.ID())
fmt.Fprintf(&buf, "%s request '%s - (%s)'.\n", direction, msg.Method(), id)
fmt.Fprintf(&buf, "Params: %s%s", msg.Params(), eor)
- maps.setServer(id, req{method: msg.Method(), start: tm})
+ set(id, req{method: msg.Method(), start: tm})
case *jsonrpc2.Notification:
fmt.Fprintf(&buf, "%s notification '%s'.\n", direction, msg.Method())
fmt.Fprintf(&buf, "Params: %s%s", msg.Params(), eor)
@@ -118,21 +120,11 @@
fmt.Fprintf(outfd, "[Error - %s] %s #%s %s%s", pastTense, tmfmt, id, err, eor)
return
}
- cc := maps.client(id, true)
+ cc := get(id)
elapsed := tm.Sub(cc.start)
- fmt.Fprintf(&buf, "Received response '%s - (%s)' in %dms.\n",
- cc.method, id, elapsed/time.Millisecond)
+ fmt.Fprintf(&buf, "%s response '%s - (%s)' in %dms.\n",
+ direction, cc.method, id, elapsed/time.Millisecond)
fmt.Fprintf(&buf, "Result: %s%s", msg.Result(), eor)
}
outfd.Write([]byte(buf.String()))
}
-
-// Writing a message to the client, log it
-func logOut(outfd io.Writer, msg jsonrpc2.Message) {
- logCommon(outfd, msg, "Received", "Received")
-}
-
-// Got a message from the client, log it
-func logIn(outfd io.Writer, msg jsonrpc2.Message) {
- logCommon(outfd, msg, "Sending", "Sent")
-}
diff --git a/internal/lsp/protocol/protocol.go b/internal/lsp/protocol/protocol.go
index 92e3d33..8dded05 100644
--- a/internal/lsp/protocol/protocol.go
+++ b/internal/lsp/protocol/protocol.go
@@ -49,9 +49,9 @@
return sendParseError(ctx, reply, err)
}
if n, ok := params.ID.(float64); ok {
- canceller(*jsonrpc2.NewIntID(int64(n)))
+ canceller(jsonrpc2.NewIntID(int64(n)))
} else if s, ok := params.ID.(string); ok {
- canceller(*jsonrpc2.NewStringID(s))
+ canceller(jsonrpc2.NewStringID(s))
} else {
return sendParseError(ctx, reply, fmt.Errorf("request ID %v malformed", params.ID))
}
