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>
This test is failing on darwin-amd64-10_12: skip it while I investigate.
Change-Id: I506f54355924e86cf4235b04fd8890f59b48ac33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232198
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.
Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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.
Fixesgolang/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>
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>
This change was unfortunately lost while rebasing, resulting in a lot of
unremoved directories.
Change-Id: Icc1b667e31ac85e617b1c3a8d7c58f78e999d151
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230314
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In order to experiment with adding more progress reporting to gopls, add
a new experimental configuration for verbose work done reporting.
Also, pass configuration in InitializationOptions when initializing the
editor.
Change-Id: Id2336790719d9c0b6d663997e1aef672aec43d28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229458
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In preparation for later changes, add support for tracking outstanding
work in the lsp regtests. This simply threads through progress
notifications and tracks their state in regtest.Env.state. A new
Expectation is added to assert that there is no outstanding work, but
this is as-yet unused.
A unit test is added for Env to check that we're handling work progress
reports correctly after Marshaling/Unmarshaling, since we're not yet
exercising this code path in actual regtests.
Change-Id: I104caf25cfd49340f13d086314f5aef2b8f3bd3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229320
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change also required the addition of a new run configuration -
WithEnv, which adds extra environment variables to the configuration.
Please let me know if this is the wrong approach.
Fixesgolang/go#37984
Change-Id: Ied2a53a443dc74c7ed723b91765eeddc1a7c1c00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229128
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
A few more updates to the regtest framework - add support for all
codeActions, not just organize imports. Also, return diagnostics from
env.Await to pass into code actions. Not sure if that's the correct way
to do it, so please let me know if there's a better way.
The test is for 1.14 only, since -modfile is only supported after 1.14.
Fixesgolang/go#38211
Change-Id: I5cdd35e2ec06e2b2487d1f3491197030008be9c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226958
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Certain regtests require referencing external data. To support this, add
the ability to use a file-based proxy populated with testdata.
To expose this configuration, augment the regtest runner with variadic
options. Also use this to replace the Runner.RunInMode function.
Add a simple regtest that uses this functionality.
Updates golang/go#36879
Change-Id: I7e6314430abcd127dbb7bca12574ef9935bf1f83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228235
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Closing the workspace has frequently been failing on Windows, due to
file locks held by the go command.
This change makes several tests more careful to check errors when
closing resources, and defers closing the regtest workspaces until the
entire test suite completes, at which point it is much more likely that
closing the workspace will succeed.
If this change results in test flakes on Windows, we should temporarily
demote errors in regtest.Runner.Close to a t.Log.
Updates golang/go#38490
Change-Id: Ibd2f7dd0e0e2faecfa0ca8c60237fc72e64f6719
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228231
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
While trying to be pragmatic, I simply locked the fake.Editor while
calling textDocument/formatting. This did indeed manifest later on as a
deadlock, so instead we release the lock but fail if the buffer version
changes while awaiting the formatting call.
Change-Id: I0ca24a502f3118e76de306a016449bbe665e70e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228230
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Configuration was not being set correctly, because the configuration
response was not honoring the configuration section order.
Change-Id: I8418535b45e6a24fd8f0605d58cb370e22664f17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228257
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change attempts to fix a concurrency error that would cause
textDocument/CodeLens, textDocument/Formatting, textDocument/DocumentLink,
and textDocument/Hover from failing on go.mod files.
The issue was that the go command would return a potential concurrency
error since the ModHandle and the ModTidyHandle are both using the
temporary go.mod file.
Updates golang/go#37824
Change-Id: I6cd63c1f75817c7308e033aec473966536a2a3bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224917
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This pushes the handler construction out to the user, allowing flexability of
use, and is the final stage of the switch to the new handler API.
Change-Id: Id2e61813a817df0d6e4d20dd47ce8c92b0ae87db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227024
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Handler is now a function type that mapps to what used to be the Deliver method.
The only handler that used other methods was Canceller, for now that still
exists as LegacyHooks. Once the handlers are fully cleaned up we should be able
to re-implement canceller as handler middleware.
Each connection is now only allowed one handler, and it is passed to the Run
method, but handlers are composable.
Change-Id: I370e0459df851bb9c9c2a679b99cff073b94489e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226479
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
One of the tricky things about asserting on conditions in regtests is
the asynchronous nature of LSP. For example, as the LSP client we cannot
be sure when we've received all diagnostics for a given file.
Currently, regtests are implemented by awaiting specific diagnostic
expectations. This means that if gopls generates diagnostics that do
not match those expectations, we can only time out the test.
Ideally, we would want to know that gopls is done generating all diagnostics
for the current file state. This is not possible without knowing the
status of diagnostics for. Barring this, we would want to know that
diagnostics are done for the current file version. Unfortunately, that
also is not possible, because a new version of file B can affect
diagnostics in file A.
So in lieu of this information, this CL exposes a few tools that can be
used to improve the experience of writing new regtests.
- A new expectation is added: AnyDiagnosticAtCurrentVersion, that is
satisfied if any diagnostics have been received for the current
buffer version.
- ExpectDiagnostics is added to Env, to help check whether the current
diagnostics matches expectations.
Updates golang/go#38113
Change-Id: I48d2c3db87c13ac3ab424d01d9444cbc285af9e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226842
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a RunGoCommand function to the workspace, which will
allow us to test how gopls responds when a certain go command is
executed. Add a test that checks behavior after "go mod init" runs.
Change-Id: I679249c1654b136d44187397b4196b8a10b5615e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226478
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
https://github.com/microsoft/vscode-go/issues/3076#issuecomment-605062933
inspired me to write a regression test for this case. Turns out we
weren't handling it correctly after all...
This change makes sure that we only rebuild the view once a new go.mod
file is saved, not just created. It also preserves the snapshot ID
number when the view is recreated so that diagnostic caching continues
to work as expected.
Change-Id: I63bd559c3bd33b91828171cd7ddb3d099c31cddb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226017
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>
Expressing regtests in terms of textual coordinates is hard to read: the
reader ends up counting lines and characters to understand the text edit
or assertion.
To address, this, add two new functions for fake.Editor: RegexpSearch
and RegexpReplace, as well as a symmetric RegexpSearch function for
workspace files and wrappers for regtext.Env.
This allows expressing edits as well as buffer locations in terms of
easily scannable regexps.
An alternative solution to this problem is to integrate markers ala
packagestest. I tried this, but it ended up being cumbersome to
implement and less usable than regexps, due to the static nature of
markers: after the buffer has been edited all markers must be
updated.
Updates golang/go#36879
Change-Id: Iad087cf0d529737034197beef7b729816a159c69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224757
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL adds support for sending progress notifications through $/progress
calls as well as being able to cancel them through window/workDoneProgress/cancel.
This feature is only supported in clients running LSP 3.15 and therefore the initialize
request will check for client capabilities for its progress support.
Updates golang/go#37680
Change-Id: Iff8c016694746a9dd553e5cc49444df7afcc21f5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222981
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
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>
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>
Not all regtests resulted in LSP shutdown, which caused temp modfiles to
be leaked. After this fix I have confirmed that /tmp is clean after a
successful run of the regtests.
Also proactively clean up the unix socket file when serving jsonrpc2
over UDS.
Change-Id: I745fbd3d2adeeb165cadf7c54fd815d8df81d4e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220061
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Create a real type for protocol.DocumentURIs. Remove span.NewURI in
favor of path/URI-specific constructors. Remove span.Parse's ability to
parse URI-based spans, which appears to be totally unused.
As a consequence, we no longer mangle non-file URIs to start with
file://, and crash all over the place when one is opened.
Updates golang/go#33699.
Change-Id: Ic7347c9768e38002b4ad9c84471329d0af7d2e05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219482
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add a new Forwarder type to the lsprpc package, which implements the
jsonrpc2.StreamServer interface. This will be used to establish some
parity in the implementation of shared and singleton gopls servers.
Much more testing is needed, as is handling for the many edge cases
around forwarding the LSP, but since this is functionally equivalent to
TCP forwarding (and the -remote flag was already broken), I went ahead
and used the Forwarder to replace the forward method in the serve
command. This means that we can now use the combination of -listen and
-remote to chain together gopls servers... not that there's any reason
to do this.
Also, wrap the new regression tests with a focus on expressiveness when
testing the happy path, as well as parameterizing them so that they can
be run against different client/server execution environments. This
started to be sizable enough to warrant moving them to a separate
regtest package. The lsprpc package tests will instead focus on unit
testing the client-server binding logic.
Updates golang/go#36879
Updates golang/go#34111
Change-Id: Ib98131a58aabc69299845d2ecefceccfc1199574
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218698
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Add a test for the bug reported in golang/go#37049: we are missing empty
diagnostics for deleted files. Doing this involved added a missing
RemoveFile method on the fake.Watcher type.
Skip the test for now, as it is failing.
Updates golang/go#37049
Updates golang/go#36879
Change-Id: Ib3b6907455cc44a2e6af00c2254aa444e9480749
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218278
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A lot of bug reports originating from LSP clients are related to either
the timing or sequence of editor interactions with gopls (or at least
they're originally reported this way). For example: "when I open a
package and then create a new file, I lose diagnostics for existing
files". These conditions are often hard to reproduce, and to isolate as
either a gopls bug or a bug in the editor.
Right now we're relying on govim integration tests to catch these
regressions, but it's important to also have a testing framework that
can exercise this functionality in-process. As a starting point this CL
adds test fakes that implement a high level API for scripting editor
interactions. A fake workspace can be used to sandbox file operations; a
fake editor provides an interface for text editing operations; a fake
LSP client can be used to connect the fake editor to a gopls instance.
Some tests are added to the lsprpc package to demonstrate the API.
The primary goal of these fakes should be to simulate an client that
complies to the LSP spec. Put another way: if we have a bug report that
we can't reproduce with our regression tests, it should either be a bug
in our test fakes or a bug in the LSP client originating the report.
I did my best to comply with the spec in this implementation, but it
will certainly develop as we write more tests. We will also need to add
to the editor API in the future for testing more language features.
Updates golang/go#36879
Updates golang/go#34111
Change-Id: Ib81188683a7066184b8a254275ed5525191a2d68
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217598
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
