Commit Graph

464 Commits

Author SHA1 Message Date
Chitoku e7de6523a2 internal/lsp: produce "method" in place of "member" as per the spec
As per the current specification, the correct token name that represents methods is `method`. The current implementation does not produce the correct set of result and results in methods being incorrectly highlighted. Furthermore, the parameter names of interface methods are treated as `method` which is not true, so a fix has been applied to handle `ast.FuncType` being `parameter`.

Spec: https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textDocument_semanticTokens

Change-Id: Id5d2fafe9f948a4a175ae19b4c94cca8f48a4b79
GitHub-Last-Rev: 9a3b65ca4dd3bb0542969c043a2042001c7fcb63
GitHub-Pull-Request: golang/tools#340
Reviewed-on: https://go-review.googlesource.com/c/tools/+/349449
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
2021-09-13 21:01:54 +00:00
Rebecca Stambler 295a39ba3c internal/lsp: remove the TODO and add a test for the hover panic
Fixes golang/go#48249

Change-Id: I86da0f185f414848bf89243737668f1d427c3e4c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/348969
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2021-09-09 21:49:46 +00:00
Rebecca Stambler c163c3172e gopls: add more instructions for working with Bazel and gopls
Change-Id: I91854ea0ad61d1b4794eacaee581888fa851d176
Reviewed-on: https://go-review.googlesource.com/c/tools/+/348430
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-09-09 13:25:25 +00:00
Robert Findley e5f719fbe6 internal/lsp/source: consider test variants when finding pkg from pos
When resolving a position to a package we must consider all packages,
including intermediate test variants. This manifests, for example, when
jumping to definition in a package that is imported as a test variant
(see golang/go#47825).

For now, fix this by threading through an 'includeTestVariants' flag to
PackagesForFile. This isn't pretty, but should be a trivially safe
change: the only effect will be to increase the number of packages
considered in FindPackageFromPos. Since we are discussing future changes
to the API for querying packages from the snapshot, now did not seem
like a good time to undertake significant refactoring.

A regtest based on the original issue is included.

This CL is joint with rstambler@golang.org.

Fixes golang/go#47825

Co-authored-by: Rebecca Stambler <rstambler@golang.org>
Change-Id: I4693ec69b50ed4acd569cff87883769c1edf332b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/347563
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-09-08 14:51:59 +00:00
Johannes Altmanninger d39bbca0a3 gopls: consistently capitalize enum variants for symbolStyle
We use a case-insensitive comparison so this doesn't really
matter but it's a bit confusing that gopls/doc/settings.md and
internal/lsp/source/api_json.go were inconsistent here.  I'm assuming
the latter also shows up as user-visible documentation somewhere,
probably in the VSCode plugin.

Change-Id: I14fa5b9d062266b6de0397aafb36e0ad84730752
Reviewed-on: https://go-review.googlesource.com/c/tools/+/344353
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-09-03 18:04:29 +00:00
Johannes Altmanninger 4e109c5706 gopls: use new "codelenses" config key in docs over deprecated "codelens"
There is a TODO to remove the "codelens" alias. This updates an
example in the documentation to use the new name.

Change-Id: Id807e99f5ba1da663eae9660fb65521596b0c488
Reviewed-on: https://go-review.googlesource.com/c/tools/+/344352
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-09-03 18:01:36 +00:00
Dan Peterson 7391fc19fa gopls/doc: update generic code section of advanced.md
Since golang.org/cl/343732 was merged the dev.typeparams branch is no
longer needed, just Go at tip.

Change-Id: Ifb2e5220bc2f544d86cfa84d43d12d93e4e7fddb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/343967
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-09-03 17:12:04 +00:00
Rebecca Stambler 3b801c8b83 internal/lsp: exclude node_modules in the workspace root by default
It is unlikely that users want gopls operating on their node_modules
directories, so we should exclude them by default. If a user wants to
include them, they can override their directory filters setting.

This doesn't exclude *any* directory named "node_modules", so we still
need to implement golang/go#46438 to exclude node_modules completely.

Change-Id: I03c42208e62390dc35e44ac5176422ddf8dc53f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/347297
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-09-02 18:21:15 +00:00
Matthew Dempsky d4cc65f0b2 gopls/internal/regtest/codelens: avoid compiler bug
Because of how the compiler internally represents variable references,
it sometimes gets position information incorrectly for them. The
codelens test hits this issue because for

	var x string
	fmt.Println(x)

the diagnostic about "x" escaping to heap (which actually refers to
the implicit conversion to "interface{}" type) should be rightly
reported at the "x" identifier within the call arguments list.
However, due to the aforementioned compiler bug, historically we
reported the diagnostic at the "(" token instead.

In -G=3 mode, the compiler (correctly) report the diagnostic at "x"
instead; but with GOEXPERIMENT=unified, the compiler intentionally
matches the original -G=0 behavior and continues reporting at "("
instead.

This CL avoids the issue entirely by changing the line to
"fmt.Println(42)", which avoids the issue because the compiler always
correctly prints the diagnostic then at "42".

Change-Id: I22d4c756ae801489f3f16c440e4339bc9b115fb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/343875
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2021-08-20 21:27:50 +00:00
Rebecca Stambler bf6c7f26e9 all: update dependencies after gopls/v0.7.1 release
Change-Id: Ifd3227e2c34798024820277b512f18768e125c38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/341809
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-08-18 17:22:40 +00:00
Rob Findley 6932d22acd internal/lsp/source: recursively search for qualified objects
When searching for references or renaming, we start from all packages
containing the current position. But as reported in golang/go#47564,
this fails if we're renaming an object in another package; we need to
start the search from the package containing the object definition.

This CL finds the missing packages by recursively searching all
locations we encounter. For now, this will cause us to consider the
object location, and may also help us behave correctly with respect to
build constraint variants in the future.

While at it, update the regtests to support renaming. This bug could
be exercised with marker tests, but it's good to have a regtest for
renaming anyway.

Fixes golang/go#47564

Change-Id: I5517e2aeaaa744fcc6b6b96ffbb0b2625b498ed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340472
Trust: Robert Findley <rfindley@google.com>
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-08-17 02:54:26 +00:00
Rebecca Stambler 9f4956114f internal/lsp: allow for multiple ad-hoc packages in the workspace
Add the scope to the command-line-arguments package ID and path so that
multiple command-line-arguments packages can coexist in the workspace.

Fixes golang/go#47584

Change-Id: Icbfe90d67627f384c54f352e46270ab2bf4240bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/341611
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-08-12 18:45:58 +00:00
Muir Manders 749a5cd4ad gopls/internal/regtest: fix completion benchmarks
I fixed a check in sandbox.go to check for size of config.Files
instead of nilness. The completion benchmarks run with an absolute
path workdir and were failing this check due to a non-nil but empty
config.Files.

I tweaked the benchmark output so it is compatible with benchstat. In
particular, the benchmark output now appears all on one line for an
imaginary benchmark named BenchmarkStatistics.

I also made a couple changes to the completion benchmarks:
- Don't modify the buffer before every completion. Type checking
  completely dominates completion, so if it has to type check every
  time then you aren't benchmarking the completion code at all.
- Don't try to exclude GC from the benchmark. I think amortized GC
  time should be included in the benchmark timing. Plus, I'm not sure
  that forcing a GC every 10 iterations was actually doing a good job
  excluding GC from the benchmark.

Change-Id: I53718a5f6e25453146ccf5bb5fdfdfc65e244df3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323251
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-08-12 16:12:36 +00:00
Rob Findley f35d7dcca5 internal/lsp/source: offer the fast fuzzy matcher as an option
Hook up the new fast SymbolMatcher as an option.

Benchmark ("test" in x/tools): 48ms->21ms (with the new matcher)
Benchmark ("test" in kubernetes): 857ms->199ms (with the new matcher)

Change-Id: Ic638eda1ed10572638f32879dd9b56467ae305ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338695
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-08-09 20:21:18 +00:00
Shoshin Nikita d529aec52f internal/lsp/cache: parse files with ParseFull mode to check if metadata reload is required
ParseHeader mode is used to parse only the package and import declarations.
However, change of go:embed directive should also invalidate metadata.
So, we must use ParseFull mode to get all file comments to compare
old and new go:embed directives.

Fixes golang/go#47436

Change-Id: If7cdb6741e895315bb6a6de2f207b404e15b269a
GitHub-Last-Rev: 64d606cead064ed5996eb7d55c3664940e7a1deb
GitHub-Pull-Request: golang/tools#333
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339469
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-08-06 19:38:52 +00:00
Rob Findley f68a40bc0d gopls/internal/regtest: fix the workspace symbols benchmark
This benchmark was using 'stressTestOptions', which avoid regtest hooks
(and therefore can't wait on IWL).

Change-Id: Id54f291ed42146e82f4d34b5db962b92fac1d6c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338690
Trust: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-08-04 17:02:12 +00:00
Rob Findley f09387104b internal/lsp/source: improve logic for finding full syntax in hover
When enriching identifier info with full syntax, it's cleaner to find
the enclosing decl. Use the full decl in hover if we were unable to find
a node in the original type-checked package.

Update the regtest to exercise hovering in a non-workspace package.

Updates golang/go#46158

Change-Id: Ic1772a38fb1758fb57a09da9483a8853cc5498f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333690
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-07-26 20:35:51 +00:00
danishprakash 0c506a2740 internal/lsp/source: evaluate bin/hex literal on hover
We currently support evaluating int literals on hover
if it's a const declaration but not if it's a var. This
change adds support for the same for var.

Fixes golang/go#45802
Change-Id: I3c4f6024b4b58fed38a5111253aa9e2ac30249fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330309
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-07-26 16:25:21 +00:00
Rebecca Stambler c740bfd9b2 internal/lsp: handle incorrect import with CRLF line endings
Going from an import line to an import block with CRLF endings did not
previously work.

Fixes golang/go#47200

Change-Id: I51334587ad51b828bd0828217ba39fb745d7e835
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334890
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-07-22 17:23:26 +00:00
Rob Findley 251092de1b internal/lsp/source: compute imports text edits from scratch
Fix the imports text edits by computing it on first principles. This
fixes at least a couple bugs:

 - Incorrect handling of positions between \r and \n with windows line
   endings.
 - Incorrect computation of edits when the imports source prefix is
   synthetically terminated with just \n, but the actual source has
   \r\n.

This is an unsatisfying solution, necessary because of the interaction
of token.File with file termination (token.File does not capture this
information).

Efforts to fix token.File proved complicated, and this is causing
problems for our users, so I think we should do this for now.

For golang/vscode-go#1489

Change-Id: I235caf3960c7201af93800d65546fbab5c6e3f4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/319129
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-22 17:13:07 +00:00
Francesco Renzi 6e9046bfcd gopls/doc: fix imports function for Neovim LSP
The documented Lua function for supporting imports with Neovim native
LSP support does work for imports, but also runs any other code action
available in the current range. For example, if the function was to run
with the user's cursor on an empty struct being initialized, the code
action for filling that struct would also run.
This change restricts the desired actions to only be organizeImports.

Fixes golang/go#47181

Change-Id: Ie4eb5961cb4405091477b79175acc7052ff16d4c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/334749
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Robert Findley <rfindley@google.com>
2021-07-15 13:35:41 +00:00
David Chase ef97713d99 gopls/doc: add vetted examples for Sublime Text
Instructions for Sublime Text + LSP + golsp,
including examples for:
  minimal configuration
  adjusting path
  per-project settings

The per-project settings include breadcrumbs helpful
to someone working on Go itself.

Change-Id: I2fe41cd60d7327578bd5bd9999f2459e79e9be4c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/325189
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-07-13 20:15:39 +00:00
Rob Findley 384460091c gopls/internal/regtest: add a flag to profile didChange handling
Using the -cpuprofile testing flag for profiling didChange handling
causes the profile to capture the IWL. Add a new -didchange_cpuprof
flag that instruments just the change handling.

Also fix a check for empty workspace files that was preventing
This inaccurate check was preventing the didChange benchmark from
working.

Change-Id: Ie9f5402960ddccda5d6b9b36ae3c111aa5b51bb8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333939
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-07-13 12:52:46 +00:00
GGCristo de44776175 gopls/doc: Neovim v0.5 is now stable
Change-Id: Ia3078f9d321d17681b92fd9b00f6dd1cb3eff6d0
GitHub-Last-Rev: 5c2023007a2e6a6471ec75871f74dc3edfe91af9
GitHub-Pull-Request: golang/tools#329
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332849
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2021-07-12 22:00:43 +00:00
Rob Findley 8e32e9f144 internal/lsp/regtest: fix a panic TestResolveImportCycle
CL 333289 introduced a panic, which was subsequently suppressed in test
error output due to the deferred t.Fatal (an interesting gotcha that I
honestly wasn't aware of).

Fix both the panic, and the suppression of regtest panics.

Also fix the regtest editor shutdown to run on a detached context, so
that shutdown doesn't fail for tests that have timed out.

For golang/go#46773

Change-Id: I080a713ae4cd4651476d8b4aab1d2291754a4f5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333510
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-07-09 17:21:04 +00:00
Rob Findley 64bd808b73 internal/lsp/cache: don't report a context error if load succeeded
If a context was canceled during load, only report it as a critical
error if the load actually failed.

Along the way, simplify evaulation of the critical error to use a switch
statement.

Also await IWL in the second Env used in shared regtests. Presumably it
is this Env that is being shutdown prior to IWL, triggering the
panicking code-path from golang/go#47030. I wasn't able to reproduce,
but all panics are occurring in regtest/misc, and this seems highly
plausible.

Fixes golang/go#47030

Change-Id: I4df65697f644cff275ab1babb783868fd9e10c2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332589
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-07-08 21:02:30 +00:00
Rob Findley 2f04284e71 internal/lsp/regtest: allow for unsent diagnostics in TestResolveImportCycle
Gopls doesn't send empty diagnostics for open files in some cases
(likely a race to context cancellation). This is probably a bug itself,
but for now don't let this cause TestResolveImportCycle to fail.

Added a TODO to investigate further.

Fixes golang/go#46773

Change-Id: I197d9b09885951b47b3f90a0480ae75679d2a1a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333289
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-07-08 19:58:58 +00:00
Rob Findley febfa9d67f internal/lsp/source: move diagnosticsDelay out of experimental
This option has been enabled by default for a while now: remove it from
'experimental' to 'advanced'.

Change-Id: Id8116bf7b8976204a61fe1bbf9dc0b8bd69c68d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309271
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-07-08 19:56:33 +00:00
Rob Findley 251f283686 internal/lsp: add a setting to batch didChangeWatchedFile notifications
Gopls's serial processing of didChangeWatchedFile notifications has
historically been a pain point for clients that don't batch file events,
when branch switching or running go generate.

It's "not that tricky" for us to debounce and batch up watched file
notifications on our end, so this CL introduces this functionality as a
new experimental setting. Truth be told it ended up being harder than
expected, due to (1) our requirement for regtests to be able to
determine when diagnostics have completed, and (2) our reliance on
jsonrpc2 for sequencing changes to the server.

To address (1), I factored out the actual processing of change
notifications into a separate method (thus increasing our surprisingly
long chain of method calls). To address (2), I guarded the processing of
file changes with a mutex.

I also guarded some places where views and snapshots were accessed in
potentially racy ways. Our interaction with session.views was rather
complicated, so I had to switch session.viewsMu to a RWMutex.

Add this to the experimental regtest mode, and more generally enable all
experimental features in this mode, rather than just the experimental
workspace module.

Fixes golang/go#41691

Change-Id: Ifccdbdf86263dbe2e37ffe9f7bbf2a2cd74218b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309269
Trust: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2021-07-08 19:55:54 +00:00
Rob Findley c979f9254a internal/lsp/cache: invalidate packages in setMetadata
When setting metadata, we must invalidate any packages that have been
computed with old metadata.

Also make setMetadata atomic, by locking around it in Load. This is just
writing memory after a Load, so should be fast and infrequent. It is
critical that our various maps related to metadata are coherent, so it
makes sense to err on the side of coarser locking, IMO.

Fix a race in TestUpgradeCodeLens.

Change-Id: Iee8003b7e52b9f21681bdba08a009824f4ac6268
Reviewed-on: https://go-review.googlesource.com/c/tools/+/331809
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-07-08 19:53:41 +00:00
pjw 55cd4804df gopls/doc: Document how gopls generates semantic tokens.
The new file gopls/doc/semantictokens.md describes how gopls presently
decides which semantic tokens and modifiers to send to clients.

Change-Id: Ifc76e13954dde2ea58b6da446517f94be038a285
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333010
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
2021-07-08 16:22:39 +00:00
Jean de Klerk 48cad5ecb1 tools/gopls: small fixes to contributing.md
Change-Id: I261e1936b96e70ad8347655a7a8d0eb792348d44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330853
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-25 15:05:05 +00:00
Rebecca Stambler da404b52bc internal/lsp: start parsing go.work file like gopls.mod file
Allow users to experiment with golang/go#45713 by adding experimental
support for the go.work file. We handle it like a special case, very
similar to the current gopls.mod file mechanism. The behavior is
undefined if both a gopls.mod and go.work file exist. Ultimately, we
will deprecate support for the gopls.mod file if the go.work file
proposal is accepted, so I don't think it's important to be careful
about handling both simultaneously.

Change-Id: Id822aeec953fc1d4acca343b742afea899dc70ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328334
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-24 04:20:31 +00:00
Rob Findley d824a7481d gopls/doc: include instructions for compiling generic code
Change-Id: Ia7855890c72f9e376816a7c2d71f9715c7f2eb9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330369
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-23 15:16:55 +00:00
Rob Findley 6d3e439322 gopls/doc: add instructions for working with generic code
Change-Id: Ib0e5e16042791d96ed0d71e900b006a2c536032a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/330031
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-06-22 19:12:15 +00:00
Rebecca Stambler 463a76b3dc internal/lsp: only reload invalid metadata when necessary
This change adds a shouldLoad field to knownMetadata so that we can be
more selective about reloading these.

If a package has invalid metadata, but its metadata hasn't changed, we
shouldn't attempt to reload it until the metadata changes.

Fixes golang/go#40312

Change-Id: Icf5a13fd179421b8f70a5eab6a74b30aaf841f49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/298489
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-16 01:55:16 +00:00
Rob Findley 3f7c32638c gopls/internal/regtest: skip the flaky TestResolveImportCycle
Also improve the description of the failing expectations.

Updates golang/go#46773

Change-Id: I9465de8a5005bb7ee719a536f8550afc54bd6044
Reviewed-on: https://go-review.googlesource.com/c/tools/+/328369
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-06-16 00:40:27 +00:00
Rebecca Stambler b12e6172dc internal/lsp/cache: don't delete metadata until it's reloaded
Retrying CL 271477, this time with parts of CL 322650 incorporated.

This CL moves to a model where we don't automatically delete invalidated
metadata, but rather preserve it and mark it invalid. This way, we can
continue to use invalid metadata for all features even if there is an
issue with the user's workspace.

To keep track of the metadata's validity, we add an invalid flag to
track the status of the metadata. We still reload at the same rate--the
next CL changes the way we reload data.

We also add a configuration to opt-in (currently, this is off by
default).

In some cases, like switches between GOPATH and module modes, and when a
file is deleted, the metadata *must* be deleted outright.

Also, handle an empty GOMODCACHE in the directory filters (from a
previous CL).

Updates golang/go#42266

Change-Id: Idc778dc92cfcf1e4d14116c79754bcca0229e63d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/324394
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-16 00:19:40 +00:00
Rebecca Stambler 4b484fb136 internal/lsp: exclude the module cache from the workspace
This change treats the GOMODCACHE like a directory filter, and it
excludes any modules under the module cache from being considered part
of the workspace. This can happen when users open their entire GOPATH.

To do this, I had to propagate the view's root and gomodcache through
the exclusion functions, and I also had to add BuildGoplsMod to the
snapshot's interface.

Change-Id: Id80b359d73d09a525b380389917451e85357b784
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326816
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-11 17:18:39 +00:00
Rob Findley 9a55cb1fbb internal/lsp/command: minor clean-up of StartDebugging description
Change-Id: I9975a98fa47f2a17a9e013d67b4a48cc6fa17598
Reviewed-on: https://go-review.googlesource.com/c/tools/+/327110
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-11 16:06:07 +00:00
Rob Findley 490eac872f internal/lsp/command: add missing doc and support for result parameters
This CL cleans up doc/commands.md to include missing command
documentation and add support for result parameters.

Included are some quick-and-dirty extensions to the command metadata
loader that handle slices and arrays of struct types.

Change-Id: I43a85e88c9c53e21b790d89a45a9de444addcc63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326909
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-06-11 15:30:39 +00:00
Rob Findley 9aa007ed16 internal/lsp/cache: invalidate broken packages when imports are deleted
In the presence of an import cycle, packages.Load will exclude the
problematic import from Package.Imports. This missing edge means that we
wouldn't correctly invalidate broken package metadata when a problematic
import is deleted. We also weren't treating import deletion as a change
that could invalidate metadata.

Fix this by invalidating any broken packages when an import path is
deleted, anywhere. This could be overly coarse, but there are various
problems with trying to do better.

Also change cycle errors from TypeError to ListError, as this
categorization was misleading.

Fixes golang/go#46667

Change-Id: Id19e7baf4009632caf3ae9365497552f9b64b5c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/326589
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-06-10 18:32:07 +00:00
Muir Manders b579874149 lsp/completion: reorganize how we track candidate type mods
"type mod" refers to agglutinative expressions such as dereference
"*", invocation "()", and slicing "[:]". When considering an object as
a completion candidate, we check whether applying a type mod would
make it a better candidate.

Previously we tracked the type mods we wanted to apply to a candidate
by setting bool fields. Now instead we keep a slice of the type mods.
This has two main advantages:
- The mods are now ordered which will allow us to format candidates
  properly when the same mods can appear in different order (e.g.
  "<-*foo" or *<-foo").
- We can now record any mod multiple times allowing for "<-<-foo" or
  "foo()()".

I changed the formatting code to always create a snippet object since
that made things simpler. I had to tweak a few snippet helper methods
to accept a snippet argument rather than creating a new snippet.

This commit's only functional change is that we no longer show any
type mods in candidate labels. For example, the user will now see
"foo" in the completion popup instead of "*foo". Showing the operators
adds noise to the candidate list, and we didn't display them
consistently.

Updates golang/go#46045.

Change-Id: I3ea7baa1ee2fee80a1f8cfe88cbae1093ae269ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323449
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
2021-06-09 15:58:13 +00:00
Rebecca Stambler 1225b6f53f internal/lsp: memoize allKnownSubdirs instead of recomputing
A lot of the time spent for every file change is recomputing the set of
known subdirectories in the workspace. We can easily memoize these known
subdirectories and avoid recomputing them on every file change. Do that
here and update the set as file creations and deletions come in.

Updates golang/go#45686
Fixes golang/go#45974

Change-Id: Ide07f7c90f0cafc3a3cc7b89ba14ab82d4e3ab28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/317410
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-04 20:34:05 +00:00
Rebecca Stambler 8f2cf6ccfc gopls/internal/regtest: clean up TestFillReturnsPanic
The test doesn't necessarily need to require exactly 2 log messages, so
the match doesn't need to be so exact.

Updates golang/go#46546

Change-Id: I6ec5dee820c76c41db7b1d4bad3925fc7afe25e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/324760
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-06-03 18:03:08 +00:00
Rebecca Stambler 384c392572 Revert "internal/lsp/cache: don't delete metadata until it's reloaded"
This reverts commit 46e69bf3b2.

Reason for revert: Still has bugs associated with it and want to do a release

Change-Id: Ifa80bde147aa23aa4029a157d5dbaf6479d53024
Reviewed-on: https://go-review.googlesource.com/c/tools/+/324290
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-02 19:45:53 +00:00
Rebecca Stambler 726034ecff Revert "internal/lsp: enable semantic tokens by default"
This reverts commit 5ab822f631.

Reason for revert: Too early to enable since we need to figure out a plan for formatting directives

Change-Id: I46567b271d4ecd7e4af574221520a72bd8b8e500
Reviewed-on: https://go-review.googlesource.com/c/tools/+/324289
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-02 18:55:23 +00:00
Rebecca Stambler 5ab822f631 internal/lsp: enable semantic tokens by default
Change-Id: Ie094874849fce1ff06988c8b1809f48a5a34b555
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323169
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
2021-05-27 19:51:27 +00:00
Rob Findley a3eb095d6a gopls/internal/regtest: skip TestDeleteModule_Interdependent
This test is flaky due to orphaned file reloading. Since it is for an
experimental feature, skip it until we understand the problem.

For golang/go#46375
Updates golang/go#46183

Change-Id: Id6b369f2b61730c8503e3532547708f7689343c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/322649
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-05-25 21:54:09 +00:00
Dominik Honnef c3e30ffe92 gopls/internal/hooks: add Staticcheck's quickfix category of checks
Staticcheck's new quickfix category contains optional code
refactorings, such as turning a chain of if/else-if into a switch
statement, or simplifying boolean expressions.

These checks produce diagnostics with the "hint" or "information"
severities. Most use "hint", which produces a subtle hint in VSCode's
UI that a refactoring is available. "information" produces a blue
underline and is only used for those checks that have a very high
likelyhood of improving code readability.

Change-Id: I8f5b4eca67eb6b4e45c17510459d83eb673384e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/322492
Trust: Dominik Honnef <dominik@honnef.co>
Run-TryBot: Dominik Honnef <dominik@honnef.co>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-05-25 19:05:14 +00:00