Commit Graph

2803 Commits

Author SHA1 Message Date
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
Robert Griesemer 6bfd3a440e Revert "internal: temporarily disable tests so we can land CL 410955"
This reverts commit 5ca4cc8b9a.

Reason for revert: appears insufficient to land CL 410955.

Change-Id: If7029c9a43568e210e89d473b738400f5155715e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410995
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-06-08 01:09:07 +00:00
Robert Griesemer 5ca4cc8b9a internal: temporarily disable tests so we can land CL 410955
For golang/go#53279.

Change-Id: I3e0a502f8f0789ff9920af504ea8c86812bf49a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@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-08 00:38:18 +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
Alan Donovan 030812f079 internal: remove unneeded FileSets
This change replaces various uses of FileSet with either
nothing (when the parameter wasn't really needed) or token.File.

Notably, astutil.Imports was being used to extract the imports
of a file (available at ast.File.Imports), forcing a number
of wrappers to have a FileSet parameter.

Also, simplify various expressions file.Position().Line to file.Line().

Change-Id: I19fe86a18aba50352275f77ed737513744d3930c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410366
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-06-06 20:47:10 +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 3dd056a8c5 internal/lsp/mod: fix broken assumptions about file base
Logic in mod.Hover relied upon ColumnMapper.TokFile being the only file
in its FileSet. Fix this to compare offsets with offsets.

Also use safetoken.InRange in semantic.go, to fix an incorrect bounds
check.

Change-Id: Ia18a0799fea1120674e3772efdf0bdb724e554e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/409934
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-06-01 22:04:06 +00:00
Robert Findley 0cdf4b588d internal/lsp/source: eliminate ColumnMapper.PointSpan in favor of Pos
It turns out PointSpan was only ever used as part of an oft repeated
pattern to get a token.Pos from a protocol position. Cut out the
middle.

Change-Id: I0b2c0fc3d335e6bbd3c1ac72c6f75e2c40c60ca5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408717
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-06-01 19:19:59 +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 dae3f4bf3c internal/lsp/diff: remove unused TokenConverter parameters
Change-Id: Icb0b4d5c2d45eafdb1f23278e3828206a7f36908
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408094
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-06-01 15:41:18 +00:00
Robert Findley de3ef4aa45 internal/lsp/source: remove workaround for newline terminated files
Eliminate the need to work around newline terminated files in
completion, by storing selection ranges as token.Pos and using an
lsppos.TokenMapper derived from the file content, which does not have
problems with newline termination.

This simplifies the completion logic, and removes the last use of
MappedRange.SpanRange, which is an inconisitent API in that it returns
positions in the compiled source, rather than edited source.

Change-Id: I65232787956325172b48fd42d85cbb598039ee5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407889
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-06-01 15:40:59 +00:00
Robert Findley 6b760fce76 internal/lsp/source: remove ineffectual memoization in MappedRange
Due to not having a pointer receiver, memoization of the computed
MappedRange.protocolRange had no effect.

Rather than fix the memoization, just remove it since it has apparently
not affected performance significantly.

Change-Id: Ib34becef44dca4074dcc7c8af93883d9a1060f8d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408716
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-06-01 15:40:28 +00:00
Robert Findley ea608150c3 internal/lsp: use the correct converter for mapped range offsets
CL 405546 introduced a latent bug in MappedRange, because it naively
used the wrong TokenConverter to convert mapped positions to offsets.
This was detected via related clean-up work in another CL. Fix this by
passing the correct converter from MappedRange.Range. Add a test that
would have demonstrated the breakage.

More cleanup is needed here. It is subtle that MappedRange.Converter
maps the adjusted position for its start and end, and there may be some
places where this invariant has been broken over the years.

Add additional documentation and bug reports.

Change-Id: If7f177894bac1242ddcc1786e79c7559455e9291
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407887
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 15:32:54 +00:00
Robert Findley 2bb78cf04e internal/lsp: add missing bug reporting, and panic during marker tests
Add additional bug reporting related to position invariants, and update
marker test runners to panic on encountered bugs.

This revealed a panic on older Go versions, where we try to create a
range for a missing package name span. Lift the check into the caller
for this case, and leave a big comment explaining that
checkForOrphanedFile probably shouldn't be so tolerant of invalid calls.

Change-Id: Ie16a07afba0f8a5682cca50ad8f04450bfa2da65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407885
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-31 14:14:38 +00:00
Hana (Hyang-Ah) Kim f38573358c internal/gocommand: skip ill-formed env vars in cmdDebugStr
Fixes golang/vscode-go#2260

Change-Id: I4dbdb1d582e3b434bce9f0e923899d8af6a43f3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408376
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-05-25 15:29:25 +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 b62b00ff9a internal/lsp: add an option to get notified of bug reports
Our mechanism for reporting internal bugs doesn't work unless we
actually notice them. Add an undocumented option to receive showMessage
dialogs on the first bug occurring server-side.

Change-Id: I259a4c13161271c350fae06dc6ab0e1621725c92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399624
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>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-05-18 01:40:46 +00:00
Tim King 960b4ce5ae internal/typeparams: adds core type implementation
Adds a function for computing the core type of a type for use within x/tools.

Updates golang/go#52940

Change-Id: I91645c0b27031506be51a5f9d53b3e125e6fd1c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/406838
Run-TryBot: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-05-17 21:56:59 +00:00
Robert Findley b55ed5ed4d internal/lsp/regtest: only print RPC logs if asked
Sometimes one wants to run the regtests with verbose output without
seeing the wall of RPC logs. Add a new flag to control the printing of
logs, rather than overloading testing.Verbose().

Change-Id: Iea5727e3079005d229319cbf86f67067f7c1f85d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405894
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-05-17 21:46:20 +00:00
Marwan Sulaiman 13bcb69cfa internal/lsp/analysis/stubmethods: recognize *ast.CallExpr
This change makes it so that the stub methods analysis can recognize
errors happening to method and function call expressions that are being
passed a concrete type to an interface parameter. This way, a method stub CodeAction will appear at the call site.

Updates golang/go#37537

Change-Id: I886d53f06a85b9e5160d882aa742bb2b7fcea139
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404655
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
2022-05-17 17:04:34 +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 ed968f66bd internal/lsp/bug: add a package for bug reporting
The existing debug.Bug mechanism for reporting internal bugs is
insufficient for several reasons:
 - It requires a context, which is not always available.
 - By being defined in the debug package, it is subject to import
   cycles.
 - It is too complicated. Listening for bugs requires understanding the
   event package.

Replace this with a simpler 'bug' package with no dependencies, that
allows reporting, listing, and listening on internal bugs. Hopefully
this will fulfill the goal of debug.Bug, to help us track down rare
bugs.

Change-Id: I30cab58429b29bd2d944d62e94f5657e40a760fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399623
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
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-16 14:45:30 +00:00
Robert Findley 090b14e850 internal/lsp/regtest: make TestResolveImportCycle robust to error order
In JSON output, the Go command was only setting the Error field for one
package involved in an import cycle. As a result, TestResolveImportCycle
was dependent on the chosen package, causing flakes when the chosen
package is not deterministic.

Arguably this behavior should be fixed, both in the go command and in
gopls, but for now make the test resilient to choice by asserting on any
of the possible errors.

Add a new AnyOf expectation to support this type of assertion and tweak the
test output formatting. Also update the test to eagerly fail once the
didOpen notification has been fully processed, so that we don't have to
wait for the assertion timeout.

Updates golang/go#52904

Change-Id: Ic209d8fdcb7308c041b287a8f122c47e96d29a96
Reviewed-on: https://go-review.googlesource.com/c/tools/+/406274
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-13 22:16:40 +00:00
Hana f918e87c18 gopls/internal/vulncheck: copy logic of govulncheck -html
reference commit: b2400d8
The latest relevant change in the code copied is CL 403075.

Change-Id: If50cb4e0096e4f33876236cf8620430e1bcfcd86
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405795
Reviewed-by: Jamal Carvalho <jamal@golang.org>
2022-05-12 21:17:31 +00:00
Hana ad497c6c68 internal/lsp/cmd: add -config option to gopls vulncheck
If -config=true, gopls vulncheck reads the package load
configuration JSON (build flags, env, tests, and later
maybe overlay info) from stdin.

And, add some logging to gopls/internal/vulncheck/command.go
that helps measuring the package loading overhead.

Update golang/go#50577

Change-Id: I5c1ce145b07f2bed03911613f42c09a3d6be6c28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404575
Reviewed-by: Robert Findley <rfindley@google.com>
2022-05-12 21:16:26 +00:00
Nooras Saba 6eb3de266b internal/lsp/analysis: fix a doc comment typo
Fixes the grammar

Updates #50262

Change-Id: I8bc524b26d6cbeda3e626d0e83601529cbf7ff6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405540
Reviewed-by: Robert Findley <rfindley@google.com>
2022-05-11 23:42:05 +00:00
Robert Findley 728485ffcb gopls/internal/regtest: add a test for using staticcheck with generics
For golang/go#52159

Change-Id: I08120331b7f5c9eb06feac0d0eeb76a9a7b629df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399914
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Le <dungtuanle@google.com>
2022-05-11 20:19:40 +00:00
Alan Donovan 93852cb65b internal/lsp: fix source.CompareDiagnostic asymmetry
In passing I noticed that this three-way comparison
is not (anti)symmetric. Such comparisons should consist
of a list of pairs of tests of this form:

  if x.key1 < y.key1 { return -1 }
  if x.key1 > y.key1 { return +1 }
  ...key2, etc...
  return 0

Also in passing:
- simplify panic-proof debug string function.
- augment doc comment of (*Server).beginFileRequest

Change-Id: Idcd0844ea4e96fc2dee5bbc270f5a200b5c27aa0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405480
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-05-11 17:43:03 +00:00
Nooras Saba 28c754d415 internal/lsp/analysis: analyzer for //go:embed directive
This CL adds a new analyzer for //go:embed directive, which
checks for the "embed" import.

Along with it, it improves doc for analysistest.Run for
comments of the the form  "//...// want..." or
 "//...// want..."

 Updates #50262

Change-Id: I60ef0ab740feadd4fff3a4d758123b27ceda0bc6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400854
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Nooras Saba‎ <saba@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-11 14:31:28 +00:00
Hana (Hyang-Ah) Kim 033cbfc76d internal/typeparams: run go generate with go1.18.2
And fix a minor issue in copytermlist.go --

copytermlist.go replaces type names from go/types with
qualified type names. Use of token.NoPos (filled with
ast.NewIdent) however confuses the go/format printer.
As a result, while transforming

  func (x *term) includes(t Type) bool

to

  func (x *term) includes(t types.Type) bool

go/format printer failed to compute the end position of
the parameter list, concluded RPAREN should be in a
different line from the parameter list, added a comma,
and printed

  func (x *term) includes(t types.Type,) bool

Reuse the replaced node's position instead. (not 100%
correct, but better than NoPos)

