Commit Graph

5933 Commits

Author SHA1 Message Date
Rob Findley 64bd808b73 internal/lsp/cache: don't report a context error if load succeeded
If a context was canceled during load, only report it as a critical
error if the load actually failed.

Along the way, simplify evaulation of the critical error to use a switch
statement.

Also await IWL in the second Env used in shared regtests. Presumably it
is this Env that is being shutdown prior to IWL, triggering the
panicking code-path from golang/go#47030. I wasn't able to reproduce,
but all panics are occurring in regtest/misc, and this seems highly
plausible.

Fixes golang/go#47030

Change-Id: I4df65697f644cff275ab1babb783868fd9e10c2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332589
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 21:02:30 +00:00
Bryan C. Mills cae92d5d68 go/packages: skip GOPATH tests in short mode
This cuts the running time for 'go test -short ./go/packages' by about
50% by skipping the tests that run in GOPATH mode.

All of the tests are still run when the -short flag is not set, so the
longtest builders should still report any GOPATH-mode regressions, and
the faster short tests should improve turnaround times (and resource
usage and associated test flakiness) on the non-longtest builders.

	~/x/tools/go/packages$ go test -short
	PASS
	ok      golang.org/x/tools/go/packages  25.379s

	~/x/tools/go/packages$ go test
	PASS
	ok      golang.org/x/tools/go/packages  42.812s

For golang/go#46764

Change-Id: Iff9a94b6a2657174cd5c60aeb732ca4f132a7897
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331052
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 21:00:45 +00:00
Rob Findley 2f04284e71 internal/lsp/regtest: allow for unsent diagnostics in TestResolveImportCycle
Gopls doesn't send empty diagnostics for open files in some cases
(likely a race to context cancellation). This is probably a bug itself,
but for now don't let this cause TestResolveImportCycle to fail.

Added a TODO to investigate further.

Fixes golang/go#46773

Change-Id: I197d9b09885951b47b3f90a0480ae75679d2a1a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333289
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 19:58:58 +00:00
Rob Findley 41a65bdc11 internal/lsp: avoid flake in TestDebouncer
TestDebouncer inherently depends on the timing of events. Specifically,
it can fail if the pause between two subsequent events is more than the
debouncing delay.

When this has proved flaky in the past I just increased the debouncing
delay. However, tests are still occasionally flaking. Rather than just
increase the delay arbitrarily, attempt to make the test more robust by
retrying up to three times if the base assumption (that goroutines are
scheduled within a reasonable amount of time) is not met.

Fixes golang/go#45085

Change-Id: Ifa32e695d64ae4bcfe9600a0413bf6358dff9b7a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309276
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 19:58:23 +00:00
Rob Findley 1c9019e64a internal/lsp/cache: fix raciness of updating the view workspace
The view workspace was written concurrently, meaning that changes to a
go.mod or go.sum could race to updating the view workspace, leaving it
in an incorrect state.

Move control over writing this workspace onto the view, and hold the
snapshotMu while writing.

Change-Id: I47f58769cc77860e9c9674c8f6bf5fd0793e7937
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309289
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2021-07-08 19:57:21 +00:00
Rob Findley febfa9d67f internal/lsp/source: move diagnosticsDelay out of experimental
This option has been enabled by default for a while now: remove it from
'experimental' to 'advanced'.

Change-Id: Id8116bf7b8976204a61fe1bbf9dc0b8bd69c68d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309271
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 19:56:33 +00:00
Rob Findley 71eae3a1b4 internal/lsp/cache: be consistent about using snapshot.FileSet
Ideally we could at some point break the snapshot->view->session->cache
reverse traversal, but for now at least don't copy this pattern around
everywhere.

Change-Id: Ib144e6d322016f5b9563f21c56a0691c1a8ec97d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309270
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 19:56:07 +00:00
Rob Findley 251f283686 internal/lsp: add a setting to batch didChangeWatchedFile notifications
Gopls's serial processing of didChangeWatchedFile notifications has
historically been a pain point for clients that don't batch file events,
when branch switching or running go generate.

