Граф коммитов

93 Коммитов

Автор SHA1 Сообщение Дата
Rebecca Stambler be796f87a6 gopls/internal/regtest: handle flake in TestCRLF
Make the test a little more stable by making sure diagnostics complete
before organizing imports in these tests.

Change-Id: Id23341569207878554902887498632d6f1ce56cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/271628
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>
2020-11-19 19:02:02 +00:00
Rebecca Stambler 25dc3e1ccc internal/lsp: handle deletion of a directory on disk
The current DidModifyFiles logic does not handle the possibility of a
directory URI in a didChangeWatchedFiles request. For creation and
changes, this is fine because only file URIs matter in those cases, but
for deletion, the deletion of a directory signifies the deletion of all
of the files in it.

Add handling for directories by looking up all of the known files in a
given directory. We then create file modifications for all of those
files, instead of the directory.

Fixes golang/go#38422

Change-Id: I26d7b93285367fe116fd1cb3e8ec13a276f9e517
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-11-19 05:40:27 +00:00
Rebecca Stambler 598b068a91 internal/lsp: fix end positions for multi-line comments with CRLF lines
The previous work-around assumed that the end of the comment would be
off by one line, when in reality, it is off by the offset of the number
of \r in the comment, which may result in the comment end position being
off by multiple lines. Work around this by scanning for the end of the
comment position using text/scanner.

Fixes golang/go#42646

Change-Id: Icc33e889546f324c6b65b55a98dea587f84c8f01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270879
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>
2020-11-18 03:03:13 +00:00
Rebecca Stambler bd56c0adb3 internal/lsp: use a marker test for struct field ranking test
Change-Id: I70faef2c789a3cc7b4dee8c7dd5dcac0e06d7568
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270880
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>
2020-11-18 00:33:11 +00:00
Chiawen Chen 74a3905de1 internal/lsp: order struct fields completions by definition
Previously struct fields completions are ordered alphabetically.
Now it is ordered by its order in definition.

Fixes golang/go#42626

Change-Id: I8c404205b97fa9f1a5a66a95a84033b6f0f48eae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
2020-11-17 23:31:01 +00:00
Rob Findley f544f6cb68 gopls/internal/regtest: eliminate log duplication
The regtest runner currently logs every framed connection, which results
in duplicate RPC tracing (one from the client perspective, one from the
server perspective).

This is unnecessarily noisy. Just log the first connection (currently
from the client perspective).

Change-Id: I4164cde8263e4267e4e45369bd54176c782c4707
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270357
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-11-17 22:57:35 +00:00
Rebecca Stambler 74f2986df7 internal/lsp: show critical error pop-ups as progress reports
We've been looking for a way to show unintrusive error status reports to
users--try using progress reports as a way of populating a status bar.
This avoids the problem of annoying the user with constant pop-ups.

Whenever an error is returns from (*snapshot).WorkspacePackages, we
start a progress report with the error message. If the error goes away
on the next call to diagnostics, or the error message changes, we will
either remove or update the progress report.

Screencast: https://drive.google.com/file/d/1tG9pc_tPsLoqkQHiJqdY8b06TzTkPVae/view?usp=sharing&resourcekey=0-CEG_LhGHYiFp9S37ut_kgw

Updates golang/go#42250

Change-Id: I8a529a911883092bc08af32246719d883dc5f5a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268677
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: Heschi Kreinick <heschi@google.com>
2020-11-17 01:39:53 +00:00
Rob Findley ac45abd4c8 gopls/internal/regtest: update TestEditFile to use a modified buffer
In CL 267577, the fake editor was updated to automatically refresh
unmodified buffers with on-disk changes. This created a race in
TestEditFile, which asserts that on-disk changes are NOT reflected in
open buffers.

Fix this by inserting a trivial modification to the open buffer.  Also
make the test deterministic, so that it fails consistently without this
modification.