Change-Id: Ia34e11562cc80c68dcf4b921ffffd926971c2215
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405536
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-05-11 13:38:50 +00:00
Robert Findley bc0e26ea12 internal/typeparams: remove examples in favor of x/exp/typeparams
The internal/typeparams directory contains an initial draft of the
documentation later published under the golang.org/x/exp/typeparams
module. Remove this draft to avoid confusion.

Change-Id: I1f40468f99c0d2885e8fc380c75669f654ba971e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405260
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-10 12:58:44 +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 cde25b3a5f internal/lsp/lsppos: add helpers for mapping token positions
For use-cases that work only with token.Pos and protocol.Position, the
span package is unnecessarily indirect, and inefficient. It also loses
information about newline termination, and handles positions within CRLF
line endings incorrectly.

The lsppos package was written to bypass this complexity, but had
limited use and lacked tests.

Add tests, and an wrapper API that operates on token.Pos. Also fix
source.TestTokenOffset to not panic, and add a temporary exemption of
the new token.Offset usage.

This change also fixes position calculation in the case of empty file
content. The mapper now finds position (0, 0) at offset 0 of an empty
file.

Change-Id: I639bd3fac78a127b1c8eddad60b890449901c68c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403678
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-05-09 17:02:03 +00:00
Hana 22b709631a internal/lsp/cmd: change vulncheck to directly call the hook
Instead of invoking the command through the LSP custom command,
call the vulncheck command hook directly. That reduces the extra
overhead of bringing up the full gopls server & package loading.
The vulncheck hook loads packages again any way, so the benefit
of running the check inside gopls's custom command framework
is not huge any more.

