Commit Graph

844 Commits

Author SHA1 Message Date
Alan Donovan 9651276d64 internal/lsp/cache: optimize Snapshot.clone
This change replaces the single large map used for snapshot.goFiles
by a map of 256 stripes, each of which becomes immutable once shared.
This optimizes the common case in which the copy is nearly identical
to the original.

We still need to visit each map entry to see whether it needs to be
deleted (which is rare) and to inherit the handle in the usual case.
This is now done concurrently.

Also, share the (logically immutable) []PackageIDs slices across
old and new snapshots. This was worth 5% of CPU and 1/3 of allocations
(all small).

Benchmark on darwin/arm64 shows a 29% reduction for DidChange.

$ go test -v ./gopls/internal/regtest/bench -run=TestBenchmarkDidChange -didchange_dir=$HOME/w/kubernetes -didchange_file=pkg/util/hash/hash.go

Before:
BenchmarkStatistics	     100	  22955469 ns/op	11308095 B/op	   47412 allocs/op
BenchmarkStatistics	     100	  23454630 ns/op	11226742 B/op	   46882 allocs/op
BenchmarkStatistics	     100	  23618532 ns/op	11258619 B/op	   47068 allocs/op

After goFilesMap:
BenchmarkStatistics	     100	  16643972 ns/op	 8770787 B/op	   46238 allocs/op
BenchmarkStatistics	     100	  17805864 ns/op	 8862926 B/op	   46762 allocs/op
BenchmarkStatistics	     100	  18618255 ns/op	 9308864 B/op	   49776 allocs/op

After goFilesMap and ids sharing:
BenchmarkStatistics          100          16703623 ns/op         8772626 B/op      33812 allocs/op
BenchmarkStatistics          100          16927378 ns/op         8529491 B/op      32328 allocs/op
BenchmarkStatistics          100          16632762 ns/op         8557533 B/op      32497 allocs/op

Also:
- Add comments documenting findings of profiling.
- preallocate slice for knownSubdirs.
- remove unwanted loop over slice in Generation.Inherit

Updates golang/go#45686

Change-Id: Id953699191b8404cf36ba3a7ab9cd78b1d19c0a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410176
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
2022-06-10 17:57:19 +00:00
Rob Findley 1d19788894 internal/lsp/cache: always compute IsIntermediateTestVariant
It is inconsistent to only compute IsIntermediateTestVariant for
workspace packages, and could be a source of bugs. Always compute it.

Change-Id: I16f40fe44a1145a74ef77fee4b7fd813abe603cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340732
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-06-09 15:02:08 +00:00
Rob Findley 4a8620f6ba internal/lsp/cache: move metadata fields to a new metadataGraph type
In preparation for making metadata immutable, move metadata-related
fields to a new MetadataGraph type. Other than instantiating this type
when cloning, this CL contains no functional changes.

For golang/go#45686

Change-Id: I7ad29d1f331ba7e53dad3f012ad7ecdae4f7d4b7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340730
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-06-08 22:15:25 +00:00
Robert Findley a3d129cecf internal/lsp/cache: extract module load errors when go.work is used
When using go.work, the go command packs module-specific errors into
synthetic results corresponding to the module query ("modulepath/...").

Extract these for use in diagnostics, by packing them into a new
moduleErrorMap error type.

Fixes golang/go#50862

Change-Id: I68bf9cf4e9ebf4595acdc4da0347ed97171d637f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405259
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-06-08 22:04:49 +00:00
Alan Donovan 63dfc2d3a9 internal/lsp/cache: two minor optimizations
1. Avoid unnecessary intermediate map updates.
2. Avoid accumulating defers in a loop when the control is simple.

Yield: -10% CPU, -37% allocs.

Typical results:

$ go test -v ./gopls/internal/regtest/bench -run=TestBenchmarkDidChange -didchange_dir=$HOME/w/kubernetes -didchange_file=pkg/util/hash/hash.go
Before:
BenchmarkStatistics	     100	  25932206 ns/op	11684109 B/op	   75458 allocs/op
After:
BenchmarkStatistics	     100	  23294195 ns/op	11293472 B/op	   47299 allocs/op