It's "not that tricky" for us to debounce and batch up watched file
notifications on our end, so this CL introduces this functionality as a
new experimental setting. Truth be told it ended up being harder than
expected, due to (1) our requirement for regtests to be able to
determine when diagnostics have completed, and (2) our reliance on
jsonrpc2 for sequencing changes to the server.

To address (1), I factored out the actual processing of change
notifications into a separate method (thus increasing our surprisingly
long chain of method calls). To address (2), I guarded the processing of
file changes with a mutex.

I also guarded some places where views and snapshots were accessed in
potentially racy ways. Our interaction with session.views was rather
complicated, so I had to switch session.viewsMu to a RWMutex.

Add this to the experimental regtest mode, and more generally enable all
experimental features in this mode, rather than just the experimental
workspace module.

Fixes golang/go#41691

Change-Id: Ifccdbdf86263dbe2e37ffe9f7bbf2a2cd74218b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309269
Trust: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2021-07-08 19:55:54 +00:00
Rob Findley c979f9254a internal/lsp/cache: invalidate packages in setMetadata
When setting metadata, we must invalidate any packages that have been
computed with old metadata.

Also make setMetadata atomic, by locking around it in Load. This is just
writing memory after a Load, so should be fast and infrequent. It is
critical that our various maps related to metadata are coherent, so it
makes sense to err on the side of coarser locking, IMO.

Fix a race in TestUpgradeCodeLens.

Change-Id: Iee8003b7e52b9f21681bdba08a009824f4ac6268
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331809
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-08 19:53:41 +00:00
pjw 55cd4804df gopls/doc: Document how gopls generates semantic tokens.
The new file gopls/doc/semantictokens.md describes how gopls presently
decides which semantic tokens and modifiers to send to clients.

Change-Id: Ifc76e13954dde2ea58b6da446517f94be038a285
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333010
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
2021-07-08 16:22:39 +00:00
Jean de Klerk 77c1b7a4f7 internal/lsp/cmd: print flags when running gopls help
Currently, flags are not being printed when users run `gopls help` because they
never get parsed in Main. This CL fixes that issue.

Updates golang/go#35855

Change-Id: Ic9882d0d2410a0f045aa0ecaa87b36c23eb569fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330854
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-07-08 16:04:41 +00:00
Cuong Manh Le 640c1dea83 go/ssa: support unsafe.Add and unsafe.Slice
Fixes golang/go#47091

Change-Id: Id59375a3083520275025f58879c8950b0560e25a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333110
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-07-08 02:47:23 +00:00
pjw fd00574339 internal/lsp/protocol: upgrade generated lsp code to beginning of July
There are no changes, other than the date and git checksum.

Change-Id: Ic7e93829f5e4cd95b7c2b5023c9c0905a07655ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332690
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-07-07 20:06:16 +00:00
Rob Findley 7edcfe5231 internal/lsp/lsprpc: add a goenv middleware
Add a middleware to enrich LSP messages with Go environment on the
forwarder.

Change-Id: Ia294b0c2fa1276b09b75542436e5f4179f81f302
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331771
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2021-07-02 16:14:50 +00:00
Rob Findley e3040f2721 internal/lsp/lsprpc: add a CommandInterceptor middleware
This will be used for starting the debug server along the serving path.

Change-Id: I19d13753ad70c179a53f348ed574aaea19c0d301
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331770
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2021-07-02 16:14:40 +00:00
Rob Findley ea370293d7 internal/lsp/lsprpc: use middleware for the V2 handshaking
The number of concerns satisfied by the lsprpc package is proving to be
a barrier to both factoring out the debug and event package, and use of
service discovery.

The new Binder API admits a nice abstraction that can help make the
lsprpc package more modular, or perhaps even unnecessary: a binder
middleware that can instrument all aspects of the connection lifecycle.
In this CL, this pattern is used to decouple the server handshake from
the actual forwarding setup. Later CLs will implement additional
functionality using this pattern.

