1
0
mirror of https://github.com/golang/go synced 2024-11-19 04:14:45 -07:00
Commit Graph

1167 Commits

Author SHA1 Message Date
Rebecca Stambler
1b668f2091 internal/lsp: disable literal completion candidates for some clients
If a client doesn't support the snippet format in completion insert
text, they can't take full advantage of the literal completion
candidates. Disable it in those cases, and remove the setting in
internal/lsp/source/options.go.

Fixes golang/go#36655.

Change-Id: Ibc045a0f2945aab753b0187194a03d0c0398dba5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-24 20:07:20 +00:00
Peter Weinberger
3f4d10fc73 internal/lsp: small change to helper.go to use ast.IsExported
Use ast.IsExported instead of explicit code.

Change-Id: I1bfabb3c575debb699a51e38f502fcb99a081203
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216297
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-24 17:05:13 +00:00
Rebecca Stambler
e0a7ba33ca internal/lsp/cache: move mod-related functions and file
This is a pure move with no code changes.
Rename parse_mod.go to mod_tidy.go since it's changed a bit.
Move the modfile.go functions into view.go since (1) Heschi doesn't
like spreading methods into multiple files and (2) it only has 2
functions in it anyway.

Change-Id: If0d7e4b50ac22c57302d90d68c2181dbb3ca8b87
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216142
Reviewed-by: Rohan Challa <rohan@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-24 14:45:41 +00:00
Peter Weinberger
9778d966fa internal/lsp: generate boilerplate stubs for type Server
go generate server.go generates server_gen.go which contains 44 boilerplate
stubs implementing the LSP protocol. The Server methods are defined
in protocol/tsserver.go and implemented in the lsp package. server_gen.go
ties the two together.

The generator is in helper/helper.go, and described in helper/README.md.

Change-Id: I120bc658e55a91ec5b07c0c0cf8c188882f0be66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215979
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-24 14:41:51 +00:00
Rebecca Stambler
5c352bb417 internal/lsp: stop returning errors when we can't find a snapshot
Change-Id: I319021a3339971809b6096b3271aa6b0d94ae336
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216139
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-24 02:10:10 +00:00
Rebecca Stambler
a4b4a6733a internal/lsp/cache: check go.mod even if tempModFile is false
Missed a pretty key case in CL 216138. Also, I accidentally made it pass
-modfile even when it's not supported.

Change-Id: Ia0d04be7e82b77e1ec3f57ee2fee04e8c14a7c90
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216140
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-24 01:37:08 +00:00
Heschi Kreinick
5ecc1643ff internal/lsp/cache: fix GOPATH vendoring
We treat package IDs and import paths as semi-interchangeable, which is
wrong when GOPATH vendoring is in use. The only place it hurts us is
during import resolution, which is fixed here. We should always have the
package loaded, so it's just a matter of finding it by searching each
possible vendor location.

Fixes golang/go#36155.

Change-Id: If789092d16fa3d3294b6d8a2bcb980264506c161
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215904
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-24 00:03:48 +00:00
Rebecca Stambler
24841a4f5f internal/lsp: eliminate redundant view.ModFile function
view.ModFiles used to not return the real mod file, even if one existed.
Now, we construct view.moduleInformation even if -modfile isn't
supported.

Change-Id: I03faf2ea521c2f404d4e1ba47f71ae48f3cb08d9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-01-23 22:07:07 +00:00
Rebecca Stambler
59ae353e8e internal/lsp: recreate the view when needed
If the initial workspace load fails (due to a lack of a go.mod file or
an invalid go.mod file), we should try to re-load as changes to the
go.mod come in. Rather than retrying within the view, we just drop the
view entirely and try to recreate it. This shouldn't lead to any
noticeable lag, as anything that has been cached can still be reused.

Fixes golang/go#36531

Change-Id: I6e157075e8b3665f0ceef35e051e56ac3c29f286
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216037
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 21:53:29 +00:00
Rebecca Stambler
e873952e15 internal/lsp: check that a file handle is unmodified before read
Address a lingering TODO. A FileHandle read should return errors if the
file has been modified on disk while in use.

Change-Id: I540de9bef575a9ca838f49500665a24b05b4f54c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215981
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 20:52:07 +00:00
Rebecca Stambler
219d3418f5 internal/lsp: batch file changes in didChangeWatchedFiles
Remove the special handling for go.mod file saves. This was only really
added to be extra careful, but our cancellation logic should cope with
this.

Fixes golang/go#31553

Change-Id: I0a69bcdeaf6369697e79aba4689a7b714484ccc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215908
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 20:51:52 +00:00
Rebecca Stambler
e54d0edf47 internal/lsp: support batched on-disk changes in source.DidModifyFiles
We don't yet propagate these batched changes in text_synchronization.go,
but this is the next step in moving towards a batched approach.

Updates golang/go#31553

Change-Id: Id6496af9d5422cc50ccb995f81c71ec1886f965a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215907
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 20:28:28 +00:00
Rebecca Stambler
a356fb7f16 internal/lsp: support multiple URIs in (*view).invalidateContent
Caught a number of unused parameters along the way. Hopefully we can
eliminate the containsFileSave boolean soon, since it's a bit annoying
to have to send that through.

Updates golang/go#31553.

Change-Id: I94319d902d329c84cb1c0676322ac04541ad53a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215906
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 20:19:14 +00:00
Rohan Challa
0043dadf92 internal/lsp: use x/mod to get edits for go.mod quick fixes
This change uses the wonderful functions from x/mod to get the proper edits for the quick fixes on a go.mod diagnostic. It also creates a goModData structure to hold the data thats gets passed into the various parse functions, this will help reduce the large function prototypes that can occur when we decompose the logic. It also refactors the Modfiles() function to return span.URIs vs FileHandles.

