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

1582 Commits

Author SHA1 Message Date
Rebecca Stambler
80cb79702c internal/lsp: don't publish duplicate diagnostics in one report
CL 242579 changed the mechanism of reporting diagnostics so that we now
combine reports from multiple sources. Previously, we overwrote existing
diagnostics if new ones were reported. This was fine because
source.Diagnostics generated all of the diagnostics for Go files, and
mod.Diagnostics generated all of the diagnostics for go.mod files.

Now, we combine diagnostics from both sources -- mod.Diagnostics can
generate reports for Go files. We may get duplicate reports for packages
with test variants, so now, we check for those and dedupe.

Change-Id: I42e98079b4eead380058dd029a3a0c72a1796ebb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 19:34:40 +00:00
Brayden Cloud
bd1e9de8d8 internal/lsp: separate test and benchmark codelens
This CL adds benchmarking by using the "-bench" flag for the test
command.

Change-Id: Idf714de1c6c3350d26a6874e7b4278927d0336fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243159
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-22 18:17:40 +00:00
Rebecca Stambler
6123e77877 internal/lsp: support go mod tidy on save without diagnostics
This change adds support for `go mod tidy` on save when users opt into
import organization on save. Previously, we supported this with the
go mod tidy command, but there's no need to do this when we already
have a ModTidyHandle available.

Change-Id: Ibb47b6a7611fd823dac2a3b4f3a43b9fbb3289b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 22:32:18 +00:00
Rebecca Stambler
b42efcd11c internal/lsp: try to parse diagnostics out of go list errors
This change attempts to parse diagnostics out of `go list` error
messages so that we can present them in a better way to the user. This
approach is definitely tailored to the unknown revision error described
in golang/go#38232, but we can modify it to handle other cases as well.

Fixes golang/go#38232

Change-Id: I0b0a8c39a189a127dc36894a25614535c804a3f0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 22:03:47 +00:00
Rebecca Stambler
0a5cd10191 internal/lsp: handle unknown revision in go.mod file
This change ensures that, when the initial workspace load fails, we
re-run it if the go.mod file changes. Previously, if a user opened a
workspace with a corrupt go.mod file, we never recovered.

To reinitialize the workspace on-demand, we use the initializeOnce field
as an indicator of whether or not we should reinitialize. Every call to
awaitInitialized (which is called by all functions that need the IWL),
passes through the initialization code. If a retry isn't necessary,
this is a no-op, but if it is, we will call the initialization logic.
Only the first attempt uses a detached context; subsequent attempts can
be canceled by their contexts.

To indicate that we should reinitialize, we call maybeReinitialize.
Right now, we only call this when the go.mod file changes. In the
future, we may need it in other cases.

Fixes golang/go#38232

Change-Id: I77eefebb0ac38fbd0fe2c7da09c864eba45b075f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242159
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 21:40:18 +00:00
Rebecca Stambler
acdb8c158a internal/lsp: handle on-disk file deletions for opened files
Previously, we only updated the opened file's overlay, but not the
snapshot. This meant that the snapshot was still operating with stale
data. Invalidating the snapshot creates a new snapshot with the correct
set of overlays.

The test is skipped because it will flake until we have a better caching
strategy for `go mod tidy` results.

Updates golang/go#40269

Change-Id: Ia8d1ae75127a1d18d8877923e7a5b25b7bd965ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-21 21:30:00 +00:00
Rebecca Stambler
5ea363182e internal/lsp: change the way that we pass arguments to command
Our approach to commands and their arguments has been ad-hoc until this
point. This CL creates a standard way of defining and passing the
arguments to different commands. The arguments to a command are now
json.RawMessages, so that we don't have to double encode. This also
allows us to check the expected number of arguments without defining
a struct for every command.

Change-Id: Ic765c9b059e8ec3e1985046d13bf321be21f16ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-21 16:30:27 +00:00
Pontus Leitzler
b8e13e1a4d internal/lsp: return the actual range from convenience code actions
Convenience code actions operates at line-by-line, but a line can
contain more than one fix.

To be able to tell them apart each response now return the actual
range as reported by the analyzer.

Fixes golang/go#40328

Change-Id: I9ffe4737653d2a5a6dafc3574e74975a71c71937
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243877
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-21 15:44:06 +00:00
Rebecca Stambler
77f530d86f internal/lsp: handle deletion of file content
We had previously been treating file changes with no content in the same
way as the deletion of content. Now, we distinguish between the two.

Fixes golang/go#38424.

Change-Id: I44b338006af0c13a8a3f842844521789ea378470
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243577
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-21 03:22:37 +00:00
Rebecca Stambler
4025ed8474 internal/lsp: move fillstruct suggested fixes out of analysis
This change moves the suggested fixes logic for fillstruct out of the
analysis and into internal/lsp/source. This logic is then used as part
of a new fillstruct command. This command is returned along with the
code action results, to be executed only when the user accepts the code
action.

This led to a number of changes to testing. The suggested fix tests in
internal/lsp doesn't support executing commands, so we skip them. The
suggested fix tests in internal/lsp/source are changed to call
fillstruct directly. A new regtest is added to check the command
execution, which led to a few regtest changes.

Also, remove the `go mod tidy` code action, as it's made redundant by
the existence of the suggested fixes coming from internal/lsp/mod.