The TestEnv helper is refactored to be more scriptable.

Change-Id: I6060bc4bba57e4ee7e161a5d6edbc40c6fccbaa8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331369
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2021-07-02 16:14:25 +00:00
Cuong Manh Le 20dafe5d60 go/ssa: allow conversion from slice to array pointer
Fixes golang/go#46987

Change-Id: Ic3b4410c3ea9f62d3cdce579a1152884167be16b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332049
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2021-07-01 08:39:23 +00:00
tkawakita f0847e0ce9 go/callgraph: change reflect.Call to reflect.Value.Call in comment
Change-Id: I672a2dcd5a50c6d6cb5bbe2c0d0c53ff56d24efd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331709
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
2021-06-30 16:45:04 +00:00
Matthew Dempsky 72e4d1bb8d go/internal/cgo: handle symlinks with $PWD, not -srcdir
In CL 293250, I changed x/tools/go/loader to invoke cgo using the
-srcdir option, rather than chdir'ing into the source directory. But
evidently cgo's -srcdir option does not affect #include processing of
relative file paths, so this ended up breaking packages that relied on
the latter.

This CL reverts back to chdir'ing into the source directory, but also
sets the PWD environment variable. This appears to be how cmd/go
invokes tools and handles symlinks.

Fixes golang/go#46877.

Change-Id: Ia538c5d8722b151a4547f73188f6537e0551136f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331409
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-29 19:12:30 +00:00
Rebecca Stambler 100b229261 internal/lsp/cache: treat load timeouts as critical errors
Lower the timeout for the go command to 10 minutes.
Also, if the first workspace load attempt fails because it times out,
treat the timeout as a critical error, since we explicitly don't cancel
the first workspace load.

Fixes golang/go#46859

Change-Id: Iccd26509177e4c47ca4b2c8ab4111df9be0f934e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330969
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-29 14:11:33 +00:00
Rob Findley 12f8456a01 internal/testenv: actually Exit if small machine for netbsd-arm*-bsiegert
Change-Id: Ie5c06bf2a0788a7dc278f5de8f56e3865d9bdbe7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331053
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-25 23:53:21 +00:00
Rob Findley 00129ffdb6 internal/lsp/lsprpc: update binder tests to handle forwarding
Update the new binder tests to run both with a standalone server, and
with a forwarding chain.

Make a few superficial improvements along the way as well.

Change-Id: Icd197698093a3f6149ab58171806b2388ed75b7f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/321134
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2021-06-25 19:21:47 +00:00
Rob Findley fe2294a1b8 internal/jsonrpc2_v2: initialize async before binding
The lsprpc package needs to wait on the newly bound connection to
perform some tear-down, so it must be safe to call conn.Wait from Bind.

Achieve this by switching async from an init pattern to a constructor.
This means moving async fields to pointers, but since they weren't safe
to use prior to initialization anyway this feels correct.

Change-Id: I4f93980915e0b568fbf2db3cd7d062adf06e4b99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330929
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2021-06-25 19:21:17 +00:00
Rob Findley 636017e131 internal/lsp/cache: fix missing pkg error on Windows
Use filepath.SplitList rather than always splitting GOPATH on ':'.

Fixes golang/go#46805

Change-Id: I27324afba96b550f75cc272b6c7f701b28825d39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328969
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-25 15:37:24 +00:00
Jean de Klerk 48cad5ecb1 tools/gopls: small fixes to contributing.md
Change-Id: I261e1936b96e70ad8347655a7a8d0eb792348d44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330853
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-25 15:05:05 +00:00
Rebecca Stambler da404b52bc internal/lsp: start parsing go.work file like gopls.mod file
Allow users to experiment with golang/go#45713 by adding experimental
support for the go.work file. We handle it like a special case, very
similar to the current gopls.mod file mechanism. The behavior is
undefined if both a gopls.mod and go.work file exist. Ultimately, we
will deprecate support for the gopls.mod file if the go.work file
proposal is accepted, so I don't think it's important to be careful
about handling both simultaneously.

