Commit Graph

6024 Commits

Author SHA1 Message Date
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 076821bd2b internal/typeparams: update MultiIndexExpr to IndexListExpr
This type was renamed in CL 348609, based on discussion in the
go/ast proposal.

Change-Id: I3addcf1bb192ad03f59f599a7aec68a579178fd5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/348629
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: Robert Griesemer <gri@golang.org>
2021-09-08 23:54:26 +00:00
Koichi Shiraishi 2cae65cc5b internal/typeparams: follow changes to Type in the go/ast and go/types
As discussed on both proposals to CL 348375 (#47781) and
CL 348376 (#47916), Go core changed the 'T' word to 'Type'.
Follows those changes as well.

Change-Id: I52c354cdc7494aaf3c63bfb136efa86159175314
Reviewed-on: https://go-review.googlesource.com/c/tools/+/348509
Reviewed-by: Robert Findley <rfindley@google.com>
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>
2021-09-08 19:08:28 +00:00
Rebecca Stambler 0a6f080615 internal/lsp: check InRange before calling token.Offset
This shows up every now and then--maybe we need a wrapper function
around token.Offset to check the range.

Updates golang/go#48249

Change-Id: I9c60bc7cc61fcfb2f4e8c6963586d8b8fbb21835
Reviewed-on: https://go-review.googlesource.com/c/tools/+/348429
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-09-08 17:59:29 +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
Cuong Manh Le 3604566214 go/analysis/passes/testinggoroutine: fix panic in goStmtFun
For function declared in other files, the identifier denoted function
will be nil, cause the analysis panics. To fix this, we just skip that
identifier for now, until golang/go#48141 is resolved.

Fixes golang/go#48124

Change-Id: I87876505ee5964639ed3d1772d541c00d091ceb6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/347089
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2021-09-04 01:07:09 +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
Zvonimir Pavlinovic 8373dc3f73 go/callgraph/vta: document explicitly CallGraph assumptions
Change-Id: I0d04bff9ab84b26a0c0a8b643fa2c3cb113ba2f0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/346591
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
2021-08-31 23:48:18 +00:00
pjw 70fb962d80 internal/lsp/semantic: semantic tokens for imports of versions
For
import "a/bar/foo"
the existing code just decides the last component is the package name.
But for
import "a/bar/v2" this is incorrect, as the packge name is 'bar'.
The new code uses the result of parsing to derive the package name
from the import string.

That is, the package name was determined syntactically, it is
now determined semantically.

Fixes https://golang.org/issue/47784

Change-Id: Iccdd25e7e3f591e6514b1e0258e9e1879af9ff2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/343909
Trust: Peter Weinberger <pjw@google.com>
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>
2021-08-31 17:42:36 +00:00
Koichi Shiraishi 3e0d083b85 internal/typeparams: update for recent API changes in go/types
This fixes the build when run with -tags=typeparams.

Change-Id: I94861e7f46e396804740a8e3c24f164380903a69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/343129
Reviewed-by: 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>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
2021-08-25 16:32:36 +00:00
smasher164 c32dd46c00 internal/imports: update stdlib index for 1.17
$ go run mkstdlib.go

Updates golang/go#38706.

Change-Id: I0ad28f7d5e1fec2d1ccc3bc0f01f43a7d2f0d817
Reviewed-on: https://go-review.googlesource.com/c/tools/+/344629
Run-TryBot: Akhil Indurti <aindurti@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
2021-08-24 14:51:57 +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
Chressie Himpel e32504ce15 go/analysis/passes/printf: fix %w for non-fmt.Errorf functions
Previously all functions that were named Errorf have been treated like a
fmt.Errorf-based function. But only fmt.Errorf (and functions based on
fmt.Errorf) accept the %w verb. Fix that.

Updates golang/go#47641.

Change-Id: Iec5d0ae674c7dc817e85291dcfa064303eafba7e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340409
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Damien Neil <dneil@google.com>
2021-08-17 17:01:38 +00:00
Rebecca Stambler 96f147c242 internal/lsp/cache: fix flakiness of load progress reports
I'm not sure why there was a goroutine in the defer here, but I think
this should fix the flakiness of this test. I wasn't able to reproduce
the flake myself locally however, so I couldn't confirm. I'll leave the
issue open until it stops happening.

For golang/go#47508

Change-Id: Ie9fdc68d70fe1634c3ad001441cf3ce0d2693d17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/342674
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-17 14:56:21 +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
Zvonimir Pavlinovic 10bcabde7c x/tools/analysis/passes/printf: fix error message for unsuppported %w
The current error message for calls to functions that not support %w
formatting directive does not explicitly say what is wrong nor why using
%w is wrong. This CL makes the message more direct.

Fixes golang/go#47690

Change-Id: I6e42ab725e5e3a989a8505ec230b4bb6218079ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/342111
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Damien Neil <dneil@google.com>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
2021-08-16 20:51:50 +00:00
Koichi Shiraishi 758a1a1db7 internal/typeparams: fix undefined *types.Named.TArgs method
Current tip Go runtime (7eaabae84d) haven't (actually removed)
implements *types.Named.TArgs() method.
Calculate and make targs using named.NumTArgs().

Change-Id: Ib6a61cf2f8da644f533891f9bbb30179f8a7df5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/342229
Reviewed-by: Robert Findley <rfindley@google.com>
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>
2021-08-16 16:11:44 +00:00
Rob Findley a55d5155d9 internal/lsp: export and move some objects related to metadata
In preparation for moving metadata related functionality to a separate
package, move around some types and export some symbols. This is purely
to reduce diffs in subsequent CLs, and contains no functional changes.

Change-Id: I24d4fbd71df78e4d7a84f6598cdf820b41d542a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340729
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-15 15:51:49 +00:00
Rebecca Stambler 45389f592f internal/lsp: add support for go.work files in file watching
This will allow us to add more support for the workspace proposal.

Change-Id: Ie557121afe0c16989ac176dc9246d82661a20c44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/341811
Trust: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-08-13 16:57:31 +00:00
Cuong Manh Le 8fae06a885 go/analysis/passes/testinggoroutine: also inspect defined/anonymous functions
Currently, testinggoroutine only inspects functions literal invoked as
"go func(){ ... }()".

For defined or anonymous functions like "go f(t)" it didn't traverse the
function body. To fix this, on encountering those kinds of functions,
retrieve the definition node then inspect it.

Fixes golang/go#47470

Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338529
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
2021-08-13 03:36:45 +00:00
Cuong Manh Le 3fce476f0a go/analysis: add slice to array pointer conversion to nilness
If we know that a nil slice is converted to a non-zero length array
pointer, warn user that this operation will always panic.

Updates golang/go#47326

Change-Id: Ic8adcc0255ddc621c5626dc5c525899b13e1c6b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/337709
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
2021-08-13 03:35:06 +00:00
Tim King 03a91dd97e go/pointer: support ssa.SliceToArrayPointer
Updates golang/go#47326

Change-Id: I6b9b59e82b1b93f7a328ba802ad473d4104d7577
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339890
Run-TryBot: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Guodong Li <guodongli@google.com>
Trust: Robert Findley <rfindley@google.com>
2021-08-13 02:31:39 +00:00
Robert Findley d52cb71cca internal/lsp/source/completion: exclude 'any' from lexical results
The predeclared 'any' type is only valid when completing constraints. We
should support that properly, but for now exclude it from results so
that our completion tests don't fail on Go 1.18.

For golang/go#47669

Change-Id: I7852f844684a6c03da90bf367d45d732e5d1e9bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/341850
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: Matthew Dempsky <mdempsky@google.com>
2021-08-12 21:18:49 +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 15a2ab852e lsp/completion: reduce garbage searching for candidates
Tweak a few things to reduce garbage:
- Pre-allocate a couple hot error objects in format.go.
- Change methodsAndFields and packageMembers to take a callback
  instead of returning a slice.
- Use two queues for breadth first search. This allows us to alternate
  between and reuse the queues for each search level instead of
  appending to a single queue indefinitely with no reuse.
- Get rid of candidate.names field. This tracked the string name of
  each object in the deep completion path. Unlike with DFS before,
  due to BFS this has to be copied for every candidate we inspect. Now
  we get the object names from each types.Object in candidate.path,
  with the addition of a new bitmask field to remember whether each
  object needs "()" appended to it.

Using TestBenchmarkFuncDeepCompletion as a benchmark:

name        old time/op    new time/op    delta
Statistics    14.2ms ± 7%    10.3ms ± 1%  -27.41%  (p=0.016 n=5+4)

name        old alloc/op   new alloc/op   delta
Statistics    4.31MB ± 1%    3.03MB ± 0%  -29.60%  (p=0.016 n=5+4)

name        old allocs/op  new allocs/op  delta
Statistics     52.7k ± 1%     44.0k ± 5%  -16.52%  (p=0.008 n=5+5)

Change-Id: I52f619d9a2e8553115be91f05cf8cc5cfa89123e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323252
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-12 17:14:32 +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
Suzy Mueller f91c4a586e internal/lsp: use LookupParent for finding scope
The lexicalLookup function is used to determine if
a use of an object would be shadowed by a different
definition after a rename. Switch to using LookupParent
which is more careful about the positions of the
identifiers.

Fixes golang/go#47583

Change-Id: I3dbdf79e537ce637d1276ddbecb094db21f1c26d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340551
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-08-11 21:39:32 +00:00
pjw 5f5a173a39 internal/lsp/semantic.go: continue even without type information
And add a case that can occur while the user is editing the file.
Fix Parsed.LineCol to get a usable answer for the final newline.

Change-Id: I6367ff52051d72431453525279d99d7eb2180703
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339772
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
2021-08-11 15:16:17 +00:00
Rebecca Stambler 92b2fbe726 internal/lsp: improve error message about unsaved files
We should mention which files gopls thinks are unsaved.

Change-Id: I291976ad9bbf52e27c84fae650c613eb7ece8e83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340469
Trust: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-08-11 04:51:32 +00:00
Rob Findley 7bc3c281e1 internal/lsp/source: fix race in workspace symbols with multiple views
Unfortunately only after merging CL 338729 did I use it in a multi-view
workspace. That CL added a goroutine per matcher to scan symbols, but
unfortunately did this for each view, resulting in a race if there are
multiple views.

The fix is straightforward.

Change-Id: I405b37921883f9617f17c1e1506ff67b4c661cbc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340970
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-10 00:38:50 +00:00
Cherry Mui 337cebd2c1 go/analysis/passes/asmdecl: support in-register result in ABIInternal
With register ABI, for an ABIInternal function, understand that
the result may be written to a register instead of to memory.

TODO: argument area size calculation is still not fixed for
register ABI.

Change-Id: I109a5dc03ff0acc4bc04502710daf32efd1b08f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339250
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2021-08-09 22:50:32 +00:00
Rob Findley fcc905b221 internal/lsp/source: parallelize workspace symbols
Especially on larger repositories, paralellizing workspace symbol
requests can significantly decrease latency.

Benchmark (fastfuzzy "test" in x/tools): 17ms->11ms
Benchmark (fuzzy "test" in x/tools): 42ms->17ms
Benchmark (fastfuzzy "test" in kubernetes): 183ms->65ms

Change-Id: I21a1d844a0fc2a5f4c2fe5762620c82870586f54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338729
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:32 +00:00
Rob Findley 28b26823c8 internal/lsp/source: don't build low scoring workspace symbols
By refactoring the symbol resolution to defer symbol construction until
after scoring, we can avoid some unnecessary string allocation.

Benchmark (fastfuzzy "test" in x/tools): 21ms->17ms
Benchmark (fuzzy "test" in x/tools): 46ms->42ms
Benchmark (fastfuzzy "test" in kubernetes): 199ms->183ms

Change-Id: I9de72eb203c9971acc1afe89657976ce193b5a5d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338696
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:27 +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
Rob Findley bfe69c31e5 internal/lsp/fuzzy: add a new fuzzy matcher optimized for Go symbols
For workspace symbol requests we use various workarounds to optimize
matches for Go symbol paths (specifically, to increase relevance toward
the right hand side of a symbol path). These workarounds have a
significant impact on performance.

The existing fuzzy matcher could also be optimized, but is hard to
modify safely. As an experiment, add a new simple-but-fast fuzzy matcher
to use as a point of reference.

Change-Id: Iacaf149dfaf75f25e13909145f9508c7eaedf1a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338689
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: Peter Weinberger <pjw@google.com>
2021-08-09 20:21:06 +00:00
Rob Findley 15eebf7e82 internal/lsp: update the fuzzy matcher to operate on chunks
We can avoid allocating strings when performing workspace symbol search
by having the fuzzy match operate directly on chunks.

When operating on a single string, this slows down the matcher slightly
(perhaps 10%) due to copying bytes rather than accessing the string
directly. We could work around this using unsafe, but this could also be
mitigated by generics.

Benchmark ("test" in x/tools): 48ms->46ms
Benchmark ("test" in kubernetes): 868ms->857ms

Change-Id: Icf0f15aaa5cc3c875cf157a7b90db801045d9ed4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338694
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-08-09 20:21:00 +00:00
Rob Findley 0d28b7d7c5 internal/lsp/source: change symbol matcherFuncs to accept chunks
Whenever possible we should avoid doing string operations when computing
workspace symbols. This CL lays the groundwork for optimizations of this
sort by changing the signature of matcherFunc to accept chunks. It is
done in a naive way though, so this doesn't yet improve performance.

Benchmark ("test" in x/tools): 40ms->48ms
Benchmark ("test" in kubernetes): 799ms->868ms

Change-Id: I171c654b914e9764cfb16f14d65ef1aed797df73
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338693
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:20:24 +00:00
Rob Findley 0f3931c766 internal/lsp: precompute workspace symbols
Coupling workspace symbols to package checking means that they do not
function when the workspace is contracted, and also forces us to do
duplicate work traversing file declarations.

This CL changes the workspace symbol implementation to precompute
symbols based only on syntactic information, allowing them to function
in degraded workspace mode, improving their performance, and laying the
groundwork for more significant performance improvement later on.

There is some loss of precision where we can't determine the kind of a
symbol from syntactic information alone, but this is minor: we fall back
on 'Class' if we can't determine whether a type definition is a basic
type, struct, or interface.

Benchmark ("test" in x/tools): 56ms->40ms
Benchmark ("test" in kuberneted): 874ms->799ms

Change-Id: Ic48df29b387bf029dd374d7d09720746bc27ae5e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338692
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:20:17 +00:00
Rob Findley 48691842f1 internal/lsp/source: use match indexes to compute dynamic symbols
Before this CL, we preferred less qualified symbols in the "dynamic"
symbol style by executing multiple matches from right to left. This is
significantly inefficient for only a marginal benefit over using the
match indexes produces by the fuzzy matcher.

Instead, change the signature of the matcherFunc to expose the matched
index to compute the dynamic symbol.

Benchmark ("test" in x/tools): 144ms->56ms
Benchmark ("test" in kubernetes): 2.6s->874ms

Change-Id: I0bf017feee436bc0d8b14bdda1e64fd227669dd7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338691
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 17:59:45 +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
Tim King f367f012d5 go/callgraph/vta: not panic on the SliceToArrayPointer instruction
No interesting type flows so the change to vta itself is to not reject the instruction.

Updates golang/go#47326

Change-Id: Ifd11a7ef854afaee3978796f3113ca3254301d19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338849
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
Trust: Roland Shoemaker <roland@golang.org>
2021-08-04 21:03:22 +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
Nicholas Asimov 309db0446d internal/lsp/lsprpc: fix returning connection error on disconnect
Fix connection error not being returned resulting in errors like `gopls: remote disconnected: <nil>`.

Change-Id: I67fa1143b2fa0fd44c946040fc1bad51b1636183
GitHub-Last-Rev: 9ebae5c3b85b7bb73eb67bb1ee8028f6f504a83d
GitHub-Pull-Request: golang/tools#334
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339709
Reviewed-by: 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>
Trust: Peter Weinberger <pjw@google.com>
2021-08-04 13:50:16 +00:00
pjw 2f64839e75 internal/lsp/protocol: bring LSP protocol up to date
No significant changes.
The only change is to CompletionItemLabelDetails, which is new in 3.17.

Change-Id: I172f0ff72f5c27c0907d7ad733f19a6320c5f510
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339089
Run-TryBot: Peter Weinberger <pjw@google.com>
Trust: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-08-03 20:45:05 +00:00
Rob Findley 32c652e336 internal/span: fix a comment about windows drive letters
Change-Id: I24e7507d32fb5e76524ed6b9aeda225ccb11e861
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339351
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-03 19:59:22 +00:00