Change-Id: Ifa0896442650f2ddbd8fe98d8f231a9e94c3d042
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215097
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-23 20:18:34 +00:00
Rohan Challa
2b7b26d7b0 internal/imports: add buildflags to ProcessEnv
This change adds a buildflags variable to the ProcessEnv inside internal/imports. When you run go list with GO111MODULE=on to get information about the package you are in, it will add a go directive to your go.mod file if there is not one. With the tempModfile=on flag, there should be no changes to a user's go.mod file.

Updates golang/go#36247

Change-Id: I817e4c46b4f433d0665fcb7585fcdf4f87049a38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215978
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 19:46:59 +00:00
Rebecca Stambler
593de60622 go/packages: handle an overlay edge case with test variants
As usual, in debugging the creation of a new file with gopls, I've
encountered a go/packages overlay bug. The issue is:

A file b/b.go with package name "b" exists on disk. A package
b/b_test.go with no content exists on disk. There is an overlay for
b/b_test.go that contains the package name "b". Running packages.Load
for file=b/b_test.go will result in a failure to load package b
[b.test]. This change adds this test to the go/packages tests.

This case is fixed by restricting the fallback logic in
runContainsQueries. We only attempt to construct an ad-hoc package if
the original package was returned with no GoFiles.

Also, a minor change to the gopls error parsing code that fixes a case
in which diagnostics were being sent without corresponding files.

Updates golang/go#36635.

Change-Id: I38680a2cc65ae9c3252294db6b942d031189faf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215743
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-01-23 02:22:18 +00:00
Rebecca Stambler
ba161d9e22 internal/lsp: add tests for references includeDeclaration setting
Make sure to test both modes, as this is the second time we've
accidentally broken this.

Fixes golang/go#36598.

Change-Id: I3993af3d106b18c76c44ada558b2c6cd9cbfcf17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 01:39:50 +00:00
Rebecca Stambler
18389cb1f4 internal/lsp: refactor (*snapshot).clone to handle multiple URIs
This is the first in a series of changes to batch file changes. First,
we have to support invalidating a snaphot with multiple files.

Updates golang/go#31553

Change-Id: I7cd83d9280e3274549a72393bb9010c47eb5dd1e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215902
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 01:31:03 +00:00
Rob Findley
fb353ca402 internal/lsp/cmd: add a test for client logging
A new test is added to verify that contextual logs are reflected back to
the LSP client. In the future when we are considering servers with
multiple clients, this test will be used to verify that client log
exporting is scoped to the specific client session.

Updates golang/go#34111.

Change-Id: I29044e5355e25b81a759d064929520345230ea82
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215739
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-22 21:55:43 +00:00
Muir Manders
9bae6688e3 internal/lsp/source: support dereferencing for completion
In cases like:

    var j *int
    var i int = <>

We will now provide "*j" as a completion candidate.

Change-Id: I1d35c2dca4864f13f7534e15b17450d784985557
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215358
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-22 21:43:17 +00:00
Rob Findley
78d067421b internal/lsp: remove the Context argument from NewSession
The passed-in Context is not used, and creates the illusion of a startup
dependency problem: existing code is careful to pass in the context
containing the correct Client instance.

This allows passing in a source.Session, rather than a source.Cache,
into lsp server constructors.

Updates golang/go#34111

Change-Id: I081ad6fa800b846b63e04d7164577e3a32966704
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215740
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-01-21 23:07:03 +00:00
Rob Findley
29f64efd55 internal/telemetry/log: correct the docstring for Error
Change-Id: I8077bbdd6f011087f7988fbf04c97c13575afec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215741
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-21 22:25:47 +00:00
Heschi Kreinick
b3205ff6ff internal/lsp/protocol: handle cancellation delivery
$/cancelRequest is handled out of band in the Request flow, but was
allowed to continue down the Deliver chain. There's no sense in letting
downstream Handlers see it and try to reply like it was a normal
request.

Fixes golang/go#36662.

Change-Id: I8471ac0fdd4f2a08acd87d6e4b83c1f077eb8600
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215738
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 21:04:57 +00:00
Rebecca Stambler
fc669b13a2 internal/lsp: eliminate getFileLocked function
Apparently, the convention is to name something "Locked" if the
implementation is mutex is already locked when the function is called.

Turns out there were no callers of getFileLocked anyway, so just delete
that function and rename to getFile.

Change-Id: Ia7a9a620c3eb8eb0ce6b0a44ddb2ed62a604484d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215737
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 20:56:39 +00:00
Muir Manders
3e306b7b28 internal/lsp/source: rename "typeInference" to "candidateInference"
This is in preparation for inferring stuff beyond just the expected
candidate type.

Change-Id: I31be9c1e4c82d82b1ff848858042a5edf46594e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215340
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-21 20:43:48 +00:00
Heschi Kreinick
d456b1cd8c internal/imports: pass dummy source for completion functions
The goimports code tries to read source from disk if none is supplied,
but the completion functions don't need source. Pass a dummy slice.

Fixes golang/go#36671.

Change-Id: Ieecc9ee75d39f8f165c7b9764ff49c8334a7fcd2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215681
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 20:03:11 +00:00
Rebecca Stambler
62545537a9 internal/lsp: use correct file identities when computing diagnostics
CL 214586 switched to using URIs in error messages instead of file
identities. It wasn't fully correct because the reports were being
allocated for the file identities in the package handle (which may be
outdated).

Fixes golang/go#36601