Change-Id: I35ca0aff1ace8f0097fe7cb57232997facb516a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241983
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-20 20:42:44 +00:00
Josh Baum
6d307edf52 internal/lsp: support extract function
Extract function is a code action, similar to extract variable. After
highlighting a selection, if valid, the lightbulb appears to trigger
extraction. The current implementation does not allow users to
extract selections with a return statement.

Updates golang/go#37170

Change-Id: I5fc3b19cf7dbca4407ecf0cc37017661223614d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241957
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-20 18:55:41 +00:00
Rebecca Stambler
9cbb971a18 internal/lsp: split only on the first = for Go environment variables
Follow-up from CL 243538.

Change-Id: I15ac8ba738fa5326b18f7b179bed8623f4c57cfd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-20 16:24:43 +00:00
Rebecca Stambler
e6549b8dc8 internal/lsp: don't re-run go env for every call to WriteEnv
This is unnecessary. It also saves a lot of time for the regtests.

Change-Id: Ifc12d4c8e0c215f336e958316d415bc7f30e5344
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243538
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-20 15:02:21 +00:00
Rebecca Stambler
6ddee64345 internal/lsp: add a configuration to enable/disable links in hover
I had previously suggested that users set LinkTarget to "" to avoid
links in the hover text. However, this work-around isn't perfect because
it also disables the documentLink behavior in other cases.

Change-Id: I3df948e2a2e4d2312998de65ccea8dfb404768ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243239
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-17 02:43:01 +00:00
Rebecca Stambler
d518495ee8 internal/lsp: log errors for compute fix edits instead of returning
Fixes golang/go#40260

Change-Id: I69032b8cd6b32a262ecd3bb746ef6d1722966a51
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243238
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-17 02:25:49 +00:00
Rebecca Stambler
58eba7e750 internal/lsp: refactor go.mod diagnostics to simplify the API
Currently, diagnostics for modules that are missing from the go.mod
appear in the Go files in which those modules are imported. As a result,
the diagnostics were previously calculated as part of the Go file
diagnostic calculations. This is convoluted and required passing around
an extra map.

This CL puts that logic in the ModTidyHandle where it belongs.
The diagnostics for the Go files are combined from the multiple sources.

Also, added a skipped test for golang/go#39784, since this CL was
originally intended to be a fix for that issue...

Change-Id: Ic0f9aa235dcd56ea131a2339de9801346f715415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-16 22:50:03 +00:00
Rob Findley
f2c07d7d8e internal/lsp/lsprpc: improvements to daemon logging
The gopls daemon had different default logging behavior than the sidecar
gopls: by default, the daemon was started with -logfile=auto.
Additionally, because most logs are reflected back to the forwarder, the
actual daemon logs have very little (if any) information.

This means that if you simply start gopls with -remote=auto, you'll get
a single logfile named /tmp/gopls-<pid>.log, which is mostly empty. This
is not a delightful experience.

Fix this via several improvements:
 + Log lifecycle events in the Daemon, to give the log a purpose.
 + Give the daemon a new default log location:
   /tmp/gopls-daemon-<pid>.log.
 + Don't pass -logfile=auto to the daemon by default.

Fixes golang/go#40105

Change-Id: I5e91ea98b4968c512bce76a596bbae441f461a66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-16 19:33:22 +00:00
Rob Findley
cf799caead internal/lsp/regtest: simpler way to invert options
This is an alternative to CL 241739, preserving the exiting variadic
options pattern and just adding a helper method to put the options up
front.

All things considered, I think this is better. CL 241739 was too
complicated, and being able to reuse "curried" runners isn't actually
important.

Updates golang/go#39384

Change-Id: I7ecd80d310cac879520b2d1feebcf5bd5e96e89b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243057
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-16 18:20:50 +00:00
Rebecca Stambler
43ed94694c internal/lsp: don't keep track of closed overlays
getFile still returns a FileHandle, even if the file doesn't exist on
disk. Work-around this by checking if the file exists before adding
the file handle to the map.

Fixes golang/go#38498

Change-Id: Ie02679068b37bf4f3d19966c6cfcf2361086b1de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242924
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-16 17:49:00 +00:00
Rebecca Stambler
e66011cbf8 internal/lsp: fix error in CL 242457
I didn't properly understand Heschi's comment, and missed a case where
we failed to return early.

Change-Id: I692c26678b2e659c6b748085bb4b16090e04472d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-16 17:41:35 +00:00
Rebecca Stambler
a8f9df4c95 internal/lsp: clean up the code lens code
This change refactors lens funcs to use only a FileHandle - each
code lens should manually compute a ParseGoHandle if it's needed. The
issue was that, if the code lens needed to also get a package, the
already parsed *ast.File was not necessarily the file used in
type-checking that package. I noticed that the code lens wasn't always
coming up.

Change-Id: Ic5a2c2b3d1010555acaa64ef8c6aa2484b8fbc9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242920
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-16 13:43:26 +00:00
Rebecca Stambler
c80dc571df internal/lsp: refactor generate code lens code
This change moves the "generic" go generate code lens code into
progress.go. Fix up a few generate-specific things in the progress
writers.

The remaining generate code isn't much, and is moved into command.go.

Change-Id: I2b7b6279da2442c0b92758b9b3e259f25787fabc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242919
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2020-07-16 13:43:19 +00:00
Rebecca Stambler
ddb87c8c46 internal/lsp: add an importShortcut configuration
This configuration deals with the fact that VS Code has the same
shortcut for go-to-definition and clicking a link. Many users don't like
the behavior of this shortcut triggering both requests, so we allow
users to choose between each behavior.

Fixes golang/go#39065

