internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
// 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 lsprpc
|
|
|
|
|
|
|
|
import (
|
|
|
|
"context"
|
|
|
|
"regexp"
|
2020-02-11 10:38:07 -07:00
|
|
|
"sync"
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
"testing"
|
|
|
|
"time"
|
|
|
|
|
2020-04-17 07:32:56 -06:00
|
|
|
"golang.org/x/tools/internal/event"
|
2020-03-30 15:09:42 -06:00
|
|
|
"golang.org/x/tools/internal/jsonrpc2"
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
"golang.org/x/tools/internal/jsonrpc2/servertest"
|
2020-02-19 08:17:48 -07:00
|
|
|
"golang.org/x/tools/internal/lsp/cache"
|
2020-02-19 08:18:21 -07:00
|
|
|
"golang.org/x/tools/internal/lsp/debug"
|
|
|
|
"golang.org/x/tools/internal/lsp/fake"
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
"golang.org/x/tools/internal/lsp/protocol"
|
2020-08-26 15:41:45 -06:00
|
|
|
"golang.org/x/tools/internal/testenv"
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
)
|
|
|
|
|
|
|
|
type fakeClient struct {
|
|
|
|
protocol.Client
|
|
|
|
|
|
|
|
logs chan string
|
|
|
|
}
|
|
|
|
|
|
|
|
func (c fakeClient) LogMessage(ctx context.Context, params *protocol.LogMessageParams) error {
|
|
|
|
c.logs <- params.Message
|
|
|
|
return nil
|
|
|
|
}
|
|
|
|
|
2020-05-14 21:15:12 -06:00
|
|
|
// fakeServer is intended to be embedded in the test fakes below, to trivially
|
|
|
|
// implement Shutdown.
|
|
|
|
type fakeServer struct {
|
|
|
|
protocol.Server
|
|
|
|
}
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
|
2020-05-14 21:15:12 -06:00
|
|
|
func (fakeServer) Shutdown(ctx context.Context) error {
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
return nil
|
|
|
|
}
|
|
|
|
|
2020-05-14 21:15:12 -06:00
|
|
|
type pingServer struct{ fakeServer }
|
|
|
|
|
|
|
|
func (s pingServer) DidOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error {
|
|
|
|
event.Log(ctx, "ping")
|
2020-02-21 15:31:45 -07:00
|
|
|
return nil
|
|
|
|
}
|
|
|
|
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
func TestClientLogging(t *testing.T) {
|
|
|
|
ctx, cancel := context.WithCancel(context.Background())
|
|
|
|
defer cancel()
|
|
|
|
|
|
|
|
server := pingServer{}
|
|
|
|
client := fakeClient{logs: make(chan string, 10)}
|
|
|
|
|
2020-02-28 08:30:03 -07:00
|
|
|
ctx = debug.WithInstance(ctx, "", "")
|
2020-07-08 11:49:07 -06:00
|
|
|
ss := NewStreamServer(cache.New(ctx, nil), false)
|
2020-02-19 08:17:48 -07:00
|
|
|
ss.serverForTest = server
|
2020-04-30 09:05:58 -06:00
|
|
|
ts := servertest.NewPipeServer(ctx, ss, nil)
|
2020-04-14 14:59:58 -06:00
|
|
|
defer checkClose(t, ts.Close)
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
cc := ts.Connect(ctx)
|
2020-04-29 06:51:37 -06:00
|
|
|
cc.Go(ctx, protocol.ClientHandler(client, jsonrpc2.MethodNotFound))
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
|
|
|
|
protocol.ServerDispatcher(cc).DidOpen(ctx, &protocol.DidOpenTextDocumentParams{})
|
|
|
|
|
|
|
|
select {
|
|
|
|
case got := <-client.logs:
|
|
|
|
want := "ping"
|
|
|
|
matched, err := regexp.MatchString(want, got)
|
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
|
|
|
if !matched {
|
|
|
|
t.Errorf("got log %q, want a log containing %q", got, want)
|
|
|
|
}
|
2020-02-11 10:38:07 -07:00
|
|
|
case <-time.After(1 * time.Second):
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
t.Error("timeout waiting for client log")
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2020-02-11 10:38:07 -07:00
|
|
|
// waitableServer instruments LSP request so that we can control their timing.
|
|
|
|
// The requests chosen are arbitrary: we simply needed one that blocks, and
|
|
|
|
// another that doesn't.
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
type waitableServer struct {
|
2020-05-14 21:15:12 -06:00
|
|
|
fakeServer
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
|
|
|
|
started chan struct{}
|
|
|
|
}
|
|
|
|
|
2020-02-11 10:38:07 -07:00
|
|
|
func (s waitableServer) Hover(ctx context.Context, _ *protocol.HoverParams) (*protocol.Hover, error) {
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
s.started <- struct{}{}
|
|
|
|
select {
|
|
|
|
case <-ctx.Done():
|
|
|
|
return nil, ctx.Err()
|
2020-02-11 10:38:07 -07:00
|
|
|
case <-time.After(200 * time.Millisecond):
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
}
|
2020-02-11 10:38:07 -07:00
|
|
|
return &protocol.Hover{}, nil
|
|
|
|
}
|
|
|
|
|
|
|
|
func (s waitableServer) Resolve(_ context.Context, item *protocol.CompletionItem) (*protocol.CompletionItem, error) {
|
|
|
|
return item, nil
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
}
|
|
|
|
|
2020-04-14 14:59:58 -06:00
|
|
|
func checkClose(t *testing.T, closer func() error) {
|
|
|
|
t.Helper()
|
|
|
|
if err := closer(); err != nil {
|
|
|
|
t.Errorf("closing: %v", err)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2020-05-14 21:15:12 -06:00
|
|
|
func setupForwarding(ctx context.Context, t *testing.T, s protocol.Server) (direct, forwarded servertest.Connector, cleanup func()) {
|
|
|
|
t.Helper()
|
|
|
|
serveCtx := debug.WithInstance(ctx, "", "")
|
2020-07-08 11:49:07 -06:00
|
|
|
ss := NewStreamServer(cache.New(serveCtx, nil), false)
|
2020-05-14 21:15:12 -06:00
|
|
|
ss.serverForTest = s
|
2020-04-30 09:05:58 -06:00
|
|
|
tsDirect := servertest.NewTCPServer(serveCtx, ss, nil)
|
2020-02-06 17:50:37 -07:00
|
|
|
|
2020-05-14 21:15:12 -06:00
|
|
|
forwarderCtx := debug.WithInstance(ctx, "", "")
|
2020-03-09 11:22:56 -06:00
|
|
|
forwarder := NewForwarder("tcp", tsDirect.Addr)
|
2020-04-30 09:05:58 -06:00
|
|
|
tsForwarded := servertest.NewPipeServer(forwarderCtx, forwarder, nil)
|
2020-05-14 21:15:12 -06:00
|
|
|
return tsDirect, tsForwarded, func() {
|
|
|
|
checkClose(t, tsDirect.Close)
|
|
|
|
checkClose(t, tsForwarded.Close)
|
|
|
|
}
|
|
|
|
}
|
2020-02-06 17:50:37 -07:00
|
|
|
|
2020-05-14 21:15:12 -06:00
|
|
|
func TestRequestCancellation(t *testing.T) {
|
|
|
|
ctx := context.Background()
|
|
|
|
server := waitableServer{
|
|
|
|
started: make(chan struct{}),
|
|
|
|
}
|
|
|
|
tsDirect, tsForwarded, cleanup := setupForwarding(ctx, t, server)
|
|
|
|
defer cleanup()
|
2020-02-06 17:50:37 -07:00
|
|
|
tests := []struct {
|
|
|
|
serverType string
|
2020-02-09 11:38:49 -07:00
|
|
|
ts servertest.Connector
|
2020-02-06 17:50:37 -07:00
|
|
|
}{
|
|
|
|
{"direct", tsDirect},
|
|
|
|
{"forwarder", tsForwarded},
|
|
|
|
}
|
|
|
|
|
|
|
|
for _, test := range tests {
|
|
|
|
t.Run(test.serverType, func(t *testing.T) {
|
2020-05-14 21:15:12 -06:00
|
|
|
cc := test.ts.Connect(ctx)
|
2020-04-01 11:19:48 -06:00
|
|
|
sd := protocol.ServerDispatcher(cc)
|
2020-05-14 21:15:12 -06:00
|
|
|
cc.Go(ctx,
|
2020-04-02 10:43:08 -06:00
|
|
|
protocol.Handlers(
|
|
|
|
jsonrpc2.MethodNotFound))
|
2020-03-30 15:09:42 -06:00
|
|
|
|
2020-02-11 10:38:07 -07:00
|
|
|
ctx := context.Background()
|
|
|
|
ctx1, cancel1 := context.WithCancel(ctx)
|
|
|
|
var (
|
|
|
|
err1, err2 error
|
|
|
|
wg sync.WaitGroup
|
|
|
|
)
|
|
|
|
wg.Add(2)
|
2020-02-06 17:50:37 -07:00
|
|
|
go func() {
|
2020-02-11 10:38:07 -07:00
|
|
|
defer wg.Done()
|
2020-04-01 11:19:48 -06:00
|
|
|
_, err1 = sd.Hover(ctx1, &protocol.HoverParams{})
|
2020-02-06 17:50:37 -07:00
|
|
|
}()
|
2020-02-11 10:38:07 -07:00
|
|
|
go func() {
|
|
|
|
defer wg.Done()
|
2020-04-01 11:19:48 -06:00
|
|
|
_, err2 = sd.Resolve(ctx, &protocol.CompletionItem{})
|
2020-02-11 10:38:07 -07:00
|
|
|
}()
|
|
|
|
// Wait for the Hover request to start.
|
2020-02-06 17:50:37 -07:00
|
|
|
<-server.started
|
2020-02-11 10:38:07 -07:00
|
|
|
cancel1()
|
|
|
|
wg.Wait()
|
|
|
|
if err1 == nil {
|
|
|
|
t.Errorf("cancelled Hover(): got nil err")
|
|
|
|
}
|
|
|
|
if err2 != nil {
|
|
|
|
t.Errorf("uncancelled Hover(): err: %v", err2)
|
|
|
|
}
|
2020-04-01 11:19:48 -06:00
|
|
|
if _, err := sd.Resolve(ctx, &protocol.CompletionItem{}); err != nil {
|
2020-02-11 10:38:07 -07:00
|
|
|
t.Errorf("subsequent Hover(): %v", err)
|
2020-02-06 17:50:37 -07:00
|
|
|
}
|
|
|
|
})
|
internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).
This change reorganizes Server instantiation as follows:
+ The lsp.Server is now purely an implementation of the protocol.Server
interface. It is no longer responsible for installing itself into the
jsonrpc2 Stream, nor for running itself.
+ A new package 'lsprpc' is added, to implement the logic of binding an
incoming connection to an LSP server session. This is put in a
separate package for lack of a clear home: it didn't really
philosophically belong in any of the lsp, cmd, or protocol packages.
We can perhaps move it to cmd in the future, but I'd like to keep it
as a separate package while I develop request forwarding.
simplified import graph:
jsonrpc2 ⭠ lsprpc ⭠ cmd
⭩ ⭦
lsp (t.b.d. client tests)
⭩ ⭨
protocol source
+ The jsonrpc2 package is extended to have a minimal API for running a
'StreamServer': something analogous to an HTTP server that listens
for new connections and delegates to a handler (but we couldn't use
the word 'Handler' for this delegate as it was already taken).
After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.
This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.
To test this change, the following improvements are made:
+ A servertest package is added to make it easier to run a test against
an in-process jsonrpc2 server. For now, this uses TCP but it could
easily be modified to use io.Pipe.
+ cmd tests are updated to use the servertest package. Unfortunately it
wasn't yet possible to eliminate the concept of `remote=internal` in
favor of just using multiple sessions, because view initialization
involves calling both `go env` and `packages.Load`, which slow down
session startup significantly. See also golang.org/issue/35968.
Instead, the syntax for `-remote=internal` is modified to be
`-remote=internal@127.0.0.1:12345`.
+ An additional test for request cancellation is added for the
sessionserver package. This test uncovered a bug: when calling
Canceller.Cancel, we were using id rather than &id, which resulted in
incorrect json serialization (as only the pointer receiver implements
the json.Marshaller interface).
Updates golang/go#34111
Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 17:34:50 -07:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2020-02-19 08:17:48 -07:00
|
|
|
const exampleProgram = `
|
|
|
|
-- go.mod --
|
|
|
|
module mod
|
|
|
|
|
|
|
|
go 1.12
|
|
|
|
-- main.go --
|
|
|
|
package main
|
|
|
|
|
|
|
|
import "fmt"
|
|
|
|
|
|
|
|
func main() {
|
|
|
|
fmt.Println("Hello World.")
|
|
|
|
}`
|
|
|
|
|
2020-02-19 08:18:21 -07:00
|
|
|
func TestDebugInfoLifecycle(t *testing.T) {
|
2020-07-22 18:06:06 -06:00
|
|
|
sb, err := fake.NewSandbox(&fake.SandboxConfig{Files: exampleProgram})
|
2020-04-14 14:59:58 -06:00
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
|
|
|
defer func() {
|
2020-04-28 22:00:52 -06:00
|
|
|
if err := sb.Close(); err != nil {
|
2020-04-14 14:59:58 -06:00
|
|
|
// TODO(golang/go#38490): we can't currently make this an error because
|
|
|
|
// it fails on Windows: the workspace directory is still locked by a
|
|
|
|
// separate Go process.
|
|
|
|
// Once we have a reliable way to wait for proper shutdown, make this an
|
|
|
|
// error.
|
|
|
|
t.Logf("closing workspace failed: %v", err)
|
|
|
|
}
|
|
|
|
}()
|
|
|
|
|
|
|
|
baseCtx, cancel := context.WithCancel(context.Background())
|
|
|
|
defer cancel()
|
2020-02-28 08:30:03 -07:00
|
|
|
clientCtx := debug.WithInstance(baseCtx, "", "")
|
|
|
|
serverCtx := debug.WithInstance(baseCtx, "", "")
|
2020-02-19 08:18:21 -07:00
|
|
|
|
2020-02-28 08:30:03 -07:00
|
|
|
cache := cache.New(serverCtx, nil)
|
2020-07-08 11:49:07 -06:00
|
|
|
ss := NewStreamServer(cache, false)
|
2020-04-30 09:05:58 -06:00
|
|
|
tsBackend := servertest.NewTCPServer(serverCtx, ss, nil)
|
2020-02-19 08:18:21 -07:00
|
|
|
|
2020-03-09 11:22:56 -06:00
|
|
|
forwarder := NewForwarder("tcp", tsBackend.Addr)
|
2020-04-30 09:05:58 -06:00
|
|
|
tsForwarder := servertest.NewPipeServer(clientCtx, forwarder, nil)
|
2020-02-19 08:18:21 -07:00
|
|
|
|
2020-02-28 08:30:03 -07:00
|
|
|
conn1 := tsForwarder.Connect(clientCtx)
|
2020-05-07 10:25:33 -06:00
|
|
|
ed1, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(clientCtx, conn1, fake.ClientHooks{})
|
2020-02-19 08:18:21 -07:00
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
2020-06-05 07:17:05 -06:00
|
|
|
defer ed1.Close(clientCtx)
|
2020-02-28 08:30:03 -07:00
|
|
|
conn2 := tsBackend.Connect(baseCtx)
|
2020-05-07 10:25:33 -06:00
|
|
|
ed2, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(baseCtx, conn2, fake.ClientHooks{})
|
2020-02-19 08:18:21 -07:00
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
2020-06-05 07:17:05 -06:00
|
|
|
defer ed2.Close(baseCtx)
|
2020-02-19 08:18:21 -07:00
|
|
|
|
2020-02-28 08:30:03 -07:00
|
|
|
serverDebug := debug.GetInstance(serverCtx)
|
2020-02-19 08:18:21 -07:00
|
|
|
if got, want := len(serverDebug.State.Clients()), 2; got != want {
|
|
|
|
t.Errorf("len(server:Clients) = %d, want %d", got, want)
|
|
|
|
}
|
|
|
|
if got, want := len(serverDebug.State.Sessions()), 2; got != want {
|
|
|
|
t.Errorf("len(server:Sessions) = %d, want %d", got, want)
|
|
|
|
}
|
2020-02-28 08:30:03 -07:00
|
|
|
clientDebug := debug.GetInstance(clientCtx)
|
2020-02-19 08:18:21 -07:00
|
|
|
if got, want := len(clientDebug.State.Servers()), 1; got != want {
|
|
|
|
t.Errorf("len(client:Servers) = %d, want %d", got, want)
|
|
|
|
}
|
|
|
|
// Close one of the connections to verify that the client and session were
|
|
|
|
// dropped.
|
2020-06-05 07:17:05 -06:00
|
|
|
if err := ed1.Close(clientCtx); err != nil {
|
2020-02-19 08:18:21 -07:00
|
|
|
t.Fatal(err)
|
|
|
|
}
|
2020-06-05 07:17:05 -06:00
|
|
|
/*TODO: at this point we have verified the editor is closed
|
|
|
|
However there is no way currently to wait for all associated go routines to
|
|
|
|
go away, and we need to wait for those to trigger the client drop
|
|
|
|
for now we just give it a little bit of time, but we need to fix this
|
|
|
|
in a principled way
|
|
|
|
*/
|
|
|
|
start := time.Now()
|
|
|
|
delay := time.Millisecond
|
|
|
|
const maxWait = time.Second
|
|
|
|
for len(serverDebug.State.Clients()) > 1 {
|
|
|
|
if time.Since(start) > maxWait {
|
|
|
|
break
|
|
|
|
}
|
|
|
|
time.Sleep(delay)
|
|
|
|
delay *= 2
|
|
|
|
}
|
|
|
|
if got, want := len(serverDebug.State.Clients()), 1; got != want {
|
|
|
|
t.Errorf("len(server:Clients) = %d, want %d", got, want)
|
|
|
|
}
|
2020-02-19 08:18:21 -07:00
|
|
|
if got, want := len(serverDebug.State.Sessions()), 1; got != want {
|
|
|
|
t.Errorf("len(server:Sessions()) = %d, want %d", got, want)
|
|
|
|
}
|
|
|
|
}
|
2020-05-14 21:15:12 -06:00
|
|
|
|
|
|
|
type initServer struct {
|
|
|
|
fakeServer
|
|
|
|
|
|
|
|
params *protocol.ParamInitialize
|
|
|
|
}
|
|
|
|
|
|
|
|
func (s *initServer) Initialize(ctx context.Context, params *protocol.ParamInitialize) (*protocol.InitializeResult, error) {
|
|
|
|
s.params = params
|
|
|
|
return &protocol.InitializeResult{}, nil
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestEnvForwarding(t *testing.T) {
|
2020-08-26 15:41:45 -06:00
|
|
|
testenv.NeedsGo1Point(t, 13)
|
2020-05-14 21:15:12 -06:00
|
|
|
server := &initServer{}
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
|
|
|
defer cancel()
|
|
|
|
_, tsForwarded, cleanup := setupForwarding(ctx, t, server)
|
|
|
|
defer cleanup()
|
|
|
|
|
|
|
|
conn := tsForwarded.Connect(ctx)
|
|
|
|
conn.Go(ctx, jsonrpc2.MethodNotFound)
|
|
|
|
dispatch := protocol.ServerDispatcher(conn)
|
|
|
|
initParams := &protocol.ParamInitialize{}
|
|
|
|
initParams.InitializationOptions = map[string]interface{}{
|
|
|
|
"env": map[string]interface{}{
|
|
|
|
"GONOPROXY": "example.com",
|
|
|
|
},
|
|
|
|
}
|
|
|
|
_, err := dispatch.Initialize(ctx, initParams)
|
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
|
|
|
if server.params == nil {
|
|
|
|
t.Fatalf("initialize params are unset")
|
|
|
|
}
|
|
|
|
env := server.params.InitializationOptions.(map[string]interface{})["env"].(map[string]interface{})
|
|
|
|
|
|
|
|
// Check for an arbitrary Go variable. It should be set.
|
|
|
|
if _, ok := env["GOPRIVATE"]; !ok {
|
|
|
|
t.Errorf("Go environment variable GOPRIVATE unset in initialization options")
|
|
|
|
}
|
|
|
|
// Check that the variable present in our user config was not overwritten.
|
|
|
|
if v := env["GONOPROXY"]; v != "example.com" {
|
|
|
|
t.Errorf("GONOPROXY environment variable was overwritten")
|
|
|
|
}
|
|
|
|
}
|