Change-Id: I4fcfc02df74d94fff49540c784ef816c357e7232
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215680
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-21 20:00:43 +00:00
Rohan Challa
bd79bb77d6 internal/lsp/cache: create infra for caching go.mod diagnostics
This change creates the infrastructure for caching diagnostics. Right now it uses a naive key of the entire snapshot which does not really add any benefit from using the cache. It also moves some code from inside the internal/lsp/mod to internal/lsp/cache.

Updates golang/go#31999

Change-Id: I599ad281baa062448d33ca0ea88ae6e09e10d73e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215021
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-21 19:43:28 +00:00
Heschi Kreinick
9375b12bd8 internal/lsp/cache: fix mod file change check
The modfiles function only works with Go 1.14. When it is enabled,
it reenters the view, causing a deadlock. Stop using it, and move the
process env under a separate lock so to break the deadlock.

Change-Id: I34c528c2be1f32c06b423ead44e90155f60c2214
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215679
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 19:24:08 +00:00
Rob Findley
13c74800b9 internal/lsp/protocol: remove unused DocumentUri type
Change-Id: I8fbc8cbbb4e62c1eca4de767d054f5d098f27edd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215497
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-21 14:52:36 +00:00
Iskander Sharipov
dbc83e6dc0 internal/lsp/source: fix typeIsValid() inf recursion
typeIsValid() intended to stop on a named type, but
since we called Underlying(), switch case never caught any
named type. To avoid that, do an early check.

Fixes golang/go#36637

Change-Id: I2700afbb8f9678b4542e2e7dccc3be59b1d9ebdf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215238
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-21 04:27:40 +00:00
Heschi Kreinick
0cba7a3a9e internal/lsp/source: enable unimported completions by default
Fixes golang/go#31906.

Change-Id: I626ff1fe94a171d2280d9deeb4578b5abc759913
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214947
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 22:05:05 +00:00
Heschi Kreinick
fe56e63357 internal/lsp/source: fix ranking of untyped completions
Claiming that untyped candidates matched the type of whatever we were
looking for messed up rankings in found(). The only other places that
use it will all work better with false. Return false.

Updates golang/go#36591.

Change-Id: I5e1e8af7cc5c27422740cbb77f9a4a20edb1e447
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215322
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 21:50:04 +00:00
Rebecca Stambler
1486df0b25 internal/lsp: do not invoke the Go command when checking common errors
There's no need to do this more than once per view.

Change-Id: I3160adc602764204155dd0e506fd554aeb55d639
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 21:23:25 +00:00
Heschi Kreinick
354bea8ca8 internal/lsp/cache: let gopls track go.mod files
We used to read the go.mod file information out of the imports.Resolver.
Now that gopls tracks go.mod itself, we can use that instead. This is a
slight regression, in that go.mods in replace targets will no longer be
watched, but I don't think that's too important.

This allows us to stop reading the ModuleResolver's internals, which
were not sufficiently locked.

Updates golang/go#36605.

Change-Id: I42939e0248cba1f6b3850a003de67fcc11ab10b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215319
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 21:20:11 +00:00
Rebecca Stambler
633b092c1e internal/lsp: add recency check to avoid sending old diagnostics
This change uses snapshot IDs instead of file versions to determine if
diagnostics are stale.

Fixes golang/go#36601.

Change-Id: I7727a30222fecf2d7733fa013c6cee6338ffd968
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215298
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 20:46:10 +00:00
Rebecca Stambler
351fb220e1 internal/lsp: invalidate directories if we have no direct IDs
Previously, we were only invalidating packages in directories if we
didn't have a file handle. We should also invalidate if we have an
unparseable file handle - that is, a file with no content or no package
name.

Fixes golang/go#36608.

Change-Id: Ia12fad962c06ddeeac382185d3220961f5c584e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215318
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 20:35:50 +00:00
Heschi Kreinick
95ece921ff internal/lsp/source: trim file very carefully
Apparently the AST will sometimes give us offsets past the end of the
file. Don't crash when it does.

Fixes golang/go#36610.

Change-Id: I3cfbf8645bfcea94a5d87bca5bef4236d657b2c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215119
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 20:34:13 +00:00
Muir Manders
a20955124b internal/lsp: consolidate completion sorting
Move completion candidate sorting into internal/lsp/source so
source_test.go and internal/lsp don't have to duplicate the logic.

Change-Id: Ifbe7ca5ad6a5b74020fd1260b4d4f775709968cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 20:30:43 +00:00
Rebecca Stambler
7ad9cd8f31 internal/lsp/cache: invalidate metadata for x_tests and test variants
Invalidating metadata for reverse dependencies isn't necessary, except
for in the case of test variants and x_tests.

An example:

The only way to reload the metadata for
"golang.org/x/tools/internal/lsp/cache
[golang.org/x/tools/internal/lsp/source.test]" is by reloading
"golang.org/x/tools/internal/lsp/source" with "[-test -deps]". That
means we have to invalidate the metadata for that
"golang.org/x/tools/internal/lsp/source_test" when we invalidating
"golang.org/x/tools/internal/lsp/cache".

Fixes golang/go#36165

Change-Id: Iff0e03a7a46158fbdafaffa091f90ca434700a97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-17 17:36:07 +00:00
Rohan Challa
ade7f2547e internal/lsp: add highlighting for import statement
This change adds highlights for imports when the cursor is over the use of that import. It also adds it for the opposite direction when the cursor is on the import, it will highlight uses of that import.

Fixes golang/go#36590

Change-Id: Ifd04d81ec9b4fdf2be1b763f31b44d0ef7d92f47
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215258
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 17:07:20 +00:00
Heschi Kreinick
6edc0a871e internal/lsp/source: score in-memory unimported candidates
We were assuming that all in-memory packages were equally useful. That's
not true for projects with a large dependency tree. Call into the
imports code to score them.