Also, move profiling logic outside the loop so that later runs
don't overwrite earlier runs. (This doesn't appear to be a problem
in practice, presumably because the last run is the big one.)

Updates golang/go#45686

Change-Id: I538ca6bb88cc18f1eefe35d2db29a62e5190280e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410697
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-06-06 21:20:40 +00:00
Robert Findley 76325da620 internal/lsp/progress: detach context for all progress notifications
Use a detached Context for all progress notifications. In particular,
using a detached Context for the window/workDoneProgress/create
notification avoids races where the $/cancelRequest notification and
create response cross paths, such that the client has created a progress
dialog but the server thinks that starting progress failed.

Also, as a matter of best practice don't store a context on the WorkDone
type, despite the fact that this Context is detached. Instead, only
close over a Context in the WorkDoneWriter, which requires a Context in
order to function but which implements the io.Writer interface.

The TestProgressBarErrors test should now pass reliably.

Fixes golang/go#46930

Change-Id: I0d115ed3a62de97fe545c8dc0403e7bb55f6e481
Reviewed-on: https://go-review.googlesource.com/c/tools/+/409936
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
2022-06-02 20:26:04 +00:00
Robert Findley 9d7bf95bad internal/lsp: factor out column mapper construction from content
This eliminates some duplication, and lays the groundwork for removing
the use of token.File within ColumnMapper.

Change-Id: I54fe570bfc4f7bca0da643b8727e890dc6343208
Reviewed-on: https://go-review.googlesource.com/c/tools/+/406135
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-06-01 19:19:49 +00:00
Robert Findley 9e1d19b13e internal/span: eliminate TokenConverter
The TokenConverter has been trimmed down to a thin wrapper around
token.File, and can now be removed.

Change-Id: I9985492274c88e6a13e6d62dadab5595c75c7840
Reviewed-on: https://go-review.googlesource.com/c/tools/+/406134
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-06-01 19:19:15 +00:00
Robert Findley ccb10502d1 internal/lsp/cache: invalidate metadata when parsing issues resolve
Package loading (at least via go list) fails when the import header
doesn't parse, so we need to invalidate metadata upon a state change
from non parsing->parsing. Refactor metadata invalidation to implement
this feature, add additional documentation, and avoid unnecessary work.

This change revealed a latent bug (via TestDeleteDirectory): when
statting a deleted directory failed, we could fail to invalidate any
package IDs, even those we already knew about. This bug was masked by
the somewhat complicated semantics of pkgNameChanged. The semantics of
pkgNameChanged are simplified to encapsulate any change to a
package->file association.

Also refactor the parsing API somewhat, and add a bug report if we don't
get a ParseGoFile while inspecting for metadata changes.

Update most regtests to panic upon receiving bug reports.

Fixes golang/go#52981

Change-Id: I1838963ecc9c01e316f887aa9d8f1260662281ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407501
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-05-23 18:14:40 +00:00
Robert Findley facb0d30e9 internal/span: eliminate Converter and FileConverter
The only real implementation of position conversion was via a
*token.File, so refactor the converter logic to eliminate the Converter
interface, and just use a single converter implementation that uses a
*token.File to convert between offsets and positions.

This change is meant to be a zero-impact refactoring for non-test code.
As such, I abstained from panicking in several places where it would
make sense. In later CLs, once the bug reporting API lands, we can
insert bug reports in these places.

Change-Id: Id2e503acd80d089bc5d73e983215784015471f04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405546
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-05-17 16:08:12 +00:00
Robert Findley d303668635 internal/lsp/cache: use cached parsed files for symbols, if available
Optimize building the symbol index for a file, in two ways:
 - use the cached full parse tree, if it already exists
 - if it doesn't exist, optimize parsing by skipping both comments and
   object resolution, which aren't necessary for symbols

This results in around 3x faster initial indexing of symbols. In my
manual testing, indexing of Kubernetes went from 16s->5s, and indexing
of x/tools went from 2.4s->700ms.

Also fix a typo in gopls/internal/regtest/bench/bench_test.go.

Fixes #52602

Change-Id: I0893e95410be96e94e5e9dee7a3aab30b59c19c5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403679
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
2022-05-09 17:04:06 +00:00
Robert Findley 0fb1abf25a internal/lsp: factor out go/token wrapper into a safetoken package
This avoids an import cycle that prevented these wrappers from being
used in the lsppos package.

