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

4186 Commits

Author SHA1 Message Date
Rob Findley
850357681a gopls/integration/govim: update build steps to capture artifacts
Govim integration tests generate a number of artifacts, including both
the govim and gopls logs, that can be useful in debugging failures. This
change updates our cloud build configuration to capture these artifacts,
along with several other minor improvements.

Notably artifacts are uploaded to GCS as a separate build step, so that
we have the potential to use its granular permission model for sharing
these artifacts. Right now, this requires temporarily swallowing the
exit code of `go test` so that the build can proceed.

Also:
 - Update govim to a newer version; we still can't use latest as there
   isn't a tagged version that contains the requisite flag change.
 - Alter the test harness to run tests from the github.com/govim/govim
   module root.
 - Switch use a major version label when referring to the test harness
   build step, to allow for breaking changes (such as the one made
   here).
 - Add a missing copyright header to run_local.sh.
 - Update run_local.sh to work with the modified harness.
 - Update documentation accordingly.

Change-Id: Ie5ddaf54e775371a36163f98c1beb90c217be931
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214577
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 14:49:07 +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
Rob Findley
e23c44e711 gopls/doc: minor fixes in design.md and implementation.md
A couple sentences are clarified in design.md, and a broken link is
fixed in implementation.md.

Change-Id: Ibbb4c2f74ce9e46c565a7e90798047e7168b076e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214798
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-14 23:33:23 +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
Adam Williams
9dc88fb721 cmd/stringer: remove redundant check in generated code
In a certain case, stringer would generate the following check for an
an unsigned integer i: "0 <= i". This changes stringer to not generate
such a check.

Also adds an additional test case for an unsigned multiple run that does
not include zero. There was already a case ("unum") that included an
unsigned multiple run starting at zero. This case's output was updated
accordingly.

Fixes golang/go#36461

Change-Id: I57e79384a0b802fa4571e2b3495db168b814bcaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214180
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
2020-01-14 01:55:19 +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
Rob Findley
78c1766344 go/analysis: remove stale documentation citing vet
This documentation references the go/analysis vet command, which was
deleted in golang.org/cl/192177.

Change-Id: I6dd5d7bfa0e5bb2dc2af07398d8a8e4293dac6d5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212864
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
2020-01-13 21:10:14 +00:00
Koichi Shiraishi
4da8041706 gopls/doc: fix 'Report the issue' bad link
Change-Id: Ie31d1f375ea93840926d23aef5232ccb77b883a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214537
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 20:22:26 +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
vovapi
ac4f524c16 go/ssa: add position data in indexed range loops
Indexed range loops are lacking position data
in their SSA representation, so add the position data
into an index operation.

This improves the nilness output by providing position data
with the message.

Updates golang/go#31008

Change-Id: I664c8668bc74207e770657e21129d20fd70e7af6
GitHub-Last-Rev: 75831d240efd2a79ff95cc18041d29d332dc94ab
GitHub-Pull-Request: golang/tools#190
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208599
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-01-09 17:47:59 +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
Rebecca Stambler
11e9d9cc00 internal/lsp/source: set default InsertTextFormat to PlainText
CL 212519 unintentionally changed the default for this option.

Change-Id: I42df51ba68e78af1493d0461ad4efff3dd55da9d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-07 18:40:32 +00:00
Muir Manders
a222fb47e2 internal/lsp/source: don't downrank builtin constant completions
We downrank untyped constant candidates so that we prefer candidates
whose type matches exactly. However, this was causing builtin
constants like "true" to be outranked by candidates that fuzzily match
"true". Fix by not downranking builtin constants.

Fixes golang/go#36363.

Change-Id: I14801688c96efdbb7ff9fee69f66028530df984c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-07 18:15:58 +00:00
Rebecca Stambler
7201abb308 internal/lsp: parallelize initial workspace load
The initial workspace load was happening when a view was created, in serial.
It should really just be kicked off in a separate goroutine once we create a
new view. Implementing this change required some other significant changes,
particularly the additional work being done by the WorkspacePackageIDs
method.

Some other changes had to be made while debugging. In particular, the
modification to the circular dependencies test was a consequence of
golang/go#36265.

Change-Id: I97586c9574f6c4106172d7983e4c6fad412e6aa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212102
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-07 18:15:18 +00:00