Change-Id: Id822aeec953fc1d4acca343b742afea899dc70ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328334
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-24 04:20:31 +00:00
Rebecca Stambler 4833ac519e internal/mod: add workfile parser
This is a minimal copy of golang.org/x/mod/modfile that adds the parser
in CL 324764. It will be used to add experimental support for go.work
in gopls.

Change-Id: I75a8171eda763506ea8b5ffa0c6d163d59010333
Reviewed-on: https://go-review.googlesource.com/c/tools/+/329069
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-24 04:20:08 +00:00
Rob Findley bfc1674319 internal/lsp/cache: fix loading of std and cmd
The go/packages queries std/... and cmd/... don't work. I believe
they're supposed to be just "std" and "cmd" based on experimentation
(and stdlib_test.go), but I didn't see this explicitly documented.

Fixes golang/go#46901

Change-Id: I3d0ed48fa64a50eefe4b5cc6074a93455cd37dc8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330529
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-24 03:06:00 +00:00
Rob Findley d824a7481d gopls/doc: include instructions for compiling generic code
Change-Id: Ia7855890c72f9e376816a7c2d71f9715c7f2eb9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330369
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-23 15:16:55 +00:00
Rob Findley 6d3e439322 gopls/doc: add instructions for working with generic code
Change-Id: Ib0e5e16042791d96ed0d71e900b006a2c536032a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330031
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-22 19:12:15 +00:00
Rob Findley 4c651fc1fc internal/lsp/source: add inferred types to generic function hover
As an experiment, this CL introduces the first gopls feature that is
specific to generics: enriching function hover information with inferred
types. This is done with no additional gating on build constraints by
using the new internal/typeparams package.

The marker tests are updated to allow tests that rely on type parameters
being enabled.

Change-Id: Ic627d64b61a6211389196814edd0abe1484491eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/317452
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-22 16:07:17 +00:00
Karthik Nayak d25f906682 internal/lsp: do not block on channel when there is an error
In `internal/lsp/cmd.(*connection).diagnoseFiles`, when we request for
`gopls/diagnoseFiles`, we are blocked on the channel even if there is an error.
In such a scenario, we've reach a deadlock since. Avoid this by checking for
error, existing if it exists and also closing the channel while we're at it.

Fixes golang/go#46251

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>

Change-Id: I4266ab3ac272a9ae3eac2084b70b6568b72c1984
GitHub-Last-Rev: 94bf2926a0f35d51d62643618a8072ef5a8b5225
GitHub-Pull-Request: golang/tools#324
Reviewed-on: https://go-review.googlesource.com/c/tools/+/329109
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-18 18:34:00 +00:00
Rebecca Stambler 463a76b3dc internal/lsp: only reload invalid metadata when necessary
This change adds a shouldLoad field to knownMetadata so that we can be
more selective about reloading these.

If a package has invalid metadata, but its metadata hasn't changed, we
shouldn't attempt to reload it until the metadata changes.

Fixes golang/go#40312

Change-Id: Icf5a13fd179421b8f70a5eab6a74b30aaf841f49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/298489
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-16 01:55:16 +00:00
Rebecca Stambler 116feaea45 internal/lsp: move the progress tracker to the session
Sometimes, we may want to report progress from functions inside of the
cache package, so move the progress tracker to the session to allow for
that.

Change-Id: I15409577a7a5080e7f0224a95d159de42856ffa7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/319330
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-16 01:01:29 +00:00
Rob Findley 3f7c32638c gopls/internal/regtest: skip the flaky TestResolveImportCycle
Also improve the description of the failing expectations.

Updates golang/go#46773