Change-Id: I9eedd256db983dfcf962edba39e3d4f3a1aabdeb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403680
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-09 17:03:37 +00:00
Robert Findley 556c550a38 internal/lsp/cache: invalidate packages that have added files
When a file is new or its package name has changed, we should invalidate
all packages for files in the current directory, not just packages
previously containing the file.

Fixes golang/go#52500

Change-Id: I9fc9857a7abcd4e730068871c899d274e1736967
Reviewed-on: https://go-review.googlesource.com/c/tools/+/401795
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
2022-05-02 22:20:22 +00:00
Robert Findley c9035635c7 internal/lsp/cache: don't cache parsed files when checking for metadata
changes

In principle, we should only parse through the cache when in workspace
mode, else we risk pinning AST in memory that we'll never need.
Alternatively: when unsure we should default to NOT parsing through the
cache.

For now, just update the logic to check for metadata changes to not
memoize its result.

Change-Id: I200af9ffb3353ba8065e46100a588dce6239dc66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/401794
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-25 23:52:26 +00:00
Robert Findley 37590b385d gopls: remove usage of golang.org/x/xerrors
As of golang/go#50827, gopls no longer supports building at 1.12, and so
usage of golang.org/x/xerrors can be replaced with the native support for
error wrapping introduced in Go 1.13.

Remove this usage as a step toward eliminating the xerrors dependency
from x/tools.

For golang/go#52442

Change-Id: Ibf459cc72402a30a6c2735dc620f76ed8a5e2525
Reviewed-on: https://go-review.googlesource.com/c/tools/+/401097
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-20 15:33:22 +00:00
Michael Matloob 5bb9c48e96 x/tools/go/packages: on Go 1.19+, explicitly ask for -json fields needed
This change uses the new feature on the go command to explicitly ask
for needed fields on the JSON output to ask for needed fields. Not
requesting most of those fields doesn't make as much of a difference,
but being able to skip Deps and Stale should add decent gains in most
cases.

Change-Id: I30e9d53fbdaf2ab485d73d5d9b9ccd60a78ed10c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/393017
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-19 17:08:06 +00:00
Russ Cox 2bbdb7a52e gopls, internal/lsp: gofmt
Gofmt to update doc comments to the new formatting.

