The error messages from view cancellation clutter up the logs when
testing, especially if you're running a single subtest.
A few quick staticcheck fixes in the CL also.
Change-Id: Ia1ed5360ac754023c589ed526ec0ed3e94a79b2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The next version of VS Code Go has to be released before this can be
released.
Fixesgolang/go#37966
Change-Id: I6b97f2a3ff9c8400ae85b815d8bf83a9b12b1069
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This allows us to register a telemetry exporter that works with mulitple active
debug instances.
It also means we don't have to store the debug information in our other objects.
Change-Id: I9a9d5b0407c3352b6eaff80fb2c434ca33f4e397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For testability, and to support the exchange of debug information across
Forwarder and server, it is helpful to encapsulate all debug information
on the instance object.
This CL moves all state in the debug package into a new 'State' type,
that is added as a field on the debug.Instance. While doing so, common
functionality for object collections is factored out into the objset
helper type.
Also add two new debug object types: Client and Server. These aren't yet
used, but will be in a later CL (and frankly it was easier to leave them
in this CL than to more carefully rewrite history...).
Updates golang/go#34111
Change-Id: Ib809cd14cb957b41a9bcbd94a991f804531a76ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220078
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Create a real type for protocol.DocumentURIs. Remove span.NewURI in
favor of path/URI-specific constructors. Remove span.Parse's ability to
parse URI-based spans, which appears to be totally unused.
As a consequence, we no longer mangle non-file URIs to start with
file://, and crash all over the place when one is opened.
Updates golang/go#33699.
Change-Id: Ic7347c9768e38002b4ad9c84471329d0af7d2e05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219482
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
The passed-in Context is not used, and creates the illusion of a startup
dependency problem: existing code is careful to pass in the context
containing the correct Client instance.
This allows passing in a source.Session, rather than a source.Cache,
into lsp server constructors.
Updates golang/go#34111
Change-Id: I081ad6fa800b846b63e04d7164577e3a32966704
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215740
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is a continuation of the discussion on
https://github.com/microsoft/vscode-go/issues/2920. Add corresponding
checks to internal/lsp/cmd/capabilities_test.go.
Change-Id: I51af05dee9e7ecea0e40733dd4c5ca3dfb8f4dd8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209859
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This is causing the "command '' not found" errors that we've been
seeing, specifically reported in microsoft/vscode-go#2920.
Also, fixed an issue with import organization in single-line files that
was caught as a result of this.
Change-Id: I2dfedb5d1b8dda976f356b0d6fcd146e53f1a650
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209219
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We've had a couple of breakages with changes to the protocol that are
bugs on our end. It's hard to review changes to the protocol, and it's
not reasonable to expect that we would remember the correct types for
everything, so we should have a test that validates some basic expectations
about the expected responses. We can add more here as issues come up.
Also, change RenameProvider back to an interface.
Updates golang/go#32703
Change-Id: Ic5f0b0ece40b05e4425cd98ab7bf18db3ad74601
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208272
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
