1
0
mirror of https://github.com/golang/go synced 2024-09-30 20:18:33 -06:00
Commit Graph

61 Commits

Author SHA1 Message Date
Rob Findley
f2c07d7d8e internal/lsp/lsprpc: improvements to daemon logging
The gopls daemon had different default logging behavior than the sidecar
gopls: by default, the daemon was started with -logfile=auto.
Additionally, because most logs are reflected back to the forwarder, the
actual daemon logs have very little (if any) information.

This means that if you simply start gopls with -remote=auto, you'll get
a single logfile named /tmp/gopls-<pid>.log, which is mostly empty. This
is not a delightful experience.

Fix this via several improvements:
 + Log lifecycle events in the Daemon, to give the log a purpose.
 + Give the daemon a new default log location:
   /tmp/gopls-daemon-<pid>.log.
 + Don't pass -logfile=auto to the daemon by default.

Fixes golang/go#40105

Change-Id: I5e91ea98b4968c512bce76a596bbae441f461a66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-16 19:33:22 +00:00
Rob Findley
e404ca24fb internal/lsp/regtest: use a common directory for regtest sandboxes
For easier debugging (and less cruft if regtests are ctrl-c'ed), root
all regtest sandboxes in a common directory.

This also tries one last time to clean up the directory, and fails on an
error. This might be flaky on windows, but hasn't been so far...

Also give regtest sandboxes names derived from their test name.

Updates golang/go#39384
Updates golang/go#38490

Change-Id: Iae53c29e75f5eb2b8d938d205fbeb463ffc82eb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-09 18:15:24 +00:00
Rebecca Stambler
f01a4bec33 internal/lsp: support opening single files
This change permits starting gopls without a root URI or any workspace
folders. If no view is found for an opened file, we try to create a new
view based on the module root of that file. In GOPATH mode, we just
use the directory containing the file.

I wrote a regtest for this by adding a new configuration that gets
propagated to the sandbox. I'm not sure if this is the best way to do
that, so I'll let Rob advise.

Fixes golang/go#34160

Change-Id: I3deca3ac1b86b69eba416891a1c28fd35658a2ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01 22:10:12 +00:00
Rob Findley
2b5917cebf internal/lsp/lsprpc: forward the go environment in initialize requests
The gopls workspace environment defaults to the process environment in
which gopls was started. This means that when switching environments,
gopls can potentially get a different environment when connecting as an
editor sidecar from when forwarding requests to the daemon.

To (hopefully mostly) mitigate this pain point, inject the Go
environment when forwarding the 'initialize' request, which contains
InitializationOptions containing the 'env' configuration. We could go
further and send the entire os.Environ(), but that seems problematic
both in its unbounded nature, and because in many cases the user may not
actually want to send their process env over the wire. Gopls behavior
should *mostly* be parameterized by gopls binary and Go env, and after
this change these should match for forwarder and daemon.

For go1.15, Explicitly set the GOMODCACHE environment variable in the
regtest sandbox. Without this, regtests were failing in the forwarded
environment because they implicitly shared a module cache.

Fixes golang/go#37830

Change-Id: Ic1b335506f8b481505eac9f74c0df6293dc07158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234109
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-16 15:43:56 +00:00
Ian Cottrell
54cf04ef09 internal/lsp: decouple client and server debug
This uses log messages to convey information to the debug system, which
has the benefit of logging even if the debug pages are not active and
also not requiring systems to reach into the debug system or require
extra lifetime tracking Not all things are decoupled yet as there are a
couple of places (notably the handshaker) that read information out of
the debug system.

Change-Id: Iec1f81c34ab3b11b3e3d6e6eb39b98ee5ed0d849
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236337
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-11 13:49:48 +00:00
Ian Cottrell
c42cb6316f internal/lsp: minor cleanup of the client and server debug
This removes the interfaces and the debug structs in the lsprpc package
in favour of just having the debug structs in the debug package.

Change-Id: I67541688444f4ef367007740c5446dcd7be6575f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236198
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:50 +00:00
Ian Cottrell
ce53dc4445 internal/lsp: clean out the debug handling
This removes all the cache/session/view hooks from the lsp and instead
uses normal introspection from the client to find all the entities
rather than keeping shadow copies of the object graph in the debug page
handler.
This required the addition only of the ability to view the sessions open
on a cache and exposing the unique identifier for all the objects, both
of which are useful and reasonable things to have in the API anyway.

Change-Id: Ic19903e7b19832ca79290954ec373d4177789742
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236197
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:14 +00:00
Ian Cottrell
7147197327 internal/lsp: now connection shutdown works, use it
This removes a TODO in the tests and has them check for shutdown
correctly.
This also required adding a Close to editor that does the Shutdown/Exit
sequence rather than just Shutdown.

Change-Id: I6433b76eb2dd1fe40b2332685fdf25ebc4fd4577
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236717
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:04 +00:00
Ian Cottrell
ef124de36d internal/jsonrpc2: change StreamServer to operate on Conn instead of Stream
Really the name is wrong now, but this is just a stepping stone towards removing
it entirely in favour of a new listener/dialer/server/client pattern, so I am
minimizing the churn by leaving the names alone for now.

Change-Id: I771d117490763ebe05ed2a8c52d490deeb4d5333
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232878
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-03 17:07:01 +00:00
Ian Cottrell
8a3674bff3 internal/lsp: change exit handling
Exit now closes the connection rather than exiting the process.
This allows things to shutdown gracefully, and removes special
cases. It also allows the tests to call CloseEditor instead of
just Shutdown, which prevents goroutine leaks.

Change-Id: I26121ba5d393ef74ce0e912611c8b3817e3691ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231798
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-03 13:14:19 +00:00
Ian Cottrell
cc40288be8 internal/lsp: change logging stream to be a framer
Change-Id: Id9e17e98ca00f31424068e875851b5f9008c6fe8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231797
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-03 13:12:46 +00:00
Ian Cottrell
9089ff1aee internal/jsonrpc2: switch to building streams on top of net.Conn
This allows us to rely on higher level functionality like timeouts and
close cancelling pending reads cleanly.

Change-Id: I3a43d21ed35d3da1eb818ea22f8d02201488a1d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230464
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-03 13:12:18 +00:00
Ian Cottrell
e84ca95fee internal/jsonrpc2: add the ability to close connections
Also the ability to wait for them to correctly close.

Change-Id: I198c9e24a21c04d5c05bae7a4a0f503429ab0346
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231699
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-03 13:11:45 +00:00
Rob Findley
0aa9f2fd80 internal/lsp/cmd: clean up remote flag descriptions
The format of descriptions for the `-remote.*` flags were inconsistent.
Clean them up.

Also remove a TODO in lsprpc.go about adding a test for telemetry. That
is enough of a separate concern (and one that is rapidly changing) that
I no longer think this TODO makes sense.

Change-Id: Id14796000634ae4be4b480d0386b1da9069737c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234113
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-15 20:48:22 +00:00
Rob Findley
b8469989bc internal/lsp/fake: fix some messiness around client hooks
While writing the fake editor, I added some state tracking without using
it (log messages, events etc). We have since duplicated this logic in
the regtest package using client hooks.

Fix two messy aspects of this:
 - remove the state tracking in the editor
 - pass in the client hooks when connecting, so that they may be used
   without locking, and so that we do not miss any hooks that may fire
   during session initialization.

Change-Id: I24c17a28e9cfa4fca32b7ddd17c7bf01cbb12e0f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232744
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 18:47:54 +00:00
Rob Findley
cb8d9cd245 internal/lsp: support configurable codeLens
Some code lenses may be undesirable for certain users or editors -- for
example a code lens that runs tests, when VSCode already supports this
functionality outside of the LSP. To handle such situations, support
configuring code lenses via a new 'codelens' gopls option.

Add support for code lens in regtests, and use this to test the new
configuration. To achieve this, thread through a new 'EditorConfig' type
that configures the fake editor's LSP session. It made sense to move the
test Env overlay onto this config object as well.

While looking at them, document some types in source.Options.

Change-Id: I961077422a273829c5cbd83c3b87fae29f77eeda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232680
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 18:47:35 +00:00
Rebecca Stambler
88e38c1d8d internal/lsp: make sure diagnostics only refer to existing files
We were previously sending diagnostics for nonexistent files, and then
adding them to the snapshot in the process. Remove this behavior, and
add a regression test. Case insensitive filesystems were too confusing
to write a test for, but fortunately, Filippo reported another instance
of this bug, so I used that for the regression test.

Fixes golang/go#38602

Change-Id: I4ef6b51944f3338e838875a5aafffd066e8392f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230315
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 02:01:22 +00:00
Rob Findley
c20a87c16a internal/lsp/fake: split up and rename the Workspace type
The Workspace type has accumulated too much additional functionality of
late: managing the Env, GOPATH, and GOPROXY in addition to the working
directory. Additionally, the name 'Workspace' can easily be confused
with 'workspaceFolder' in the LSP spec, and they're not quite
equivalent.

Split off a Proxy type to be responsible for the fake module proxy, and
a Workdir type to be responsible for working with the temporary
directory. Rename what remains of 'Workspace' to a more appropriate name
for such a collection of resources: Sandbox.

This is mostly just moving things around, with one significant change in
functionality: previously our three temporary directories (workdir,
gopath, and goproxy) were in separate toplevel directories below
$TMPDIR. Now they are all below a new sandbox temp directory, so that
they are correlated in the filesystem and can be cleaned up with one
call to os.RemoveAll.

Change-Id: I1e160a31ae22f0132355117df941fe65822900eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230758
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-06 18:17:57 +00:00
Rob Findley
9bfbc38543 internal/lsp: fix incorrect format strings when calling event.Error
I noticed that in a couple places, event.Error was called with a
message containing formatting verb. This was my likely done out of
habit, but is an incorrect use of the API: err is not formatted in the
message but is rather applied as an event label.

Remove the unused formatting verbs.

Change-Id: I52f914da81e338013c7449066e5d9ffa40a0a4c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232137
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-04 19:35:31 +00:00
Ian Cottrell
535e1470ec internal/lsp: use %w in error wrappers
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.

Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 14:52:14 +00:00
Ian Cottrell
7b212d60a1 internal/event: renaming the main event API functions
event.Log removed
event.Print -> event.Log
event.Record -> event.Metric
event.StartSpan -> event.Start

In order to support this core now exposes the MakeEvent and Export functions.

Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:21:36 +00:00
Ian Cottrell
cf0cb92717 internal/telemetry: renaming to internal/event
internal/telemetry/event was renamed to internal/event/core
Some things were partly moved from internal/telemetry/event straight to
internal/event to minimize churn in the following restructuring.

Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:20:48 +00:00
Ian Cottrell
434f7a8fef internal/jsonrpc2: add concrete types for Call, Notify and Response
The previous implementation was exposing the details of the wire format
and resulted in non idomatic go, detecting the presence of absence of
values in fields to deterimine the message type.
Now the messages are distinct types and we use type switches instead.
Request still exists as an interface to expose the shared behaviour of
Call and Notification, as this is the type accepted by handlers.
The set of messages is deliberately closed by using a private methods on the
interfaces.

Change-Id: I2cf15ee3923ef4688670c62896f81f760c77fe04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228719
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-20 21:05:32 +00:00
Rob Findley
92fa1ff4b1 internal/lsp/regtest: add support for custom test proxy data
Certain regtests require referencing external data. To support this, add
the ability to use a file-based proxy populated with testdata.

To expose this configuration, augment the regtest runner with variadic
options. Also use this to replace the Runner.RunInMode function.

Add a simple regtest that uses this functionality.

Updates golang/go#36879

Change-Id: I7e6314430abcd127dbb7bca12574ef9935bf1f83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228235
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-16 19:38:27 +00:00
Rob Findley
405595e0b5 internal/lsp/fake: be more careful when closing the workspace
Closing the workspace has frequently been failing on Windows, due to
file locks held by the go command.

This change makes several tests more careful to check errors when
closing resources, and defers closing the regtest workspaces until the
entire test suite completes, at which point it is much more likely that
closing the workspace will succeed.

If this change results in test flakes on Windows, we should temporarily
demote errors in regtest.Runner.Close to a t.Log.

Updates golang/go#38490

Change-Id: Ibd2f7dd0e0e2faecfa0ca8c60237fc72e64f6719
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228231
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-16 19:25:41 +00:00
Ian Cottrell
1f08ef6002 internal/jsonrpc2: split reply from request
reply is now passed to handlers separately from request, which allows it to be
substituted by handlers.
This also makes the handler signature much closer to http (which has
ResponseWriter)

Change-Id: I12be2e8e8b9bd508982ba43c9092709429284eaf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227839
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-13 01:58:12 +00:00
Ian Cottrell
b854406a88 internal/jsonrpc2: make the wire structures private
The wire structures do not need to be public, and making them so might make it
harder to keep the package correct without breaking changes if the protocol
changes in the future.

Change-Id: I03a5618c63c9f7691183d4285f88a177ccdd3b35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227838
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-13 01:57:29 +00:00
Ian Cottrell
2595e72532 internal/jsonrpc2: replace NewErrorf with fmt.Errorf
We utilize error wrapping to recover the error codes when needed.
The code constants are also replaced by fully declared errors with
human readable messages.

Change-Id: I8edeb05f5028e99966e4ca28151f644f008d4e7d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227837
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-13 01:57:14 +00:00
Ian Cottrell
ae9902aceb internal/lsp: remove the CallCanceller
This required changing the jsonrpc.Conn.Call signature to also return the
request ID so it can be cancelled.
The protocol package now declares the Call function which wrapps up
Conn.Call and then sends a cancel message if the context was
cancelled during the call.
There is a small chance that a context can be cancelled on a
request that has already completed, but it is safe to do so.

Change-Id: Ic8040c193e1dd4ef376ad21194b1d0ea82145976
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-10 13:26:12 +00:00
Ian Cottrell
ccaaa5c26f internal/jsonrpc2: dont add any handlers by default
This pushes the handler construction out to the user, allowing flexability of
use, and is the final stage of the switch to the new handler API.

Change-Id: Id2e61813a817df0d6e4d20dd47ce8c92b0ae87db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227024
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-06 14:44:07 +00:00
Ian Cottrell
17cc17e0bb internal/jsonrpc2: remove the legacy interface
We can do cancelling at the top level handler now, it can drop the cancel
messages themselves before they enter the queue stage, and also track
all the events as they flow through it.
The ugly part is the OnCancelled interface, which is a bit clunky.

Change-Id: I3fa972198625fb3517fdecc740d1a3fdb19a188a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226959
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-06 14:43:58 +00:00
Ian Cottrell
44c82bac18 internal/jsonrpc2: make it an error to fail to call Reply
It is now a programatic error to have a handler registered to a connection that
does not call reply for all messages, including notifications.
This normalizes the flow making the code easier to understand  and fixes a
couple of long standing hard to find bugs.

Change-Id: If41c39ece70e3bc64420abefac75ec647a8f8b37
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226838
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-06 13:49:19 +00:00
Ian Cottrell
6dc6d5718f internal/jsonrpc2: change handler to a function type
Handler is now a function type that mapps to what used to be the Deliver method.
The only handler that used other methods was Canceller, for now that still
exists as LegacyHooks. Once the handlers are fully cleaned up we should be able
to re-implement canceller as handler middleware.
Each connection is now only allowed one handler, and it is passed to the Run
method, but handlers are composable.

Change-Id: I370e0459df851bb9c9c2a679b99cff073b94489e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226479
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-06 13:48:45 +00:00
Ian Cottrell
80f63e2b9b internal/lsp: rewrite the stats using the newer telemetry features
This allows us to reduce the handler interface and delete the telemetry handler.
It is also safer and faster, and can be easily disabled along with the rest of
the telemetry system.

Change-Id: Ia4961d7f2e374f7dc22360d6a4020a065bfeae6f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225957
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 00:50:33 +00:00
Ian Cottrell
8dcfad9e01 internal/telemetry: switch metrics to use the event system
Change-Id: If036530ffe47e7df925bcf6592f664d6020a3d65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223997
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:44:30 +00:00
Rob Findley
9a0fabac01 internal/lsp: fix errors found by staticcheck
While experimenting with different static analysis on x/tools, I noticed
that there are many actionable diagnostics found by staticcheck. Fix the
ones that were not false positives.

Change-Id: I0b68cf1f636b57b557db879fad84fff9b7237a89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222248
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-19 19:20:54 +00:00
Ian Cottrell
540150da73 internal/telemetry: add type safe tag keys
This changes the way keys work, there is still a single internal key
implementation for performance reasons, but the public interface is a set of key
implementations that have type safe Of and Get methods.
This also hides the implemenation of Tag so that we can modify the storage form
and find a more efficient storage if needed.

Change-Id: I6a39cc75c2824c6a92e52d59f16e82e876f16e9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223137
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:29:43 +00:00
Ian Cottrell
04208b9e8a internal/lsp: move the telemetry package
Move the lsp specific telemetry package that just declares the labels under the
debug package and call it tag, to make all the usages much more readable.

Change-Id: Ic89b3408dd3b8b3d914cc69d81f41b8919aaf424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222850
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:49 +00:00
Rob Findley
30fd94b347 internal/lsp/lsprpc: expose configuration for auto-started daemon
Three new flags are added to the serve command, and threaded through to
the LSP forwarder:
 -remote.listen.timeout: -listen.timeout for the auto-started daemon
 -remote.debug: -debug for the auto-started daemon
 -remote.logfile: -logfile for the auto-started daemon

As part of this change, no longer enable debugging the daemon by
default.

Notably none of this configuration affects serving, so modifying this
configuration has been chosen not to change the path to the automatic
daemon. In other words, this configuration has effect only for the
forwarder process that starts the daemon: all others will connect to the
daemon and inherit whatever configuration it had at startup. This should
be OK, because in the common case this configuration should be static
across all clients (e.g., many Vim sessions all sharing the same
.vimrc).

Exposing this configuration made the signature of lsprpc.NewForwarder
a bit hard to understand, so I decided to go ahead and switch to a
variadic options pattern for initializing both the Forwarder and
StreamServer, the latter just for consistency with the Forwarder.

Updates golang/go#34111

Change-Id: Iefb71e337befe08b23e451477d19fd57e69f36c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222670
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-03-12 19:43:16 +00:00
Rob Findley
bd435c612c internal/lsp/cmd: add an inspect verb
Add a new toplevel `inspect` verb to the gopls command, to expose the
internal state of a running gopls server. For now, this verb exposes a
single subcommand `sessions`, which lists some information about current
server sessions on a gopls daemon.

This can be used even if the debug server is not running, which will
become the default in a later CL.

Updates golang/go#34111

Change-Id: Ib7d654a659fa47280584f9a7301b952cbccc565a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222669
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-12 17:21:45 +00:00
Ian Cottrell
206ec5b82a internal/lsp: migrate telemetry to using the event package
Change-Id: Idc662385ed08bd62593ccd1d54afd3fa8c1a7d29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 03:59:16 +00:00
Rob Findley
aa4048aca1 internal/lsp/lsprpc: don't connect to sockets owned by different users
When running gopls as a forwarder it attempts to forward the LSP to a
remote daemon. On posix systems, by default this uses a unix domain
socket at a predictable filesystem location.

As an extra precaution, attempt to verify that the remote socket is in
fact owned by the current user.

Also, change the default TCP listen address used on windows to bind to
localhost.

Updates golang/go#34111

Change-Id: Ib24886d290089a773851c5439586c3ddc9eb797d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222246
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-09 18:08:59 +00:00
Rob Findley
d1d1f200c6 internal/lsp/lsprpc: use Setsid on POSIX GOOSes, to avoid SIGTERMs
When the gopls daemon is automatically managed (-remote=auto), it will
be started by one of the forwarder gopls processes that was in turn
started by the editor. By default, this puts it in the same process
group as the forwarder gopls.

Some editors (at least Vim) send SIGTERM to the process groups of
sidecar processes when exiting. This can cause the gopls daemon to
terminate, thereby losing state.

Rather than ignore SIGTERM (which is bound to be editor dependent
anyway), let's just put the gopls daemon in a separate session.

Updates golang/go#34111

Change-Id: I71386fb54b8c2efe1c565f59763f46693a7d48b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221220
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-05 21:44:44 +00:00
Ian Cottrell
4183ba16a9 internal/lsp: move the debug.Instance onto the Context
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>
2020-03-03 22:54:53 +00:00
Pontus Leitzler
066e0c0245 internal/lsp/lsprpc: use localhost for remote gopls debug interface
When using the experimental feature where gopls runs as deamon, the
remote is spawned with debug enabled. It currently exposes the debug
interface to all network interfaces. This should be configurable in
the future, but until then we should at least keep it on the
localhost interface.

This change starts the debug on the local interface instead of all.

Updates golang/go#34111

Change-Id: I3184268dd434ae11ff5f8c3c57a229d22c158196
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221697
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-01 22:23:51 +00:00
Rob Findley
45849e8256 internal/lsp/lsprpc: wait for the handshake before running clientConn
By running the client connection before the forwarder-remote handshake
completes, we introduce a race in the TestDebugInfoLifecycle: the editor
may connect and initialize before the handshake, and assertions on the
debug state fail.

Requiring that the handshake complete before running the client
connection fixes this flakiness. It also seems like a good thing to
have the handshake complete before proceeding with any LSP.

Fixes golang/go#37444

Change-Id: I07187797b4759bcaf99f5b0a0b3cd7ac7de34f43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220903
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-25 18:38:23 +00:00
Rob Findley
daf22849ce internal/lsp/lsprpc: add a 1m timeout to auto-started gopls
It would be bad behavior if a gopls forwarder process started the gopls
daemon and it stuck around indefinitely. Add an idle timeout by default
for the gopls daemon.

Updates golang/go#34111

Change-Id: I0408a1e6a3b89d862803ae5c439d6aa0d8ed9494
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220521
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 23:36:36 +00:00
Rob Findley
63caf62cea internal/lsp/lsprpc: clean up client session on disconnection
Gopls behavior on disconnection is currently somewhat undefined, because
it hasn't mattered when there was a single gopls session per binary
invocation. With golang/go#34111, this changes.

Checks are added to ensure clients and sessions are cleaned up when an LSP
connection closes. Also, normal client disconnection is differentiated
with the jsonrpc2.ErrDisconnected value.

Updates golang/go#34111

Change-Id: I74d48ad6dcfc30d11f7f751dcffb20c18a4cbaa3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220519
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 23:23:05 +00:00
Rob Findley
a208025ccb internal/lsp/lsprpc: automatically resolve and start the remote gopls
Most users will not want to manage their own gopls instance, but may
still want to benefit from using a shared instance.

This CL adds support for an 'auto' network type that can be encoded in
the -remote flag similarly to UDS (i.e. -remote="auto;uniqueid"). In
this mode, the actual remote address will be resolved automatically
based on the executing environment and unique identifier, and the remote
server will be started if it isn't already running.

Updates golang/go#34111

Change-Id: Ib62159765a108f3645f57709b8ff079b39dd6727
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220137
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 22:51:55 +00:00
Rob Findley
20f46356b3 internal/lsp/lsprpc: add a handshake between forwarder and remote
In the ideal future, users will have one or more gopls instances, each
serving potentially many LSP clients. In order to have any hope of
navigating this web, clients and servers must know about eachother.

To allow for such an exchange of information, this CL adds an additional
handler layer to the serving configured in the lsprpc package. For now,
forwarders just use this layer to execute a handshake with the LSP
server, communicating the location of their logs and debug addresses.

Updates golang/go#34111

Change-Id: Ic7432062c01a8bbd52fb4a058a95bbf5dc26baa3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220081
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 22:51:04 +00:00