While I'm here, score the main module above direct deps.

Updates golang/go#36591.

Change-Id: I07c56dd3ff7338e76f3643e18d35abc1b52d6763
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215023
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 01:23:04 +00:00
Michael Matloob
84cebe1034 go/packages: fix incorrect needtypes and needsrcs logic
This logic is really fiddly and I'm not really 100% sure it's right, though
I've thought about it for quite abit (and added comments to help me reason
through it).

Also always request CompiledGoFiles when NeedTypes is true because we might
need to fall back to loading from source when type data is incorrect.

Fixes golang/go#36441
Fixes golang/go#36547

Change-Id: I1cc27ca2e4401a9abc8502990b0da7d0480f6f84
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214943
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 22:59:55 +00:00
Rebecca Stambler
28ed04f882 go/packages: internally expose ForTests in go/packages
This change eliminates our need to guess what the package under test is
in gopls, since `go list` always knows the answer.

Change-Id: I16e482ed3b452bd57cd478b1f8280fcea56474d3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215020
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 22:11:50 +00:00
Heschi Kreinick
1c4842a210 internal/lsp/cache: refresh imports cache in the background
Now that we can detach scans, it's easy to kick off a background refresh
that doesn't block the user. Performance may be a bit worse until the
scan finishes, but that's the price we pay for freshness.

Adaptively rate-limit the refresh rate so that we don't turn the user's
computer into a hot plate.

Change-Id: Icbe8f384f44a219b57465da22d9becc19001eab8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212022
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 20:36:08 +00:00
Rebecca Stambler
872a348c38 internal/lsp: don't log context.Cancelation in diagnostics
Fixes golang/go#34103.

Change-Id: I76e713d54422d31b587cedd0e4359c8b3bc7845e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215019
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 18:16:51 +00:00
Muir Manders
0508ad4c83 internal/lsp/source: return obj decl first in find-references
Restore previous behavior where the object's declaration is returned
as the first reference in find-reference calls.

Fixes golang/go#36598.

Change-Id: Ibdeaf9971aa5cb1f3244f6a888fe77bdf386e563
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215057
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-16 16:57:51 +00:00
Muir Manders
473961ec04 internal/lsp/source: improve completion for "range" and "<-"
Now that we understand object "kind" for builtin generic functions, we
can apply it to a couple more places as well:

// prefer rangeable object kinds
for i := range <> {
}

// prefer channels
<- <>

Change-Id: If9cfba3a06b3abde073a9d397000bb3f3b0e9853
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214678
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 06:24:25 +00:00
Muir Manders
f80fb1dfa1 internal/lsp: refactor find-references and rename
The main goal is to push the package variant logic from internal/lsp
into internal/lsp/source so all users of internal/lsp/source benefit.

"references" and "rename" now have top-level source.References() and
source.Rename() entry points (as opposed to hanging off
source.Identifier()). I expanded objectsAtProtocolPos() to know about
implicit objects (type switch and import spec), and to
handle *ast.ImportSpec generically. This gets rid of special case
handling of *types.PkgName in various places.

The biggest practical benefit, though, is that "references" no longer
needs to compute the objectpath for every types.Object comparison it
does, instead using direct types.Object equality. This speeds up
"references" and "rename" a lot.

Two other notable improvements that fell out of not using
source.Identifier()'s logic:

- Finding references on an embedded field now shows references to the
  field, not the type being embedded.