(There are so many files in x/tools I am breaking up the
gofmt'ing into multiple CLs.)

For golang/go#51082.

Change-Id: Ife11502fe1e59a04d53dba9edccd3043e57f9ae8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399358
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-12 17:41:31 +00:00
Dan Kortschak 4077921f14 all: fix spelling
Change-Id: I13cf73d7e043dda1a06c28bb09e413a76a68df1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/391934
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-04-04 19:50:34 +00:00
Robert Findley 86b02b36c4 internal/lsp/cache: check for nil WorkFile.Go
Fixes golang/vscode-go#2121

Change-Id: Icf44c8bb4079c0bcba83a9512f939260bbb5b6de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/393856
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-03-21 13:26:21 +00:00
Suzy Mueller 779dfa4fb2 internal/lsp: invalidate package on go.mod change with go.work
If a go.mod file is updated, the packages
in that module may have updated type check
errors since the go version can affect errors.

Fixes golang/go#51732

Change-Id: I3a8cd8b9ab267336ccc5c841a14c5ec5f6c986e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/393534
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-03-17 21:30:21 +00:00
Hana 877ec07e2d internal/lsp/cache: remove unused code
Change-Id: I7bafb1555ab7d35b9c49138c915b6c6c781e92c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/392656
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-03-17 13:53:34 +00:00
Michael Matloob 622cf7b338 internal/lsp/cache: copy workFile when invalidating workspace
to avoid losing workFile information when that happens. This fixes an
issue where diagnostics, hover, etc didn't show up after the initial
load when some changes were made to go.work files.

Change-Id: I42e2dcfd94a5b4726856ab0a4d8dfc9c1efc48b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/391257
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-03-10 22:50:41 +00:00
Robert Findley c773560c36 internal/lsp/cache: set GOWORK=off when mutating modfiles
Commands will fail with -mod=mod when GOWORK is set. Explicitly set it
to off.

Fixes golang/go#48941

Change-Id: I39f9d95526ca6f3b0414ff273cecd443d69afa61
Reviewed-on: https://go-review.googlesource.com/c/tools/+/391518
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-03-10 21:45:24 +00:00
Suzy Mueller b105aac570 internal/lsp: use regexp to add go mod edit -go quick fix
Fixes golang/go#51086

Change-Id: Iebe51355e016809442af37debf140d9ba2f3317a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/390615
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Suzy Mueller <suzmue@golang.org>
2022-03-09 21:08:21 +00:00
Robert Findley b59c441bdb internal/lsp/cache: always consider go.work files for ws expansion
If a go.work file is present, we activate the workspace mode regardless
of the experimentalWorkspaceModule setting.

Fixing this also revealed a long-standing bug in traversing parent
directories. Amazingly I have no recollection of this bug being
reported, but it is far more likely to be hit with go.work files, when
users regularly open nested subdirectories.

Fixes golang/go#51477

Change-Id: I310a11b14f3bbc18fdd604c5c0ec33d7ee8f1174
Reviewed-on: https://go-review.googlesource.com/c/tools/+/389300
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-03-07 16:11:49 +00:00
Michael Matloob 121d1e448f internal/lsp: report diagnostics on go.work files
Report diagnostics on use lines where the directory doesn't have a
go.mod file, and on syntax errors in go.work.

For golang/go#50930

Change-Id: Idab36b43d86c4842f8eecd5c071ce0587e6f27b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/389317
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-03-04 18:29:58 +00:00
Michael Matloob 3286927895 internal/lsp/cache: construct workspace even when go.work has error
Before this change, newWorkspace would fall back to other workspace
types if there was an error parsing go.work files, or one of the
workspace modules did not exist. Instead, in those cases, newWorkspace
now successfully builds a *workspace with an error set.

This respects the user's intent when working with a go.work file so that
gopls can surface a diagnostic with the error.

Change-Id: Icfc2cad270111c03d53137fa8f1b2ee9b75093c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/389314
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-03-03 21:30:30 +00:00
Suzy Mueller c2ddf3dda2 internal/lsp: add quick fix for unsupported feature
Adds a command to run go mod edit -go to allow
users to easily upgrade their go directive.

Doing this change also revealed that changing
the go directive does not invalidate the type check
data and there may be stale diagnostics for a package.

Updates golang/go#51086

Change-Id: I659a216059c489a88e29cd51b944c3a0274f3700
Reviewed-on: https://go-review.googlesource.com/c/tools/+/386875
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-28 18:28:20 +00:00
Suzy Mueller b7525f4396 internal/lsp: hash go version into package key
The go directive affects type checking errors so
it should be included in the package key.

I manually tested that this fixes the issue in golang/go#51325

Fixes golang/go#51325

Change-Id: I109859ae65e7e4d5fdefc63a0a7a838117ee02cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/387914
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-02-24 19:04:43 +00:00
Robert Findley 5210e0ca15 gopls: wire in LangVersion and ModulePath for gofumpt formatting
If available, pass the LangVersion and ModulePath provided by
go/packages to gofumpt. This allows gofumpt to apply additional
formatting rules that require this information.

Also add a regression test for gofumpt formatting.

Fixes golang/go#51327

Change-Id: I47c8c96d595d62e1c444285ce69ce6a4e61fa74c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/387634
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-24 18:08:50 +00:00
Robert Findley 897bd77cd7 internal/gocommand: remove support for -workfile
Remove support for passing the -workfile flag now that the go command no
longer supports it.

Updates #51215

Change-Id: I95d73fb1a3a6d9bcfaaae5e22e44722118d12c03
Reviewed-on: https://go-review.googlesource.com/c/tools/+/386536
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-18 14:51:54 +00:00
Hana c6776771dd internal/lsp/cache: let Session.getGoEnv query GOWORK
This will make GOWORK value appear in the gopls log
when initializing the workspace. This CL does not include
GOWORK in `environmentVariables` (in view.go) (yet).

Change-Id: I16224d1ac50ac8209def008966371c6be858d05a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/385894
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2022-02-16 20:15:32 +00:00
Michael Matloob be40034de7 internal/lsp: add support for formatting go.work files
Wired through support for calling x/mod's go.work formatter on go.work
files into LSP. Tested it by hand in editor using the "Format Document"
command. Added a test case to workspace_test regtest, though I'm not
totally sure the test is correct.

For golang/go#50930

Change-Id: Ied052ded514bb36f561737698f0e2d7b488158e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/383774
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-02-15 17:15:32 +00:00
Heschi Kreinick 164402db9d internal/lsp/cache: set types.Config.GoVersion
Set the language version from the controlling go.mod file's go version,
if any. Also verify that we properly surface a diagnostic if the version
is invalid.

I didn't add any quick fixes.

Fixes golang/go#50688.

Change-Id: Ic472502d1224a1decb5b989d51110b837020e998
Reviewed-on: https://go-review.googlesource.com/c/tools/+/383394
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-07 18:19:30 +00:00
Robert Findley 597b165f5f internal/lsp/cache: use -workfile on 1.18
When using go 1.18 with go.work files, don't fake workspaces by creating
a workspace module: just run from the workspace root and pass -workfile.

For golang/go#44696

Change-Id: Iaa1979d26b1ce67c6e18e0bf26546a504069da8c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382241
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-04 14:03:59 +00:00
Robert Findley 8305295d5d gopls: recognize the go.work file type
For golang/go#50930

Change-Id: I5bae56853f1e913fccd559d2a553ced4200bcb35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382237
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: Gopher Robot <gobot@golang.org>
2022-02-04 14:03:20 +00:00
Robert Findley 3e30e21b53 gopls: remove the workspace_metadata command
Remove the workspace_metadata command, as VS Code no longer needs this
to run workspace commands (it can use go.work instead).

Updates golang/go#44696

Change-Id: Ife579a15e64969c4301e4508e18b7c8a8b633b9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382235
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: Gopher Robot <gobot@golang.org>
2022-02-04 14:02:58 +00:00
Robert Findley 25d2ab26ce internal/lsp/cache: fixes for workspace invalidation
This CL fixes a several bugs in workspace invalidation:
 - When invalidating the workspace due to a change in gopls.mod or
   go.work files, we should not update the workspace if the change does
   not parse. There was a comment to this effect, but it was not
   properly implemented.
 - Check go.work before gopls.mod, consistent with our intitial
   workspace load.
 - If we get an 'unchanged' change (such as a save) to go.work, don't
   continue looking for gopls.mod.

A regression test that inadvertently relied on our mishandling of broken
go.work files is updated to have the correct syntax. A new regtest
expectation is added to assert on a clean workspace.

For golang/go#44696

Change-Id: I2b7e739573e225cadfbf8cc892a0b5daf0191e40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382115
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-04 14:02:15 +00:00
Robert Findley 8d915b1238 internal/lsp/cache: simplify workspace construction
Remove some duplication in the construction of cache.workspace
instances.

For golang/go#44696

Change-Id: Ic955fa3f9a574781f627c550694f0b87ad2013e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382114
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-04 14:02:00 +00:00
Robert Findley 77f5fa5142 internal/lsp/cache: replace old go.work parser with x/mod
Gopls was using an old version of workfile support in x/mod that did not
support the 'use' syntax.

Upgrade to x/mod@master, and update tests accordingly.

For golang/go#44696

Change-Id: I87841cd1322d7b1aa0fbc929838746ea600bee77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382078
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-04 14:01:19 +00:00
Robert Findley 4da8ceb6f1 internal/lsp/source: remove the unused UpdateUserModFile invocation flag
Change-Id: I4325417e4d97d3ab82551c2fa0382aa3d962a781
Reviewed-on: https://go-review.googlesource.com/c/tools/+/382077
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-02-03 22:05:07 +00:00
Robert Findley e7c9de23ff internal/lsp/cache: remove unsynchronized write to snapshot.files
snapshot.GetVersionedFile already memoizes the file handle in
snapshot.files. There is no reason to do another write, and this write
is unsynchronized.

Fixes golang/go#50747

Change-Id: Ib196395135de6f22dd59169accb756665ad10e58
Reviewed-on: https://go-review.googlesource.com/c/tools/+/380214
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-01-21 21:27:57 +00:00
Robert Findley 6299a6dbc2 internal/lsp/cache: keep going on per-file workspace symbol errors
I'm not sure how to hit this in practice, since it appears that symbols
should only return errors with some type of corruption, but we should
keep going when searching for symbols in snapshot files.

Change-Id: I9fcf6cc92fdc943520e8890ece51d28ec46e8812
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376363
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-01-19 15:06:41 +00:00
Robert Findley 7424a4db47 internal/lsp/cache: fix check for excluded paths in locateTemplateFiles
The pathExcludedByFilter helper function expects path to be relative to
the workspace folder. This is a confusing API, and led to us passing
absolute paths when checking for excluded template files.

Fix it, add a regtest, and add a TODO to clean up the API.

Fixes golang/go#50431

Change-Id: Iedcf0dd9710e541c9fb8c296d9856a13ef3fbcb6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/378398
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-01-14 17:01:02 +00:00
Robert Findley 4e31bdea78 internal/lsp/cache: avoid panic in mod diags with redundant requires
modfile.File.SetRequire panics on duplicate requires with conflicting
versions. Avoid the panic by returning an error in this case.

Skip directness diagnostics that run into this error, rather than
invalidating all diagnostics.

Fixes golang/go#50425

Change-Id: Ic6379ecc48581e7fd7b470ed295e449833c351dd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376394
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: Gopher Robot <gobot@golang.org>
2022-01-13 21:34:46 +00:00
Robert Findley 99c83b8bac internal/lsp: find templates based on file kind
Now that we honor the template file kind provided by LSP clients, we
need to use this to derive the set of active templates.

For golang/vscode-go#1957

Change-Id: If0883b2acff746fe6d187124b939f3a2d0e0d4f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/378354
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-01-13 20:55:50 +00:00
Robert Findley 3f6aab191a internal/lsp: honor the file kind provided by clients for overlays
Honor the file kind provided by clients for overlays, by passing the
FileHandle into View.FileKind and checking for overlays.

For golang/vscode-go#1957

Change-Id: I4a767cb64dc5205f1d10c3126a2cbe67c21a34e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/378314
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
2022-01-13 20:54:45 +00:00
pjw 68b574acb9 internal/template: identify template files by the templateExtensions option
Make the language id (sent from the client) 'gotmpl' equivalent to 'tmpl'

Wherever a view is known, use its options to determine which files
are template files. Whenever the client sends an explicit
languageID, use that.

Partially fixes golang/vscode-go#1957

Change-Id: I04cd630d6c6c80e0a78c2fafb6ddc1166ce86829
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376854
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-01-13 17:30:17 +00:00
ye.sijun 72cd390b55 x/tools: improving the error message for check fact type
Fixes golang/go#50235

Change-Id: Ic9f61cb61fbf1e2cd198cc5d20bcba794c3181dc
GitHub-Last-Rev: 32d212d858102507ab6d6e510264feca9b78787e
GitHub-Pull-Request: golang/tools#356
Reviewed-on: https://go-review.googlesource.com/c/tools/+/374016
Reviewed-by: Alan Donovan <alan@alandonovan.net>
Run-TryBot: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Trust: Keith Randall <khr@golang.org>
2022-01-04 19:51:13 +00:00
Bryan C. Mills e212aff8fd internal/memoize: do not allow (*Generation).Acquire to fail
The Acquire method is nearly instantaneous; it only potentially blocks
on a small, constant sequence of cache misses, so there is no need to
avoid blocking in it when a Context is cancelled.

An early return when the passed-in Context is canceled was added in CL
242838 to avoid incrementing the Generation's WaitGroup after its
destruction has begun; however, that early return also bypasses the
WaitGroup accounting that blocks Destroy while the generation is still
in use. Instead, we need the invariant that Acquire is not called in
the first place after Destroy, which we can ensure by nilling out
the View's snapshot when we begin destroying it.

I was not able to reproduce golang/go#48774 locally, but I believe
that this CL will fix it. (It may, however, expose other races or
deadlocks that may have been masked by the early return, which we can
then fix separately.)

Fixes golang/go#48774

Change-Id: Iac36fceb06485f849da5ba0250b44b55f937c44b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/367675
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-12-02 03:25:35 +00:00