Commit Graph

85 Commits

Author SHA1 Message Date
Suzy Mueller a4274a8a0e gopls: add diagnostics for non-stdlib vulnerabilities
Show the vulnerabilities found by the runvulncheck codelens in the
go.mod file using diagnostics. This uses a similar strategy to
upgrade codelenses to store the vulnerabilities in the view so the
diagnostics can be calculated at a later time.

Change-Id: Ie9744712d9a7f8d78cbe3b54aa4cd3a380a304bc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/433537
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
2022-09-28 19:25:30 +00:00
Robert Findley bddb3720ba gopls: deprecate three experimental features
Deprecate the following features, which never made it out of
experimental:
- experimentalWorkspaceModule (golang/go#52897)
- experimentalWatchedFileDelay (golang/go#55268)
- experimentalUseInvalidMetadata (golang/go#54180)

See the associated issues for rationale behind the deprecations.

With this change, any users configuring these settings will get a
ShowMessageRequest warning them of the deprecation.

Fixes golang/go#52897
Fixes golang/go#55268
Fixes golang/go#54180

Change-Id: I49f178e68793df4e4f9edb63e9b03cad127c5f51
Reviewed-on: https://go-review.googlesource.com/c/tools/+/434640
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-09-28 16:54:30 +00:00
Suzy Mueller 2f04713366 gopls: fix out of bounds bug in extract
There was a bug where the code would try to access an out of bounds
index when trimming extra space and comments in extract. This change
also adds a regtest for extract.

Fixes golang/go#55271

Change-Id: I7da716a6a68a9678011abf15def47acdea0b33fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/432135
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-09-21 16:48:39 +00:00
Robert Findley eeaf4eb24c internal/lsp/fake: add rename file support for testing
This CL implements the fake.Editor.RenameFile method, which mimic's the
behavior of VS Code when renaming files. Specifically:
- open buffers affected by the renaming are closed, and then reopened
  with new URIs
- files are moved on disk
- didChangeWatchedFile notifications are sent

Along the way, add missing comments and fix one place where the editor
mutex was held while sending notifications (in Editor.CreateBuffer).
Generally, the editor should not hold any mutex while making a remote
call.

For golang/go#41567

Change-Id: I2abfa846e923de566a21c096502a68f125e7e671
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427903
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-09 15:30:36 +00:00
Robert Findley b15dac2b88 gopls: migrate internal/lsp to gopls/internal/lsp
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>
2022-09-07 16:44:44 +00:00
Hana (Hyang-Ah) Kim eb8352e307 gopls/internal/govulncheck: sync x/vuln@62b0186
VulnDB OSV schema was changed recently
  https://go-review.googlesource.com/c/vulndb/+/424895
to fix the misinterpretation of 'affected.package.name',
and the database entries were repopulated with the new schema.
We need to update the client library to pick up the change.
We also need to update the fake vulndb entries used in tests.

gopls/regtest/misc/testdata/vulndb was copied from
  golang.org/x/vuln/cmd/govulncheck/testdata/vulndb @ 62b0186
(the version updated in cl/424895)

Also reverse golang.org/cl/425183 which includes the position
information in the SummarizeCallStack result. Like in govulncheck -v,
the position info is already available in the callstack, thus
this is unnecessary for us. Since x/vuln is currently frozen
until the preview release, revert it from gopls/internal/vulncheck.

Ran go mod tidy -compat=1.16; otherwise, the transitive dependency
on github.com/client9/misspell from golang.org/x/vuln breaks go1.16
build.

Updated copy.sh script to copy x/vuln/internal/semver package
(golang/go#54401) and add the build tags back to all go files.
Gopls's builder builds&tests packages with old go versions,
so we still need go1.18 build tag.

Fixes golang/go#54818

Change-Id: I37770d698082378656a7988d3412a4ca2196ca7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427542
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
2022-09-02 15:10:33 +00:00
Robert Findley 4ccc73cbb5 internal/lsp/tests: simplify comparison of markdown at go1.18
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>
2022-08-31 21:08:24 +00:00
Robert Findley 5a26068387 internal/lsp/source: remove utm_source from pkgsite links
This query parameter is not needed.

Change-Id: Id45d7be0b1cbe5d383bcc6768ef20df26de3e7b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422901
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
2022-08-15 16:22:50 +00:00
Robert Findley 96d05aa120 gopls/internal/regtest: fix TestFailingDiagnosticClearingOnEdit
This test was asserting immediately on the non-presence of published
diagnostics, and therefore was racing with the diagnostics calculation
pass. Following https://go.dev/cl/420539 this became a flake, because
once gopls has calculated diagnostics for the open event, it will
actually publish empty diagnostics for the opened file.

Update the test to use a OnceMet so that we only evaluate the
expectation once we've finished the diagnostics pass. As a result, the
test deterministically failed, which was fixed by using the
EmptyOrNoDiagnostics expectation.

Fixes golang/go#54271

Change-Id: Id3f220ce44c7996132699a724b6c627f034e367f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/422136
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>
2022-08-09 15:36:31 +00:00
Hana (Hyang-Ah) Kim 81c7dc4e4e internal/lsp: polish vulncheck progress messages
gopls.run_vulncheck_exp runs `gopls vulncheck`
(fork of govulncheck) and pipes its stderr (logs)
as progress messages. The default log format
includes timestamp and that is too long for progress
message. Tell gopls vulncheck to omit timestamp
in the log message.

Use "govulncheck" as the progress message prefix,
instead of the long "Checking vulnerabilities".

Change-Id: I92fe9958b20d0260711a42af9b5f9f399e267587
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420998
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 20:05:03 +00:00
Hana (Hyang-Ah) Kim 6c27717f2a internal/lsp/mod/code_lens: add "run govulncheck" codelens
And, make gopls.run_vulncheck_exp show an information/error
message popup after a successful run. This is temporary.
We plan to publish the results as diagnostics and quick-fix.

Finally, changed the stdlib vulnerability info id in
testdata to GO-0000-0001 which looks more like a vulnerability
ID than STD.

Changed TestRunVulncheckExp to include tests on codelens
and use the command included in the codelens, instead of
directly calling the gopls.run_vulncheck_exp command.

Change-Id: Iaf91e4e61b2dfc1e050b887946a69efd3e3785b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420995
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 20:03:56 +00:00
Robert Findley 763f65c3d2 gopls/internal/regtest/misc: simplify shared edit tests
In order to avoid shutdown races we must always close the second editor
created for simultaneous edit tests.

Rather than introduce additional indirection, just merge the two tests
into one.

For golang/go#54241

Change-Id: I6604141baa77d07f6281d3a90efa13c02b94079a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/421258
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-04 19:57:25 +00:00
Hana (Hyang-Ah) Kim b5fd08821a internal/lsp/command: replace VulncheckArgs Dir with URI
commandHandler.run expects a valid file for forURI and
forURI is necessary to fully populate commandDeps.
Our use of directory URI does not work.

We plan to use this custom command triggered through
codelenses on documents (e.g. go.mod), so we use
the document's URI.

Change-Id: I4de6d488dec5a4b71716499e7fc5e8328ecf3e49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420994
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-08-04 18:58:55 +00:00
Hana (Hyang-Ah) Kim bceee4b059 internal/lsp/command: let RunVulncheckExp call gopls vulncheck
By making gopls.run_vulncheck_exp (RunVulncheckExp implements)
call `gopls vulncheck`, we achieve

  - gopls.run_vulncheck_exp can run asynchronously and be cancellable
  - log information can be forwarded as progress messages
  - isolate any failures during vulncheck execution

In this CL, we also changed not to include test files in the analysis
(match the default of govulncheck). We will add an option in the future.

TODO:
 - prevent concurrent gopls.run_vulncheck_exp
 - convert the gopls vulncheck output to diagnostics and publish it
 - remove timestamps from the `gopls vulncheck` log messages
   for simplify progress messages
 - add test to check vulnerability in third-party dependencies

Change-Id: I21592e03794cd9e9d96ed3989973a2ab7d75c538
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420717
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-04 18:21:12 +00:00
Robert Findley 87f47bbfb4 gopls/internal/regtest/bench: refactor and improve benchmarks
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>
2022-08-04 14:58:59 +00:00
Robert Findley 0d04f65da9 internal/lsp: re-send diagnostics on file events
Fix golang/go#50267 by ensuring that diagnostics are re-sent following
didOpen or didClose events. Additionally, introduce a new hidden
'chattyDiagnostics' option that causes diagnostics to be resent on
*every* file change event. This latter option is for LSP clients that
get confused when diagnostics are not re-sent for later file versions.
For now, be conservative and only force diagnostic publication on
didOpen and didClose.

Update tests whose 'NoDiagnostics' assertions were broken by the new
behavior.

Fixes golang/go#50267

Change-Id: I6332d66a1851e0d8261599d37020a03b4c598f7d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420539
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-08-02 18:52:36 +00:00
Dylan Le bd3f524777 internal/lsp: rename all the package names in the renamed package
This CL contains a partial implementation of package renaming. Currently gopls is only able to rename references to the renaming package within the renaming package itself.

prepareRename is still expected to return an error for renaming a package.

For golang/go#41567

Change-Id: I3683a0a7128bba7620ef30db528f45b753e6c08f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420219
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dylan Le <dungtuanle@google.com>
2022-08-01 17:28:00 +00:00
Dylan Le 9580c84d57 internal/lsp: Check if user's editor support rename operation
Change-Id: Iadda768e93eda1d53fa00a5ff8a28013a575ef57
Reviewed-on: https://go-review.googlesource.com/c/tools/+/419774
Run-TryBot: Dylan Le <dungtuanle@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-29 20:19:23 +00:00
Robert Findley 39a4e36475 internal/lsp/regtest: only run /default tests with -short
Due to shared caching, the "default" tests can run faster than other
execution modes and still retain most of the test coverage. Update the
test runner to only run the singleton tests if testing.Short() is true,
independent of GOOS.

On the other hand, we lost noticeable coverage when disabling the
Forwarded testing mode. Now that the regtests are lighter weight in
general, reenable it on the longtests builders.

While at it, clean up tests that used the server-side Options hook to
instead use Settings, since clients would configure their server via
Settings and Options can't work with a shared daemon.

Updates golang/go#39384

Change-Id: I33e8b746188d795e88841727e6b7116cd4851dc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418774
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-26 21:20:53 +00:00
Robert Findley f157068c1b internal/lsp/regtest: allow sharing memoized results across regtests
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>
2022-07-26 21:20:42 +00:00
Robert Findley a2a24778ba gopls/internal/regtest: externalize shouldLoad tracking
The fundamental bug causing TestChangePackageName to fail has been
fixed, yet unskipping it revealed a new bug: tracking whether or not a
package should be loaded requires that we actually store that package in
s.meta. In cases where we drop metadata, we also lose the information
that a package path needs to be reloaded.

Fix this by significantly reworking the tracking of pending loads, to
simplify the code and separate the reloading logic from the logic of
tracking metadata. As a nice side-effect, this eliminates the needless
work necessary to mark/unmark packages as needing loading, since this is
no longer tracked by the immutable metadata graph.

Additionally, eliminate the "shouldLoad" guard inside of snapshot.load.
We should never ask for loads that we do not want, and the shouldLoad
guard either masks bugs or leads to bugs. For example, we would
repeatedly call load from reloadOrphanedFiles for files that are part of
a package that needs loading, because we skip loading the file scope.
Lift the responsibility for determining if we should load to the callers
of load.

Along the way, make a few additional minor improvements:
 - simplify the code where possible
 - leave TODOs for likely bugs or things that should be simplified in
   the future
 - reduce the overly granular locking in getOrLoadIDsForURI, which could
   lead to strange races
 - remove a stale comment for a test that is no longer flaky.

Updates golang/go#53878

Change-Id: I6d9084806f1fdebc43002c7cc75dc1b94f8514b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417576
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-22 21:31:20 +00:00
Robert Findley 459e2b88fc internal/lsp/progress: actually close over Context in WorkDoneWriter
CL 409936 eliminated cases where we close over a Context during progress
reporting, except in one instance where it wasn't possible: the
WorkDoneWriter that must implement the io.Writer interface.

Unfortunately it contained a glaring bug that the ctx field was never
set, and the regression test for progress reporting during `go generate`
was disabled due to flakiness (golang/go#49901).

Incidentally, the fundamental problem that CL 409936 addressed may also
fix the flakiness of TestGenerateProgress.

Fix the bug, and re-enable the test.

Fixes golang/go#53781

Change-Id: Ideb99a5525667e45d2e41fcc5078699ba1e0f1a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417115
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-07-12 22:14:18 +00:00
Robert Findley 6e6f3131ec internal/lsp/regtest: simplify, consolidate, and document settings
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>
2022-07-12 16:56:46 +00:00
Robert Findley e1ec1f3230 internal/imports: use a module resolver if GOWORK is set
Previously, gopls would fall back on a gopath resolver when running
goimports from a directory containing go.work (but not go.mod). Fix this
by update the code to recognize that GOWORK also puts goimports into
module mode.

All the work to _support_ go.work had already been done, but the tests
were only passing because they were setting GO111MODULE=on explicitly
(and therefore GOMOD=/dev/null was satisfying the pre-existing check).

Also add a test for the regression in gopls.

Fixes golang/go#52784

Change-Id: I31df6f71a949a5668e8dc001b3ee25ad26f2f927
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413689
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
2022-06-24 21:03:09 +00:00
Dylan Le 59bd4faed9 internal/lsp: find references to package
Update References to detect if the package is referenced and a regtest to test within and external package references.

Updates golang/go#41567

Change-Id: I607a47bf15f1c9f8236336f795fcef081db49d6a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408714
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-06-21 19:39:47 +00:00
Rob Findley 697795d6a8 internal/lsp/regtest: don't run the connection on the test context
When test awaiting fails, we often fail to shut down the server because
the pipe is closed. Fix this by using a detached context for running the
connection.

Also clean up some unnecessary context arguments.

Change-Id: I535c1cc1606e44df5f8e2177c92293d57836f992
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340850
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>
2022-06-10 12:48:42 +00:00
Robert Findley ccb10502d1 internal/lsp/cache: invalidate metadata when parsing issues resolve
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.

Fixes golang/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>
2022-05-23 18:14:40 +00:00
Robert Findley 728485ffcb gopls/internal/regtest: add a test for using staticcheck with generics
For golang/go#52159

Change-Id: I08120331b7f5c9eb06feac0d0eeb76a9a7b629df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399914
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Le <dungtuanle@google.com>
2022-05-11 20:19:40 +00:00
Robert Findley 4a3fc2182a internal/lsp: only linkify urls with http, https, and ftp schemes
Fixes golang/go#43990

Change-Id: I0ea26d521b2432238b05c26bfaccef6fc773dcf2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/393854
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-02 22:00:12 +00:00
Dylan Le 559469ae16 internal/lsp: render package documentation when hovering over a package import
Update FindHoverContext to retrieve & render package documentation for a hovered package. Add a regtest for this hovering feature.

Updates golang/go#51848

Change-Id: If57396d59be9c4cf7e09b64e39832de6f996c7ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400820
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-25 17:29:41 +00:00
Robert Findley b2552ef554 internal/lsp: run go mod vendor exclusively to avoid file contention
Running go mod vendor can result in vendor/modules.txt being transiently
deleted while it is being updated. On Windows this introduces potential
problems with file locking, if modules.txt is being read by another go
process, such as an ongoing package load.

Change the command to use RunGoCommandPiped, which executes serially
within the gopls process.

For golang/go#49646

Change-Id: If2d1fe5431122ba05014051a0e9c303cf7ee9cb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399714
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-11 22:06:59 +00:00
Robert Findley 7cc24c2ba3 gopls: upgrade staticcheck to v0.3.0
Upgrade staticcheck to v0.3.0 to pick up support for generics. Since
this version only supports Go 1.17+, increase the version at which we
install staticcheck to 1.17 (from 1.15). This change is likely to cause
confusion for users on Go 1.16, so show a warning when setting
the "staticcheck" option to an unsupported value. Slightly refactor our
setting of options along the way.

Fixes golang/go#52159

Change-Id: Id9b4cee340e31988c64ca712d98573343aaf5848
Reviewed-on: https://go-review.googlesource.com/c/tools/+/396974
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-04-07 14:59:24 +00:00
Hana 54af36eca2 internal/lsp: handle invalid URL args in command params
beginFileRequest returns a nil error when file request url is
not a valid file url format. This is reported with a separate 'ok'
boolean return value.

This bug caused custom gopls commands to return success with
an empty result, instead of reporting the problem as an error.

Change-Id: I8eed39368087fa9928bcc68853225ea432f500d2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/395814
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-03-25 15:45:26 +00:00
Robert Findley 4adadedf93 internal/lsp/source: handle empty strings in directoryFilters
Avoid a panic when a directoryFilters entry is empty.

Fixes golang/go#51843

Change-Id: I50948894ce60d83441c3fc698c2e7d80d1d2b50e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/395675
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>
2022-03-24 21:16:24 +00:00
Robert Findley 5210e0ca15 gopls: wire in LangVersion and ModulePath for gofumpt formatting
If available, pass the LangVersion and ModulePath provided by
go/packages to gofumpt. This allows gofumpt to apply additional
formatting rules that require this information.

Also add a regression test for gofumpt formatting.

Fixes golang/go#51327

Change-Id: I47c8c96d595d62e1c444285ce69ce6a4e61fa74c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/387634
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-24 18:08:50 +00:00
Robert Findley fd59bdfe0d internal/lsp/source: adjust object position when formatting full AST
Thread through an adjusted object position when we format an re-parsed
AST that wasn't type-checked to produce the object. Also remove some
unnecessary handling of the object type.

Fixes golang/go#46158

Change-Id: Ic8bee2bb4cb992d577d085599213fca1deab3b4e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/384697
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-17 17:37:03 +00:00
Heschi Kreinick 414ec9c3f0 internal/lsp: add ListImports
The VS Code extension uses information about imports to figure out
whether a given function is a Testify test. As of writing, it asks:
- Does the file import Testify?
- Does the package it's in do so?
To answer these questions, add ListImports, which tells you about the
packages imported by the current file, including their import name,
plus the import paths of all imports in the entire package.

I suspect the latter may be wrong in the presence of GOPATH vendoring,
but that should be a relatively rare situation at this point so I didn't
bother testing.

Fixes golang/go#40514.

Change-Id: I4c69e1db80dce6e594bdb595a81aade1ddec4d29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/383354
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>
2022-02-04 22:09:29 +00:00
Robert Findley 21ca3b3a93 internal/lsp/source: parse symbol queries when using fastfuzzy
Hook up the fastfuzzy symbol matcher to our fzf-style query parsing, for
consistency with the (slow) fuzzy matcher.

In the past I had wanted to implement this natively inside the
SymbolMatcher, but it is much simpler to keep using combinators. In the
common case we'll just be using fuzzy matching.

For golang/go#50016

Change-Id: I1c62c8c8e9d29da570cb1e4034c2b10782529081
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376362
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: Peter Weinberger <pjw@google.com>
2022-01-19 15:06:07 +00:00
Robert Findley 3737ecd836 internal/lsp/fuzzy: consider whole word matching in SymbolMatcher
In the new SymbolMatcher, add an additional scoring feature that
considers whether the matching pattern streak is part of a whole-word
match. This differentiates matches of "foo" against "pkg.foo" and
"pkg.foobar".

For golang/go#50016

Change-Id: Ib84ff13eee0b7ec23143325592cef9a41be07375
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376361
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-01-19 15:05:25 +00:00
Robert Findley a222cdb107 gopls/internal/regtest: add regression tests for template diagnostics
Add some additional regressions tests for our loading of templates based
on language ID and/or the configured templateExtensions. Move these
tests to a new regtest package "templates", so that they may be easily
run together.

Fixes golang/vscode-go#1957

Change-Id: Ic83454725e9aec41b3c1f5202bb68d97cc73c839
Reviewed-on: https://go-review.googlesource.com/c/tools/+/378394
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: Suzy Mueller <suzmue@golang.org>
2022-01-13 20:57:08 +00:00
Robert Findley c4cfc425f2 gopls: change the default value for templateExtensions to be empty
There is no standard for go template file extensions, and users may not
want this functionality. Make template support opt-in by changing the
default value of templateExtensions to be [].

Updates golang/vscode-go#1957

Change-Id: I7e37d22b1bc63d8300634b3b0394b4036b43fa49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/375874
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-01-13 20:56:24 +00:00
Robert Findley 0a91d41f6f internal/lsp/source: sort workspace symbol results for stability
Add additional order invariants to workspace symbol results to
differentiate between items of equal score: sort next by symbol length,
and then finally by lexical ordering.

For golang/go#50016

Change-Id: Ic5cda2628f57cecfe972b7585525c49b0f8518bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376360
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: Paul Jolly <paul@myitcv.org.uk>
2022-01-11 13:58:16 +00:00
Robert Findley f234b3d4ab internal/lsp/source: keep collecting symbols on metadata errors
Workspace symbols requests fail when a file in the workspace is missing
metadata, such as if a build-tagged file is open in an overlay. We keep
going if metadata is missing, but not on metadata errors (such as
packages.Load errors).

Change this to log and keep going on errors.

Fixes golang/go#50536

Change-Id: I41331e864bf21e4f39674248bc30bfc02a4c3d1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/377234
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: Suzy Mueller <suzmue@golang.org>
2022-01-10 19:19:50 +00:00
Shoshin Nikita feb39d0cd7 internal/lsp/source: don't format generated files
Fixes golang/go#49555

Change-Id: I53e3c0d34be6a545386d6766371fbeda8a2b2ffb
GitHub-Last-Rev: 516168765671c8135523fcf801f50455b352bdab
GitHub-Pull-Request: golang/tools#349
Reviewed-on: https://go-review.googlesource.com/c/tools/+/365295
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Robert Findley <rfindley@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2021-12-06 16:11:25 +00:00
Robert Findley f64c0f46fa internal/lsp/analysis/fillreturns: update fillreturns for new type errs
The error messages produced by go/types related to an incorrect number
of return values have changed, and have been repositioned. Update the
fillreturns analyzer to handle the Go 1.18 form of these errors.

Ideally we'd use error codes here, but can't because they are not
forwarded by go/packages. Added a TODO to revisit when error codes are
exposed.

Change-Id: I4c594fd2fd1cb7423f37ff9c6a57e9490dcbc2a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/367714
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: Suzy Mueller <suzmue@golang.org>
2021-12-03 18:55:02 +00:00
Bryan C. Mills 3c63f30840 gopls/internal/regtest/misc: temporarily skip TestGenerateProgress
For golang/go#49901

Change-Id: Id41c6437fadf1c539488662fc0ca5bc746b8a498
Reviewed-on: https://go-review.googlesource.com/c/tools/+/368216
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-12-01 18:47:44 +00:00
Robert Griesemer 6e52f51f5b x/tools: temporarily skip a couple of tests
Skip tests so we can get CL 367196 submitted. After that
reenable tests (and adjust as needed).

Change-Id: Id0a6d6b873a0760ae438f6f0cb147c20d00b1ca6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/367574
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-11-29 20:12:13 +00:00
Bryan C. Mills 771dabf433 gopls/internal/regtest/misc: skip TestInconsistentVendoring on Windows
This test is probably detecting a real bug that exclusively affects
Windows. To avoid masking other bugs, let's skip the test on that
platform until the bug is fixed.

For golang/go#49646

Change-Id: I38c615d47454ecfd72b416fff2018973b3ae9259
Reviewed-on: https://go-review.googlesource.com/c/tools/+/364676
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: Robert Findley <rfindley@google.com>
2021-11-17 17:12:49 +00:00
Robert Findley 96715ada81 internal/lsp: check for invalid URIs in definition
URI.Filename() panics on non-file URIs. Just pass UnknownKind to
beginFileRequest to allow handling template files.

Fixes golang/go#49223

Change-Id: Ic2add49d3d0c04855bf25583712102a50fb425ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/359734
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-10-29 19:33:54 +00:00
Robert Findley d5520c2912 internal/lsp/source: check for nil objects in call hierarchy
Fixes golang/go#49125

Change-Id: Id91415377240ab87aeaddb9d668281509d073510
Reviewed-on: https://go-review.googlesource.com/c/tools/+/358549
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-10-27 16:11:50 +00:00