Change-Id: I760c99c1fade4d157515d3b0fd647daf0c8314eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-15 23:00:36 +00:00
Rebecca Stambler
4cea89713c internal/lsp: handle nil pointer in PackageStats
Fixes golang/go#40224

Change-Id: I9f3f194b1c115ff0242cbce89a36e20d9e21a41f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-15 19:57:38 +00:00
Rebecca Stambler
b42590c1b1 internal/lsp/source: handle nil pointer in newBuiltinSignature
Fixes golang/go#40230

Change-Id: I457dd3ef009c9df9293f66871acdd90dcc6e1dcc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242798
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-15 19:52:07 +00:00
Peter Weinbergr
9048b464a0 internal/lsp: avoid panic caused by assuming file ends with newline
importPrefix in format.go computes the end of the package statement
including a trailing newline. This causes a panic if the user has
not typed the newline. The code now checks for that case.

Fixes golang/go#40208

Change-Id: I4c5118a5de78027e6c5e6ed91dfddca81e38f7e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242638
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-14 19:07:37 +00:00
Hyoyoung CHANG
6acd2ab80e internal/lsp: always show signature as the top line in hover
The current behavior is inconsistent across different configurations.
The signature is the top line for FullDocumentation, but the last line
for synopsis documentation.

Change-Id: I2fe1e1c7bfa38ae46fda06b0ecd4c7712b0e74af
GitHub-Last-Rev: d3b768558f21f518d1296b6fc776cf3974f441d9
GitHub-Pull-Request: golang/tools#241
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242104
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-13 23:52:42 +00:00
Rebecca Stambler
62a0bb781d gopls, internal/lsp: support an extra formatting hook for gofumpt
This change reworks CL 240118 to apply gofumpt directly as a formatter,
not an analyzer. Depending on how gofumpt changes, we may be able to use
it as an analyzer in the future, but for now it's just easier to add it
as a formatting hook.

Fixes golang/go#39805

Change-Id: I227fde4b1916d8a82557f30dfca88e155136dff5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-13 21:42:47 +00:00
Rebecca Stambler
f8240f79c3 internal/lsp: add changeMethods logic to rename check
This logic is directly copied from the refactor/rename package. See
https://github.com/golang/tools/blob/master/refactor/rename/rename.go#L321.

Fixes golang/go#39269

Change-Id: Ibe335aab37c495d2a960cb9da254b24b6fbac8e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242158
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-13 19:50:33 +00:00
Rebecca Stambler
01425d7016 internal/lsp/source: add Vendor to list of supported commands
I wanted to add a regtest for this, but then immediately got stumped on
how to do it well. The best I've got is to add a helper that wraps
the client's call to executeCommand, but it's used in so many places that
an easier fix was to put it in executeCommand itself.

Fixes golang/go#40101

Change-Id: Iafb804fb77e57e6ac01e3de1d115058b21ac1954
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241984
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-13 19:07:48 +00:00
Rebecca Stambler
fd294ab11a internal/lsp: watch go.{mod,sum} files, as well as Go files
Not sure how this hasn't come up earlier. We aren't noticing file
changes on-disk for go.mod/go.sum files.

Fixes golang/vscode-go#297

Change-Id: I4532a252f330404515efec244a9c266a765e6bcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242160
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-13 01:13:07 +00:00
Rebecca Stambler
7342f9734a internal/lsp/cmd: add a -vv flag for higher verbosity
Add a higher log level for the command-line. This uses the verboseOutput
setting.

Updates golang/go#40139

Change-Id: I9b7edcda12b0431058c9cfe1413b7c5fc016c026
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241857
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-11 15:58:55 +00:00
Rebecca Stambler
f1c4188a97 internal/lsp, go/packages: reproduce and fix golang/go#39646
This test exposes a lot of fundamental issues with `go list` overlays.
Now that we have the regression test framework, we can copy it over and
make any flakes completely reproducible by waiting for each change to
complete.

The issue here ended up being that initial workspace load returns
workspace packages with no associated files due to golang/go#39986.
Working around this allows invalidation to proceed as usual.

In the process of debugging this, I also noticed that packages can get
loaded as command-line-arguments even when we can get the correct
package path for them. It's pretty complicated to fix this, so
restructured the code a tiny bit and left a TODO. I'd like to come back
to this at some point, but it's not pressing since I was able to fix
this bug.

Fixes golang/go#39646.
Updates golang/go#39986.

Change-Id: Id6b08a5e92d28eddc731feb0ef3fd3b3fc69e64b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-07-10 04:28:08 +00:00
Rebecca Stambler
125cc70a40 internal/lsp: add new go.mod requires to files with unused require
This change addresses an underlying issue with the go.mod code, which is
that it was modifying go.mod files without cloning them. This could've
resulted in some ugly race conditions.

We also handle the fact that new dependencies weren't being added
cleanly to files that already had unused dependencies.

Fixes golang/go#39041

Change-Id: I96ee0052d8d29a25e24f0bda9688e780a0fa7442
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241443
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-10 04:15:23 +00:00
Rebecca Stambler
55a0fde516 internal/lsp: fix rename with Windows line endings
Replacing the text in the comment line-by-line prevents issues to do
with CRLF/LF line endings.

No test, because txtar expects LF line endings, and I didn't think it
was worth more invasive changes to handle this.

Fixes golang/go#39364.

