1
0
mirror of https://github.com/golang/go synced 2024-11-05 16:26:11 -07:00
Commit Graph

92 Commits

Author SHA1 Message Date
Heschi Kreinick
e31c80b82c all: rework goimports environment, support GOMODCACHE
This CL got away from me a little.

For a number of reasons, the existing goimports API of passing in values
for various GO* values was not working. For one, the number of necessary
variables kept growing. For another, we tried to avoid calling `go env`
in GOPATH mode by using `build.Default`, but that turns out to be buggy;
see golang/go#39838. And finally, it created massive confusion about
whether the values were intended to be read from the OS environment, or
fully evaluated by the `go` command.

There are only two users of the internal imports API, so there really
shouldn't need to be more than two modes. For the command line tool, we
have to call `go env` to deal with the `go/build` bug. So we just do it.
Tests use that same path, but can augment the enviroment to set
themselves up. In contrast, `gopls` needs to fully specify the
environment. It can simply pass in the fully evaluated GO* values.

Finally, make the change I was actually here to make: propagate
GOMODCACHE and use it where appropriate.

Fixes golang/go#39761.

Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-25 17:33:20 +00:00
Rebecca Stambler
20e05c1c8f internal/lsp: don't use -modfile for go mod commands
These are commands whose changes should be reflected in the existing
go.mod file, as they do not provide edits. Add a third way of running
the go command, explicitly without -modfile. Update the regression test
accordingly.

Change-Id: I866b5db83b504fae190e58c306c01a7a4296672d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-22 20:30:43 +00:00
Rebecca Stambler
6023b8da91 internal/lsp: use -modfile for import organization
Also add a regression test.

Fixes golang/go#39681.

Change-Id: I685fbe4255b9e8e067dacd84a6d464d0a2bbe1f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238737
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-19 17:44:56 +00:00
Rob Findley
20370b0cb4 internal/lsp: honor GOPRIVATE in documentLinks and go.mod hovers
Several fixes related to GOPRIVATE handling and links:
 + In Go source, fix links matching GOPRIVATE for external modules.
   Previously, in these cases we'd try to match <mod>@v1.2.3/<suffix>,
   which wasn't the correct input into the GOPRIVATE matching algorithm.
 + Similarly check GOPRIVATE for go.mod require statement hovers.
 + Likewise, for documentLink requests (both mod and source).
 + Move the existing hover regtest to link_test.go, and expand to cover
   all these cases.

Along the way, I encountered a couple apparent bugs, which I fixed:
 + Correctly handle the case where there is only one require in a go.mod
   file. This was exercised by the regtest, so took some debugging.
 + Only format links [like](this) if the requested format is actually
   markdown.

Fixes golang/go#36998

Change-Id: I92011821f646f2a7449dcca619483f83bdeb54b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238029
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-18 13:42:42 +00:00
Heschi Kreinick
87be026d38 internal/lsp/regtest: move and re-enable TestRegenerateCgo
Revert https://golang.org/cl/234480, which was unnecessary, and move it
to a more appropriate file.

Change-Id: I3f5a3eccaf0ffe324fee8e27945a2e5ece2ff12c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238597
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-17 21:29:13 +00:00
Heschi Kreinick
6222995d07 internal/lsp: handle deletion of workspace packages
When a workspace package is deleted, we need to stop trying to load it.
Check that every workspace package still has at least one .go file when
we copy them between snapshots.

I think this is correct, but even if it's not, orphaned file loading
should patch it up.

Fixes golang/go#38977.

Change-Id: I0b11010a40aac09f619f54b5ba02e2467b15a36c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238028
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-17 16:12:49 +00:00
Rebecca Stambler
7f3f4b10a8 go/packages: correct package IDs for overlaid x tests
Bad go/packages results were being cached, causing unexpected behavior
when new x tests are created. This fix also allows us to enable a
regression test that had been previously disabled.

Change-Id: I93c05df2f4d32c6c8a43e9f5aaeeb30bc4a32f3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238058
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-17 04:29:24 +00:00
Rob Findley
b1f3cdd652 internal/lsp/regtest: add a regtest for hover handling of GOPRIVATE
Add a regtest to verify that GOPRIVATE identifiers are not given a link
to pkg.go.dev. For efficiency, as well as to exercise dynamic
configuration, do all this in a single regtest.

