When caching file data specific to a session (anything with a Version or
tied to a view), we now need to be more careful about the existence of
multiple sessions.
This change fixes a few places where we appear to cache session data
without explicitly referring to the session. In principal this could
cause data corruption in multi-session gopls instances, but I have not
been able to force this to occur in either manual or automated testing.
Also fix a data race to the unsaved overlays:
https://storage.googleapis.com/go-build-log/588ee798/linux-amd64-race_d0762522.log
Updates golang/go#34111
Change-Id: I47117da1aba1afc2e211785544ad3f8b3416d15d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reading the docstring, it looks like the sense of the type assertion was
accidentally inverted in this function. Fix it to what I believe was
intended.
Fixesgolang/go#37687
Change-Id: Ief44a996f1a262527c2916d6b78b81dd35b2cf9e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222217
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
This means that tags also become cheap if there is no exporter and cleans up the
mess with how spans, tags and logs were related.
Also fixes the currently broken metrics that relied on the span tags.
Change-Id: I8e56b6218a60fd31a1f6c8d329dbb2cab1b9254d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222065
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
We need to use the filename derived from the symbol information, not the
one from CompiledGoFiles, in case of cgo packages.
No tests because workspace symbols are entangled with document symbols,
and the latter doesn't work in cgo packages.
Fixesgolang/go#37659.
Change-Id: Ic32293c542830a49b37c25ebf3b231771c3a4225
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222060
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change attaches a modTidyHandle to the snapshot instance and tries to reuse it as often as possible. It also modifies the modTidyKey to include the files that have not yet been saved. This was necessary because `go mod tidy` only runs using contents on disk.
So, if a user decides to add an import that is not in their modcache, we run diagnostics as normal and the ModTidyKey's imports field gets updated with the new import, we run `go mod tidy` but since the file was not saved yet, nothing changes. Then when the user eventually saves their file, we do not rerun `go mod tidy` because the imports hash has not changed from the time the file was in overlay to the time the file was saved on disk. To be able to account for this, we need the invalidate the ModTidyKey when imports change between saves.
Change-Id: I9e210a15cf009d222cecd7824c2a1a927957483b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219477
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We no longer use the span as the core type of tracing, instead that is an
artifact of the exporter, and start and end tracing is just event based.
This both makes the interface normalized, and also means the null exporter case
is considerably cheaper in memory and cpu.
See below for benchstat changes
name old time/op new time/op delta
TracingNoExporter-8 4.19µs ±12% 2.71µs ±11% -35.33% (p=0.000 n=20+20)
Tracing-8 24.1µs ± 3% 5.1µs ±17% -78.66% (p=0.000 n=16+20)
name old alloc/op new alloc/op delta
TracingNoExporter-8 2.32kB ± 0% 0.40kB ± 0% -82.76% (p=0.000 n=20+20)
Tracing-8 6.32kB ± 0% 2.32kB ± 0% -63.30% (p=0.000 n=20+20)
name old allocs/op new allocs/op delta
TracingNoExporter-8 35.0 ± 0% 15.0 ± 0% -57.14% (p=0.000 n=20+20)
Tracing-8 215 ± 0% 35 ± 0% -83.72% (p=0.000 n=20+20)
Change-Id: I3cf25871fa49584819504b5c19aa580e5dd03395
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221740
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
These are just like the logging benchmarks but for tracing instead.
Also makes the log writer write out tracing events as well if it is
not in only errors mode
Change-Id: Ie00d7c80f7e2b9433787603261950f70ab1c1e9d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221739
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This adds a type to events and makes the logging calls use it.
Change-Id: Iaa50fe2e332caae611b1e00424c878a3bc479feb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221741
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
The Tagger interface allowed for specifying a key in a tag list and having the
value be aquired from the context automatically. It was almost never used, and
the alternative (using the key to get the value) is not that much more long
winded, so it was not holding it's own weight from a complexity persective alone
but the performance cost of having to use a list of interface rather than a list
of tags was very large. See the benchstat improvements below for the difference
it makes to both speed and memory usage, especially in the no exporter case.
name old time/op new time/op delta
Baseline-8 341ns ± 2% 342ns ± 1% ~ (p=0.139 n=19+18)
LoggingNoExporter-8 2.46µs ± 5% 1.97µs ± 3% -19.88% (p=0.000 n=19+20)
Logging-8 13.3µs ± 2% 12.8µs ± 2% -3.42% (p=0.000 n=17+20)
LoggingStdlib-8 5.39µs ± 6% 5.34µs ± 3% ~ (p=0.692 n=20+19)
name old alloc/op new alloc/op delta
Baseline-8 80.0B ± 0% 80.0B ± 0% ~ (all equal)
LoggingNoExporter-8 728B ± 0% 440B ± 0% -39.56% (p=0.000 n=20+20)
Logging-8 2.75kB ± 0% 2.46kB ± 0% -10.53% (p=0.000 n=17+20)
LoggingStdlib-8 568B ± 0% 568B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
Baseline-8 5.00 ± 0% 5.00 ± 0% ~ (all equal)
LoggingNoExporter-8 32.0 ± 0% 23.0 ± 0% -28.12% (p=0.000 n=20+20)
Logging-8 88.0 ± 0% 79.0 ± 0% -10.23% (p=0.000 n=20+20)
LoggingStdlib-8 28.0 ± 0% 28.0 ± 0% ~ (all equal)
Change-Id: Ic203ad0c5de7451348976b999a0d038ac532dc39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
We change the main exporter to be stored and fetched using atomics rather
than aquiring a mutex for a mild (but significant in the disabled case) speedup.
Also has the benefit of not holding a global lock over all telemetry operations.
benchstat of logging benchmatks before and after:
name old time/op new time/op delta
Baseline-8 329ns ± 2% 327ns ± 2% ~ (p=0.181 n=19+17)
LoggingNoExporter-8 3.08µs ± 3% 2.42µs ± 2% -21.42% (p=0.000 n=20+19)
Logging-8 13.7µs ± 2% 13.2µs ± 1% -3.49% (p=0.000 n=19+19)
LoggingStdlib-8 5.39µs ± 3% 5.41µs ± 2% ~ (p=0.177 n=19+20)
This is a replacement for https://go-review.googlesource.com/c/tools/+/212244
but built on the single exporter principle rather than the exporter list.
Change-Id: Icc99319c4357e0bcb63386c64372a733e8a76796
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221218
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@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>
Not sure why I thought it was a good idea to enable extra logging
by default, but this was added in CL 184198.
Fixesgolang/go#37636
Change-Id: I1840a9e53625db99c9097f2c23500ae20d6dac1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221918
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
This change adjusts where the start and end of the arguments list are when determining which parameter is the active paramter. Rather than use the first and last argument's position, we will use the start and ending parentheses.
Fixesgolang/go#37271
Change-Id: I70bc5c8b4bdb5242fc35a20e63b9a8860cb1d6bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221817
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change fixes an issue with concurrent map reads and writes that occurred when determining if an import is missing the corresponding dependency in the go.mod file.
Change-Id: Ic5cf3a620b4fd84eb4a377d0e4c22edbc8f37540
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221897
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In all cases, use a Logf field to configure debug logging. Non-nil means
that logging is enabled through the given function.
Fixes accidental debug spam from goimports, which had a separate Debug
flag that was supposed to guard logging, but wasn't used when creating
the gocommand.Invocation.
Change-Id: I448fa282111db556ac2e49801268d0affc19ae30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221557
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a test for diagnostics not disappearing when you create the first .go file for a project and add an import, then save.
Updates golang/go#37195
Change-Id: I871e77d9ab129f13be58931fe009e4297c7f2b38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221222
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds an upgrade all dependencies codelens on the go.mod file if there are available upgrades.
Updates golang/go#36501
Change-Id: I86c1ae7e7a6dc01b7f5cd7eb18e5a11d96a3acc1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221108
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We were panicking when a LHS assignee's type was nil, such as:
// "foo" has not been declared
foo = 123
A recent refactoring changed (*completer).typeMatches() to no longer
gracefully handle a nil types.Type for its first parameter. That
behavior seems fine, so fix the problematic caller of typeMatches to
check for nil before calling.
Change-Id: Ie11e4a2d374ab1efbf6fd13fbe214e06d359fca0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221020
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now "implementations" supports finding interfaces implemented by the
specified concrete type. This is the inverse of what "implementations"
normally does. There isn't currently a better place in LSP to put this
functionality. The reverse lookup isn't an important feature for most
languages because types often must explicitly declare what interfaces
they implement.
An argument can be made that this functionality fits better into find-
references, but it is still a stretch. Plus, that would require
find-references to search all packages instead of just transitive
dependents.
Updates golang/go#35550.
Change-Id: I72dc78ef52b5bf501da2f39692e99cd304b5406e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219678
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add two new fake editor commands: Formatting and OrganizeImports, which
delegate to textDocument/formatting and textDocument/codeAction
respectively. Use this in simple regtests, as well as on save.
Implementing this required fixing a broken assumption about text edits
in the editor: previously these edits were incrementally mutating the
buffer, but the correct implementation should simultaneously mutate the
buffer (i.e., all positions in an edit set refer to the starting buffer
state). This never mattered before because we were only operating on one
edit at a time.
Updates golang/go#36879
Change-Id: I6dec343c4e202288fa20c26df2fbafe9340a1bce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221539
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Helper functions on the regtest.Env wrapping workspace or editor
functionality are moved to a new wrappers.go file.
Also, rename 'WriteBuffer' to 'SaveBuffer', for less confusion with
'WriteFile'.
Change-Id: Ide9a5cf919dcee6e4a4fbfdb167eddf751a26eeb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221538
Reviewed-by: Rohan Challa <rohan@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There are missing nil checks in a few places when using the gopls
debug page. It will result in panics when trying to access a debug
page that no longer exist (for example a closed session).
This change adds nil checks and updates the template to not render
non-existing debug address.
Change-Id: Ic9163f3727fd8c51122cbd4ab4374fc4d55477c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221699
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
It is never invoked anyway, and it provices no useful benefit while complicating
the implementation, as it is the only non context aware method.
Change-Id: Id5a99439fedafdf4d71285e36103b4854cf3635a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221540
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Darwin's syscall.Dirent is declared as larger than the kernel may
actually return. Don't assume that we can copy the whole thing out of a
buffer.
Fixesgolang/go#37269
Change-Id: I7f2b273f3a14071df8b5ff5bd70e59d784f6d187
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221381
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Originally I decided to use t.Parallel() in hopes of uncovering new
bugs. That may have worked... but manifested as rare flakes that are
difficult to diagnose (golang.org/issues/37318).
Since this level of parallelism is extremely unlikely in normal gopls
workloads, I'm going to remove the t.Parallel() calls in hopes of
eliminating this flakiness. I'd rather be able to continue running these
tests.
Also, don't run in the 'Shared' execution mode by default: normal gopls
execution is either as a sidecar (the Singleton execution mode), or as a
daemon (the Forwarded execution mode).
Un-skip the TestGoToStdlibDefinition test, as hopefully it will no
longer flake.
Updates golang/go#37318.
Change-Id: Id73ee3c8702ab4ab1d039baa038fbce879e38df8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221379
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Related to https://github.com/microsoft/vscode-go/issues/3063.
A small collection of fixes: (1) Don't reload packages with missing
dependencies unless imports change. (2) Show the error message from the
initial workspace load to the user.
Also, a small fix for deletion suggested fixes - upgrading to the latest
version of the VS Code language client revealed that I had made a
mistake there.
Change-Id: I7ab944ed27dc3da24a79d5d311531a1eb2b73102
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221217
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Instead we only have a single exporter, and it must delegate behaviour
to any other exporters it wants to include.
This removes a whole collection of suprises caused by init functions adding
new exporters to a list, as well as generally making things faster, at the small
expense of needing to implement a custom exporter if you want to combine the
features of a few other exporters.
This is essentially the opposite of https://go-review.googlesource.com/c/tools/+/212243
which will now be abandoned in favor of this approach.
Change-Id: Icacb4c1f0f40f99ddd1d82c73d4f25a3486e56ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220857
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The highlight tests imported golang.org/x/tools/internal/lsp/protocol
package, which doesn't exist when the testdata is set up in the test
harness. This causes gopls to try to reload the metadata for the
package. I'm not sure why this wasn't an issue before, since we should
re-run go/packages.Load every time we have missing dependencies.
Fixesgolang/go#37365
Change-Id: I8ebcbbf78b7e6fcdac9ab83bef3f5a0c9a50a361
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221107
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
I had meant to continue instead of break this loop. This caused us to
invalidate all IDs (except for those for one file) every time a snapshot
was cloned.
Picked up a staticcheck fix along the way.
Change-Id: I8fb3b2bdd6b58ac21130e01cb0d32fa6a57e6b73
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221103
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
This change implements support for textDocument/hover when it comes to go.mod files.
Updates golang/go#36501
Change-Id: Ie7da0194bb972955b7ab9cf7b9c9972bd9f4b8a9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220359
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change implements support for textDocument/documentLink when it comes to go.mod files.
Updates golang/go#36501
Change-Id: Ic0974e3e858dd1c8df54b7d7abee085bbcb6d4ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219938
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We had previously only added to a list of package IDs, rather than
invalidating package IDs along with metadata. This is because we loaded
packages by file, rather than by package path or workspace. Now that we
load by workspace, we can invalidate package IDs. This will avoid the
issue of lingering "command-line-arguments" IDs.
Fixesgolang/go#37213
Change-Id: I21830219efaf0df9531e9d811ccbc3f141c0cbcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Searching with an empty string shouldn't return every symbol in the
workspace -- nobody wants that. Limit to 100 results to avoid breaking
editors. (VS Code locks up for like 30 seconds on my workspace.)
Change-Id: I1e0be476e8eeaef9e69767bfa04a89d40bd3a6e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220939
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We want the go command to get a chance to do its cleanups. Send it
Interrupt before Kill.
Fixesgolang/go#37368.
Change-Id: Id891d2f4f85aae30d2aaa19b9c854d2346ec6e1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220738
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Over time we have accumulated a disturbing number of ways to run the Go
command, each of which supported slightly different features and
workarounds. Combine all of them.
This unavoidably introduces some changes in debug logging behavior; I
think we should try to fix them incrementally and consistently.
Updates golang/go#37368.
Change-Id: I664ca8685bf247a226be3cb807789c2fcddf233d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220737
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix various things related to regtest execution:
+ Check the error from OpenFile in fake.Editor.GoToDefinition.
+ Add an error-checked wrapper to env for CloseBuffer.
+ Use env wrappers in TestDiagnosticClearingOnClose.
+ Use os.Executable to get the test binary path.
+ Add a -listen.timeout to the remote gopls process, so that it is
cleaned up.
Updates golang/go#36879
Change-Id: I056ff50bdb611a78ad78e4f5e2a94a4f155cc9de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220902
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Various bits of the module code use HasPrefix, which only works if all
paths are clean. Make sure they are.
No test; this is pretty esoteric and it's not easy to test.
Fixesgolang/go#36193.
Change-Id: I8c8e87d1882d149a1a129d8b7123bc95c115b2ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220659
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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.
Fixesgolang/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>
I'm still not sure if we need to handle any other non-standard package
path apart from "command-line-arguments".
Also, a couple of staticcheck fixes.
Change-Id: I0bb3e60f6ffe104ff9027dbebb628020caaa1af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Make all the benchmarks cleanly pass around the context to remove a non logging
difference.
Rename the non logging calls benchmark to Baseline
Change-Id: I24a33b0a817df403fb43c53b5f097bc1e9418af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220520
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
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>
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>
When running gopls against an automatically started remote instance, we
want the lifecycle of the remote to be detached from that of its
clients, so that it doesn't shut down while clients are still connected.
On the other hand, a gopls process can consume significant resources, so
we don't want it to remain when there are no more connected clients.
The jsonrpc2 package is updated to support the concept of idle timeout:
a duration after which the server is shut down when there are no
connected clients. This is exposed in the gopls serve command via the
-listen.timeout flag.
Update golang/go#34111
Change-Id: Id62b3d4a2fa66de2c9306d130ca431717f01d1e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220281
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
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>
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>