- Finding references on an imported object now works
  correctly (previously it searched the importing package's dependents
  rather than the imported package's dependents).

Finally, I refactored findIdentifier() to use pathEnclosingObjNode()
instead of astutil.PathEnclosingInterval. Now we only need a single
call to get the path because pathEnclosingObjNode() has the
"try pos || try pos-1" logic built in.

Change-Id: I667be9bed6ad83912404b90257c5c1485b3a7025
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211999
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 06:24:15 +00:00
Heschi Kreinick
7042ee646e internal/lsp/source: always look up mapper when building ranges
Any file could have //line directives in it, which means that we should
never trust a mapper that was looked up for a whole file. Remove the
range conversion helpers that accepted a mapper and look it up on the
spot.

Change-Id: Ic518891fcc1a682b31cbc6d1d4e1e1af1ef21962
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214949
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 01:10:02 +00:00
Muir Manders
49797d04a8 internal/lsp/source: improve completion in type assertions
In cases like:

var foo *someType = bar.(some<>)

We will now complete "some" to "*someType". This involved two changes:

1. Properly detect expected type as *someType in above example. To do
   this I just removed *ast.TypeAssertExpr from
   breaksExpectedTypeInference() so we continue searching up the AST for
   the expected type.

2. If the given type name T doesn't match, also try *T. If *T does
   match, we mark the candidate as "makePointer=true" so we know to
   prepend the "*" when formatting the candidate.

Change-Id: I05859c68082a798141755b614673a1483d864e3e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212717
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 01:06:23 +00:00
Rebecca Stambler
96555e0fa5 internal/lsp/cache: initialize view before LookupBuiltin
This will crash otherwise.

Change-Id: I4fbce813283291792ed21fa5d83186ec59543ff1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214948
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 00:42:58 +00:00
Rebecca Stambler
a7dab0268b internal/lsp: diagnose the snapshot on every text synchronization event
This change moves to our ultimate approach of diagnostics the snapshot
on every file change, instead of carefully picking which files and
packages to diagnose. Analyses are shown for packages whose files are
open in the editor. Reverse dependencies are no longer needed for
source.Diagnostics because they will be invalidated when the snapshot is
cloned, so diagnosing the entire snapshot will bring them up to date.

This even works for go.mod files because all of workspace-level `go list`s
will be canceled as the user types, and then we trigger an uncancellable
go/packages.Load when the user saves. There is still room for improvement
here, but it will require much more careful invalidation of metadata for
go.mod files.

Change-Id: Id068505634b5e701c6f861a61b09a4c6704c565f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 23:07:48 +00:00
Rebecca Stambler
97cd989a76 internal/lsp: remove boolean for publishEmpty in diagnostics
This parameter was added to avoid sending a slew of empty diagnostics
for the initial workspace load. It's actually not needed anymore, as we
can just detect if we have previously sent diagnostics for the given
file, and if not we shouldn't send an empty diagnostic. This is true for
all cases, not just the initial workspace load.

Change-Id: I34a323bc0c0df79edd39630cd25577238b256266
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214287
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 22:25:09 +00:00
Rebecca Stambler
f04f2c82d0 internal/lsp/cache: refactor initialization for builtins
This change combines the two packages.Load calls that happen on view
creation. Builtins can be loaded along with the rest of the workspace.

To avoid race conditions, create a builtinPackageHandle type for
builtins and use it to create the data.

Updates golang/go#36531

Change-Id: I7aa342c463a0b7718e1ad5fee507622310d8443b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214877
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 21:04:26 +00:00
Rebecca Stambler
497c7f156c internal/lsp/cache: construct package handles as part of IWL
Package handles should be cached on the snapshot as part of the initial
workspace load, otherwise this cached data will be repeatedly lost and
reconstructed during tests and regular execution.

Fixes golang/go#36556

Change-Id: If7676685db17519c998b857a812467c7f3cc6003
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214799
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-15 19:55:33 +00:00
Heschi Kreinick
3ded1b734d internal/lsp: add and use nonstandard gopls/diagnoseFiles
Switch the command line client, and its tests, away from the hardcoded
30-second timeout and to a newly-added custom request.

Inconveniently for us, the jsonrpc2 package only serializes requests,
not replies. (Notifications are requests for this purpose.) So, for a
flow like this:

diagnoseFiles -->
  <-- publishDiagnostics
  <-- publishDiagnostics
diagnoseFiles <-- (reply)

...there's actually no guarantee that the incoming requests will be
processed before the reply comes in -- it gets to jump the
serialization. The only way to guarantee previous notifications have
been processed is to send another request. I didn't feel like adding
nonstandard notification support, so I just send a fake diagnostic.

Error handling for untyped JSON is hideous so for now we just panic.
Nobody else should be calling these, or if they do it's at their own
risk.

Fixes golang/go#36518.

Change-Id: I63f8df5bb627c9783314a0d38d2dac8721b3c320
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214583
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 19:23:06 +00:00
Rohan Challa
de0b176007 internal/lsp: show dependency quick fixes for go.mod diagnostics
This change adds quickfixes for unused dependencies and dependencies that should not be marked as indirect. It also updates the positions for the diagnostics to make more sense relative to the warning message. There is a testing harness now for suggested fixes.

Updates golang/go#31999

Change-Id: I0db3382bf892fcc2d9ab3b633020d9167a0ad09b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213917
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 16:51:05 +00:00
Rob Findley
98c82cf1f4 internal/lsp: add server instance to debug info
When debugging multiple instances of gopls simultaneously, it is useful
to be able to inspect stateful debugging information for each server
instance, such as the location of logfiles and server startup
information.

This CL adds an additional section to the /info http handler, that
formats additional information related to the gopls instance handling
the request.

Updates golang/go#34111

Change-Id: I6cb8073800ce52b0645f1898461a19e1ac980d2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214803
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-15 14:58:21 +00:00
Rob Findley
7fd989f60c internal/lsp/tests: correct typo 'CompletionCaseSensitve'
CompletionCaseSensitve is renamed to CompletionCaseSensitive.

Change-Id: I060d02cd72d271d0a1f5595ca35c55e7e142c996
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214802
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-15 14:33:14 +00:00
Rob Findley
df87866820 internal/lsp,internal/telemetry: correct stale docstrings
Several docstrings reference earlier names for the symbols they
document. This CL corrects those that I noticed while reading the lsp
code.

Change-Id: I1968459feff7011e070333c99eb149e72d3302de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214801
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 14:25:42 +00:00
Rebecca Stambler
831fdb1e18 internal/lsp: push initialization tasks into one function
This change moves as much view initialization code into the
initialization function, instead of having it happen on view create.
Also, the `go env` variables that are collected at inconsistent times
are all collected on view creation. That is sufficient, since the view
is recreated if the environment changes.

I had originally hoped that the initial call to `go env` and the
-modfile detection could become part of this parallel initialization as
well, but you can't create a *packages.Config until the temporary
modfile has been set up, so it still makes sense to do that on view
create. This is, however, the reasoning behind the refactorings in
the -modfile detection in this CL. The main changes are a few renamings
and a split between snapshot.ModFiles and view.modFiles to maximize the
amount of work done in the view. I changed view.modfiles to moduleInformation
because I thought we might want to store additional information there at some
point. Rohan, please let me know if you disagree with any of the changes I made,
and I can revert them.

Fixes golang/go#36487

Change-Id: I504db5a4f41b79bee99ebd391e32e7b520a19569
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-01-15 04:46:56 +00:00
Rob Findley
7ae403b6b5 internal/lsp: finish renaming CheckPackageHandle to PackageHandle
In golang.org/cl/209419, CheckPackageHandle was renamed to
PackageHandle, but a number of references to CheckPackageHandle remained
in function names and comments.

This CL cleans up most of these, though there was at least one case
(internal/lsp/cache.checkPackageKey) where the obvious renaming
conflicted with another function, so I skipped it.

Change-Id: I517324279ff05bd5b1cab4eeb212a0090ca3e3ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214800
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-14 23:56:10 +00:00
Heschi Kreinick
3d14842334 internal/imports: load test exports of package under test
x_tests can access exports from the test variant of the package under
test. Change loadExportsFromFiles to understand that mode, and use it
where appropriate.

I didn't want to come up with a cache key for for the test variant, so
for now we bypass the cache in these situations.

Fixes golang/go#29979.

Change-Id: I9959a08da97bbee64c5bcd56e06f548486693156
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213221
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 23:54:10 +00:00
Rebecca Stambler
f986e617de internal/lsp: make sure diagnostics with analyses are not overwritten
The initial workspace load does not send analyses with diagnostics, but
subsequent diagnostic requests do. If we've already sent diagnostics
with analysis for a file at a given version and snapshot, do not resend
diagnostics for the same file version with analyses.

Change-Id: I6979aa308e8846ff0cb66bd23e0dac488eae5446
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214587
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 22:43:10 +00:00
Rohan Challa
579903b3ad internal/lsp: add mapper for go.mod files
This change adds a protocol.ColumnMapper when parsing go.mod files. This will prevent us from having to worry about line and column offsets, specifically when converting from the x/mod/modfile position to a span.Span.

Updates golang/go#31999

Change-Id: Iacdfb42d61dfea9b5f70325cf5a87c9575f8f345
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214699
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 22:04:35 +00:00
Heschi Kreinick
3be912efc3 internal/lsp/protocol: support nonstandard requests
Support arbitrary client->server requests in the generated code. This is
primitive, with no strong typing, but should be good enough for simple
requests. We can do something fancier later if we want.

Change-Id: Ib83ff6aa49418bda5222b37cde3b150a26552221
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214582
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 21:52:24 +00:00
Rebecca Stambler
9c46c2c3da internal/lsp: fix flaking internal/lsp/cmd tests
Reloading metadata on demand fails for some our test packages,
because I don't understand how to construct arguments to commands.

Change-Id: Ib18454a09772e854a612528af898d06ce14133c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214717
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-14 21:42:16 +00:00
Rebecca Stambler
189207f339 internal/lsp: use URIs instead of FileIdentity in errors
This change modifies the source.Error type to have a URI instead of a
FileIdentity associated with an error.

Change-Id: I8056bdc881dd3ec43af02cca1366815c0bc6dfcd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214586
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 19:14:11 +00:00
Muir Manders
d31a08c2ed internal/lsp/source: improve completion support for args to builtins
We now understand what "kind" of type is expected when using various
builtins. For example, when completing "close(<>)" we prefer channels,
and when completing "delete(<>)" we prefer maps.

I also added some code to infer the expected type for the second
argument to "delete()" and for the args to "copy()":

delete(map[someType]int{}, <>) // expect "someType"

copy([]int{}, <>) // expect "[]int"
copy(<>, []int{}) // expect "[]int"

And I marked "new()" as expected a type name, and it infers the type
name properly:

var _ *int = new(<>) // expected type at "<>" is "int"

Fixes golang/go#36326.

Change-Id: I4295c8753f8341d47010a0553fd2d0c2586f2efa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212957
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 05:24:53 +00:00
Rebecca Stambler
3b9e235283 internal/lsp: fix context cancellation
Change-Id: Ib932722b0e66910b8cd23031e756f16099ee82dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214581
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 01:26:48 +00:00
Rebecca Stambler
294c8f8251 internal/lsp: use latest file versions in diagnostics
This change makes sure that diagnostics are sent with the most recently
seen version for a file, instead of a cached version.

Fixes golang/go#36476

Change-Id: Ibac2757099fdfc71989987cf5851a678f589aadf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214422
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 00:45:52 +00:00
Rebecca Stambler
5a294e27f3 internal/lsp: allow subdirectories of module roots when checking errors
Previously, we would surface a warning message if a user had a missing
dependency in a subdirectory of their module root. This is not
necessary, so do a better job checking for that case.

Change-Id: Ib6fcdcbf6ac191478b9bb1f5f8a55d154fd30b5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214420
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-14 00:24:04 +00:00
Rohan Challa
544dc8ea2d internal/lsp: change go1.14 check to be more lenient
This change adds the -e flag to ensure that we get the release tags when we are checking if the go version is at least 1.14. This also adjusts the check to be more lenient when it comes to processing the output of the version check.

This also fixes another issue where if the version is not 1.14 we were publishing empty diagnostics for an empty uri. This arose because we did not check if there was a valid go.mod file before we published the reports.

Fixes golang/go#36488

Change-Id: I5a8057c153f49167811e2bf8e4ade52bf6171d6b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214290
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-13 22:38:16 +00:00
Rebecca Stambler
533f309ed4 internal/lsp: reload workspace package metadata on demand
This change removes functions from the snapshot that return package IDs.
We prefer PackageHandles, since getting PackageHandles in a granular
fashion is not effective and causes us to spawn many `go list`
processes. By only ever returning PackageHandles, we can batch metadata
reloads for workspace packages. This enables us to add a check to
confirm that the snapshot is in a good state before returning important
data, like reverse dependencies and workspace package handles.

Change-Id: Icffc8d8e0449864f207c15aa211e84cb158c163f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214383
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 20:10:07 +00:00
Rebecca Stambler
8f45075ebc internal/lsp: merge completion options into source.Options
This change flattens the completion options type into UserOptions and
DebuggingOptions, which will enable us to generate documentation for
these options more effectively. This results in some modifications in
the tests.

Additionally, the fuzzyMatching and caseSensitive boolean flags are
merged into one setting, matcher, which can be used to specify the type
of matcher that is used for completion. Other requests (notably
workspaceSymbols) may need to use a matcher in the future.

Change-Id: I185875e50351be4090c7a2b3340d40286dc9f4a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212635
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 20:09:44 +00:00
Rebecca Stambler
6316571e2c internal/lsp: don't require type-checking for formatting
We had previously required a type-checked package for formatting
requests, in order to determine if the package contained any parse
errors. We can get this information directly from the ParseGoHandle, so
there is no need to check the package. This will prevent `go list`
errors from making their way into formatting requests, for which a `go
list` really is not needed.

Updates golang/go#36511

Change-Id: I297f8880367e800d2343f468535efa80d2937071
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214421
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 19:43:37 +00:00
Rebecca Stambler
86d412b4c6 internal/lsp: fix support for watching changed files
This is the beginning of the CLs to refactor the file watching code with
the normal text synchronization code. This hasn't yet been tested other
than with some minimal local testing, so follow-up CLs will be needed.

Updates golang/go#31553

Change-Id: Id081ecc59dd2903057164171bd95f0dc07baa5f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214277
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 19:19:55 +00:00
Rebecca Stambler
4a54ec1d38 internal/lsp: remove view.FindPosInPackage and view.FindMapperInPackage
There is no reason for these functions to live on the view. They make
more sense as unexported functions in internal/lsp/source.

Initially, I had to propagate contexts through a lot of functions in
internal/lsp/source, but instead I removed the unused contexts forom
snapshot.GetFile.

Change-Id: I8323419d0356feb2010091fe8d3ed35e511f801a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214384
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 18:51:11 +00:00
Rohan Challa
30cae5f2fb internal/lsp: surface diagnostics for invalid go.mod files
This change will surface errors that come from the mod package. It will handle incorrect usages, invalid directives, and other errors that occur when parsing go.mod files.

Updates golang/go#31999

Change-Id: Icd817c02a4b656b2a71914ee60be4dbe2bea062d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213779
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-13 15:48:38 +00:00
Muir Manders
eac381796e internal/lsp: fix completion ordering workaround
Our loop to make all candidates use the same "filterText" wasn't
including the final candidate. This was causing strange ordering of
candidates in VSCode when the "worst" candidate happened to match the
prefix exactly (causing VSCode to reorder it to the top).

Fixes golang/go#36519.

Change-Id: Ie2119ca94d13f337edd241fbde862706bdeee191
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214184
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-13 04:08:37 +00:00
Rebecca Stambler
a7a6caa82a internal/lsp: initialize the view on creation
This change moves the initialization of the view into the view's
creation, instead of forcing the tests to call WorkspacePackageIDs to
initialize.

Change-Id: Iccea820ec268b1851d58821481d92c7a3d4772c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214279
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:31:25 +00:00
Rebecca Stambler
3e6f5d44f4 internal/lsp: don't invalidate workspace when a mod file is opened
Opening a mod file is not sufficient cause to invalidate in the
workspace, so don't.

Change-Id: I2b7703008528e4469be312165deb17fe6856fd74
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214259
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:20:43 +00:00
Rebecca Stambler
945ed6e034 internal/lsp: fix link handling for links without schemes
Unfortunately, this can't be tested until golang/go#35880 is resolved,
because only the third-party xurls library detects links without
schemes.

Change-Id: I620581d6ce99c79ae89f3f22ea8ee634c3850a8e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214280
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:19:44 +00:00
Rebecca Stambler
e8a26f4160 internal/lsp: prevent initial workspace load from being canceled
In one of the many iterations on CL 212102, the contexts propagated
through the initial workspace load were allowed to be canceled. This
should not be allowed because the initial workspace load has to be
completed.

Change-Id: I6c6273b4e58fb9041af518f329f4766ed5f1f81b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213641
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 21:08:40 +00:00
Rohan Challa
428f1ab0ca internal/lsp/mod: test go.mod is unchanged when tempModfile=true
This change adds a test to ensure that your go.mod file remains unchanged when the tempModfile flag is activated. Specifically, it adds a test to ensure that a go directive does not get added to a user's go.mod file when there was not one included before.

Updates golang/go#36247

Change-Id: If8db5508ace5b7222112408255ffa66e4d38797f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214260
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-10 14:27:00 +00:00
Rebecca Stambler
e2f26524b7 internal/imports: create listener map after clearing for new scan
The module resolver needs to recreate the listener map if it's cleared
for a new scan.

Change-Id: If5e945d4f2059f2a79aef3129f963a2c50e90229
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214278
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-10 04:28:03 +00:00
Heschi Kreinick
563860d11d internal/imports: fix use of uninitialized resolvers
Resolvers are lazy initialized. That worked fine until the addition of
the scan semaphore -- it's not a good idea to create that lazily, since
you can't synchronize on a channel that doesn't exist.

Specifically, this caused a gopls hang when completion finished without
needing to use the resolver. In that case, we'd call ClearForNewScan/Mod
on an uninitialized resolver, and then hang receiving from a nil
channel.

Instead, eagerly initialize where convenient, and particularly the scan
semaphore.

Change-Id: I3adb1ae76b751650995e50f50346e06fd9c9f88d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214258
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-10 00:40:31 +00:00
Rebecca Stambler
0a1579a33b internal/lsp: don't run full workspace diagnostics on mod file change
Minimize the issues at master by not running workspace-level diagnostics
on mod file changes. Once the initial workspace load stabilizes we will
be able to go back to that approach.

Also, a couple of minor changes along the way while debugging.

Change-Id: Ib3510e15171326a1b89f08ef0031a3ef7d9ac4ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214257
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-10 00:12:05 +00:00
Heschi Kreinick
5d34a75004 internal/imports: don't block completions on walks
Filesystem walks of large GOPATHs/module caches can take seconds,
especially on systems with slow filesystems like MacOS and WSL. We don't
want to block completion requests on walks finishing. At the same time,
cancelling a walk midway through results in an unusable cache, where we
don't know which parts have been scanned so far.

The best option is to run the walks in a separate goroutine. Then we can
detach and let them finish. On the other side, we need to be able to
reattach for the next completion request.

Introduce a new method on caches, ScanAndListen, which first processes
all the items in the cache, then notifies of any new items. This allows
us to reattach to an existing scan without missing anything.

The background scan introduces concurrency to the resolvers where there
wasn't any before. We can't use mutexes, because there's no way to stop
Lock() when a context expires. Use a 1-element semaphore channel to
accomplish the same effect.

Along the way: Only rescan GOPATH if the resolver has been cleared. None
of this makes sense for GOPATH without that. Fix a bug where we were
scanning the main module twice in module mode. Stop loading exports in
module tests, it slows them down a ton.

Change-Id: I978efae733ccba0c0cdc8e8fe6892bf5f15feac8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213217
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-09 22:24:46 +00:00
Rebecca Stambler
a9a43c4726 internal/lsp: store workspace package IDs with package paths
A test variant for a package can only be reloaded by running go/packages
on the non-test variants import path with the -test flag. We need to
cache this import path in order to be able to reload a test package
on-demand.

Also, always ignore test main packages by detecting them in the
metadata.

Fixes golang/go#36473

Change-Id: I6e5a61c8e73689212303ae09fa3aed4a2ee8762a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213660
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 22:04:34 +00:00
Rebecca Stambler
8a1a3df092 internal/lsp: allow diagnostics to be computed per-file or per-package
This change splits source.Diagnostics into source.FileDiagnostics and
source.PackageDiagnostics. There isn't much difference in functionality,
but this just simplifies the process of getting diagnostics if you only
have a PackageHandle.

Change-Id: I62c0de8778065a4c46cc5672e2834ce5c63fe012
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213644
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:49:23 +00:00
Rebecca Stambler
700ee2612c internal/lsp: propagate errors from find references
We were ignoring errors in a few cases and returning incorrect errors in
the case of the command-line interface (no identifier found when there
were just no references). I think it is better for references to return
an error than incomplete results.

Change-Id: Id90bca58ebdd9f6a910853cb4ac5b6ab6bec57f8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:23:09 +00:00
Rebecca Stambler
dfcf57064e internal/lsp: stop requiring file kind when fetching a file
This change consolidates the FileKind into only the FileHandle.
Previously, it had been set in multiple places, which required users to
pass in a FileKind when fetching a file. This resulted in confusion,
particularly in places when users did not have access to the file kind.

Change-Id: I9e07d7320c46a21d453ffe108d1431a615706a71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213459
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:19:36 +00:00
Rebecca Stambler
487733262f internal/lsp: don't send diagnostics for old file versions
This isn't a strictly necessary change, but it simplifies things for
open files when the initial workspace load diagnostics come in too late.

Updates golang/go#36452

Change-Id: I85a9c201876b7c63a97d8dca020fc396b3c57706
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213820
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 21:16:13 +00:00
Rebecca Stambler
675cf513f4 internal/lsp: warn user in case of a multi-file ad-hoc package
This change surfaces a warning to the user if they might be coding in a
multi-file ad-hoc package.

Updates golang/go#36416

Change-Id: Ifaa15a0777ea97e62c1477fb33911636b13e073e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213458
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-09 20:44:29 +00:00
Rohan Challa
f270e23f6a internal/lsp: add warning diagnostics for unused dependencies in go.mod files
This is the first step to surfacing potential fixes and suggestions to a user's go.mod file. Specifically, it will show a warning if you have a dependency that is not used, or if a dependency is declared as indirect when it should be direct and vice versa.

This CL adds functionality for version of Go that are >= 1.14.

Updates golang/go#31999

Change-Id: Id60fa0ee201dcd843f62e2659dda8e795bd671db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211937
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-09 20:05:13 +00:00
Muir Manders
89082a3841 internal/lsp/source: fix some invalid literal candidates
We were marking all literal candidates as addressable so we were
getting invalid candidates like "&int()". Fix it to only mark literal
struct, array, slice and map types as addressable.

I also fixed the unnamed literal candidate to pass the dereferenced
expected type. For example, if the expected type was "*[]int" we were
passing a literal type of "*[]int" which wasn't working anymore. Now
we pass "[]int" and take its address as "&[]int{}".

Change-Id: I5d0ee074d3cc91c39dd881630583e31be5a05579
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212677
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-08 20:36:44 +00:00
Heschi Kreinick
316d2f2484 internal/lsp/source: only add names to imports when necessary
As usual, I forgot to clear out the import spec's name when it matches
the import path.

Change-Id: I4ddd49b70e0db95fcd30d2968b098327fac39a92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213222
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Muir Manders <muir@mnd.rs>
Reviewed-by: zikaeroh <zikaeroh@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-08 19:54:15 +00:00
Rohan Challa
acbc0e3ba1 internal/lsp: modify how we check for go 1.14 for modfiles
When the tempModfile flag was enabled, there would be some go
commands that would run without the -modfile flag. This change adds
the config's buildFlags to the invokeGo command in go/packages and
updates how we are checking if the go version is 1.14 within the
modfileFlagExists function.

Fixes golang/go#36247

Change-Id: I436666c3fcad33e85ba00aec5f227fe4a0e638b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212477
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-08 19:23:01 +00:00