Updates golang/go#36998

Change-Id: I9102a11312db5c334fdbd30cce9ca2d2e19e9ac2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237938
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-16 15:44:21 +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
Heschi Kreinick
1725ffee6d internal/lsp/cache: hide diagnostics for ignored dirs
The go command ignores go files in specially-named directories like
testdata. We probably still want to try to offer services like
autocomplete and jump-to-definition there, but diagnostics are less
likely to be helpful. As a compromise, just squash diagnostics.

Note that the rules we use are slightly wrong; see the comment on
ignoreFiles for details.

Fixes golang/go#39563.

Change-Id: I0bc00639e68bd71ea55d867af36e07ef4ec780a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237638
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-15 17:27:23 +00:00
Rob Findley
782c6b3cc7 internal/lsp/regtest: fix context for shared server
The shared server was executing on ctx.Background(), which meant it
didn't have a debug.Instance. This resulted in logs being printed to
stderr, due to the fallback behavior of the global exporter.

Fixes golang/go#39130

Change-Id: Ibb968f78d69752452bec71a7abeff808b1cccb04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237583
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-06-11 19:17:43 +00:00
Rob Findley
c1702b46e6 internal/lsp/regtest: print RPC logs when verbose output is enabled
Use the -v flag to control whether RPC logs are always printed, rather
than a regtest specific flag.

Updates golang/go#39130

Change-Id: Ie6293815adee4b59defd80cfc015838cfbf2b3e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235920
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-06-11 19:16:16 +00:00
pjw
308beac283 internal/lsp: add a way for regtests to look at the diagnostics
regtests can use Await to wait for diagnostic expectations. But sometimes
it is useful (or more robust) to then look at the specific diagnostics.
This change introduces env.DiagnosticsFor, which returns the current
diagnostics for a file.

Change-Id: Iea35d28f6679289795bc853f156aae351279b205
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236837
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-09 09:53:42 +00:00
Rebecca Stambler
5123702d80 internal/lsp: fix update code lens and add a regression test
CL 235619 introduced a bug into the command code, resulting in code
lenses suggesting `go mod get` commands. Fix this issue and add an
end-to-end code lens regression test.

Fixes golang/go#39446

Change-Id: I3646aec881b69f43e5320adcb0976b3516a46761
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236841
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-08 16:37:37 +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
Rebecca Stambler
4d5ea46c79 internal/lsp: support go mod vendor as a command
In addition to adding a `go mod vendor` command option, which can be
exposed via an editor client frontend, we show a suggestion to users who
experience the "inconsistent vendoring" error message.

The main change made here is that we save the view initialization error,
and we return it if the view has absolutely no metadata. This seems
reasonable enough, but my fear is that it may lead to us showing
outdated error messages. I will spend some time improving the handling
of initialization errors in follow-up CLs.

Updates golang/go#39100

Change-Id: Iba21fb3fbfa4bca956fdf63736b397c47fc7ae44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235619
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-04 18:33:45 +00:00
Ian Cottrell
0310561d58 internal/lsp: change the hover test to use normal editor methods
It was directly generating messages and sending them on the conn, now it
just uses an editor method like all the other tests.
It was also broken because it never opened the file it was hovering in, so I am
not sure it was testing anything useful before.

Change-Id: I7a1b444015c95c82a0a137d3bb1da661ed9331af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232983
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:13 +00:00
Ian Cottrell
d3396bb197 internal/jsonrpc2: change jsonrpc2.Conn to be an interface
This will allow varying implementations and wrappers, and more
closely matches the concepts used in the net library.

Change-Id: I4be4c6efb3def0eda2693f482cbb0c6f776e5642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232877
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:06:45 +00:00
Rob Findley
a45abac6c9 internal/lsp/regtest: print completed work when a regtest fails
Change-Id: Ib1ea26c352f6e8b397ffda282f45a425a4278ab5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236169
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-03 13:19:21 +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
Rebecca Stambler
2caf76543d internal/lsp/regtest: add a test that reproduces golang/go#38878
This test partially reproduces some strange behavior with creating
new tests files. In particular, it creates a new x test in a package
that already has a test variant and adds content with a missing import.

