This is just to ensure that arguments are always ordered when passed to
packages.Load.
Change-Id: I2c74673eff31efc31854dc0d306809890f633789
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The go command gets mad if go.mod has changed since it started, e.g.
because a new dependency was added by a concurrent go list call. Retry
loads if they hit a concurrency problem. See the comment for more
details.
Testing this is awkward. I ran a background script that constantly
modified the go.mod file and observed that gopls waited until it was
killed and then recovered.
Updates golang/go#36772.
Change-Id: I5636c99a5a94b415c4a6fbb71869b07e31d3fed0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216543
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change sets up the infrastructure to surface the dependencies that are missing in a go.mod file. In a follow up CL, we will use these to add diagnostics to the imports of .go files telling the user to add the dependency to their go.mod file.
Updates golang/go#31999
Change-Id: I697df0d7c6eabfdc2f0c75cb36aa35794647fd13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214700
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were reloading all known files, which resulted in modifications to
user's go.mod files.
Change-Id: I14e86af896d1e75f3fdaaa00b9af8d7fb1d1e9e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216542
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mostly because I find it very annoying.
But I imagine lots of people shadow variables intentionally, and it's
very noisy.
Also, fix an error caught by the nilness check while I'm thinkng about
analyses.
Change-Id: I1867c8613194028815666efd879899bb5065c9d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216541
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The command line tool doesn't go through JSON RPC, so it retains type
information that's stripped in the tests. Cut the parameters down to
basic types manually for now. But I don't know why the command line
tests send RPCs if the real thing doesn't.
Fixesgolang/go#36769.
Change-Id: I428b40478557ca35a7dae8d02bbcd990411f714f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216539
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The go/packages workaround to hide errors for overlay packages doesn't
seem to work well. It's easier to just hide list errors in gopls
diagnostics unless the package genuinely failed to type-check. Check if
the package has missing dependencies as an approximation of if it is
well-typed.
This required some additional special casing for the import cycle error
detection, which now causes them to have duplicate diagnostics. It's a
rare enough case that this doesn't concern me, but we should clean this
up at some point.
Fixesgolang/go#36754.
Change-Id: If12c92fb9a0e0b69b711ae9a509ecb1b2a32255c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216310
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now that we are reloading metadata for workspace packages by package
path, ad-hoc packages get their metadata reloaded incorrectly. If an
ad-hoc package has no metadata, reload it by reloading the entire
directory.
Fixesgolang/go#36753
Change-Id: Ie440f6f76a220009d487b7ceadcf40594643e969
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216307
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I had originally thought I might be able to use exprAtPos for this,
which is why I ended up eliminating that function when I saw it only had
one use.
One test also had to change in order to fit better with the spec.
Specifically: "If [the active parameter is] omitted or the value
lies outside the range of `signatures[activeSignature].parameters`
it defaults to 0 if the active signature has parameters."
Fixesgolang/go#36766.
Change-Id: I400d5b2db2985bfaa5efbcd91225151ca8b5f46a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216309
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The LSP already supports a bunch of analyses we have less confidence in
than the vet suite so we should add these too.
Updates golang/go#36639
Change-Id: Ifc37d09e3acd73de021be7b45b3d80fe8c00e0d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215677
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There are a number of new RPCs, for CallHeirarchy, SemanticTokens, and
WorkDoneProgress. Some of the messages now have two slashes, like
'textDocument/semanticTokens/edits'. A few unused RegistrationOptions
are no longer present.
Only generated code has changed. There are no changes any other .go
code.
The typescript code needed a new heuristic for finding the RPCs
Change-Id: Ie6942abac3a0cd60e86afe3fdbc6f6b0e9b30cb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216537
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
CL 215738 didn't work because the canceller was embedded in the
serverHandler, which already had a Deliver method. Add it as an actual
handler instead.
Change-Id: I0c79f1bee67aa3b4da53d92547804de859f1938c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216303
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that the view can report its own validity, we no longer need to have
an extra function to handle that.
Change-Id: Icd22f9b08601bd0fe18be064c43d21be2d6782e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216144
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The view should really be able to determine if it's valid, not the
source package. Expand moduleInformation to be buildInfo, and use it to
collect additional details.
Use this information to determine if we should load a view's
subdirectories as part of the initial workspace load. If a module is
initialized, we will recreate the view, so we should be fine. Not sure
what will happen if the directory is moved into GOPATH, but that should
be less of a concern (I think).
Fixesgolang/go#35818.
Change-Id: Ic8ceedd37386b1653b8965c64d9ba8953778ab78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216143
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
go/packages overlay handling only really works for contains queries
(file=), so our approach of reloading packages by package path (for
workspace packages) wasn't handling newly created packages that need to
be handled through overlays. Workaround this by reloading metadata for
individual files that are missing it by running extra contains queries
(only after the first metadata load for package paths). Be careful not
to reload the same file multiple times if the first load did not succeed.
Somewhat related, clear out `go list` errors in packages that go
through overlay handling, since they will often be rendered irrelevant.
I'm not sure if this is the right move, but if it's not, then we will
have to do extra work to disregard those errors in gopls.
Fixesgolang/go#36661.
Fixesgolang/go#36635.
Change-Id: Ib83cffcdf8a3e07da0f30e734d5e2c89691e1aba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216141
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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.
Fixesgolang/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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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.
Fixesgolang/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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
Make sure to test both modes, as this is the second time we've
accidentally broken this.
Fixesgolang/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>
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>
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>
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>
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>
$/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.
Fixesgolang/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>
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>
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>
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).
Fixesgolang/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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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.
Fixesgolang/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>
Apparently the AST will sometimes give us offsets past the end of the
file. Don't crash when it does.
Fixesgolang/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>
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".
Fixesgolang/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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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.
Fixesgolang/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>
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>
Test variants and test mains can result in multiple packages being
loaded for a single ID. Handle this case in the PackageHandle function
instead of returning an error.
Change-Id: Ic0024c5bded162a3e78a9cdcb9566449f3683e35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When dealing with cgo files, we need to pass a Mapper for the authored
file. I got most of the sites in CL 208501, but missed this one.
I was nervous about starting to propagate errors, but all the tests
passed, so shrug.
Change-Id: I7505e670e9c01d719c72c3f99d01c8153c3d2ff5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212862
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There were a few cases that we were missing when caching diagnostics.
The first was caching empty diagnostics from the initial workspace load,
so we were resending old empty diagnostics. The second missing case was
resending diagnostics for the same version of a file.
Fixesgolang/go#36340.
Change-Id: I5c55b47a60fe94b10e76be99714cb983f9b84b54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213122
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
CL 212102 contains a few cleanup-type fixes that are unrelated to the
actual content of that CL. Pull them out to make the diffs simpler.
Change-Id: I6b7e34320f2889d05179c8aeb8cb7f8f4c505a3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212917
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Packages that have already been loaded by gopls are more likely to be
used, and have full type information. Check them for completion
candidates before scanning the disk.
Also, minor bug fixes: add a missing mutex, and use a lower-than-usual
score for typed unimported completions.
Change-Id: I46388802913f9a89342fb47290f704b471154ec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212860
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In scan implementations, stop after cancellation, and swallow the
context's error for convenience.
In the module implementation specifically, try to avoid scanning if the
cache is enough to satisfy the user. When we do have to scan, prioritize
module dependencies before the whole cache.
Change-Id: I23dc98df016f9fca4f31c7ded3d11bc257c29b94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212857
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We only need to return a relatively small number of completions to the
user. There's no point continuing once we have those, so switch the
completion functions to be callback-based, and cancel once we've got
what we want.
Change-Id: Ied199fb1f41346819c7237dfed8251fa3ac73ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212634
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I want to stop sorting unimported completions. We still want to show
users something reasonable, so use label as a tiebreaker for score in
the higher level completion function.
To maintain the current sorting, we need to adjust scores by search
depth (height?) for lexical completions. A few tests are really ties,
and need sorting in the test case.
Change-Id: Ie2d09fdcbebf6fda4ab33a2f16c579d12b0f26ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212633
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
No point in constructing the defaults in three places.
Change-Id: I2b0776910a933a7250245bd82dc27e63c34df18a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212632
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We have multiple use cases for scanning: goimports, import completion,
and unimported completions. All three need slightly different features,
and the latter have very different performance considerations. Scanning
everything all at once and returning it was not good enough for them.
Instead, design the API as a series of callbacks for each
directory/package: first we discover its existence, then we load its
package name, then we load its exports. At each step the caller can
choose whether to proceed with the package. Import completion can stop
before loading exports, goimports can apply its directory name
heuristics, and in the future we'll be able to stop the scan short once
we've found all the results we want for completions.
I don't intend any significant changes here but there may be some little
ones around the edges.
Change-Id: I39c3aa08cc0e4793c280242c342770f62e101364
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212631
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Unimported completions are always low-priority. If the user already
has 100 completion options, the unimported ones are probably not useful.
There's no point in calculating any of them.
Also, only do unimported completions for package members when they're
enabled. Oops.
Change-Id: I7535a22ad56bed869dceb6cd0ffdfc6390cf8eb5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212629
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The metadata for the workspace packages may not be available when we
need it, so we should allow loading a single package ID. This can be
improved in follow-up CLs by consolidating the individual IDs into one
call to packages.Load. Some adjustments from CL 212102 were split out
into this CL.
Change-Id: I173a79a3cb136530bc99d093f1c2be189eac8ce2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212628
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change treats text of the format golang/go#1234 as a link to the Go
issue tracker. This will improve the readability of TODOs that include a
link to an issue, since it doesn't have to be an actual link.
Change-Id: I27606ceb9cbb15bc6bfb1d7aa660d3f4fdd08739
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change cleans up the structure of the Options struct in order to
clearly delineate which options should be configurable for the user.
Follow-up work is needed to refactor the completion options to fit into
this structure, as well as to make sure that the name of the field is
the name of the setting. This will make it easier to generate markdown
documentation from the code.
Also, remove options that are no longer in-use and mark them as
deprecated.
Change-Id: Ib34ae25789e21b76077a564601e487fbebfc5f48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Our current implementation isn't robust, and it doesn't seem worth it to
invest significant effort in improving it when this library exists.
Also, make the protocol part of the default URL regex non-optional, as
the alternative is that any string of the format "foo.bar" will appear
to be a link.
Updates golang/go#33505
Change-Id: Ia430a1c193eded394f8af12050bdd4dc2a9ccc94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212517
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
VSCode doesn't like (read: ignores) candidates whose filterText begins
with "&", so trim it off.
I also tweaked "addressed" candidates to include the "&" prefix in the
item label as well so the user can see what they will get.
Change-Id: I85840d036e379a202b72e28c5257807a069ae45d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212406
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We now support taking the address of objects to make better completion
candidates. For example:
i := 123
var p *int = <> // now you get a candidate for "&i"
This required that we track addressability better, particularly when
searching for deep candidates. Now each candidate knows if it is
addressable, and the deep search propagates addressability to child
candidates appropriately.
The basic propagation logic is:
- In-scope *types.Var candidates are addressable. This handles your
basic "foo" variable whose address if "&foo".
- Surrounding selector is addressable based on type checker info. This
knows "foo.bar.<>" is addressable but "foo.bar().<>" isn't
- When evaluating deep completions, fields after a function call lose
addressability, but fields after a pointer regain addressability. For
example, "foo.bar()" isn't addressable, but "foo.bar().baz" is
addressable if "bar()" returns a pointer.
Fixesgolang/go#36132.
Change-Id: I6a8659eb8c203262aedf86844ac39a2d1e81ecc4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212399
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is the start of a significant refactoring to implementations,
references, and rename (i.e. everything that searches across
packages). The main goal of the refactoring is to push the package
variant logic from internal/lsp into internal/source so that all users
of source benefit, not just internal/lsp. It also makes it easier to
write tests for various cases because the source tests invoke the
source package directly (so previously did not include all the package
variants).
Currently source.Identifer() handles lots of disparate use cases.
Things like definition and hover don't care about package variants but
do care about other random bits of info that may not apply to
implementations or references. So, I'm splitting implementations out
from source.Identifier. As I work through references and rename
hopefully things will end up separated into smaller chunks.
I also improved implementation deduping to happen earlier. I thought I
could dedupe using obj.Pos(), but mirror objects in package variants
have different positions (suggesting they aren't reusing the
same *ast.File). Instead I used token.Position to dedupe.
Change-Id: I81c2b3ec33bf12640accb852be9ecdea4aa24d69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211718
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Make the score and import info be fields on "candidate" since they are
properties of the candidate. There shouldn't be a functional change
here; I'm just consolidating things in preparation for an additional
piece of candidate metadata.
Change-Id: I4c7c8ef40e8e5db7b52691cca21490ba13c17642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212398
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If the enclosing value spec specifies a type on the LHS, we now prefer
completions of that type on the RHS. For example:
i := 123
var foo int = // prefer "i" since we know we want an int
I also added a special case to lexical() to know that we can't offer
objects defined on the LHS as completions on the RHS. For example:
var foo int = // don't offer "foo" as completion
Change-Id: I8e24245a2bc86a29887360e7f642a4cbb87fa6ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212401
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit will delete the temporary go.mod file that is attached
to a view when the session is shutdown.
Updates golang/go#31999
Change-Id: I0f6b0663b0a814ce45d9b12468669f415f8c36aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212479
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 212037 introduced a bug with saving overlays. Since VS Code sends nil
contents for a file on save, we were deleting overlay contents on save.
This resulted in very strange behavior.
Also rename overlay.data to overlay.text to match variable names.
Fixesgolang/go#36224
Change-Id: I7f2d12e369aa7f6daa2c9f36c33468ec6bf61930
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212199
Reviewed-by: Michael Matloob <matloob@golang.org>
In one of my previous refactoring changes, I lost the fact that the
context should be detached before invalidating a file's contents. If
this function is canceled, we will be in a bad state.
Also, small change to return ctx.Err() instead of a custom error message
from (*packageHandle).check.
Change-Id: I19e513e09e438feee105fdd89cb7364a0c3c5e7f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212104
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now that `go list` errors are sufficient for us to determine a circular
dependency, we don't need to cache errors on dependency packages.
Change-Id: I0633aeb356f93d21afed3371d61d7eae7de255ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change merges the small helper functions that modified overlays
into a single function and removes the openFiles sync.Map in the view.
Change-Id: Id94c7d86228c9628b7373fab0030ad0c8018dda5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212037
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change eliminates the extra step of calling GetFile on the view and
getting the FileHandle from the snapshot. It also eliminiates the
redundant source.File type. Follow up changes will clean up the file
kind handling, since it still exists on the fileBase type.
Change-Id: I635ab8632821b36e062be5151eaab425a5698f60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
On startup gopls runs go/packages over the entire workspace. The log
message in question outputs each package found along with all the
package's filenames. Obviously in a large project this produces an
incredible amount of output. Fix by putting the log message behind the
"verboseOutput" flag when invoking go/packages in the "dir/..." mode.
I also added the go/packages "query" string to the
once-per-go-packages-call log message so it is more useful.
Change-Id: I651419e34a855325056bca6720eda8671f7d5fa8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210739
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There is an issue when highlighting a single character identifier if the cursor is on the right of it. This problem
is a result of an assumption made in astutil.PathEnclosingInterval relating to the arguments passed in. Specifically,
if start==end, the 1-char interval following start is used instead. As a result, we might not get an exact match
so we should check the 1-char interval to the left of the passed in position to see if that is an exact match.
Updates golang/go#34496
Change-Id: If689fdf695df6ec1bc1935088e50d3de055bd5db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL makes sure that code.ts will generate the latest version
of tsprotocol.go. It has a more succinct way of deciding which fields
need to be pointers.
Change-Id: I6854cb2f096d3707bc3b828a9601f9384638f475
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212140
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL adds a "disableTempModfile" boolean that can be turned on or off.
While we are adding support for go.mod files in gopls, this flag will allow
users to opt out of using the -modfile flag that is enabled in Go 1.14. This
flag might be removed at a future point, the decision still needs to be made.
Updates golang/go#31999
Change-Id: Ic90229333e988fcc6d461ab1ee47bce1114bd965
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212139
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The import cycle not allowed error should be returned as a ListError not
an UnknownError.
Fixesgolang/go#35964
Change-Id: Ibc575f92d926ff715c0da67a4fceda05badcc652
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212138
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It wasn't infinite, but gopls would sit at 100% cpu for ~25 seconds
whenever I made a change to a package imported by essentially
everything in my project.
Change-Id: Ifa253a4de06897260e0791888284527258e8de48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212000
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The situation in golang/go#35638 was as follows:
didOpen main.go creates a snapshot that knows main.go is in package
"mod.com".
didChange main.go creates a snapshot. When a file changes, we discard
its contents by leaving the file handle out of the "files" map.
didOpen const.go creates a snapshot, and attempts to invalidate the
metadata for packages in the same directory.
The way we detect packages in the same directory is by iterating through
the files in the snapshot. But we threw away the only file in "mod.com"
in step 2 when its contents changed. If a diagnostics run happened to
get in between the two steps, it would re-load main.go and the bug would
go away. If not, step 3 would find no files and fail to invalidate
"mod.com".
The best way to fix this is to insert the new file handle eagerly during
cloning. That way there's no confusion.
Fixesgolang/go#35638.
Change-Id: I340bd28a96ad7b4cc912032065f3c2732c380bb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211578
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Eliminate the file watcher, since it led to a lot of confusion and
difficulty reasoning about the flow of a file action. This change splits
a file invalidation into the two logical steps - 1) things that affect
the overlay, and 2) things that affect the view. It is based on top of
CL 211757, so the diffs will look better once that CL is merged.
Change-Id: I277475569b61f3c80feaa6b6fe457b4bace82e35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change has no code modifications. Just move the handling for
overlays and debugging into separate files to make them easier to find.
Also, add some missing copyrights.
Change-Id: I7256f704c017457fa3418818d03f89f061af6fc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Print the debug port to stderr when the debug server listens on a
dynamic port so that clients can determine which port to use when
launching a browser to the debug view.
Change-Id: I92f5e334df5cefdf54f7242ac2328b026852c70e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211798
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In the upcoming Go 1.14 release, there is an introduction of the -modfile
flag which allows a user to run a go command but choose where to direct the
go.mod file updates. The information about this can be found here: golang/go#34506.
This change starts setting up the infrastructure to handle the seperate modfile
rather than keep changing a user's go.mod file. To support versions of Go that are
not 1.14, we run a modified "go list" command that checks the release tags to see
if 1.14 is contained.
Updates golang/go#31999
Change-Id: Icb71b6402ec4fa07e5f6f1a63954c25520e860b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211538
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Clear out a few things I noticed aren't really used anymore.
Change-Id: I03f955015632197b56230ae0443cfb3871b54db2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211717
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change fixes the link anchors for fields within a struct or
composite literal by getting the enclosing types.Type.
Fixesgolang/go#36138
Change-Id: I534a900fad6fa6fa1b1acaa5a63ca264c5d34c39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211582
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Generally speaking, if the imports package can operate on a file, we can
assume that it's in good enough shape for the rest of our work. That
means it's important we run imports first. On one code path, we weren't.
Fixesgolang/go#36162.
Change-Id: I750aff31e0c3706aeb798ceb60c35ea17ba95943
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211580
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a continuation of the discussion on
https://github.com/microsoft/vscode-go/issues/2920. Add corresponding
checks to internal/lsp/cmd/capabilities_test.go.
Change-Id: I51af05dee9e7ecea0e40733dd4c5ca3dfb8f4dd8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209859
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Cloning is complicated enough without worrying about concurrency, so
hold the snapshot's lock during the entire process.
Consolidate everything into one function. I don't think that the split
was making it easier to understand, and I was able to see and clean up
some extra complexity once it was all in one place. Let's discuss
options if you think the result is too long.
I don't intend any semantic changes in this CL.
Updates golang/go#35638.
Change-Id: I05c4b28875976293f5fcd56248d9c9e468f85cc6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211537
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We weren't returning promoted methods as implementations when the
promoted method was defined in a different package than the type
implementing the interface.
Fix by properly mapping the implementer types.Object to its containing
source.Package.
I generalized the implementations() result to just contain the
implementer objects and their containing package. This allowed me to
get rid of some result prep code in Implementation().
Fixesgolang/go#35972.
Change-Id: I867f2114c34e2ad39515ee3c8b6354c1cd35f7af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210280
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Descriptions of certain commands were changed to not start with capital
letter.
All of the commands were splitted into so called main
commands and feature commands.
Package tool did have a limitation that revealed itself when command
was invoked only with `-h`, i.e. `gopls -h`. Limitation was that in
above mentioned case, FlagSet.Parse() was intercepting `-h` flag and
printing just default usage.
Updates golang/go#35855
Change-Id: I9bd27fc72e8fb8d18025d95ebcae974dd5583e91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210359
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In a previous change I inadvertently added completion candidates like:
var f func(int)
f = <> // useless candidate "func(int)(<>)"
Ignoring the fact it is a syntax error without more parens around the
signature, it isn't a useful candidate because you don't need to cast
when assigning a named signature type to an unnamed type.
Change-Id: Ic261817af344ee47193240a11dca5d3a32cbd293
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211319
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If the cursor is within an argument that is within a callExpr which is
in a return statement, we only want it to highlight the ident that the cursor
is in. We do not want it to highlight the entire function.
Updates golang/go#34496
Change-Id: If4025660a99fd5df90098e0560a5e9e7260e33c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211338
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Created an analogous data structure for go.mod files when we parse them
using the golang.org/x/mod package. Gopls can now access the data
within a go.mod file using a parseModHandle and the corresponding
parseModData object. This will help down the road when it is time
to implement the lsp functions for go.mod files.
Updates golang/go#31999
Change-Id: Ibd4d64569bbe3df61b203490b63399d479e7d794
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211303
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
After the addition of golang/go#35964, the import cycle error now
has the import stack attached in the message. This CL parses that
stack and attached the import cycle diagnostic to the import versus
just adding it to the first character of the .go file.
Fixesgolang/go#33085
Change-Id: I6f5f067c338879b898829951236f816aa63d9dfa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210942
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The current handling of error messages did not address the case that a
user may have multiple directories listed in their GOPATH.
Fixesgolang/go#36120
Change-Id: I8e68a45d3dd902975a0ccf4177ec2698accdf69d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211304
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If there are less than 3 args for @item, it does an early
return without recording it for a given pos.
This leads to a panic when tests runner can't lookup
a non-nil item for a pos.
Since other collect* methods seem to use t.Fatal for error
handling, do the same in collectCompletionItems().
Change-Id: I21960731f532b93029e6e06800e0484dc7d599df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211257
Run-TryBot: Iskander Sharipov <quasilyte@gmail.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When the expected type is a basic type, we will now offer a
corresponding type conversion candidate. For example:
var foo int64
foo = // offer "int64(<>)" as a candidate
The type conversion candidate will be ranked below matching concrete
candidates but above the sea of non-matching candidates.
This change broke almost every completion test. I added a new
completion option for literal candidates so tests can selectively ask
for literal completions.
Updates golang/go#36015.
Change-Id: I63fbdb33436d662a666c1ffd3b2d918d840dccc7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210288
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change refactors some of the logic that builds a link anchor for
a given symbol, pushing the actual Link into the HoverInformation struct.
This is necessary because type information is needed to build up that
link in certain cases, like methods.
The last step will be to correctly display struct fields.
Updates golang/go#34240Fixesgolang/go#36031
Change-Id: I7f989faddbaa07f91838a870b4477bf78ce8ddf7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210201
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Having nil ranked normally causes it to show up as the top candidate
in cases like:
context.WithCancel(<>) // "nil" shows up before "context.Background()"
"context.Background()" gets a slight score penalty since it is a deep
completion, so "nil" is ranked highest.
Sometimes you do want "nil", but it's such a short identifier you
probably aren't leaning too heavily on autocompletion. I think it
makes sense to optimize for the case when you want something non-nil.
Change-Id: I537927db2b573535e751380c4cba5c9873dfe524
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210539
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Cache delivered diagnostics on the server so that we can determine if
they should be resent. To be careful about this, we only reuse cached
diagnostics if they are for a greater version, or if we don't know
the file's version and it is unchanged.
Fixesgolang/go#32443
Change-Id: I4ba22d85e5b21a8ad6cc62f74cd83c07d3c220cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208261
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change is the next step in unification of text synchronization
methods. The logic really belongs in the internal/lsp/cache package
rather than the internal/lsp package.
Pulled out a function to run diagnostics on a file (diagnostics are still
run async).
Change-Id: I5e237411a02af210ad386b37a6c2aa62ef723567
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210784
Reviewed-by: Heschi Kreinick <heschi@google.com>
We previously searched the reverse dependencies of the "widest"
package that contained out starting identifier, but if our package has
tests then the widest package is the ".test" variant, and it has no
reverse dependencies. Fix by searching through all of the packages
that contain our starting identifier.
For example:
-- foo/foo.go --
package foo
func Foo() {}
-- foo/foo_test.go --
package foo
func TestFoo(t *testing.T) {}
-- bar/bar.go --
import "foo"
func _() {
foo.Foo()
}
We would start searching from the foo.test variant, but we wouldn't
search package bar at all because bar does not import foo.test, it
imports plain foo. Now we search from both foo and foo.test (you still
need search foo.test to find references within foo_test.go).
Fixesgolang/go#35936.
Change-Id: I5fd2f7bb130a421ed6fad92da11179995c99a2cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When we are processing a go.mod file, we are calling go/packages.load
when we should not be. It will always return 0 packages since it is
not a .go file. This CL adds branching inside each internal/lsp protocol
function and also adds a check in snapshot.PackageHandles for the file type
and returns an error. This will prevent `go list` from running on go.mod files for now.
Updates golang/go#31999
Change-Id: Ic6d0e9b7c81e1f404342b98e10b9c5387adde2ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210757
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Say you have foo.go and foo_test.go yielding packages "foo" and
"foo.test". Previously when you changed foo_test.go we would
invalidate the foo.test and foo packages. Invalidating foo is not
necessary since it does not depend on any test files. Furthermore, it
caused problems because nothing would refetch foo's metadata until
foo.go changed, so various things (such as finding implementations in
packages that depend on "foo") would be broken.
Now we only invalidate metadata from packages that contain the
modified file. We only invalidate type info from packages that contain
the modified file, or from such packages' transitive reverse
dependencies.
Change-Id: I23d1af91bcdf22fad4faa1b048afe17ef4e403a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210460
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL teaches lsp to report `**T` instead of `**invalid type`,
`func (badParam) badResult` instead of `func (invalid type) invalid type`, etc.
To do that, we need to detect "invalid type" inside any part of a type.
I've added typeIsValid() function for that.
To simplify type formating code in resolveInvalid(), formatNode
function is added that can also format *ast.StarExpr (of any depth).
Since we already used AST printer in the same file, I
added formatNode function that is now used in both places.
While at it, replaced bytes.Buffer to strings.Builder there.
Change-Id: I3bb84c58c417b175cceefb410e238c48425f7cee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210357
Run-TryBot: Iskander Sharipov <quasilyte@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When the go.mod file changes, we should invalidate all the files that are
contained in the package for the mod file. This will allow the files to recheck
their packages in case new packages were added in the go.mod file.
This still does not fix issue where changes to go.mod files do not trigger recalculation of diagnostics.
Updates golang/go#31999
Change-Id: I6d8f1531f5c28ed2dca7fb8ad4ee0317fada787b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210557
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We previously only searched for implementations of the object we found
in the "widest" package variant. We instead need to search all
variants because each variant is type checked separately, and
implementations can be located in packages associated with different
variants.
For example, say you have:
-- foo/foo.go --
package foo
type Foo int
type Fooer interface { Foo() Foo }
-- foo/foo_test.go --
package foo
func TestFoo(t *testing.T) {}
-- bar/bar.go --
package bar
import "foo"
type impl struct {}
func (impl) Foo() foo.Foo { return 0 }
When you run find-implementations on the Fooer interface, we
previously would start from the (widest) foo.test's Fooer named
type. Unfortunately bar imports foo, not foo.test, so bar.impl
does not implement foo.test.Fooer. The specific reason is that
bar.impl.Foo returns foo.Foo, whereas foo.test.Fooer.Foo returns
foo.test.Foo, which are distinct *types.Named objects.
Starting our search instead from foo.Fooer resolves this issue.
However, we also need to search from foo.test.Fooer so we match any
implementations in foo_test.go.
Change-Id: I0b0039c98925410751c8f643c8ebd185340e409f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210459
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When a file is changed, we invalidate various cached data so we
re-type check and refetch metadata as needed. Previously when a file
changed we would delete the metadata for all transitive reverse
dependencies. This broke all-packages-in-workspace features since we
could no longer fetch the package handle for packages without
metadata.
Fix by only deleting metadata for the packages that the file being
changed belongs to. It doesn't seem like a package's metadata contains
anything that is sensitive to changes in the package's dependencies.
Change-Id: I6a2d5df49ecd4d627b37689e48ed48fe78ce658d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210458
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This should provide simple name completions for comments
above exported variables.
Can be activated with `ctrl+space` within a comment.
Pretty new, so all help is welcome.
Fixes#34010
Change-Id: I1c8f71baa3beaa22ec5fd9fd4a531284a8d125f3
GitHub-Last-Rev: a9868eb69dc587cb4579268b2c3ae46932702641
GitHub-Pull-Request: golang/tools#166
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197879
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When a new file is opened, the first time we learn about it will be a
didOpen event. We need to invalidate the package's metadata in that
case too.
Fixesgolang/go#35638.
Change-Id: I36c6171e9c959c48ede9e125679e8dd1be7609f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210559
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The control flow highlighting is taking precedence when
you are highlighting a key:value expression within the return statement.
Expected behavior is to just highlight all instances of the key or value and ignore
the control flow statement when inside the scope.
Fixesgolang/go#36057
Change-Id: If4b254151c38d152f337833c55a456f8dce18be7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210558
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove the unused code that was tracking concrete-type =>
interface-type mappings. It isn't clear if there is a good spot for
this in LSP.
I also made it skip interface types when looking for implementations.
It doesn't seem useful to be shown other interface types/methods when
you are looking for implementations of a given interface type/method.
Change-Id: Ib59fb717e5c1a181cc713581a22e60ed654b918c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210279
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
- Add test count to golden file so test count gets checked.
- Make @implementation note take a list of marks similar to completion
tests.
- Get rid of unnecessary intermediate test data type.
Change-Id: I741eb14b77b0b8ed08e86c634ed39457116e8718
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210278
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I had mistakenly forgotten to return a snapshot along with the view.
Fixesgolang/go#36020
Change-Id: I1fc802b8924fccec1d6aaa110640eaed490c3aa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210215
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the common case that a file has imports, we're going to diff just the
import block. That means that ApplyFixes doesn't need to produce the
whole formatted file, which is a huge speedup. We will do more work twice
for files with no imports, but those are presumably pretty short.
Hat tip to Muir for pointing towards this in
https://go-review.googlesource.com/c/tools/+/209579/2/internal/imports/imports.go#87
even if I didn't catch it.
Updates golang/go#36001.
Change-Id: Ibbeb4d88c6505eac26a36994de514813606c8c79
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210200
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Building unimported completions requires re-parsing and formatting at least
some of the file for each one, which adds up. Limit it to 20; I expect
people will just type more rather than scroll through a giant list.
Updates golang/go#36001.
Change-Id: Ib41232b91c327d4b824e6176e30306abf356f5b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210198
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Previously, (*IdentifierInfo).References was returning the declaration
of the identifier among the reference results. This change alters the
behavior of this function to only ever return non-declaration
references. Declarations can be accessed through the
IdentifierInfo.Declaration field.
Fixesgolang/go#36007
Change-Id: I91d82b7e6d0d51a2468d3df67f666834d2905250
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210238
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change will provide a more useful error when you
are self importing a package. It has TODOs in place to propagate the
"import cycle not allowed" error from go list to the user.
Updates golang/go#33085
Change-Id: Ia868a7c688b0f0a7a9689cfda5ea8cea8ae1faff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209857
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The invalidateContent function does not acquire a snapshot's mutex to
avoid blocking other work (even though it probably should since it's
only called after a context is canceled). A case was added to iterate
through files when a file is created, and it did not respect the fact
that the snapshot's mutex was not locked, resulting in a concurrent map
read and write. This change makes sure that the access of the snapshot's
files map is guarded by a mutex.
As a follow-up, we should just acquire snapshot.mu in invalidateContent.
Updates golang/go#36006
Change-Id: Idd904ae582055ce786062df50875ac7f0896fd1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210199
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We don't distinguish between genuine errors and context cancellation in
diagnostics, which often results in superfluous logging of these errors.
Avoid spamming the logs with them by checking.
Also, remove the logic for sending undelivered diagnostics. It's a relic
of old bugs and isn't useful.
Change-Id: I7df226539b9b1eb49ab3aae8d7b0a67f59fb3058
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Apparently I should've had staticcheck on. We were only reading the
metadata in updateMetadata to calculate unused imports, but that's now
at a higher level.
Change-Id: Id3d54fa736062bbbf1c207b8739e87ed5a90293d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210078
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A long time ago I only fixed golden generation for lsp/source. Get lsp/
and lsp/cmd too.
We have import tests that aren't formatted correctly, so we can't use
goimports to generate goldens. Just trust got.
Change-Id: If924503c0c0f6c60cd31fce194a8c1216001035b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209981
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This adds a link to documentation to the hover contents for the
current symbol if it is exported.
Updates golang/go#34240
Change-Id: I19c66e91e46f79284bfd0006c53f518eda4edef7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200604
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change is the first step in reorganizing the logical flow of file
changes in the internal/lsp package. A new function,
(*server).didModifyFile does the bulk of the work, but we will be able
to push most of its details into the cache layer in a follow-up.
Also, some refactoring of the applyChanges function to flow more
logically. It was unnecessarily convoluted.
Change-Id: Icc1b8642a4cb04d309338b0f8840fe58133d3df1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209979
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously, we returned CheckPackageHandles when creating a new view.
Now, return the view's snapshot. Also, add a WorkspacePackageIDs
function in order to run diagnostics on them.
Fixesgolang/go#35548
Change-Id: Ica7e41f5a7aa3ee9413feb4de4ee16e1825db2e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209418
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We weren't maintaining our ancestor node list correctly. This caused
us to fail to make AST repairs in certain cases. Now we are careful to
always append to the ancestors list when recursing.
Updates golang/go#34332.
Change-Id: I9b51ec70572170d9f592060d264c98b1f9720fb8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209966
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
"x/tools/gopls" appears to be the currently used prefix for gopls
issues. Make "gopls bug" use this prefix instead of just "gopls" to
avoid needing to edit titles before/after submitting.
Change-Id: I7244aa5539332cc361870f49ae4f27b2a2441571
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209964
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We had previously been ignoring many errors in
textDocument/documentSymbols, which led to errors appearing in the VS
Code extension logs (see
https://github.com/microsoft/vscode-go/issues/2919 for more context). We
should return errors so that we can more easily debug these issues in
gopls directly.
Change-Id: Ieef7c9f0bc8296f7e12d8c84e60d8b978d311651
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209858
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously, we would reload if a user's import list decreased or simply
changed order. This is not necessary. Now, we only re-run if a new import
needs to be loaded.
Updates golang/go#35388
Change-Id: I47874afe773dddb835ac27b18895e7a082950dc7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209057
Reviewed-by: Heschi Kreinick <heschi@google.com>
When searching for implementations we look at all packages in the
workspace. We do a full parse since we need to look for non-exported
types and look in functions for type declarations. However, we always
type check a package's dependencies in export-only mode to save work.
This leads to what I call the "two world" syndrome where you have both
the export-only and full-parse versions of a package in play at once.
This is problematic because mirror objects in each version do not
compare equal.
For example:
-- a/a.go --
package a
type Breed int
const Mutt Breed = 0
type Dog interface{ Breed() Breed }
-- b/b.go --
package b
import "a"
type dog struct{}
func (dog) Breed() a.Breed { return a.Mutt }
---
In this situation, the problem is "b" loads its dependency "a" in
export only mode so it gets one version of the "a.Breed" type. The
user opens package "a" directly so it gets fully type checked and has
a second version of "a.Breed". The user searches for "a.Dog"
implementations, but "b.dog" does not implement the fully-loaded
"a.Dog" because it returns the export-only version of the "a.Breed"
type.
Fix it by always loading in-workspace dependencies in full parse mode.
We need to load them in full parse mode anyway if the user does find
references or find implementations.
In writing a test I fixed an incorrect import in the testdata. This
uncovered an unrelated bug which made a different implementation test
very flaky. I disabled it for now since I couldn't see a fix simple
enough to slip into this commit.
Fixesgolang/go#35857.
Change-Id: I01509f57d54d593e62c895c7ecb93eb5f780bec7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209759
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Lack of context in error messages is making my life difficult. Add
context to a few, refactoring out some duplicate code along the way.
Change-Id: I3a940b12ec7c82b1ae1fc477694a2b8b45f6ff71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209860
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
As far as I can tell, the code I removed in from load did roughly
nothing -- returning nil metadata didn't suppress type checking as I
think was intended. Throwing away the metadata also created the race in
Pull the check for missing import changes up to PackageHandles, where it
is non-racy and can cause type checking to be skipped. Simplify and
refactor.
Fixesgolang/go#35951.
Change-Id: Id4b32b86569afb36863aaf982616b2b3727b0e83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209737
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"
"Fixes golang/go#33505"
Corrects the regexp definition
Change-Id: I587d611f26a3f3c5ea89eda7b2c3ccf369e8bb2f
GitHub-Last-Rev: 740ffca04dd16b36a96f03781d58ff727e39ae79
GitHub-Pull-Request: golang/tools#154
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194661
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Sometimes the prefix of the thing you want to complete is a keyword.
For example:
variance := 123
fmt.Println(var<>)
In this case the parser produces an *ast.BadExpr which breaks
completion. We now repair this BadExpr by replacing it with
an *ast.Ident named "var".
We also repair empty decls using a similar approach. This fixes cases
like:
var typeName string
type<> // want to complete to "typeName"
We also fix accidental keywords in selectors, such as:
foo.var<>
The parser produces a phantom "_" in place of the keyword, so we swap
it back for an *ast.Ident named "var".
In general, though, accidental keywords wreak havoc on the AST so we
can only do so much. There are still many cases where a keyword prefix
breaks completion. Perhaps in the future the parser can be
cursor/in-progress-edit aware and turn accidental keywords into
identifiers.
Fixesgolang/go#34332.
PS I tweaked nodeContains() to include n.End() to fix a test failure
against tip related to a change to go/parser. When a syntax error is
present, an *ast.BlockStmt's End() is now set to the block's final
statement's End() (earlier than what it used to be). In order for the
cursor pos to test "inside" the block in this case I had to relax the
End() comparison.
Change-Id: Ib45952cf086cc974f1578298df3dd12829344faa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209438
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Added a check to make sure that highlighting the control flow of
a function only continues if the cursor is actually in a function.
Change-Id: Idac90d9e55c09c3dcb9ae938585157658acc95e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209581
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When the cursor is on a return statement or in the function declaration
it will highlight the control flow for the function. It will also highlight
individual fields and results if the cursor is specifically in one.
Fixes#34496
Change-Id: I71d460cd174a8fbc61d119b9633c3c3ecbde2af9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208267
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The new code generates all 3 files (tsprotocol.go, tsserver.go, tsclient.go)
at once and tries not to generate unneeded types. This is the code that
generated the checked-in files currently being used (including CL208272
and 209219).
README.md has been modified correspondingly.
Change-Id: I719781a09f27bf0a426f8da3e45f7fa2eb0a7484
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208665
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We were previously returning errors when we failed to load/check a
user's workspace folder, but now we suppress all errors. We shouldn't
disable gopls functionality if something is broken in a user's workspace
folder, rather, we should fall back to the file= queries that will run
when a user edits a file.
Change-Id: Iae05174ca80d2573c0222ac42f29e5556bda0134
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209420
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is causing the "command '' not found" errors that we've been
seeing, specifically reported in microsoft/vscode-go#2920.
Also, fixed an issue with import organization in single-line files that
was caught as a result of this.
Change-Id: I2dfedb5d1b8dda976f356b0d6fcd146e53f1a650
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209219
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 208272 made one occurrence of RenameProvider an interface{}.
This CL reflects the effect of making that change in the code
generator. (That is, the other occurrence of RenameProvider is now
an interface{} too.) gopls seems to still work, and all tests pass.
Change-Id: Icfc4a5639192c46b564509a601b8c03bbe2665a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209158
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We've had a couple of breakages with changes to the protocol that are
bugs on our end. It's hard to review changes to the protocol, and it's
not reasonable to expect that we would remember the correct types for
everything, so we should have a test that validates some basic expectations
about the expected responses. We can add more here as issues come up.
Also, change RenameProvider back to an interface.
Updates golang/go#32703
Change-Id: Ic5f0b0ece40b05e4425cd98ab7bf18db3ad74601
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208272
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Find implementations sometimes returns no results, as it prematurely returns when it
finds an invalid object. Instead the behavior should be to check all the objects in case
a later object is a valid interface.
Fixes#35602
Change-Id: I0e3e2aa8d3afeaa34e392c2fe3ef8cdcd13b3d1e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208959
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Finding implementations adds the same implementation multiple times, this commit
removes the duplicates and ensures that only one instance of each implementation
gets returned. Also moves the sorting of results to the test file to ensure that
the tests are deterministic.
Fixes#35600
Change-Id: I244d36a46b7e31bf3c1f845e241239de05d45e6f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208668
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds command line support for highlight.
Provided with an identifier position, it will display
the list of highlights for that within the same file.
Example:
$ gopls highlight ~/tmp/foo/main.go:3:9
$
$ 3:9-6:0
$ 10:22-11:32
$ 12:10-12:9
$ 12:20-30:0
Updates golang/go#32875
Change-Id: I5de73d9fbd9bcc59a3f62e7e9a1331bc3866bc75
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207291
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Running staticcheck on the entire workspace causes a slowdown, and most
likely users don't want to see staticcheck reports for every
subdirectory of their workspace. Only run staticcheck on open files.
Also, fixed a staticcheck warning that showed up along the way. Filed
golang/go#35718 to remind ourselves to fix all of the staticcheck warnings
that showed up when we ran gopls with staticcheck on x/tools.
Finally, made sure that we don't send empty diagnostics when diagnosing
the snapshot on start-up, as that is not necessary.
Change-Id: Ic51d1abfc80b1b53397057f06a4cfd7e2dc930f9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The import formatting code tries to extend the range it's diffing to
the next line after the last import so that it's working with whole
lines. Make sure that the next line actually exists before trying to use
it.
Fixesgolang/go#35604.
Change-Id: I18fe61843aa11e62ed311a9ddff62ff876888a15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208672
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Finding implementations of an interface should not give the interface itself
as an implementation.
Fixes#35601
Change-Id: Id3352619ece90fb7ca906ddabca613742d156c76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208667
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Expose ImportPathToAssumedName (internally) and use it in an LSP
completion case that doesn't go through the usual imports code.
Fixesgolang/go#35401.
Change-Id: If87912072e11e22c542f7474841e53467a33ef2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206890
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The early return logic for didOpen events in
(*snapshot).invalidateContent was preventing the creation of a new
snapshot, which was in turn stopping the versions from being updated.
This exposed a fundamental issue in the way we were calculating
workspace diagnostics. Since we weren't waiting for diagnostics to be
completed for an entire snapshot before replying that the server had
been initialized, snapshots were being cloned without any type
information. For quickfix code actions, we assume that we have all
information cached (since we need to have sent the diagnostics that the
quickfix is mapped to), so we were not finding the cached analysis
results.
To handle this in the short-term, we key analyses by their names, and
then regenerate results as-needed for code actions. This is technically
more correct than simply assuming that we have the analyses cached. In a
follow-up CL, I will send a follow-up that will make sure that
snapshots "wait" on each other to be fully constructed before being
cloned.
Change-Id: Ie89fcdb438b6b8b675f87335561bf47b768641ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208265
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We now have pretty good support for users of cgo packages. Add tests.
Closesgolang/go#35720.
Change-Id: Icdc596038bc6fca1c08eacd199def12264cf512d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208503
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When packages.Load'ing cgo packages, the authored files show up in
GoFiles, and the generated files show up in CompiledGoFiles. We need the
AST and type information for the latter, since they're the only thing we
can type check. But we also need the contents (and column mapper) for
the authored file so that we can navigate into it.
Store GoFiles in package metadata and checked Packages. Parse the extra
files, just for their mappers. Refactor the View functions a little bit,
since there's only one place that actually needs to find the mapper for
a file.
Updates golang/go#35720.
Change-Id: I9f96872a9a592bf0e11da27ebd8976c6db8752c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208502
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When //line directives are in play, the ast.File's Offset function will
return offsets in the generated file. We want offsets in the authored
file, so we need to pass a Converter for the authored file, in addition
to the ast.File for the generated file. For the same reason, we have to
start (Range).Span() by translating into positions in the authored file,
then calculate offsets from that.
A lot of call sites outside of the LSP don't pass the Converter, but
they probably don't matter much. I think everything inside does because
it ends up using mappedRange.
Updates golang/go#35720.
Change-Id: I7be09b3a50720b078e862d48cfdb02208f8187ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208501
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In cases like:
var foo []bytes.Buffer
foo = append(foo, <>)
you will now get a literal candidate "bytes.Buffer{}". Previously we
were skipping all literal candidates at the variadic position, but the
intention was to only skip literal slice candidates (i.e.
"[]bytes.Buffer{}" in the above example).
I also improved the literal struct snippet to not leave the cursor
inside the curlies when the struct type has no accessible fields.
Previously it was only checking if the struct had no fields at all.
This means after completing in the above example you will end up with
"bytes.Buffer{}<>" instead of "bytes.Buffer{<>}", where "<>" denotes
the cursor.
Change-Id: Ic2604a4ea65d84ad855ad6e6d98b8ab76eb08d77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Type aliases don't work well with types.TypeString. Work around that by
using the AST to build this information. Follow up from CL 201677.
Fixesgolang/go#33500
Change-Id: I8b2d4ea238eb5d284a419f2b0bbf9655e69d434d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I'm not sure why this was being managed by the view, but delete the code
that handles tracking a file's first change. It is only used to avoid
spamming the user with error messages.
Change-Id: Id95089478ffb7e189d38cbc147e3dde6a1c55c5e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208274
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Right now, we request analyses for files in ParseExported mode, which
doesn't actually produce any meaningful facts. Disable it until we
resolvegolang/go#35089, since right now, all this is doing is wasting
memory and CPU.
Change-Id: I6ffb7bdf6c915159b55753b51289cef4bd937603
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208270
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
