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>
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.
Fixesgolang/go#36441Fixesgolang/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>
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>
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>
Restore previous behavior where the object's declaration is returned
as the first reference in find-reference calls.
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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.
Fixesgolang/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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
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"
Fixesgolang/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>
This change makes sure that diagnostics are sent with the most recently
seen version for a file, instead of a cached version.
Fixesgolang/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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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).
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
