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>
Instead of making all completion candidates look like perfect matches
to VSCode, we now make all candidates match exactly like the first
candidate. This still causes VSCode to maintain the ordering from
gopls, but it fixes the absolute match score of gopls's top
candidates. This fixes interplay with other sources of VSCode
completions, such as user defined snippets.
Fixesgolang/go#35782.
Change-Id: Ie7e489b76a02fb1353b63fa3c6fa42afee2e1441
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208439
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
except the race was a symptom of a larger problem, so the fix
actually invovles cleaning up the way we run command line tests
totally to have common shared infrastructure, and also to clean up
the way we handle errors and paths into the temporary directory
Fixes: golang/go#35436
Change-Id: I4c5602607bb70e082056132baa3d4b0f8df6b13b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208269
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change modifies the behavior of the GetReverseDependencies function
used for diagnostics. Since we now return diagnostics for the entire
workspace, we don't have to worry if a file is open to show errors in
it. This change requires the addition of a new (*snapshot).PackageHandle
function that gets a CheckPackageHandle for a given package ID. This
function does not cause a re-load of the package metadata, though if we
feel that this is something we need in the future we can add it.
Change-Id: I863bdf284d15f2317d8fae395928a90b9455146b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208102
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Since diagnostics are published with the URI separately, there's no need
for us to keep the FileIdentity around in two places.
Change-Id: I5724b9582e5eee49f66fcf9f08625f14a69e3fc0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208263
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When the cursor is on a "for" statement or on any branch statement inside
the for loop. It will highlight the control flow inside the for loop.
Updates #34496
Change-Id: Idef14e3c89bc161d305d4a49fd784095a93bbc03
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208337
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change uses the FileIdentity when reporting an error message, so
that the version number can be propagated to through the
publishDiagnostics notification.
Change-Id: I6a2103e304717ca09895008ea40336e3ace3c66d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208260
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now that we run diagnostics on the entire workspace, we don't need to
hide diagnostics when a file has been closed.
Change-Id: I98d43820ff2ec2f9eb66bb4a1b6e59372ba7fc27
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208237
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I introduced a bug in the way that diagnostics are computed after a
didChange event in CL 208099. This will fix it. Whoever sees this first
tomorrow, feel free to LGTM and submit this CL.
Change-Id: I324c1185df456c2c5c2423fc7322b959e0117218
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208262
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
This change cleans up internal/lsp/source/view.go to have a more logical
ordering and deletes the view.CheckPackageHandle function. Now, the only
way to get a CheckPackageHandle is through a snapshot (so all of the
corresponding edits).
Also, renamed fuzzy tests to fuzzymatch. Noticed this weird error when
debugging - I had golang.org/x/tools/internal/lsp/fuzzy in my module
cache and it conflicted with the test version.
Change-Id: Ib87836796a8e76e6b6ed1306c2a93e9a5db91cce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
As we improve support for cgo we'll need to reference GoFiles, not just
CompiledGoFiles. "Files" is right out.
I think I got everything that needs renaming but please let me know if
not.
Updates golang/go#35720.
Change-Id: I97a6ebf5b395535de0d5f4f8b3f84b46ca34643f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208101
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When line directives are in use, we want the logical file name, not the
one we found the bytes in. This matters most for cgo, where the file we
parsed is not the one the user wants to see.
Updates golang/go#35720.
Change-Id: I495328071d8865e6895cb731467f1601f11e93db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208100
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
None of the godef tests were running due to a mistake in the test
harness code. Fix them and re-enable.
We decided that the range for an import statement should be the whole
import path, not just the first character, so make that change and
adjust the PrepareRename tests accordingly.
Change-Id: I45756a78f2a1beb3c5180b5f288ce078075624bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207900
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Change 207598 overenthusiastically (and incorrectly) changed the Range field
in a TextDocumentContentChangeEvent from type *Range to type Range,
which created a bug in text_synchronization.go.
This CL attempts to repair the damage.
Change-Id: I19e7418cd5ebaedf5448adfcc60ca86e71eb9b2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208097
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Modified the way highlights are tested to allow for author to explicitly
mark the matches. Also added highlighting for fields and methods. Used
type checking in addition to ast to get better matching. Worked with
@stamblerre
Updates #34496
Change-Id: I462703e0011c4e0a4b98016e9c25af9bf1ead0b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207899
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change runs diagnostics on all packages in the workspace, instead
of just open files. We also want to avoid invalidating the type
information for a newly-opened file (since we should have it be default
now), so handle that case.
This causes a large increase in memory usage in the
internal/lsp/cmd tests, so to handle that, share an app between all of
the tests, rather than creating one per-test type.
Change-Id: Ifba18d77a700cda79ec79f66174de0e7f13fe319
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207353
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
textDocument/didChange events need to indicate if the change includes
the full file content or just a diff. Previously, the
contentChange.Range field was a pointer, so if it was nil, then we would
conclude that the file change was for the whole file. Now, the best we
can do is compare it to an empty range, but this still doesn't work if
you are at the beginning of a file. I think that the range needs to be a
pointer for this to work correctly.
Also, some minor changes that came up along the way while debugging:
(1) Don't close over the *cache variable for fear of pinning anything in
memory
(2) Improve the error message when the token.File is nil
(3) Check for a nil token.File earlier
Change-Id: If9f310e92b7fb740b45e6cd3f9ca678a6fb52ff6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207906
Reviewed-by: Heschi Kreinick <heschi@google.com>
Rather than copying this package to another repository, let's promote
this one out of internal.
Change-Id: I6f9cc1ada1577a720905271f7471c3afe05a2b41
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207905
Reviewed-by: Ian Cottrell <iancottrell@google.com>
we only construct a cache as we build a server, rather than for each instance
of Application now.
Change-Id: Ic18966906f8f61b18b71fc6d6f7ccc8e9cebbd29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207904
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Paul Jolly observes that returning interface{} is not helpful.
Now CodeAction() returns []CodeAction.
The type in typescript is (Command | CodeAction)[] | null
but the choice is up to gopls, which returns []CodeAction.
Fixesgolang/go#35688, golang/go#35679
Change-Id: I91c22bb0752431954ae2f993cb7b44726cf60e5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207898
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Quick fix to (*server).CancelRequest, but we haven't implemented actual
support for it. Filed golang/go#35679 to track this.
Change-Id: Ic0de01d49b779c4f0656587584fbd2bf8791d0ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207718
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new corrected type is map[string][]TextEdit. The old type
was incorrectly struct{}.
Change-Id: I3cb64eee90c5281b3fb40e543026cd308c55c49a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207717
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Code generation has been unified, so that tsprotocol.go and tsserver.go
are produced by the same program. tsprotocol.go is about 900 lines shorter,
partly from removing boilerplate comments that golint no longer requires.
(And partly by generating fewer unneeded types.)
The choice made for a union type is commented with the set of types. There
is no Go equivalent for union types, but making themn all interface{}
would replace type checking at unmarshalling with checking runtime
conversions.
Intersection types (A&B) are sometimes embedded (struct{A;B;}, and
sometimes expanded, as they have to be if A and B have fields with the
same names.
There are fewer embedded structs, which had been verbose and confusing to
initialize. They have been replaced by types whose names end in Gn.
Essentially all the generated *structs have been removed. This makes
no difference in what the client sends, and the server may send a {}
where it previously might have sent nothing. The benefit is that some
nil tests can be removed. Thus 'omitempty' in json tags is just
documentation that the element is optional in the protocol.
The files that generate this code will be submitted later, but soon.
Change-Id: I52b997d9c58de3d733fc8c6ce061e47ce2bdb100
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207598
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In cases like:
var foo []io.Writer
var buf *bytes.Buffer
foo = append(foo, <>)
we weren't giving "buf" a good score. When comparing the candidate
type *bytes.Buffer to the (variadic) expected type []io.Writer we were
turning the candidate type into []*bytes.Buffer. However, of course,
[]*bytes.Buffer is not assignable to []io.Writer, so the types didn't
match. Now we instead turn the expected type []io.Writer into
io.Writer and compare to *bytes.Buffer.
I fixed the @rank test note to check that the candidates' scores are
strictly decreasing. Previously it would allow candidates with the
same score if they happened to be in the right order. This made it
easier to right a test for this issue, but also uncovered an issue
with untyped completion logic. I fixed it to do the untyped constant
check if _either_ the expected or candidate type is
untyped (previously it required the candidate type to be untyped).
Fixesgolang/go#35625.
Change-Id: I9a837d6a781669cb7a2f1d6d3d7f360c85be49eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207518
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rather than panicking when we have not created any views for the packages,
we should show a reasonable error to the user. This change propagates the
errors to the user.
Updates golang/go#35599
Change-Id: I49789d8ce18e154f111bc3584488f468a129e30c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207344
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
go/parser.ParseFile can return both an AST and errors. We should still
be able to do import organization even if the AST contains errors, as
long as they are below the portion of the file that contains the import
block.
Change-Id: Id6b86171fca3e15d02910d1c6f4ce25e803754d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207261
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When looking for references, look in the entire workspace rather than
the same package. This makes the references query more expensive because
it needs to look at every package in the workspace, but hopefully
it shouln't be user-noticable. This can be made more efficient by only
checking packages that are transitive reverse dependencies. I don't think a
mechanism to get all transitive reverse dependencies exists yet.
One of the references test have been changed: it looked up references
of the builtin int type, but now there are so many refererences that
the test too slow and doesn't make sense any more. Instead look up
references of the type "i" in that file.
Change-Id: I93b3bd3795386f06ce488e76e6c7c8c1b1074e22
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206883
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Instead of using the entire import node as the range for the
link, use only the link text in the path node itself. This looks
better when using a _ or named import, as well as constraining
the link to inside the quotes.
Fixesgolang/go#35565
Change-Id: Ie93d9df993fbd8e0106ca6c3b40e0885355be66b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207137
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
This change is the first step in centralizing control of modifications
to different files, either within the workspace or outside of it. We add
a source.FileAction type to pass into the internal/lsp/cache package and
handle the difference between opening and creating a file.
Now that we load all packages in a workspace by default, we no longer
need to re-load a file on open. This CL should enable CL 206883 to work
correctly.
Change-Id: I2ddb21ca2dd33720d668066e73283f5629d02867
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206888
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change propagates the versions sent by the client to the overlay
so that they can be used when sending text edits for code actions and
renames.
Fixesgolang/go#35243
Change-Id: I8d1eb86fe9f666f7aa287be5026b176b46712c97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205863
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Add more detailed instructions for installing and running node
and the typescript compiler on Linux and MacOS.
Change-Id: Id549760dd186d88cfa9b137e6f46dfd4d60d6322
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206887
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change makes sure that we only return files that contain the given
position. There are a few instances of needing to look up files by URI
in the internal/lsp/cache package, so use an unexported package for
that. This allows us to remove some code in the implementations code.
Change-Id: Ifa7a62c67271826e6c632e4c88667d60f8b760c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206880
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change adds support for returning versions along with file URIs, so
that the client can know when to apply changes. The version is not yet
propagated along to the internal/lsp/cache package, so this change will
have no effect (VS Code ignores a version of 0 and still applies the
changes).
A few minor changes made in the rename code (to remove the view
parameter). Some minor staticcheck fixes.
Updates golang/go#35243
Change-Id: Icc26bd9d9e5703c699f555424b94034c97b01d6f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206882
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
If a user is typing fast, they will quickly invalidate many snapshots.
We don't want to stack up a bunch of stale type check and analysis
operations, so we should propagate cancellation through the cache.
Handles are long-lived, so we may cancel an operation only to
restart it again later. Also, there may be multiple operations waiting on
the same computation, and just because one is cancelled doesn't mean we
should necessarily stop. The easiest way to support all that was to add
an explicit state to each handle, and track the number of waiters.
See the code for more details on Handle life cycles.
As far as I can tell, the rest of gopls is prepared for this behavior.
I added an explicit check to the type checking code, where I was worried
it might get overly confused. But long-term it would probably be good to
return an error from Get.
Change-Id: I3ea6e141b52b94300a41248d3f2e039b023709d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206879
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We used to need our own copy of astutil.AddNamedImport to use during
completion for a variety of reasons, but I think the major one was
needing to not format the whole file. The same problem applied to using
the imports package.
Happily, that was resolved in CL 205678. Now we can use the same
implementation on both paths. In addition to removing a bunch of code,
that means that unimported completions now add their imports in the
right place, respecting goimports grouping and the local configuration
setting.
Fixesgolang/go#35519.
Change-Id: I693c2e8b5ced9bac62b1febf1e2db23c770e5a7a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206881
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Look in all packages the snapshot knows of (through a new method on snapshot called
KnownPackages) and see if any of those packages contain implementations. Before,
the Implementation call only looked in the current package.
Much of the new complexity in implementation.go is routing through the Type to
Package data in the implementsResult.pkg field so the identifier can be looked up
in its correct package.
Fixesgolang/go#32973
Change-Id: Ifa7115b300f52fb4fb55cc00db2e7f339e8c2582
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206518
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
It uses it to update the config of all active views cleanly
Fixesgolang/go#32258
Change-Id: I7a849941d5022499d48ad640c5b7bc9cf79eb9b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206148
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
It attempts to detect changes that would invalidate the view and replace itself
with a new view when that happens
Change-Id: I0f1a8cd3bd6ddcef115fedc6c57ae0398b16d3b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206147
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds command line support for foldingRange.
Provided with a file, it will display a list of folding
ranges within that file, with 1-indexed positions using
the format
{startingLine}:{startingChar}-{endingLine}:{endingChar}
Example:
$ gopls folding_ranges ~/tmp/foo/main.go
$
$ 3:9-6:0
$ 10:22-11:32
$ 12:10-12:9
$ 12:20-30:0
Updates golang/go#32875
Change-Id: Ib35cf26088736e7c35612d783c80be7ae41b6a70
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206158
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We don't want to return an error for the whole package when we are just
building out error positions.
Change-Id: I56b5b88ff2b4b44da8a372ade81cd9b1534235c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206597
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Even if the packages.Load of the directory the NewView is being created for
fails, create and add the view. But also return the error from NewView, just
after the new view has been added.
Fixesgolang/go#35468
Change-Id: I76c2d3cbe1a508ad0794a6fcd3bc67cd48c97e22
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206497
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change copies the code in guru's implements implementation
that finds implementations of methods over to gopls, and uses
the information determined to resolve implements requests on
methods. Implements still only works only within packages.
Updates golang/go#32973
Change-Id: I0bd7849a9224fbef7ab8385070b18fbb30703e2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206150
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Change tsprotocol.go so that the types are in alphabetical order, which
will make it simpler to see what has changed. (In this version only the
git hash and date have changed. The Go code has only been rearranged.)
The typescript code with this tiny change will be submitted in
another CL.
Change-Id: I7055b31075c7b3cda6e23b467205683423529c33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205499
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Add a special case for append() arguments so we infer the expected
type from the append() context. For example:
var foo []int
foo = append(<>)
We now infer the expected type at <> to be []int. We also support the
variadicity of append().
Change-Id: Ie0ef0007907fcb7992f9697cb90970ce4d9a66b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205606
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I assumed that f.Pos() would be the first byte of the file, but it's the
position of the package declaration. This kills the file. Just use 0.
Fixesgolang/go#35458.
Change-Id: Ic77c93344c71435ef8e5624c2f2defb619139a15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206145
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Treat it as okay if no packages are found when loading all the packages in a
workspace. Users may open workspaces that don't have any Go files, either because
they are workspaces for other languages, or because no Go files have been created
yet.
Fixesgolang/go#35455
Change-Id: I60912472ec8930649996edc150d1d19cd74a0a2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206140
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Previously, if we failed to find an item's documentation, we would not
return the item at all. It seems better to do a best-effort approach,
i.e. return the item without documentation.
Fixesgolang/go#35406
Change-Id: I896ffda2fc79b9210f0d83311808114d0e686820
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205862
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Add a source.Scope type that can be used to refer to directories or
files, and modify (*snapshot).load to take source.Scope.
Then call load in NewView.
Change-Id: I8f03c7b271d700b162100d2890d23219ef9578c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204822
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We want people to add imports as they need them. That means we probably
don't want adding an import to reformat your whole file while you're in
the middle of editing it.
Unfortunately, the AST package doesn't offer any help with this --
there's no good way to get a diff out of it. Instead, we apply the
changes, then diff a subset of the file. Picking that subset is tricky,
see the code for details.
Also delete a dead function, Imports, which should have been unused but
was still being called in tests.
Fixesgolang/go#30843.
Change-Id: I09a5344e910f65510003c4006ea5b11657922315
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205678
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Previously we were erroneously suggesting a "func() {}" literal in
cases like:
http.Handle("/", <>)
This was happening because saw that the http.HandlerFunc type
satisfied the http.Handler interface, and that http.HandlerFunc is a
function type. However, of course, you can't pass a function literal
to http.Handle().
Make a few tweaks to address the problem:
1. Don't suggest literal "func () {}" candidates if the expected type
is an interface type.
2. Suggest named function types that implement an interface. This
causes us to suggest "http.HandlerFunc()" in the above example.
3. Suggest a func literal candidate inside named function type
conversions. This will suggest "func() {}" when completing
"http.HandlerFunc(<>)".
This way the false positive func literal is gone, and you still get
literal candidates that help you use an http.HandlerFunc as an
http.Handler. Note that this particular example is not very compelling
in light of http.HandleFunc() which can take a func literal directly,
but such a convenience function may not exist in other analogous
situations.
Change-Id: Ia68097b9a5b8351921349340d18acd8876554691
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Improve candidate ranking when completing the variadic parameter of
function calls.
Using the example:
func foo(strs ...string) {}
- When completing foo(<>), we prefer candidates of type []string or
string (previously we only preferred []string).
- When completing foo("hi", <>), we prefer candidates of type
string (previously we preferred []string).
- When completing foo(<>), we use a snippet to add on the "..."
automatically to candidates of type []string.
I also fixed completion tests to work properly when you have multiple
notes referring to the same position. For example:
foo() //@rank(")", a, b),rank(")", a, c)
Previously the second "rank" was silently overwriting the first
because they both refer to the same ")".
Fixesgolang/go#34334.
Change-Id: I4f64be44a4ccbb533fb7682738c759cbca3a93cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205117
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In CL 205501 I thoughtlessly set import name to package name, but really
we only want to name imports when goimports would do it. For now, it's
better to not name them and let the usual imports code add a name if
necessary.
Fixesgolang/go#35397.
Change-Id: Id0df866f95e5e86ed72b25fbd1a7224c79ee8084
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205657
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add a new "verboseOutput" config flag (defaults to "false") to enable
verbose go/packages and imports output. Previously this output was
always present.
The go/packages output would dump out the entire (humongous) "go list"
JSON response which would lock up my editor for a second whenever
something triggered a go/packages call.
The imports output would produce a bunch of "gopathwalk" debug
messages that aren't useful in general and in particular add noisy
output to tests.
Change-Id: Ie4693d074cb84f1397e0e51d7346dc9391bd1278
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205138
Reviewed-by: Koichi Shiraishi <zchee.io@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Unimported completions now try to pull Packages from everywhere, not
just the transitive dependencies of the current package. That confused
the import formatting code, which only looked at deps. Pass the Package
along with the import suggestion, and use it when it's present.
Also change some error messages to be different for diagnostic purposes.
Fixesgolang/go#35359.
Change-Id: Ia8ca923e46723e855ddd2da7611e6eb13c02bb4f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205501
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The special handler was dropped in cl/191963 which moved the logging functionality, but
was still needed for the rpc metrics
Change-Id: I494ef47646fe0d705709694378dbc981b549622a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205164
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There was a dealock introduced in cl/190737 on all the internal structure
debug pages.
The object getters all protect with the mutex already, it should not also
be done in the outer Render function
Change-Id: I5c85dc8e2ec489e59ca5a80128f2649dd7753983
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205165
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Loading completion suggestions can be slow, especially in GOPATH mode
where basically anything can change at any time. As a compromise, cache
everything for 30 seconds. Specifically, after a completion operation
finishes, if the cache is more than 30 seconds old, refresh it
asynchronously. That keeps user-facing latency consistent, without
chewing up CPU when the editor isn't in use. It does mean that if you
walk away for an hour and come back, the first completion may be stale.
In module mode this is relatively benign. The only things the
longer caching affects are the main module and replace targets, and
relevant packages in those will generally be loaded by gopls, so they'll
have full, up-to-date type information regardless.
In GOPATH mode this may be more troublesome, since it affects
everything. In particular, go get -u of a package that isn't imported
yet won't be reflected until the cache period expires. I think that's a
rare enough case not to worry about.
Change-Id: Iaadfd0ff647cda2b1dcdead9254b5492b397e86e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205163
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Packages that aren't imported in the current file will often have been
used elsewhere, which means that gopls will have their type information
available. Expose loaded packages in the Snapshot, and try to use that
information when possible for unimported packages.
Change-Id: Icb672618a9f9ec31b9796f0c5da56ed3d2b38aa7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204824
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When a user completes rand.<>, propose rand.Seed (from math/rand) and
rand.Prime (from crypto/rand), etc.
Because we don't necessarily have type checking information for
unimported packages, I had to add shortcut cases to a number of
functions around the completion code. Better suggestions welcome.
Change-Id: I7822dc75c86b24156963e7bdd959443f4f2748b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204819
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Muir Manders <muir@mnd.rs>
The metadata was being added to the cache before it was fully computed.
Change-Id: I6931476a715f0383f7739fa4e950dcaa6cbec4fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204562
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This way, we can tell if someone is building gopls at head, or if they
are using a tagged version. We should only change the version string on
a release branch.
Change-Id: I55eba534baa2e171315fe72242a400e2d2794314
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205159
Reviewed-by: Ian Cottrell <iancottrell@google.com>
A lot has changed since golang/go#32794 was filed, and we now have many more
tests for the command line.
Fixesgolang/go#32794
Change-Id: Ib268865a2345fd6676b2679bd76197c2d8658a85
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204818
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Some analyses may panic, so we should be careful to recover.
Change-Id: Ie13df07aca1a21f4e6a66648cd4890b6a31966cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
If the identifier doesn't have type info, don't try to access it.
Change-Id: I7e7c834c2863ec82a1269e60f7c50b0ce03f4692
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204740
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds command line support for symbols.
Symbols are formatted as '{name} {type} {range}', with
children being preceded by a \t.
Example:
$ gopls symbols ~/tmp/foo/main.go
$
$ x Variable 7:5-7:6
$ y Constant 9:7-9:8
$ Quux Struct 29:6-29:10
$ Do Method 37:16-37:18
$ X Field 30:2-30:3
$ Y Field 30:5-30:6
Updates golang/go#32875
Change-Id: I1272fce733fb12b67e3d6fb948f5bf3de4ca2ca1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203609
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This adds support for calling links from the gopls command line,
e.g.
$ gopls links ~/tmp/foo/main.go
Optional arguments are:
-json, which emits range and uri in JSON
With no arguments, a unique list of links are emitted.
Updates golang/go#32875
Change-Id: I1e7cbf00a636c05ccf21bd544d9a5b7742d5d70b
GitHub-Last-Rev: 7ed1e4612186bce4077d3c73f2407cf6def211d9
GitHub-Pull-Request: golang/tools#181
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203297
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The approach of using ASTs to determine error ranges had more
complications than I anticipated. Revert it for now to go back to the
old user experience, while we consider a better approach.
Change-Id: I5b23f0147c26089330f8a4bbf7b6914ae66cf59a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204561
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change stops the memory leak from happening. It does not stop the
large number of allocations done by analysis, however, so these still
need to be prevented through correct cancellation.
Change-Id: I1cf7fbf6f85ed413af1f2ea55f91862a6d7f2db7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204558
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change adds command line support for signatureHelp.
If the location provided corresponds to a function, that
function signature is displayed. In case that function is
documented the related comment is shown as well.
Example:
$ gopls signature ~/tmp/foo/main.go:7:5
$
$ Next(n int) []byte
$
$ Next returns a slice containing the next n bytes from
$ the buffer, advancing the buffer as if the bytes had been
$ returned by Read.
Note that linebreaks shown in the comment are just to adhere
commit message guidelines. The command prints documentation
comments on one line.
Updates golang/go#32875
Change-Id: Ib0dcc3267c594f95d80b74f289c1235c2c0c5f64
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204057
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds support for the LSP implemention call, based
on the guru code for getting implementations. The guru code
did much more than we need, so some of the code has been
dropped, and other parts of it are ignored (for now).
Fixesgolang/go#32973
Change-Id: I1a24450e17d5364f25c4b4120be5320b13ac822b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203918
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This adds support for calling suggestedfix from the gopls command line, e.g.
$ gopls suggestedfix ~/tmp/foo/main.go
Optional arguments are:
-w, which writes the changes back to the original file; and
-d, which prints a unified diff to stdout
With no arguments, the changed files are printed to stdout.
Wasn't sure if the command should be `suggestedfix` or just `fix` or `quickfix`?
Also this applies all changes to a file, does not allow for selective fixes.
Updates golang/go#32875
Change-Id: I8b75f9824be82974f6edb7c03383b4d56116943c
GitHub-Last-Rev: 070fcda33ac3494bfe8f19c2cd78c089c713ed98
GitHub-Pull-Request: golang/tools#174
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202480
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When proposing packages to import, we can propose more relevant packages
first. Introduce that concept to the pkg struct, and sort by it when
returning candidates.
In all cases we prefer stdlib packages first. Then, in module mode, we
prefer packages that are in the module's dependencies over those that
aren't. We could go further and prefer direct deps over indirect too,
but I didn't have the code for that handy.
I also changed the alphabetical sort from import path to package name,
because that's what the user sees first in the UI.
Updates golang/go#31906
Change-Id: Ia981ee9ffe3202e2a68eef3a36f65e81849a4ac2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204203
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
go/parser has switched from reporting no position for the end of a
broken file to reporting an invalid position. This broke on of our tests
that contains broken code. Change the test case as a result.
Change-Id: I4feb7790539994e593c56d5ae84929364c1eec1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204202
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When our expected type is a named type from another package, we now always
search that other package for completion candidates, even if it is not currently
imported.
Consider the example:
-- foo.go --
import "context"
func doSomething(ctx context.Context) {}
-- bar.go--
doSomething(<>)
"bar.go" doesn't import "context" yet, so normally you need to first import
"context" through whatever means before you get completion items from "context".
Now we notice that the expected type's package hasn't been imported yet and give
deep completions from "context".
Another use case is with literal completions. Consider:
-- foo.go --
import "bytes"
func doSomething(buf *bytes.Buffer) {}
-- bar.go--
doSomething(<>)
Now you will get a literal completion for "&bytes.Buffer{}" in "bar.go" even
though it hasn't imported "bytes" yet.
I had to pipe the import info around a bunch of places so the import is added
automatically for deep completions and literal completions.
Change-Id: Ie86af2aa64ee235038957c1eecf042f7ec2b329b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201207
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Closing over the checkPackageHandle creates a cycle that forces the
checkPackageHandle not to be garbage collected until the value is
created. If a value is never created, the handle will not be collected.
Change-Id: I0f94557da917330ebe307a0e843b16ca7382e210
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204079
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change encompasses the refactorings needed to correctly implement
CL 204079. The goal of this CL is to make the actual relevant diffs more
clear.
Change-Id: I38acfd436e2380be790910e01b6e37d8280e9100
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204139
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This adds support for calling import from the gopls command line,
e.g.
$ gopls imports -w ~/tmp/foo/main.go
Optional arguments are:
-w, which writes the changes back to the original file; and
-d, which prints a unified diff to stdout
With no arguments, the changed file is printed to stdout.
Updates golang/go#32875
Change-Id: I12f980d977fe12c16e51b024c9dd28c33ba6c002
GitHub-Last-Rev: c3fdd90e25204e7a12a94e9dfde389b7674e7e6d
GitHub-Pull-Request: golang/tools#176
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202624
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change eliminates the need for the importer struct. We should no
longer need the "seen" map for cycle detection. This is because
go/packages will not return import maps with cycles, and we fail in the
Import function if we see an import we do not recognize.
Change-Id: I06922c74e07eb47ce63b56fa2ac2099e7fc8bd8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This adds (or makes exported) a convenience function for reporting diagnostics with a
node directly (which is what folks usually want).
Change-Id: Ieb7ef2703f99d3a24ba7e48a779be62a7761cd0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/180237
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In cases like:
type myInt int
const (
a = 1
b myInt = 2
)
var foo myInt = <>
We now prefer "b" over "a" since b's type matches the expected type
exactly.
Change-Id: I675934761cc17f6b303b63b4715b31dd1af7cea1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202737
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now a budget of 0 disables mean unlimited and tests no longer set the
budget. Hopefully the deep completion tests will stop flaking.
Updates golang/go#34617
Change-Id: Icdff5e78dcf1cc3d3fcbf0326716b39b00f0a8c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203338
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, address minor comments that I ignored from Heschi because I am
obnoxious.
Change-Id: I99dcac38578585af2cdd951dd2b9755732ef945f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203281
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that we are using the memoize package to cache analysis results, we
can use that cache for suggested fixes.
Change-Id: I42905a6fe575f49d38979d53d58ea8ec59210ae0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203278
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
There is no need to cache the pass on the actionHandle, as it does not
need to be reused and does not live outside the exec function.
Change-Id: I1737271383776b35718df3475b4f888232d57ae4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We now expect a type name when in the key or value of a *ast.MapType.
I also added an extra filter to expect a comparable type for the key.
Change-Id: I647cf4d791b2c0960ad3b12702b91b9bc168599b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197439
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We don't need to worry about a package's errors unless it is the
top-level package. Also fix some fallback logic in the type error range
computation.
Change-Id: Ib26b5e25bd70193ea24ec4a197811eedf69b0e2c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202622
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
When VSCode applies its own fuzzy matching/filtering/ranking logic to
completion candidates, it can end up reordering and even omitting some
of our candidates. It is mainly a problem with deep completions (i.e.
VSCode downranks or completely hides deep completion candidates that
should be ranked at the top).
We now trick VSCode into not reordering our candidates by setting each
candidate's "filterText" to the completion prefix. This makes every
candidate look like an identically perfect match, so VSCode just
maintains the order specified by "sortText".
Note that we don't do this trick if server side fuzzy matching and
deep completions are disabled. In this case unimpeded client side
candidate filtering is necessary.
Change-Id: I677047bca12b9ce05a953016d0d89182f1fe44d6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202717
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This change effectively reverts CL 202039. This CL was a mistake, as it
creates a cycle. Snapshots hold CheckPackageHandles, which in turn hold
pkgs.
Change-Id: I944304cb365f0ef98b5e54ea38edea6cece40453
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202740
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
If there are no imports that need organizing, don't send the "Organize
Imports" code action.
Change-Id: Id01521edd1524fb3f7372fd787d6c90418740cf3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202825
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
*ast.ArrayTypes are type expressions like "[]foo" or "[2]int". They
show up as standalone types (e.g. "var foo []int") and as part of
composite literals (e.g. "[]int{}"). I made the following
improvements:
- Always expect a type name for array types.
- Add a "type modifier" for array types so completions can be smart
when we know the expected type. For example:
var foo []int
foo = []i<>
we know we want a type name, but we also know the expected type is
"[]int". When evaluating type names such as "int" we turn the type
into a slice type "[]int" to match against the expected type.
- Tweak the AST fixing to add a phantom selector "_" after a naked
"[]" so you can complete directly after the right bracket.
I split out the type name related type inference bits into a separate
typeNameInference struct. It had become confusing and complicated,
especially now that you can have an expected type and expect a type
name at the same time.
Change-Id: I00878532187ee5366ab8d681346532e36fa58e5f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change modifies the invalidContent function to take a file change
type. This allows us to eliminate the separate invalidateMetadata
function. The logic of watching changed files is then further pushed
into the caching layer.
Updates golang/go#34218
Change-Id: Id31b3931c45ec408b6e7b4a362e00f9091ba4f70
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201221
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Originally the fuzzy matcher required a match in the final candidate
segment. For example, to match the candidate "foo.bar", the input had
to have at least one character that matched "bar". I previously
removed this requirement as it is too restrictive for deep completions
to be useful.
However, there was still some lingering final-segment favoritism in
the matching algorithm. In particular, there were penalties for not
matching the final segment's first character and for not matching the
final segment's word initial characters. However, these penalties only
made sense when we also required a final segment match. Consider this
example:
User input: "U"
Candidate "ErrUnexpectedEOF" - with only a single segment, we got big
penalties for not matching the leading "E" (since it is the final
segment).
Candidate "ErrUnexpectedEOF.Error" - "ErrUnexpectedEOF" is no longer
the final segment, so we didn't get penalties. And we didn't get
penalties for the final segment "Error" because we finished matching
after the first "U". As a result, this candidate slips through with a
higher score.
Fix by simplifying the skip penalty. Now we only penalize for skipping
the first character of the first or final segment (and the penalty is
lower). For deep completions, the first and final segment are both
"important" segments, so I think it makes sense to focus on both of
them. We don't want to penalize all segment starts because that makes
it harder to match deeper candidates where you often "ignore"
intermediate segments.
I had to adjust a few scores in the tests, but I don't think the
impact will be too big other than fixing the bug.
Fixesgolang/go#35062.
Change-Id: Id17a5c80bf0f80ce252fe990ccfbd51c1bac1c72
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202638
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove the input type option. Now everything behaves as "symbol".
We don't use the "text" or "filename" input types, and I don't foresee
us using them. Removing them simplifies the code a bit, but simplifies
the tests a lot. It was tedious to make changes to the matcher logic
because you had to fret over test failure details that didn't actually
matter because we didn't use that functionality.
Change-Id: I651debde9e63ee283d7bc3ad718d22f4b9a127c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202637
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For *ast.Ident completion requests, this checks the parent node to
see if the token begins a statement and then based on the path adds
possible keyword completion candidates. The test lists some cases where
this approach cannot provide completion candidates.
The biggest thing missing is keywords for file level declarations
Updates golang/go#34009
Change-Id: I9d9c0c1eb88e362613feca66d0eea6b88705b9b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196664
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change allows us to hanel cancel messages as they go into the queue, and
cancel messages that are ahead of them in the queue but not being processed yet.
This should reduce the amount of redundant work that we do when we are handling
a cancel storm.
Change-Id: Id1a58991407d75b68d65bacf96350a4dd69d4d2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200766
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A continuation of CL 202298, only for analysis errors.
Change-Id: I957d52cef31938ef66be73463e92695a5b56869c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202540
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Since a user's module cache is generally going to be much bigger than
their main module, one would expect that caching just information about
the module cache would be sufficient. It turns out that's not correct.
When we discover something in the module cache, we have to make sure
that a different version of it isn't already in scope. Doing that can
require information about the main module or replace targets, so that
needs to be cached too.
Concretely, when I'm working in x/tools, if a scan discovers a version
of x/tools in the module cache, it should usually ignore that version.
But that might not be true in more complicated cases, especially those
involving nested modules whose boundaries change.
So, cache everything except GOROOT. Since the new data is mutable,
we store it separately from the module cache data so that it can be
discarded easily between runs.
Change-Id: I47364f6c0270fee03af8898fec6c85d1b9c8d780
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202045
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