Change-Id: I9465de8a5005bb7ee719a536f8550afc54bd6044
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328369
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-16 00:40:27 +00:00
Rebecca Stambler b12e6172dc internal/lsp/cache: don't delete metadata until it's reloaded
Retrying CL 271477, this time with parts of CL 322650 incorporated.

This CL moves to a model where we don't automatically delete invalidated
metadata, but rather preserve it and mark it invalid. This way, we can
continue to use invalid metadata for all features even if there is an
issue with the user's workspace.

To keep track of the metadata's validity, we add an invalid flag to
track the status of the metadata. We still reload at the same rate--the
next CL changes the way we reload data.

We also add a configuration to opt-in (currently, this is off by
default).

In some cases, like switches between GOPATH and module modes, and when a
file is deleted, the metadata *must* be deleted outright.

Also, handle an empty GOMODCACHE in the directory filters (from a
previous CL).

Updates golang/go#42266

Change-Id: Idc778dc92cfcf1e4d14116c79754bcca0229e63d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/324394
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-16 00:19:40 +00:00
Rebecca Stambler 4b484fb136 internal/lsp: exclude the module cache from the workspace
This change treats the GOMODCACHE like a directory filter, and it
excludes any modules under the module cache from being considered part
of the workspace. This can happen when users open their entire GOPATH.

To do this, I had to propagate the view's root and gomodcache through
the exclusion functions, and I also had to add BuildGoplsMod to the
snapshot's interface.

Change-Id: Id80b359d73d09a525b380389917451e85357b784
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326816
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-11 17:18:39 +00:00
Rob Findley 9a55cb1fbb internal/lsp/command: minor clean-up of StartDebugging description
Change-Id: I9975a98fa47f2a17a9e013d67b4a48cc6fa17598
Reviewed-on: https://go-review.googlesource.com/c/tools/+/327110
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-11 16:06:07 +00:00
Rob Findley 490eac872f internal/lsp/command: add missing doc and support for result parameters
This CL cleans up doc/commands.md to include missing command
documentation and add support for result parameters.

Included are some quick-and-dirty extensions to the command metadata
loader that handle slices and arrays of struct types.

Change-Id: I43a85e88c9c53e21b790d89a45a9de444addcc63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326909
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-11 15:30:39 +00:00
Rob Findley 9aa007ed16 internal/lsp/cache: invalidate broken packages when imports are deleted
In the presence of an import cycle, packages.Load will exclude the
problematic import from Package.Imports. This missing edge means that we
wouldn't correctly invalidate broken package metadata when a problematic
import is deleted. We also weren't treating import deletion as a change
that could invalidate metadata.

Fix this by invalidating any broken packages when an import path is
deleted, anywhere. This could be overly coarse, but there are various
problems with trying to do better.

Also change cycle errors from TypeError to ListError, as this
categorization was misleading.

Fixes golang/go#46667

Change-Id: Id19e7baf4009632caf3ae9365497552f9b64b5c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326589
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-10 18:32:07 +00:00
Zvonimir Pavlinovic f6327c5b22 vta: adds the VTA call graph construction
This CL uses the results of propagating types and functions over the VTA
graph to create a VTA-based call graph.

Change-Id: I1481641dc6682f8fb874823de3073bf81187f6b7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323051
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-06-10 17:33:43 +00:00
Rob Findley 13cf844527 go/ast/astutil: fix panic when rewriting multi-argument type instances
Use the typeparams helper package to fix AST rewriting in go/ast/astutil
when encountering the new ListExpr type.

This is liable to break in the future when the go/ast API changes, but
at least it will be easy to find by virtue of using internal/typeparams.

Fixes golang/vscode-go#1551

Change-Id: Id34bbcdede9024ed9818bef6d73a19e993dd76a8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326131
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-09 21:40:20 +00:00
Zvonimir Pavlinovic 937957b6d9 vta: adds VTA graph propagation functionality
The propagation consists of two steps. First, VTA graph is converted to
a DAG by computing SCCs (resolving any graph loops) and then types and
higher-order functions are propagated through the DAG.