Still `gopls vulncheck` is useful - editors don't need to install
another binary for vulncheck feature, and it will output the
result in the format easier to handle than what `govulncheck`
currently offers.

Updates golang/go#50577

Change-Id: Ia21e6d7e0c37c4a1b02dc8bbca860143524c3d1b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404574
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-05-06 16:33:28 +00:00
Robert Findley d7e01c038e internal/lsp/source/completion: use typeutil.Map for short-circuiting
While working on golang/go#52715, I discovered an infinite recursion in
gopls' completion logic: eachField assumes a finiteness of type pointers.

It is almost certainly a go/types bug that type-checked types expand
infinitely, but nevertheless we should use the more accurate
typeutil.Map for short-circuiting our search.

Change-Id: Ib1c7125e624f42882869acd4e0476e317d4da056
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404335
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
2022-05-06 16:24:14 +00:00
Suzy Mueller 30fbd19e84 internal/lsp: fix fillstruct for structs with unsafe.Pointer
Provide a default value for unsafe.Pointer in fillstruct.

Fixes golang/go#52640

Change-Id: I10a1878fbf53b082f83f44e0ba2788ead14439d6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403535
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: David Chase <drchase@google.com>
2022-05-05 17:08:12 +00:00
Alan Donovan 0ebacc111e internal: remove pre-go1.12 conditionally compiled files
Change-Id: I496ad24654990341eae9508587e950abde05b540
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404334
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
2022-05-05 16:48:28 +00:00
Alan Donovan 45c8a71312 internal/tool: implement structured help command
'gopls help cmd...' is redirected to 'gopls cmd... -h'.
The tool.Run operation for each subcommand already has
logic to show the usage message when it parses a -h flag.