Change-Id: Ia26b311a851396e4dde1954ebfc1b40c0a3c04fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-10 04:00:52 +00:00
Rob Findley
e327e1019d internal/lsp/regtest: standardize on 20s timeout
The default regtest timeout was increased to 60s a while ago to try to
avoid flakes.

We think we've figured out the flakes, so drop it back down.  Start with
20s, since TestGOMODCACHE (which had overridden its timeout to 5s)
occasionally fails on freebsd:
https://build.golang.org/log/6c607c7fd3e886cf23127c958ae17637440fbc63

Change-Id: I10e6927985b8fb1d149070e204bd8ca120309c7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240060
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-09 18:17:11 +00:00
Rob Findley
e404ca24fb internal/lsp/regtest: use a common directory for regtest sandboxes
For easier debugging (and less cruft if regtests are ctrl-c'ed), root
all regtest sandboxes in a common directory.

This also tries one last time to clean up the directory, and fails on an
error. This might be flaky on windows, but hasn't been so far...

Also give regtest sandboxes names derived from their test name.

Updates golang/go#39384
Updates golang/go#38490

Change-Id: Iae53c29e75f5eb2b8d938d205fbeb463ffc82eb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-09 18:15:24 +00:00
Heschi Kreinick
df98bc6d45 internal/lsp/regtest: remove stray short timeout
Apparently 5 seconds is too short for the freebsd-race builder and we
care about that.

Fixes golang/go#40090.

Change-Id: I6dad57402ed18e7c9e01f695ada7cc8b0cf35fb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241521
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-08 18:38:56 +00:00
Rebecca Stambler
3544e8c95d internal/lsp/regtest: add a regtest for golang/go#39296
Looks like the issue described in
https://github.com/golang/go/issues/39296#issuecomment-652058883 is
resolved with Go 1.15.

Fixes golang/go#39296

Change-Id: Ibe694281d4d4d40df3dc7bacc5837f593bb1dc5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241398
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-08 18:30:29 +00:00
Heschi Kreinick
31b9a74881 internal/lsp/cache: handle missing mod file
If the config changes in non-module mode, don't look at the nonexistant mod file.

I actually caught this case after review but didn't finish the job.

Fixes golang/go#40121.

Change-Id: Ic888f7eab5f882e6ca79b046b5a3fb37b3b19e5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241520
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-08 18:08:30 +00:00
Rebecca Stambler
7370b03448 internal/lsp: check if analysis enabled in convenience fixes
Change-Id: I49c7808ee07340cea0381f324b1fb113ec405213
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-08 18:02:25 +00:00
Rebecca Stambler
134513de88 internal/lsp/regtest: await IWL before running tests
This change makes these tests a little more resilient by making sure
that the initial workspace load is completed before we run them.

Change-Id: I24735da31cc045cb78dfb280e88136b5835351a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241276
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-08 00:37:08 +00:00
Rebecca Stambler
d5a745333d internal/lsp/source: disable fillstruct by default
The discussion on golang/vscode-go#299 has uncovered the fact that
go/printer is very expensive, and we're calling it frequently on file
changes. Analyzers whose suggested fixes require the original file
content are generally posing issues; this is being discussed on
golang/go#40110.

Updates golang/vscode-go#299

Change-Id: I5cb370c9cb508203e463e7243fc781e75876fe30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-08 00:01:38 +00:00
Heschi Kreinick
065b96d36c internal/lsp/cache: fix race in RunProcessEnvFunc
Changing build flags (-modfile) while work is happening in the
background causes races. Explicitly detect relevant configuration
changes and only modify the ProcessEnv then, when the resolver is
inactive after the call to ClearForNewMod.

This still leaves a very small window for a race: if refreshProcessEnv
has already captured env but not yet started priming the cache, it may
race with the modification. But I don't expect it to be a problem in
practice.

Fixes golang/go#39865.

Change-Id: I31c79f39be55975fee14aa0e548b060c46cdd882
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241317
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-07 22:21:32 +00:00
Josh Baum
9c9572d6f9 internal/lsp: extract highlighted selection to variable
I add a code action that triggers upon request of the user. A variable
name is generated manually for the extracted code because the LSP does
not support a user's ability to provide a name.

Change-Id: Id1ec19b49562b7cfbc2cd416378bec9bd021d82f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240182
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-07 21:12:28 +00:00
Heschi Kreinick
416e8f4faf internal/imports: require valid options, move LocalPrefix up
CL 239754 eagerly initialized the environment. This turns out to be a
problem for gopls, which calls ApplyFixes with no ProcessEnv.
Reinitializing it every time seriously harmed the performance of
unimported completions. Move back to lazy initialization.

Working with invalid options has caused a lot of confusion; this is only
the most recent. We have to maintain backwards compatibility in the
externally visible API, but everywhere else we can require fully
populated options. That includes the source byte slice and the options.

LocalPrefix is really more of an Option than an attribute of the
ProcessEnv, and it is needed in ApplyFixes where we really don't want to
have to pass a ProcessEnv. Move it up to Options.

Change-Id: Ib9466c375a640a521721da4587091bf93bbdaa3c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241159
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-07 20:02:13 +00:00
Josh Baum
9e0a013e85 internal/lsp/analysis/fillstruct: support anonymous structs
The previous implementation did not support filling anonymous structs.
This is distinct from filling a struct that has an anonymous
struct as a field. That field will still not be filled.
golang/go#39929 has been opened to address this limitation.

Fixes: golang/go#39803
Change-Id: Ia7ba9fda2bdee092d182e0a23829b9a6d9a4b986
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240463
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-07 13:47:15 +00:00
Rebecca Stambler
682c4542fd all: update dependencies in go.mod file
CL 226639 changed the positions of the go.mod diagnostics (in a
negligible way), but the tests are quite brittle and can't handle the
different position. Rewrite this test as a regression test to handle it.

The special // indirect marker can be removed from the go/expect package
as a result, since it was only used in this one place.

Change-Id: I7d9a62e32e53d477838e65673635ed231c07b659
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240691
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-06 23:19:48 +00:00
Søren L. Hansen
95bc2bdf7e internal/lsp/source: handle nil pointer in comment completions
c.pkg.GetTypeInfo().ObjectOf(node.Name) will sometimes return nil.
Check for that and a few other things that c.found also checks for.

Fixes golang/go#40043

Change-Id: I4a2b40adbbd740323e10b3460f025b29cff74130
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241019
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-06 18:08:31 +00:00
zikaeroh
0cc1aa72b3 gopls/internal/hooks, internal/lsp/source: only match full words in link regexes
The current URL regexes (including xurls) match links in any part of a
string. However, this leads to odd underlining within words which just
so happen to have valid domain names as substrings. For example, a
comment which contains "reflect.DeepEqual" will match the URL regex as
"reflect.de" is a domain name.

Ensure that the URLs only match full regex words by adding \b around the
expressions. For xurls, this is done by pulling the expression out of
Relaxed() and recreating the Regexp.

While I'm here, make the non-gopls module expression prefer the longest
match, as xurls does.

Change-Id: I403db970fa1661c443b0693c03f8dee114f8eaff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240738
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-02 04:49:44 +00:00
Rebecca Stambler
f01a4bec33 internal/lsp: support opening single files
This change permits starting gopls without a root URI or any workspace
folders. If no view is found for an opened file, we try to create a new
view based on the module root of that file. In GOPATH mode, we just
use the directory containing the file.

I wrote a regtest for this by adding a new configuration that gets
propagated to the sandbox. I'm not sure if this is the best way to do
that, so I'll let Rob advise.

Fixes golang/go#34160

Change-Id: I3deca3ac1b86b69eba416891a1c28fd35658a2ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01 22:10:12 +00:00
Rebecca Stambler
ea7be8d74e internal/lsp/regtest: add regression tests for on-disk file changes
There are still many more cases to check, but this is a good starting
point. A few tests have skips in them because I encountered bugs, which
I plan to go back and fix.

Change-Id: I0b7bbeb632d38c09d6bdb1f4866d81a1690d6ca7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238917
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01 22:06:37 +00:00
Muir Manders
1837592efa internal/lsp/source: speed up completion candidate formatting
Completion could be slow due to calls to astutil.PathEnclosingInterval
for every candidate during formatting. There were two reasons we
called PEI:

1. To properly render type alias names, we must refer to the AST
   because the alias name is not available in the typed world.
   Previously we would call PEI to find the *type.Var's
   corresponding *ast.Field, but now we have a PosToField cache that
   lets us jump straight from the types.Object's token.Pos to the
   corresponding *ast.Field.

2. To display an object's documentation we must refer to the AST. We
   need the object's declaring node and any containing ast.Decl. We
   now maintain a special PosToDecl cache so we can avoid the PEI call
   in this case as well.

We can't use a single cache for both because the *ast.Field's position
is present in both caches (but points to different nodes). The caches
are memoized to defer generation until they are needed and to save
work creating them if the *ast.Files haven't changed.

These changes speed up completing the fields of
github.com/aws/aws-sdk-go/service/ec2 from 18.5s to 45ms on my laptop.

Fixes golang/go#37450.

Change-Id: I25cc5ea39551db728a2348f346342ebebeddd049
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221021
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-01 04:11:22 +00:00
Rebecca Stambler
9a0e069805 internal/lsp/cmd: change pre-filled issue header for gopls bug
Change-Id: I221f2e6a0dc41cfd8325df64607b5c625d69ca17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240503
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2020-07-01 03:39:15 +00:00
Rebecca Stambler
a32c0cb1d5 internal/lsp: decorate error message from workspace/configuration
Fixes golang/go#39896

Change-Id: Ib63e3b4b716f6005b3cbbc92ede40fdbfc8ca22f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240619
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01 00:03:37 +00:00
TheSecEng
b2d8b03366 internal/lsp: remove VS Code-specific completion hack
This hack was previously being used to correct the rankings for VS Code
Go users. This logic has since been moved into the VS Code Go plugin.

Change-Id: Ia6bff53eb2c6dbe479faa1fd0667d763837eea78
GitHub-Last-Rev: e7fed76b3f85b0db3b705cbb6945cecd855d2c7b
GitHub-Pull-Request: golang/tools#225
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231098
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-30 15:48:51 +00:00
Rebecca Stambler
4bdfe1a3b7 internal/lsp/cache: handle a few possible panics in PackageStats
Change-Id: I50dbe8bf7d801e7325bdbe413368e85779511d85
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240183
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-30 15:45:28 +00:00
Rob Findley
aa94e735be internal/lsp/source: add a new symbolStyle configuration option
Add a symbolStyle configuration option, and use it to parameterize the
following behavior when computing workspace symbols:

 + package (default): include package name in the workspace symbol.
 + full: fully qualify the symbol by import path
 + dynamic: use as the symbol the shortest suffix of the full path that
   contains the match.

To implement this, expose package name in the source.Package interface.
To be consistent with other handling in the cache package, define a new
cache.packageName named string type, to avoid confusion with packageID
or packagePath (if confusing those two identifiers was a problem, surely
it is a potential problem for package name as well).

Change-Id: Ic8ed6ba5473b0523b97e677878e5e6bddfff10a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236842
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
2020-06-26 17:13:37 +00:00
Paul Jolly
84cfedeb1e internal/lsp: fully qualify workspace Symbol matches
Fully qualify (using a package's import path) symbols when matching
them. This is a minimal change in preparation for later changes to add
more advanced query functionality to the workspace Symbol method.

Add more comprehensive regtest tests (covering case sensitive, case
insensitive and fuzzy matchers) in addition to updating lsp tests.

Change-Id: I675cde2f7b492158988cf9c3206a0a55fe29622a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228123
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-26 16:54:09 +00:00
Rebecca Stambler
bcbc01e07a internal/lsp: add a new regtest to reproduce golang/go#39646
Change-Id: I51d8c66a83ecae1c8fc1f39c0e90a03a732c263b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240063
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-26 03:28:29 +00:00
Rob Findley
6506e20df3 internal/lsp/fake: explicitly set GOPACKAGESDRIVER=off in regtests
In the past, changes to GOPACKAGESDRIVER have led to some confusing
regtest failures. Explicitly set it off.

Updates golang/go#39384

Change-Id: I303a58380a5e46e6621c19b2edc40d43199bb343
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240058
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-25 21:18:23 +00:00
Rebecca Stambler
fadf93ffb2 internal/lsp: watch all files in the module and replace target
Previously, our file watching only considered the root directory of the
view, which may not include the entire module or its replaced
dependencies. Now we expand our watching to include the whole module.

As part of testing this, I noticed that VS Code's file watcher actually
only sends updates for files in the workspace, even if we request
notifications for all files in the module. I filed an issue to ask about
this: https://github.com/microsoft/vscode-languageserver-node/issues/641.

Change-Id: I9499d31aff273f69e9c117511e7985ff58b7fdc4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239198
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-25 21:08:52 +00:00
Heschi Kreinick
e31c80b82c all: rework goimports environment, support GOMODCACHE
This CL got away from me a little.

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

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

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

Fixes golang/go#39761.

Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-25 17:33:20 +00:00
Josh Baum
727c06e3f1 internal/lsp/analysis/fillstruct: correct pointer to builtin values
The current implementation correctly calls 'new' when filling a
pointer to a builtin type.

Fixes: golang/go#39854
Change-Id: I0c2b27bb57fd865c4376279059ad060608d48ba3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239978
Run-TryBot: Josh Baum <joshbaum@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-25 16:34:51 +00:00
Rebecca Stambler
aa12c9ebf5 internal/lsp: handle panics due to line numbers in fillstruct
Change-Id: I90f3fd2daf180705048d494476acac5a213f5fb3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239751
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Baum <joshbaum@google.com>
2020-06-25 15:39:20 +00:00
Rebecca Stambler
7a9acb0a45 internal/lsp/source: fix panic in test code lens
Change-Id: I727b2772c63752cb5d3bb4b9165f984b64adc842
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239752
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-24 22:30:20 +00:00
Rebecca Stambler
25775e59ac internal/memoize: add an error return to (*handle).Get
Fixes golang/go#36004

Change-Id: I8da7c21eaa9cf6ffac12aabdd6803d06781cef32
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239564
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-24 16:33:19 +00:00
Rebecca Stambler
f8e0ea3a3a internal/lsp/analysis/fillstruct: add indentation for struct fields
Getting the right indentation for the text in the AST proves to be a
little complicated. The most reasonable approach seems to be writing
out the AST, getting the lines with the struct definition on it,
and trimming whitespace to get the current indent. Then, we add this
indent to the struct fields in the new text.

Change-Id: I1cc3421d95edae61cfb662254ff3fb759b5c487f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239199
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-23 20:47:33 +00:00
Josh Baum
456ad74e14 internal: fill struct with initialized values instead of nil values
The previous implementation filled map, slice, channel, signature, and
pointer types with nil values rather than initializing these types.

Change-Id: I558446af31fdd8877db5eb3cd92b4004f5d5ab22
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239039
Run-TryBot: Josh Baum <joshbaum@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-23 18:51:56 +00:00
Rebecca Stambler
1745ac5bc6 internal/lsp: refactor various module-specific handles in cache
This change separates out different functions of mod handles.
Previously, we had ModHandle and ModTidyHandle. ModHandle was used to
parse go.mod files and get the results of `go mod why` and possible
dependency upgrades.

Now, we factor this out into 4 handles: ParseModHandle, ModWhyHandle,
ModUpgradeHandle, and ModTidyHandle. This allows each handle to be
specific to its own functionality. It also simplifies the code a bit,
as the handles can be written in terms of ParseModHandles instead of
FileHandles.

I may have some follow-up CLs to refactor the `go mod tidy` logic out of
the cache package, though I'm no longer certain that that's a good
choice.

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

Change-Id: I866b5db83b504fae190e58c306c01a7a4296672d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-22 20:30:43 +00:00
Heschi Kreinick
4fd1c64487 internal/lsp/cache: fix ignored file check
The return of IgnoreFile. We continue using go list's ignore rules,
but only apply them to the part of the path underneath the source root.

This should be fixed to include replace targets once we have them.

Change-Id: I054fee8c12fc860a279b0d0c1fd670f44d78a63f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239288
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-06-22 19:29:24 +00:00
Josh Baum
0f592d2728 internal/lsp/analysis/fillstruct: use AST nodes to fill struct
The current implementation writes strings to the diagnostic instead
of creating new AST nodes.

Change-Id: Ibb37a93a3c43fb74ec5dc687091601e055c6e1b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238198
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-19 21:01:11 +00:00
Rebecca Stambler
6c6fd98e38 internal/lsp: fix panic in computing diagnostics for module
We were returning empty diagnostics for an empty file.

Fixes golang/go#39696.

Change-Id: Idfd50980f4bf771f76ad7b7fb6180a5f712a92bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239040
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-19 20:49:21 +00:00
Heschi Kreinick
8b7669898d internal/lsp/debug: save per-package stats in memory debug files
The per-package stats have proven pretty useful, and I don't want to
have to teach users how to save them. Create zip files and add them in.

Since some users may be sensitive about revealing any information about
the code, generate two variants: one with package names, and one
without.

Change-Id: Icc5631b4cebbbabfdd2fcea4a4cdf4f205dbcab9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239037
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-19 18:00:37 +00:00
Rebecca Stambler
6023b8da91 internal/lsp: use -modfile for import organization
Also add a regression test.

Fixes golang/go#39681.

Change-Id: I685fbe4255b9e8e067dacd84a6d464d0a2bbe1f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238737
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-19 17:44:56 +00:00
Pontus Leitzler
aaae6734f9 internal/lsp/cmd: add example to cli help for -ocagent flag
I've forgotten to specify scheme a few times, and gopls is quiet
about invalid ocagent addresses.

Add an example to the CLI help output aid the users with the format
implicitly.

Change-Id: I89ba8a16c18dabff2b4b87b2235755c22ae71117
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239057
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-19 16:26:05 +00:00
Pontus Leitzler
037be6a065 internal/lsp/source: support highlight of switch statements
Placing the cursor on a switch statement or corresponding break and call
document.Highlight will now highlight them.

Fixes golang/go#39275

Change-Id: Ib7e3ba0c6e78141ed3dd37cfd3b72b567b857247
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238478
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-19 02:36:21 +00:00
Rebecca Stambler
c7475b9d7f internal/lsp/source: refactor highlighting code
The current implementation has a number of helper functions that have to
thread through context, errors, converters, etc. Simplify the code by
factoring out the conversion to a protocol range into a final step.
There are probably further improvements that can be made too.

As part of debugging, I noticed that the cmdtest server does not get
shutdown once tests finish, which leads to excessive logging. Is closing
the test server connection enough to trigger shutdowns? It doesn't seem
to be.

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

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

Fixes golang/go#36998

Change-Id: I92011821f646f2a7449dcca619483f83bdeb54b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238029
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-18 13:42:42 +00:00
Rob Findley
5fddd300b6 internal/lsp/cmd/test/cmdtest: shutdown connections on test completion
To prevent misleading errors from outstanding go command invocations at
test completion, properly shutdown the LSP connection before cleaning up
exported files.

Change-Id: I9ad175060fefc5b914e544c5f58b9b6658405edc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238546
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-18 11:25:59 +00:00
Rebecca Stambler
d15173dcc7 internal/lsp/cache: separate modTidyHandle out of modHandle
While working in this code, I was confused by the fact that modHandle
and modData were used for both ModHandle and ModTidyHandle. Separate out
the two so it's a little more clear. This is mostly a copy-paste, with a
few fields renamed to simplify them.

Change-Id: I20ea11c32a624fd250eabf901d09ebb05fbad062
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238337
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-18 03:14:02 +00:00
Rebecca Stambler
47c907e258 internal/lsp: use a new temporary go.mod for every go list call
Refactor internal/lsp/cache to use a new temporary go.mod file for each
go command invocation. This cleans up the abstraction in the source
package, as we no longer are aware of temporary go.mod files.

This will also fix the raciness of reusing the same temporary go.mod
file for each invocation.

Updates golang/go#37318.
Fixes golang/go#39504.

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

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

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

Fixes golang/go#38977.

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

Change-Id: I93c05df2f4d32c6c8a43e9f5aaeeb30bc4a32f3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238058
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-17 04:29:24 +00:00
Heschi Kreinick
b7b89dcb81 internal/lsp/cache: support minimal module compatibility in GOPATH
In GOPATH mode, you can import example.com/foo even if it has a go.mod
that says its path is example.com/foo/v2. Adjust our import path
resolution to support that case.

Fixes golang/go#39560.

Change-Id: I758a2220c579c4374084365c3a78a3a2bbd14b01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238260
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-16 21:57:26 +00:00
Pontus Leitzler
dc31b401ab internal/lsp/source: avoid panic in rename check
In a case where the type info for an ast.CompositeLit isn't found in
forEachLexicalRef(...) gopls will panic.

e.g. trying to rename "foo" where it is declared in this case:
func fn() {
	var foo bool
	make(map[string]bool
	if foo {
	}
}

Note the missing ')' after make.

This change will skip ast.CompositeLits if the type can't be found, and
that fixes the panic. But it also do rename the identifier as long as it
is possible, and I'm not convinced that we should allow rename at all if
the source can't be compiled.

Updates golang/go#39614

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

Updates golang/go#36998

Change-Id: I9102a11312db5c334fdbd30cce9ca2d2e19e9ac2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237938
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-16 15:44:21 +00:00
Rob Findley
2b5917cebf internal/lsp/lsprpc: forward the go environment in initialize requests
The gopls workspace environment defaults to the process environment in
which gopls was started. This means that when switching environments,
gopls can potentially get a different environment when connecting as an
editor sidecar from when forwarding requests to the daemon.

To (hopefully mostly) mitigate this pain point, inject the Go
environment when forwarding the 'initialize' request, which contains
InitializationOptions containing the 'env' configuration. We could go
further and send the entire os.Environ(), but that seems problematic
both in its unbounded nature, and because in many cases the user may not
actually want to send their process env over the wire. Gopls behavior
should *mostly* be parameterized by gopls binary and Go env, and after
this change these should match for forwarder and daemon.

For go1.15, Explicitly set the GOMODCACHE environment variable in the
regtest sandbox. Without this, regtests were failing in the forwarded
environment because they implicitly shared a module cache.

Fixes golang/go#37830

Change-Id: Ic1b335506f8b481505eac9f74c0df6293dc07158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234109
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-16 15:43:56 +00:00
Rebecca Stambler
6aa8f57aac internal/lsp: un-export (*snapshot).Config to limit it to cache
Limiting the Config to the view seems reasonable, considering that it is
only used to run the `go` command. I prefer just having the cache run
go commands, so that source doesn't have to deal with the environment.

This also enables CL 237517.

Change-Id: I639c082592de30e9682dc25cdd12c7751ddb4f97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-15 22:28:25 +00:00
Rebecca Stambler
9fe02d1961 internal/gocommand: refactor API to always use *gocommand.Runner
We should always go through the runner to avoid load serialization
errors. This change also updates the comments and uses pointers to
Invocations consistently. This causes a few updates throughout x/tools.

This change also results in a few modifications to the go:generate code
lenses, as they operate on directories rather than file URIs.

Change-Id: I6306e761e68dfdd23f1b410e44aab0ffa85d234c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237685
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-15 22:00:55 +00:00
Rebecca Stambler
e31b568ad1 internal/lsp: plumb fillstruct through analysis
Now that fillstruct is an analyzer, we can simplify the code that calls
it in code_action.go. We introduce a new class of analyzer --
convenience analyzers, which are closer to commands. These represent
suggestions that won't necessarily improve the quality or correctness of
your code, but they offer small helper functions for the user.

This CL also combines the refactor rewrite tests with the suggested fix
tests, since they are effectively the same.

For now, we only support convenience analyzers when a code action was
requested on the same line as the fix. I'm not sure how to otherwise
handle this without bothering the user with unnecessary diagnostics.

Change-Id: I7f0aa198b5ee9964a907d709bae6380093d4ef21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237687
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-15 21:22:08 +00:00
Heschi Kreinick
a2fa627c4b internal/lsp/cache: don't skip x_tests
We were marking normal tests as workspace packages, but pruning x_tests.
Don't do that.

Fixes golang/go#39578.

Change-Id: Ia8eceea38e63ac17e40f50d6837de9c7a668469b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237943
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: zikaeroh <zikaeroh@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-15 19:08:35 +00:00
Heschi Kreinick
1725ffee6d internal/lsp/cache: hide diagnostics for ignored dirs
The go command ignores go files in specially-named directories like
testdata. We probably still want to try to offer services like
autocomplete and jump-to-definition there, but diagnostics are less
likely to be helpful. As a compromise, just squash diagnostics.

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

Fixes golang/go#39563.

Change-Id: I0bc00639e68bd71ea55d867af36e07ef4ec780a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237638
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-15 17:27:23 +00:00
Josh Baum
a1e2396bbd internal/lsp: port fill struct into analysis framework
The current implementation of the fill struct tool is not a part of
the analysis framework. This commit moves the functionality from the
source directory to the analysis directory.

Change-Id: Ibe37b57f3e6680c8729932dbbe010a4642600e4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237258
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-12 20:33:59 +00:00
Heschi Kreinick
4651fa3054 internal/lsp/debug: show per-package memory usage
Calculate and display very crude memory usage statistics. This is
complicated by various levels of sharing and indirection, so the numbers
should be taken with *large* grains of salt and interpreted mostly by
experts.

Still, the results are interesting and helpful.

Change-Id: Ia9aff974c7d5fddd63df0cfd5cecc08ead33cf84
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236163
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-12 20:10:52 +00:00
Rebecca Stambler
3c1b287bbd internal/lsp: await the initial workspace load in ModHandle
ModHandle races with the initial workspace load if the go.mod file does
not yet exist. We should await for the initial workspace load to
complete before proceeding with update codelenses, etc.

Part of trying to figure out the flakes in golang/go#39504.
Also a few staticcheck fixes, and fix the Windows line endings in
fill_struct.go, because `git gofmt` complains.

Change-Id: Ide21a47137390792d1afb924740cff0bb6f0b764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-12 18:21:55 +00:00
Rebecca Stambler
9001f16f58 internal/lsp: cancel the initial workspace load when view shuts down
The error messages from view cancellation clutter up the logs when
testing, especially if you're running a single subtest.

A few quick staticcheck fixes in the CL also.

Change-Id: Ia1ed5360ac754023c589ed526ec0ed3e94a79b2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-12 18:06:40 +00:00
Ian Cottrell
742c5eb664 internal/lsp: fix the cache debug page
The cache does not know the sessions associated with it, so the
debug template cannot display them.

Change-Id: I9b043592351ddce097f4e74f3e9aa7a6ad24e613
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237618
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-12 02:23:31 +00:00