The results of this propagation are used in the final step to create a
call graph. That CL comes after this one.

Change-Id: Ib66a9401885d4105f3b6a68d5a0007f306882144
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323050
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-06-09 16:53:08 +00:00
Muir Manders e0b9cf74f6 lsp/completion: support completing to Elem() types
For array, slice, maps and chan candidates, we now support
transforming them to their element type in completions. For example:

    var m map[string]int
    var _ int = m<>

At <> we complete to "m[]" because we see that the map value type
matches our expected type.

Fixes golang/go#46045.

Change-Id: Ibee088550193a53744f93217cc365f67f301ae90
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323451
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-09 15:58:30 +00:00
Muir Manders 16e5f55009 lsp/completion: search deeper for candidate type mods
Now we search up to three levels for candidate modifiers. For example,
we can now complete "foo" to "foo()()" (double invocation).

Granted this is rarely useful, but it generalizes and simplifies the
searching we did for dereference modifiers.

Updates golang/go#46045.

Change-Id: Ibf0be8158e16a0a26a6344a346f34af8fe182bb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323450
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
2021-06-09 15:58:24 +00:00
Muir Manders b579874149 lsp/completion: reorganize how we track candidate type mods
"type mod" refers to agglutinative expressions such as dereference
"*", invocation "()", and slicing "[:]". When considering an object as
a completion candidate, we check whether applying a type mod would
make it a better candidate.

Previously we tracked the type mods we wanted to apply to a candidate
by setting bool fields. Now instead we keep a slice of the type mods.
This has two main advantages:
- The mods are now ordered which will allow us to format candidates
  properly when the same mods can appear in different order (e.g.
  "<-*foo" or *<-foo").
- We can now record any mod multiple times allowing for "<-<-foo" or
  "foo()()".

I changed the formatting code to always create a snippet object since
that made things simpler. I had to tweak a few snippet helper methods
to accept a snippet argument rather than creating a new snippet.

This commit's only functional change is that we no longer show any
type mods in candidate labels. For example, the user will now see
"foo" in the completion popup instead of "*foo". Showing the operators
adds noise to the candidate list, and we didn't display them
consistently.

Updates golang/go#46045.

Change-Id: I3ea7baa1ee2fee80a1f8cfe88cbae1093ae269ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323449
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
2021-06-09 15:58:13 +00:00
Suzy Mueller 890984ba22 internal/lsp: change generated variable names to be more verbose
If new functions or variables need to be created during an extract
call, then we choose the new names. This change makes those names
more descriptive to make the generated names easier to read and
understand.

Before:
cond0, ret0 := fn0()
if cond0 {
    return ret0
}

After
shouldReturn, returnValue := newFunction()
if shouldReturn {
    return returnValue
}

Change-Id: I44795dc45185c75d5bf65e48378aa54d6c156212
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326112
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-09 14:15:55 +00:00
Suzy Mueller 9f230b5628 internal/lsp: fix extract bug choosing available identifiers
When choosing variable names, extract makes sure that the chosen
name does not conflict with any existing variables. By avoiding these
conflicts, we may actually have a conflict with the other names we
are choosing. This change removes this conflict by sending the next
index to use as the suffix of the function name.

Change-Id: Icd81b67db29db2503e214d24ec19ca1065cda090
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326111
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-09 14:15:26 +00:00
Rebecca Stambler 4e58f8f09d internal/lsp: handle out of range panic in template parsing
Fixes golang/vscode-go#1548

Change-Id: I95911a417cecc514151e75a60253ef119803885e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326089
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-08 16:06:52 +00:00
Zvonimir Pavlinovic 716319fe59 vta: finalizes VTA graph construction by adding support for function calls
These include call and return statements as well as closure creations
and panics/recovers.

Change-Id: Iee4a4e48e1b9c304959fbce4f3eb43eecd8cb851
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323049
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-06-07 18:10:59 +00:00