This CL changes the default behavior of gopls to always publish
diagnostics for new file versions. In practice, this avoids stale
diagnostics in multiple LSP clients (golang/go#54983 has more details).
After this change, TestDownloadDeps was failing because it asserted on
the non-existence of published diagnostics. Update the test to treat an
empty diagnostic set the same as an unreceived diagnostic set.
Fixesgolang/go#54983
Change-Id: I41ed2f859b748e14585e7feb53702d3f38dcd599
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429935
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This CL was created using the following commands:
./gopls/internal/migrate.sh
git add .
git codereview gofmt
For golang/go#54509
Change-Id: Iceeec602748a5e6f609c3ceda8d19157e5c94009
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426796
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In several places throughout the x/tools codebase, the internal/lsp/diff
package is used to provide clearer test output when comparing large,
multi-line strings. In some places, this is implemented via the
tests.Diff helper function, but in others it is not (presumably due to
import cycles).
Factor out this pattern into a diff.Pretty function, and add commentary.
Also remove the *testing.T argument, as diffs should never fail; opt to
panic instead. Also add a test.
Using this function, simplify the logic to comparing our 1.18 markdown
output with 1.19 golden content, by normalizing differences between the
two.
This is necessary as markdown content will change as a result of moving
internal/lsp to gopls/internal/lsp.
For golang/go#54509
Change-Id: Ie1fea1091bbbeb49e00c4efa7e02707cafa415cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426776
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Metadata reloading has been significantly refactored recently. Unskip
this test to see if it still flakes.
For golang/go#51352
For golang/go#53878
Change-Id: I9f2ae1835bbace1b5095c2d45db082c4e709437b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/423974
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Significantly refactor the gopls benchmarks to turn them into proper
benchmarks, eliminate the need for passing flags, and allow running them
on external gopls processes so that they may be used to test older gopls
versions.
Doing this required decoupling the benchmarks themselves from the
regtest.Runner. Instead, they just create their own regtest.Env to use
for scripting operations. In order to facilitate this, I tried to
redefine Env as a convenience wrapper around other primitives.
By using a separate environment setup for benchmarks, I was able to
eliminate a lot of regtest.Options that existed only to prevent the
regtest runner from adding instrumentation that would affect
benchmarking. This also helped clean up Runner.Run somewhat, though it
is still too complicated.
Also eliminate the unused AnyDiagnosticAtCurrentVersion, and make a few
other TODOs about future cleanup.
For golang/go#53992
For golang/go#53538
Change-Id: Idbf923178d4256900c3c05bc8999c0c9839a3c07
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419988
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
I believe the races described in the issue have been fixed: we should
invalidate mod tidy results on any metadata change. If this invalidation
doesn't work due to a race, we want to know about it.
Update the test to wait for file-related events to complete before
removing files, in an attempt to avoid windows file-locking issues.
For golang/go#40269
For golang/go#53878
Change-Id: I91f0cb4969851010b34904a0b78ab9bd2808f92e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420718
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Per the explanation at golang/go#54069, allow network access whenever
loading the workspace. Enable one test that exercises this behavior.
More tests will be added in subsequent CLs.
Updates golang/go#47540
Updates golang/go#53676
Updates golang/go#54069
Change-Id: I9c3bb19d36702bc6b8051bee6b7cddaec5b97c0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419500
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Each regtest does a significant amount of extra work re-doing things
like parsing and type-checking the runtime package. We can share this
work across regtests by using a shared cache, significantly speeding
them up at the cost of potentially hiding bugs related to timing.
Sharing this work still retains most of the benefit of the regtests, so
implement this in the default mode (formerly called "singleton" and now
renamed to "default"). In a subsequent CL, modes will be cleaned up so
that "default" is the only mode that runs with -short.
Making this change actually revealed a caching bug: our cached package
stores error messages extracted from go/packages errors, but does not
include these errors in the cache key. Fix this by hashing all metadata
errors into the package cache key.
Updates golang/go#39384
Change-Id: I37ab9604149d34c9a79fc02b0e1bc23fcb17c454
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417587
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Configuration of LSP settings within the regression test runner had
become a bit of a grab-bag: some were configured via explicit fields on
EditorConfig, some via the catch-all EditorConfig.Settings field, and
others via custom RunOption implementations.
Consolidate these fields as follows:
- Add an EnvVars and Settings field, for configuring environment and
LSP settings.
- Eliminate the EditorConfig RunOption wrapper. RunOptions help build
the config.
- Remove RunOptions that just wrap a key-value settings pair. By
definition settings are user-facing and cannot change without
breaking compatibility. Therefore, our tests can and should set the
exact string keys they are using.
- Eliminate the unused SendPID option.
Also clean up some logic to change configuration.
For golang/go#39384
Change-Id: Id5d1614f139550cbc62db2bab1d1e1f545ad9393
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416876
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Package loading (at least via go list) fails when the import header
doesn't parse, so we need to invalidate metadata upon a state change
from non parsing->parsing. Refactor metadata invalidation to implement
this feature, add additional documentation, and avoid unnecessary work.
This change revealed a latent bug (via TestDeleteDirectory): when
statting a deleted directory failed, we could fail to invalidate any
package IDs, even those we already knew about. This bug was masked by
the somewhat complicated semantics of pkgNameChanged. The semantics of
pkgNameChanged are simplified to encapsulate any change to a
package->file association.
Also refactor the parsing API somewhat, and add a bug report if we don't
get a ParseGoFile while inspecting for metadata changes.
Update most regtests to panic upon receiving bug reports.
Fixesgolang/go#52981
Change-Id: I1838963ecc9c01e316f887aa9d8f1260662281ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407501
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
This test is somewhat noisy on the builders.
I'd like to skip it until someone can look into it in more depth.
For golang/go#51352.
Change-Id: I2aa5c9c279807b57872324d84c1bb585cdf34d06
Reviewed-on: https://go-review.googlesource.com/c/tools/+/398699
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Commands will fail with -mod=mod when GOWORK is set. Explicitly set it
to off.
Fixesgolang/go#48941
Change-Id: I39f9d95526ca6f3b0414ff273cecd443d69afa61
Reviewed-on: https://go-review.googlesource.com/c/tools/+/391518
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Set the language version from the controlling go.mod file's go version,
if any. Also verify that we properly surface a diagnostic if the version
is invalid.
I didn't add any quick fixes.
Fixesgolang/go#50688.
Change-Id: Ic472502d1224a1decb5b989d51110b837020e998
Reviewed-on: https://go-review.googlesource.com/c/tools/+/383394
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
There are two possible quick fixes for a missing go.sum entry, and the
regression tests always run all available fixes. That never made sense,
but I never got around to fixing it because it didn't cause a problem.
Now that it turns out to be the cause of the problem described in CL
315152, fix it and roll that CL back.
Change-Id: I49430429a99a412f43bd11b30afe8903db99a694
Reviewed-on: https://go-review.googlesource.com/c/tools/+/315910
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
For some reason the "bad" case of this test fails when the main module
is lazy. Since the go.mod file lacks a 'go' version directive, as of
CL 315210 it is upgraded to a lazy module the first time it is edited
by an invocation of cmd/go.
I don't know why this case doesn't exhibit the expected failure mode
when lazy loading is enabled — I guess it's because we attribute the
checksum error to the individual package that needs it instead of the
module as a whole? — but I can't follow this test well enough to
figure out whether there is actually a real problem for lazy modules
here. If there is, we can handle that by adding a new, separate test.
For golang/go#36460
Change-Id: I0949e1f9f5cb1b6c884706e50a9694232308387b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/315152
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Note: this only moves the regtest framework, not the gopls regtest
tests.
Change-Id: Ia70d2e97df8a8bd48a042e5b037c1e56a210b594
Reviewed-on: https://go-review.googlesource.com/c/tools/+/312412
Trust: Paul Jolly <paul@myitcv.org.uk>
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Previously, we were running `go get` only on modules, which led to us
not downloading dependencies of packages. This resulted in further
go.mod diagnostics that users could not resolve with quick fixes. Now,
download packages directly so that dependencies are downloaded.
Fixesgolang/go#44307
Change-Id: Id764ea5a2f7028e238eadaaba0ca3cfc765b85b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/293729
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>
Our handling of go command errors was cobbled together, leading to
unexpected gaps and duplication. Refactor it to be more coherent.
Our goal is to turn every go command error into a diagnostic in the
relevant location. The errors don't contain error positions, so we have
to guess where they belong using the module names mentioned in the
error. If we can't find any reference to those modules, we are forced to
add diagnostics to all go.mod files.
I may have destroyed the intent of TestMultiModule_OneBrokenModule but
I'm not sure what to do about it.
Some cleanup along the way:
- Stop parsing modfile.Parse error text: it returns structured errors
and we can just use them.
- Return CriticalErrors from awaitLoadedAllErrors, and do error
extraction lower in the stack. This prevents a ridiculous situation
where initialize formed a CriticalError, then awaitLoadedAllErrors
returned just its MainError, and then GetCriticalError parsed out
a new CriticalError from the MainError we got from a CriticalError.
- In initialize, return modDiagnostics even if load succeeds: we are
missing packages and should not silently fail, I think?
- During testing I tripped over ApplyQuickFixes' willingness to not
actually do anything, so I made that an error.
Fixesgolang/go#44132.
I may also have fixedgolang/go#44204 but I haven't checked.
Change-Id: Ibf819d0f044d4f99795978a28b18915893e50c88
Reviewed-on: https://go-review.googlesource.com/c/tools/+/291192
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>
Reviewed-by: Robert Findley <rfindley@google.com>
We currently write directly to go.mod/sum via the go command, expecting
that editors will pick up the changes. While that's true for VS Code,
vim doesn't necessarily reload unchanged buffers. Change to send
explicit edits instead, but only if the file is open. Behavior when
using Go versions that don't support -modfile is unchanged.
Fixesgolang/go#44035.
Change-Id: Ie4e5cf60673b860f5dfcbdb726bee0efe6aae405
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290189
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The only thing that the mod tidy diagnostics use the network for is
adding dependencies, and we already have quick fixes for those. The one
exception is the case covered by TestBadlyVersionedModule, a dependency
that fails to declare one of its own dependencies and therefore requires
an indirect dependency in the workspace module. That only triggers an
error on the dependency's import statement, which the user will never
see.
Fortunately, the go command does expose these problems in the DepsErrors
field of the list response. Add an internal API to access that, and turn
it into diagnostics on both the file and the controlling go.mod.
Refactor the go get diagnostic generation so that it applies to both
modules and packages.
Fixesgolang/go#38462.
Change-Id: Ie2af940087654682a40de9ecfccd46f404a88b60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289309
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>
The Typescript source is still at version 3.16, but there are new
requests, more detailed client capabilities, and an attempt to be
more specific about ranges of number in the Typescript code.
Vscode defines types integer and uinteger (32-bit signed and unsigned),
so the Go code now uses int32 and uint32.
They've changed the use of TextDocument, so version information is sometimes
missing. cache/session.go:625 was changed correspondingly.
This CL also make CodeAction.Disabled into a pointer.
New requests or notifications:
DidCreateFiles, DidRenameFiles, DidDeleteFiles (notifications)
ShowDocument, WillCreateFiles,WillRenameFiles, WillDeleteFiles (request)
It's a lot of code; I've probably missed something.
Change-Id: I8449ad8473ac00947d0344c5f6133f9bd73b9e10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286192
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Now that workspace module mode generates a combined go.sum there are
relatively few blockers to enabling -mod=readonly. Fix them and do it.
This CL is a bit of a grab bag, but the fixes are relatively separate. I
can split it into multiple CLs if desired.
- If module A depends on module B at v1.0.0, the go command will want to
upgrade the workspace module from v0.0.0-goplsworkspace to v1.0.0. To
prevent that, use vN.999999.0 as the base pseudoversion, adjusting v0 to
v1 where appropriate. A few test cases needed updating as a result.
- For old Go versions, sort the generated workspace module and
synthesize a go statement from the maximum go version declared in the
workspace.
- Some regtests need go.sum files created.
- matchErrorToModule created incorrect quick fixes: it would try to
download the top-level module mentioned in the error message, not the
one that actually caused the problem. Now it issues quick fixes for the
lowest-level module.
- TestMultiModuleModDiagnostics accidentally included the same module
in the workspace twice. Fix it, and make that an error.
Fixesgolang/go#43346.
Change-Id: I605f762a4d23bedd914241525e64c1b3ecc42150
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287032
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Regtests have gotten large, and started timing out on some slow
builders. They also can't yet be run in parallel, due to some
process-level shared state (e.g. gc_details). Furthermore, there seems
to be some per-process resource leak that we don't yet fully understand
causing slowdown as the tests progress.
Address these problems by splitting the regtests up into multiple
packages. This also makes it easier to run only relevant tests during
development.
For golang/go#42789
For golang/go#39384
Change-Id: I1a74e4c379f3a650f4c434db44f9368e527aa459
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287572
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Robert Findley
Bryan C. Mills
Heschi Kreinick
Paul Jolly
Rebecca Stambler
pjw