Change-Id: If20af2a3e8f33a64a7514ae3fb5469bd9c59a9c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270244
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: 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>
2020-11-16 00:27:33 +00:00
Rebecca Stambler 9712d02be3 internal/lsp: add ?utm_source=gopls to links to pkg.go.dev
This extra query needs to be added before any link anchors (#), so it's
a little more complicated to add than expected. Separating out the
anchor from the link path simplifies the code a little bit and allows
us to create the correct links.

Fixes golang/go#42602

Change-Id: Ia72d50e20b1446149f1ddd36b12f6d968a9b06cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270097
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>
2020-11-16 00:15:58 +00:00
Heschi Kreinick 1643af1435 internal/regtest: refactor diagnostic expectation implementation
The various diagnostic expectation functions had a huge amount of
duplicate code between them. Make the DiagnosticExpectation type more
concrete make the functions thin wrappers around it.

I think there's probably an argument to be made for a fluent interface
here, rather than half a dozen functions, but not now.

This rewrites NoDiagnosticWithMessage, which as far as I can tell made
no sense. Please correct me if I'm wrong.

Change-Id: I16d713ed3dccb7c3cf456d9c293c184fb8e83950
Reviewed-on: https://go-review.googlesource.com/c/tools/+/269319
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-11-13 20:20:37 +00:00
Heschi Kreinick f6c1dd6914 internal/lsp/cache: suppress Load log spam
moduleLoadScope triggers a load of many packages, just like
viewLoadScope, and should not log each loaded package.

Change-Id: Ifd59b41a496da8eb57421f948b9327519e82d2ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268538
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-11-10 18:16:08 +00:00
Rob Findley 53e0aa8268 gopls/internal/regtest: add an option to nest the workdir
Add a regtest run option NestWorkdir, which causes the working directory
to be nested in the editor workspace, and use it in some modfile tests
to exercise the new workspace logic.

For now we only run the nested tests while using experimental workspace
modules. In a later CL, the 'legacy' mode will be updated to find a
solitary nested module, at which point we should be able to run nested
in all modes.

Fixes golang/go#42111

Change-Id: I0bd3b31969684bc1ba1935633cbb9a3f26de1587
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257138
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
2020-11-10 18:07:23 +00:00
Heschi Kreinick 78b1585853 internal/lsp/fake: reflect on-disk changes in clean buffers
If an open buffer is unmodified from the on-disk version, and that
on-disk content changes, VS Code will update the buffer with the change.
This is relevant for our quick fixes that use the go command to make
changes to go.mod/sum -- we want tests to pick up the changes without
needing to close/reopen the buffer.

For whatever reason, VS Code doesn't do this for deletions, and it made
the implementation easier, so I followed suit. I also mimicked its
behavior of sending both in-memory and on-disk change notifications.

Change-Id: I838a64b2f48f3cbe1a86035293923951b53aecf3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267577
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-11-05 22:03:10 +00:00
Heschi Kreinick 22bd85271a internal/lsp: remove organize imports action for go.mod
Per our discussion, it's too slow for a save hook.

Fixes golang/go#38209. (for real this time?)

Change-Id: I264c6d1590a374eff09b09cb1a9162e8e5ff2dc7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267682
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-11-04 19:38:57 +00:00
Heschi Kreinick aefe0b7421 internal/lsp: set correct directness when adding new requires
When adding a require, we should add an `// indirect` comment if that's
what go mod tidy would do.

It's possible I should split Add out from Update and Remove, but this
was quick and easy and I'm not too worried about it for now.

Also minimize the test that covered this case, which was way more
complicated than it needed to be AFAICT.

Fixes golang/go#38914.

Change-Id: I89c44f8573873227c4c9e637d1d31d8c1a6530aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267578
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-11-04 18:37:02 +00:00
Rob Findley 330dc7d2a4 internal/lsp/cache: assign a static temp workspace dir to the first view
Editors need a way to run commands in the same workspace that gopls
sees. Longer term, we need a good solution for this that supports
multiple workspace folders, but for now just write the first folder's
workspace to a deterministic location:
  $TMPDIR/gopls-<client PID>.workspace.

Using the client-provided PID allows this mechanism to work even for
multi-session daemons.

Along the way, simplify the snapshot reinitialization logic a bit.

Fixes golang/go#42126

Change-Id: I5b9f454fcf1a1a8fa49a4b0a122e55e762d398b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/264618
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
2020-11-04 18:18:50 +00:00
Rebecca Stambler 1f28ee6820 internal/lsp: change `go mod vendor` warning into a diagnostic
This CL uses the approach of a source.ErrorList for the `go mod vendor`
error--rather than a ShowMessageRequest. The suggested fix is a command
that runs `go mod vendor`, so I added a CommandFix to the source.Error
type.

I'm not sure if a diagnostic is better than a ShowMessageRequest--
perhaps it should be both?

Fixes golang/go#41819
Fixes golang/go#42152

Change-Id: I4ba2d7af0233f13583395e871166caed83454d6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261737
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>
2020-11-02 00:57:53 +00:00
Rob Findley d463eb0e41 internal/lsp/cache: introduce a workspace abstraction
When incorporating the gopls.mod file, the invalidation logic for
workspace module information becomes quite complicated. For example:
 + if a modfile changes we should only invalidate if it is in the set of
   active modules
 + the set of active modules can be provided by either the filesystem or
   gopls.mod
 + if gopls.mod changes, we may gain or lose active modules in the
   workspace
 + if gopls.mod is *deleted*, we may need to walk the filesystem to
   actually find all active modules

Doing this with only concrete changes to the snapshot proved
prohibitively complex (at least for me), so a new workspace type is
introduced to manage the module state. This new abstraction is
responsible for tracking the set of active modules, the workspace
modfile, and the set of workspace directories. Its invalidation logic is
factored out of snapshot.clone, so that it can be tested and to
alleviate some of the growing complexity of snapshot.clone.

The workspace type is idempotent, allowing it to be shared across
snapshots without needing to use the cache. There is little benefit to
the cache in this case, since workspace module computation should be
infrequent, and the type itself consumes little memory.

This is made possible because the workspace type itself depends only on
file state, and therefore may be invalidated independently of the
snapshot. The new source.FileState interface is used in testing, and so
that the workspace module may be computed based on both the session file
state as well as the snapshot file state.

As a result of this change, the following improvements in functionality
are possible:
 + in the presence of a gopls.mod file, we avoid walking the filesystem
   to detect modules. This could be helpful for working in large
   monorepos or in GOPATH, when discovering all modules would be
   expensive.
 + The set of active modules should always be consistent with the
   gopls.mod file, likely fixing some bugs (for example, computing
   diagnostics for modfiles that shouldn't be loaded)

For golang/go#41837

Change-Id: I2da888c097748b659ee892ca2d6b3fbe29c1942e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261237
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>
Trust: Robert Findley <rfindley@google.com>
2020-10-30 19:58:30 +00:00
Heschi Kreinick 2c115999a7 internal/lsp: use the go command to fix go.mod files
Modifying go.mod files directly leaves go.sum unchanged, and therefore
in need of updates later. Leaving work for the users to clean up isn't
ideal, so it'd be better to use the go command to make modifications.

Unfortunately, the go command has something of a mind of its own. The
most obvious problem is that using go get to add a new require adds a
// indirect comment to that new require, and there's no way to prevent
it. The only thing we can do is add the require first, then use go get
to do nothing but update the go.sum file.

The other inherent problem is that the go command operates on files as
they exist on disk, not the in-memory versions. As discussed, we issue
an error for this case. The alternative would be to work on temporary
files based on the in-memory contents, but that would be much larger
change, so I'd rather not at least right now.

To support Commands for quick fixes, add a new Command field to
source.SuggestedFix, and use it when forming the CodeAction response.

Fixes golang/go#38209.

Change-Id: I0c13ea39199368623e7494e222ba38587323d417
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265981
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-28 22:47:54 +00:00
Peter Weinbergr c12dc663cb gopls: regtest to demonstrate gopls confused by //line directives
2 examples of //line directives confusing gopls, for hovers and
for diagnostics.

Change-Id: I88838f66ccf23ff191acb68ef81b6019cdfa6135
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262700
Run-TryBot: Peter Weinberger <pjw@google.com>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-21 12:08:33 +00:00
Rob Findley 2d76fa4fd5 gopls/internal/regtest: delete OpenFileWithContent
OpenFileWithContent is identical to CreateBuffer. Delete it in favor of
the latter, which (in my opinion) has the clearer name.

Change-Id: Icf0f017a9a71a273ace6d697c7d669a2df2e3ba8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263206
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>
Trust: Robert Findley <rfindley@google.com>
2020-10-16 23:30:35 +00:00
Rebecca Stambler de28cda03d internal/lsp: remove staticcheck and gofumpt from all experiments
Change-Id: I7b60a06ca5442ce2736d3c1a2d55a0f91b4d44fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263197
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-10-16 20:25:18 +00:00
Jean de Klerk 7c89c52d2f x/tools/gopls: add a skipped test for renaming of package
This adds a test that asserts that gopls gracefully recovers from a bad package
name. It does not include the fix for the test, and so this test is skipped: I
took a stab at it but it was a deep spelunk that I wasn't able to make enough
progress on.

Updates golang/go#41061

Change-Id: I86d817396dae5b0211e633721867e811c7d6cf40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262019
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jean de Klerk <deklerk@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-16 15:57:21 +00:00
Rob Findley a5d9e455e9 internal/lsp/source: be consistent about command identifiers
The primary identifier for gopls commands was changed from Command.Name
to Command.ID(), but this change was not made in the commands passed
back to the client via ExecuteCommandOptions.

Fix this, and ensure that our regtests actually check commands against
the list of supported commands that has been sent.

For golang/go#41985

Change-Id: If566584f157e8a86d26eac6353dbfd772b298cfc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262597
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: 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>
2020-10-15 18:20:29 +00:00
Rebecca Stambler 0112737ef1 gopls/internal/regtest: add a test for switching from modules to GOPATH
Fixes golang/go#40487

Change-Id: I79457a8de559da2a9a3ffabdb315f4d35345c8a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261738
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: Heschi Kreinick <heschi@google.com>
2020-10-13 18:32:36 +00:00
Rebecca Stambler ec925d8b1d gopls/internal/regtest: add a failing test for swig
Fixes golang/go#37660

Change-Id: I4607142af03224820e8350a85eca722586b150b5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261140
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>
2020-10-13 01:55:53 +00:00
Rebecca Stambler 5bd0538631 internal/lsp: move the workspaceMode into the snapshot
Workspace mode makes more sense as a property of the snapshot, since
it is determined based on the modules in the workspace. Move it to the
snapshot and enable the GOPATH to modules test. The mode switch means
that we may run `go mod` commands before a `go.mod` is on-disk, so add
handling for that case.

Also, remove the code added in CL 258121 to treat packages starting with
a "_/" the same way as command-line arguments--that's not actually
correct because perfectly valid packages can also have a "_/" package
path prefix.

Fixes golang/go#40340

Change-Id: I35044f5d108983ba00df1359698bf14217caa982
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260078
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: Heschi Kreinick <heschi@google.com>
2020-10-12 19:26:20 +00:00
Rebecca Stambler 96877f285f internal/lsp, gopls: require a "gopls_" prefix on all commands
Updated the generator to check for this. Necessary to fix command name
collision in VS Code Go. Not the nicest solution, but seemed like the
least invasive one.

The codelens configuration is a little strange now, with the "gopls_"
prefixes, but the alternative is adding the prefix when processing the
config and that would make the default look different from the example.

Fixes golang/go#41187

Change-Id: I5cf42f4a96f6252016dcd5c40a4ac401e1b30a8f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259204
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: Heschi Kreinick <heschi@google.com>
2020-10-09 03:22:23 +00:00
Rebecca Stambler ffec97847f internal/lsp: handle major versions above v0/v1 in workspace module mode
Workspace mode did not yet support major versions other than v0/v1. To
do so, we have to check the major version before creating the fake gopls
workspace pseudoversion that goes in the workspace module.

Fixes golang/go#41807

Change-Id: I108fe504fdf9e9a0ce23f7102991c9ae78f12a9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260004
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: Heschi Kreinick <heschi@google.com>
2020-10-08 17:44:24 +00:00
Rebecca Stambler 9df69603ba internal/lsp: don't pass build flags to `go mod` commands
This is a temporary fix--golang/go#41826 is a better approach.

Fixes golang/go#41803

Change-Id: Ia055c5e171fe5d4f6d67e9e2e9e7c85fe254605e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260000
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-10-08 02:52:39 +00:00
Rebecca Stambler d5f20aad3b gopls: add regtest mode for experiments, like workspace module tests
This CL adds another regtest mode for trying out experiments, like the
workspace module mode. Before this CL, we lost coverage for default
mode when we turned experimental module mode on by default.

Removed TestNoWorkspaceModule, as it was just a placeholder until this
mode was available.

Change-Id: Ie3e4c2daec3e635c188bafa25b33dd178da875a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258518
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>
2020-10-07 01:32:53 +00:00
Rebecca Stambler 45061abf50 internal/lsp: add support for an "enable all experiments" settings
This setting automatically turns on all of the off-by-default gopls
features so that users can opt in to all experiments.

Fixes golang/go#41763

Change-Id: Ia6998128649a081c2a1e8eb08c2db6d795a73143
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260002
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2020-10-07 01:14:59 +00:00
Rebecca Stambler a00137c514 internal/lsp: default to workspace module mode even with vendor dirs
We were previously opting out of workspace module mode when any module
had vendor directories. While we still haven't decided how to handle
vendoring, it simplifies things to opt experimentalWorkspaceModule users
in to the workspace module mode when they have modules with vendoring.

Temporarily require default mode for the inconsistent vendoring test
(golang/go#41819).

Fixes golang/go#41725

Change-Id: Ifa494daea51a2a3bb2e6bc3026bfb9e8118d31a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259623
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: Heschi Kreinick <heschi@google.com>
2020-10-07 00:01:20 +00:00
Heschi Kreinick 23a3aa10a2 internal/lsp: improve handling of files not in views
didModifyFiles and DidModifyFiles were tightly coupled but also repeated
each other's work a bit, and inconsistencies in the implementation led
to golang/go#41777.

Push all the work of assigning a "best view" down to the Session, and
always assign it somewhere, matching the logic in ViewOf. This would
in principle allow us to diagnose random files, but we only diagnose
workspace packages.

Fixes golang/go#41777.

Change-Id: I6dab32b98bdff6edd07032d84a8fec1b82ecd283
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259877
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-06 18:39:45 +00:00
Rebecca Stambler 576e169c3d internal/lsp: fix hover for builtin error method (Error)
Looks like this may never have worked. "Error" does not appear in the
builtin package's scope, so we have to look up "error" and then find
the method. A little convoluted, but it works.

Change-Id: I5fe4e96d5c51a1fdc683e44b9a80e0cbdab85422
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259143
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: Peter Weinberger <pjw@google.com>
2020-10-05 18:50:03 +00:00
Rebecca Stambler 39ee293a30 gopls: add test for `go mod tidy` diagnostics in multiple modules
Just a simple test that confirms that diagnostics work in multi module
mode. There are still a lot of edge cases with interdependent modules
that should be covered.

Change-Id: I3a489071346d3fce12b21c48a727ca1db4cfc57e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259140
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>
2020-10-02 18:24:19 +00:00
Rob Findley 08f19738fa gopls/internal/regtest: only run in singleton mode
singleton mode is naturally less flaky and faster than forwarded mode,
and provides 98% of the coverage.

Regtests have started to time-out in some particularly slow
environments.  Rather than not be able to write more regtests, let's
first run only in singleton mode by default.

I'll set up Kokoro CI to run in all modes.

Change-Id: I6b1c11b75f6c8fe002c123ab75b02f674e67e442
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259137
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-02 16:18:17 +00:00
Rebecca Stambler 22683886a9 internal/lsp: fix go.mod creation without experimental workspace module
We were previously adding modules to the snapshot, even if they weren't
relevant without the workspace module mode. Now, check that the modules
are relevant before adding them.

Change-Id: Ib7600482992d538db2f7451863fee5709a35ffb3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258719
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-10-02 14:15:43 +00:00
Rebecca Stambler 0d28ed0cbe gopls: add a test that mirrors govim's -mod=readonly test
This isn't strictly necessary, but we've broken it a number of times,
and it's easier to have a regtest for it. When golang/go#41437 is resolved, we
might consider making this a -mod=mod test or something.

Also, added a wrapper for RunGoCommand.

Change-Id: I75db585549d84dacde9d3efbc6e02a7ba4e005aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258803
Reviewed-by: 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>
Trust: Rebecca Stambler <rstambler@golang.org>
2020-10-02 05:59:58 +00:00
Rebecca Stambler 4e032a7e1e internal/lsp: fix and add a test for non-workspace module mode
This issue was reported on Slack. We need to run more tests in the
default mode, but this is a quick fix.

Change-Id: Ic29d0332aa0410b6e80f77d894c8c007c7f251fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258657
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: Heschi Kreinick <heschi@google.com>
2020-10-01 21:53:43 +00:00
Rebecca Stambler 1e3611d215 internal/lsp: remove logic for re-creating a view when a go.mod changes
This is no longer necessary now that we modify the modules field based
on newly created/deleted modules.

There was a race condition setting the metadata--we were reusing old
metadata that may have already been cached in some instances. Now, we
always override metadata.

Also, disabled the TestUseGoplsMod test -- there seems to be an issue
with it. Will discuss this offline in the AM.

Change-Id: Ie8c97557d4f0a319051256e5f130b9cdae479928
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258121
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: Heschi Kreinick <heschi@google.com>
2020-10-01 18:12:13 +00:00
Rebecca Stambler 2e5f0cfadf internal/lsp: remove all but one use of the view's modURI field
The view reinitialization logic appears to be broken, and so needs one
remaining use of the modURI, which be fixed in a follow-up. Every other
use of view.modURI is removed.

Updates golang/go#32394

Change-Id: Ic051ed848c30e6981d42a576fb35f40efbeb17a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257417
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-09-30 20:43:29 +00:00
Rob Findley a6f32d173b gopls/internal/regtest: allow cleanup to fail on windows
Due to Windows' default file locking and the fact that regtests shell
out to the go command, cleanup sometimes fails.

This is causing trybot flakes, increasingly as of late. Since the
tempdir will eventually be cleaned up on the trybots anyway, don't fail
on windows.

For golang/go#38490

Change-Id: I136d97143baba1d98777db51daa062cf0e42e33e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258315
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>
2020-09-30 20:10:28 +00:00
Rebecca Stambler f1e51e6b94 internal/lsp: stop using modURI as much as possible
This change switches over load and RunProcessEnvFunc to use the
snapshot's modules instead of the view's modURI. These do not seem to
have been the racy parts of CL 257417.

Change-Id: I317a350fc4b0c62d77858455a0e2e61148804ecd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257969
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: Heschi Kreinick <heschi@google.com>
2020-09-29 19:10:02 +00:00
Rob Findley 5272f303b6 gopls: fix various staticcheck errors
Do a pass of unused code cleanup and other staticcheck errors.

Change-Id: Iaf5d27c4f5405d4cce3e48fa06a5d46ec757de40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257965
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>
Trust: Robert Findley <rfindley@google.com>
2020-09-29 17:30:36 +00:00
Rob Findley d7fc70abf5 gopls/internal/regtest: always await InitialWorkspaceLoad
There's very little reason not to always await IWL, and none of our
tests actually need to be able to execute before it completes.

Await IWL before running the test func.

Change-Id: Ia0626aa4e4c6cfa89dc5d03075b6bd797574a8dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258078
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-09-29 16:13:45 +00:00
Rob Findley 19e0367891 internal/lsp/cache: use gopls.mod for the workspace module if it exists
When building the workspace module, prefer a gopls.mod file located at
the root of the view if it exists.

Also do some minor documenting/cleanup along the way.

For golang/go#32394

Change-Id: If87729a766d37e6c834fefe40cfb47b67a36a60c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256582
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>
Trust: Robert Findley <rfindley@google.com>
2020-09-28 18:20:47 +00:00
Rob Findley 42b62fc938 gopls/internal/regtest: fix data race when printing logs
Lock around log access in regtests. This flake was relatively rare in
regtests because of the serial nature of our jsonrpc2 implementation,
but was nonetheless racy.

Fixes golang/go#41653

Change-Id: I08674f42b05624a69d33885c2232c9e31866375b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257957
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: 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: Rebecca Stambler <rstambler@golang.org>
2020-09-28 11:28:10 +00:00
Rebecca Stambler 5d1fdd8fa3 internal/lsp: allow multiple go.mod files in a view
This change allows a view to have multiple go.mod files associated
with it. This doesn't actually make any changes in internal/lsp/cache
with regards to the view's modURI, but it does do the necessary plumbing
in the client packages.

The next CL will delete modURI.

Updates golang/go#32394

Change-Id: I2312efd69c364aed4244ee3769679885a1ebc7e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256941
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: Heschi Kreinick <heschi@google.com>
2020-09-25 19:12:24 +00:00
Rebecca Stambler 0eae6ac92e internal/lsp: add a test for gc annotation details code lens
This change adds a test to check the functionality of the GC details
code lens, including the behavior of toggling it on and off. This
exposed a Windows bug that was mentioned on Slack, which can be fixed by
adjusting the URI.

I also refactored a bit of the code to use a JSON decoder, which
simplifies things a little bit.

Change-Id: I7737107cf020fa35bca245485a3b1a1416920fd2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256940
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: Peter Weinberger <pjw@google.com>
2020-09-25 14:57:16 +00:00