In the test, the import is never added. However, in my own experience
debugging this in VS Code, I see the import get added but the diagnostic
never get removed. One thing at a time though...

Updates golang/go#39315

Change-Id: I724a145688b915d04abd1f21efc6f9a7506be043
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235581
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-01 17:56:30 +00:00
pjw
41453949f3 internal/lsp: regtests for removing files outside the editor
When a file with errors is removed outside the editor, sometimes its
errors are cleared by the editor and sometimes they are not. If the file
is still open in the editor gopls does not clear the errors, taking the
editor's version as the truth. Otherwise the errors are cleared.
(This behavior depends on the editor sending gopls a notification that
the workspace changed.)

There seems to be no good way yet to test that gopls takes no action after
receiving the didChangeWatchedFiles notification.

Updates golang/go#38878

Change-Id: Ie418dd786d4c5f827cf0665a31f0f9913f7cfdc0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235377
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-29 13:51:22 +00:00
Heschi Kreinick
af9456bb63 all: remove version-specific test files
Now that we're not using build tags any more we can consolidate files.
Do so.

I tried a little to find good places for the moved code, but only a
little.

Change-Id: I6b66afb7cad870471d7d4a3d86c13fadb94a40e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235457
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-28 17:13:50 +00:00
Heschi Kreinick
8e7acdbce8 all: replace build tags in tests with testenv helper
Many tool features, particularly modules-related, require particular Go
versions. Build tags are unwieldy, requiring one-off test files which
break up test organization.

Add a suite of testenv functions that check what Go version is in use.
Note that this is the logical Go version, as denoted by the release
tags; it should be updated at the beginning of the release cycle per
issue golang/go#38704.

For ease of reviewing, I'll merge/delete files in a followup CL.

Change-Id: Id85ce0f83387b3c45d68465161cf88447325d4f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234882
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-27 18:32:53 +00:00
pjw
253fce384c internal/lsp: fix formatting edge cases (36824)
One line legal code like `package x; import "os"; func f() {}` was
being misformatted. In these cases the parse flag ImportsOnly loses
important parts of the code, while full parsing works. Presumably
all these cases are short enough that there is no appreciable penalty
from the extra parsing.

Fixes https://github.com/golang/go/issues/36824

Change-Id: I9a4581d67c590578f8fdea5ed2a2a58e0bc3c40b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234900
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-26 20:56:00 +00:00
Rebecca Stambler
91d71f6c2f internal/lsp/regtest: add a t.Skip for golang/go#36824 regtest
Switching to using a t.Skip means we are more likely to remember to
actually re-enable the test at some point.

Also picked up a staticcheck fix along the way.

Change-Id: I382eaa8d204bee74a7ff46e8a1b11dab567b83ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-05-21 15:57:04 +00:00
pjw
8979540587 internal/lsp: simplify and correct fixing imports for CodeAction
The code was introducting syntax errors for some edge cases (example in
regtest/import_test.go), and I found it hard to follow.

The new code passes all the tests. There are new regtests to guarantee
no CodeActions are returned for some cases that vim testing noticed.

Change-Id: Ia09da667f74673c11bfe185e4f76a76c66940105
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233117
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-18 22:54:12 +00:00
Rob Findley
4bde419ae8 internal/lsp/regtest: Skip failing/flaky TestRegenerateCgo
Something is making this test deterministically fail in some
environments, such as @bcmills' desktop.

Skip it while I build go at tip and debug.

Updates golang/go#39135

Change-Id: I1bf8c55c5cfca471a904de85936f504313094807
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234480
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-18 19:26:01 +00:00
Heschi Kreinick
c79c01b1c5 all: consolidate cgo requirement checks
Many tools test check for the ability to compile cgo programs.
Consolidate them all into testenv.NeedsTool("cgo").

Change-Id: I62c96e7b4dc72df34b8fdbf10326c7d19e0613e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234108
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-18 17:24:58 +00:00
Heschi Kreinick
d3bf790afa internal/lsp: add Regenerate Cgo code lens
Now that we support authoring cgo packages better, we need a way to
regenerate the C definitions. Doing it automatically is very difficult;
in particular, referencing a new symbol from the C package may require
regeneration, but we don't want to do that for every typo.