Examples:
   $ gopls help
   $ gopls help remote
   $ gopls help remote sessions

Fixes golang/go#52598

Change-Id: I5135c6e7b130d839f880d0613534ac08de199bcf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403677
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: 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-05-04 22:58:41 +00:00
Alan Donovan 4911e4af7d internal/testenv: remove darwin/arm case from NeedsGoBuild
This function was derived from internal/testenv.HasGoBuild
in the standard library, which has since been updated
(in https://go-review.googlesource.com/c/go/+/260719)
to permit darwin/arm64 builds.

Change-Id: Ifc793b52b65ee3eedb2f41fbcbfbe36a20cfdbcc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403795
Reviewed-by: Robert Findley <rfindley@google.com>
2022-05-04 20:40:54 +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 4a3fc2182a internal/lsp: only linkify urls with http, https, and ftp schemes
Fixes golang/go#43990

Change-Id: I0ea26d521b2432238b05c26bfaccef6fc773dcf2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/393854
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-05-02 22:00:12 +00:00
Robert Findley aaffface24 internal/lsp/source: avoid panic in HoverIdentifier
Fixes golang/go#52211

Change-Id: I2ab5dbba4bb06092a234269d1ee65493f60fc6fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399622
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-04-29 18:06:40 +00:00
Muir Manders b44fad8412 lsp/completion: fix func literals with type params
Give placeholders for type params in func literal completions. For
example:

    func foo[T any](func(T) T) {}
    foo(<>)

Will now give "func(<T>) <T> {}" where <> denotes a placeholder.

Change-Id: Iadde73ed6b88e1410c28dfa33a20ab6a51235c93
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400616
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2022-04-26 14:19:52 +00:00
Muir Manders 5bb9a5ecb1 lsp/completion: fix literal completions with type params
In cases like:

    type foo[T any] struct{}
    func bar[T any](foo[T]) {}

    bar[int](<A>)
    bar(<B>)

At <A> we will now offer "foo[int]{}". At <B> we will now offer a
snippet "foo[<T>]{}" which lets the user fill in the type arg.

Note that we have no knowledge of type inference, so you can be left
with superfluous type args after completion.

Change-Id: Ia7d63284f3317d9367864fdae3e3f9ae68fdff1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400615
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2022-04-26 02:30:39 +00:00
Muir Manders 0941294088 lsp/completion: further improve generic func arg ranking
In cases like:

    func foo[T int | string](T) {}
    foo[int](<>)

Previously at <> we would favor int and string candidates. This is
because go/types doesn't instantiate foo in this case (for some
reason). Work around the issue by using types.CheckExpr to re-check
the *ast.CallExpr.Fun. CheckExpr seems to do a better than a full type
check in the face of errors.

Updates golang/go#52291
Updates golang/go#52503

Change-Id: Ide436428f3232db2e06ea3cc22ea250edbf28685
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400614
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: 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>
2022-04-26 02:29:50 +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
Dylan Le 559469ae16 internal/lsp: render package documentation when hovering over a package import
Update FindHoverContext to retrieve & render package documentation for a hovered package. Add a regtest for this hovering feature.

Updates golang/go#51848

Change-Id: If57396d59be9c4cf7e09b64e39832de6f996c7ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400820
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-25 17:29:41 +00:00
Muir Manders fa7afc95f2 lsp/completion: improve generic func arg ranking
In cases like:

    func foo[A int|string](a A) {}

    foo[_](<>)

We now prefer ints and strings at <> by matching against the type
constraint. Note that even if "_" is replaced with "int", we still
prefer strings since the type checker doesn't seem to want to
instantiate foo unless the params check out.

Change-Id: I0e7acfef0775752a96fcfe23e7e2e3d939820eee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/394017
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Auto-Submit: Peter Weinberger <pjw@google.com>
2022-04-22 20:11:24 +00:00