For now, give the user a button and make them push it. We attach a
code lens to the import "C" line. This is vulnerable to the usual
user-didn't-save glitches.

Updates golang/go#35721.

Change-Id: Iaa3540a9a12bbd8705e7f0e43ad0be1b22e87067
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234103
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-15 22:01:28 +00:00
Richard Miller
866d71a317 internal/lsp/regtest: don't build unix_test for Plan 9
TestBadGOPATH tests for a empty element in the GOPATH list using
"GOPATH=:/path/to/gopath", assuming that ':' is the path list
separator. On Windows the test fails, because Windows path list
separator is ';'. Therefore this test isn't built for Windows.

On Plan 9, os.PathListSeparator is '\000', so the test fails
there too, and should not be built for Plan 9.

Change-Id: Icdc2f8de098c1415103ec6124906ad6c578ad183
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233718
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-13 12:28:04 +00:00
Rebecca Stambler
aaeff5de67 internal/lsp/regtest: add regression test for golang/go#36960
It's in the build tagged tests because the fix is only in 1.14.

Fixes golang/go#36960

Change-Id: I606dfb4f73735c6db8c48f67f19720d340be8361
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232991
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-12 00:15:01 +00:00
Paul Jolly
0bd3dbed90 internal/lsp/fake: define Symbol method on Editor
In preparation for later changes to the implementation of the workspace
Symbol method, we add the Symbol method to fake.Editor. This requires
the definition of a number of associated fake types (editor-friendly,
byte-offset-based versions of protocol UTF16-based types) for example
fake.SymbolInformation and the types it references.

We also implement a basic regtest for the Symbol method, exposing Symbol
on regtest.Env like other LSP server methods. To aid with the writing of
Symbol result assertions, we provide some helper functions to simplify
the process of defining matches that are evaluated against the result
set.

Change-Id: If73b493e1e791c8201423a303af8041f5a15ccfc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228121
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 20:27:07 +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
Rebecca Stambler
002d754683 internal/lsp/regtest: add test for a GOPATH that's missing an element
Test the case described in
https://github.com/fatih/vim-go/issues/2673#issuecomment-622307211.

Change-Id: I55ff3b8719fc255ec0901cf3778e68b48630323d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232360
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 01:58:00 +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
1d9c21a7db internal/lsp/regtest: add support for testing generate commands
Our editor interaction for running `go generate` was untested. Add
support for triggering generate from the fake editor, and a simple test.

To enable this, some helpers were added to list Workspace files and
check for file state changes, to avoid having to synthetically create
file events. This workaround is not ideal as it results in a leaky
abstraction: in other cases the regtest may assume that FileEvents are
triggered by workspace interactions (e.g. ws.WriteFile), but in this
case it cannot. Unfortunately the only real solution for this would be
to make file watching more realistic, by polling file state on an
interval or using an actual file watching library. Neither of those
options seemed worthwhile just to keep the fake.Editor API pristine.

A new debugging option is added, SkipCleanup, to allow inspecting
regtest working directories after a test with minimal code change.

Change-Id: I64dceeb21a4eb9eff2b6936e44f80f4bd24b82da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230313
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 19:25:30 +00:00
pjw
ed308ab3e7 internal/lsp: avoid showing no-GOPATH-nor-module message so much
If the user's workspace is neither in GOPATH nor a module, and there
are errors in their code, send a message (with ShowMessage) only on
the first load, or when the configuration changes. The previous
behavior sent the message more frequently.

There is a regtest, and two new Expectations for when the fake
editor sees (or does not see) a ShowMessage notification.

Fixes golang/go#37279

Change-Id: I076bd95105359b9310dcf97019e3559159271356
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230897
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-02 20:28:11 +00:00
pjw
2658dc0cad internal/lsp: add a regtest for formatting one-line files
Issue https://github.com/golang/go/issues/36824 complained about
legal go code (e.g., 'package a; func f() {}') that was mishandled
(by being rewritten just as 'package a'). This bug seems to have been
partially fixed, as certified by the new regtests. The comment on
OneLineImports36824 says that the bug would be fixed if gopls
formatted the file before fixing the imports, but it doesn't.

Change-Id: If27fa738e54d9434d5b2f17ed4e52d555cb7c499
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229303
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-01 15:50:19 +00:00
Rebecca Stambler
ab2804fb9c internal/lsp: don't offer suggested fixes for generated files
As suggested on Slack, a better fix for golang/go#38467 would be to hide
suggested fixes on generated files. This way, the diagnostics are still
visible but files are not unintentionally modified.

Also, deleted the SuggestedFixes field on source.Diagnostic, since it's
entirely unused.

Change-Id: I10756471e0f913465b1cccd7f222eea0f4de77fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230999
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-01 06:56:59 +00:00
Rob Findley
127c98bd79 internal/lsp/regtest: rearrange runner.go for readability
This is purely moving code: the getRemoteSocket, getTestServer, and
AddCloser funcs were above the more important RunOptions and runner.Run.
Move them closer to their usage.

Change-Id: I6d5b5577ca8ba5cbd5226b195541696967433dec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230998
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-29 21:33:35 +00:00
Heschi Kreinick
290e0af1a2 internal/lsp/regtest: cosmetic fixes
Add issue comments to tests for issues, move a couple constants closer
to their use.

Change-Id: I34fa2643195ae81e463a070952a1a1a4af2c6132
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230997
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-29 21:16:28 +00:00
Rob Findley
0c9eba77bc internal/lsp/regtest: add a OnceMet combinator for Expectations
A common problem when writing regtests is that if you have an error in
your expectations, you must wait until the regtest times out to see what
went wrong.

With the integration of additional progress reporting in the LSP server,
we know when diagnostic work should have been completed, and we should
be able to fail tests early once we know that our diagnostic
expectations will never be met.

This CL adds a new OnceMet Expectation, which combines two expecations:
the first is a precondition that must be met before checking the second.
The second is an arbitrary expectation, but is translated as follows:
once the precondition is met, the second condition is checked and any
Unmet verdicts are translated into Unmeetable.

Change-Id: Ie8c677229a347c624e2659a3ef9104304b175243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229977
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 21:14:28 +00:00
Rob Findley
dbf5ce1eac internal/lsp/regtest: add expository package documentation
Update the regtest docstring to further explain how the package works,
and give a sense for why it exists.

Fixes golang/go#36879

Change-Id: I4df4a286dc835ae56386b6d92128f7ea6c77ffb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229782
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 21:10:48 +00:00
Rob Findley
12a1c85843 internal/lsp/regtest: rename and document some symbols
Minor cleanup for the regtest package:
 - EnvMode is renamed to Mode, because it's really a server mode and not
   directly related to the Env type.
 - Modes are better documented.

Change-Id: Ia3aedfc70b665ea75a66731a72e4e87ae79db298
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229781
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 21:10:38 +00:00
Rob Findley
317da45f2f internal/lsp/regtest: extract Runner to a new file
The file length of env.go is getting hard to manage, so factor out the
test Runner to a new file.

Also move Runner.Close to the bottom of the file to have a more logical
progression.

Change-Id: Ifebea3277d826e9456aa02507a8349746386eb03
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229780
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 20:47:08 +00:00
Rob Findley
46dc332f25 internal/lsp: instrument work done reporting to use in regtests
In order for regtests to wait until file diagnostics are complete,
instrument diagnostics with verbose WorkDone reporting. In order for
this to be granular enough for use, the modification source needed to be
threaded through to the didModifyFiles function (which is where the
diagnostic goroutine is spun off).

A new expectation is added: CompletedWork, to allow specifying that a
specific work item has been completed. The problem with using
NoOutstandingWork was that it required a continuous chain of work to
prevent the regtest from succeeding when the bug was present, meaning
that by the time we have sent the didChange notification successfully
the server must have started work on its behalf. This was inherently
racy, and too tricky to get right.

Additionally, a couple bugs are fixed:
 - EmptyDiagnostics is corrected to account for the case where we have
   received zero diagnostics for a given file.
 - A deadlock is fixed in Await when expectations are immediately met.

Updates golang/go#36879
Fixes golang/go#32149

Change-Id: I49ee011860351eed96a3b4f6795804b57a10dc60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229777
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 20:46:32 +00:00