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>
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>
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>
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>
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>
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>
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>
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 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>
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>
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>
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>
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>
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 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>
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>
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>
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>
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>
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>
*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>
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>
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>
Scan most sources, including GOPATH, the module cache, the main module,
and replace targets as appropriate. Use the cached stdlib instead of
scanning GOROOT.
We heavily cache the contents of the module cache, so performance is
decent. But we have to look at all the modules not in the module cache
too to get the right versions of modules (see
(*ModuleResolver).canonicalize), which currently isn't cached at all,
even just for a single run. That ends up being pretty expensive.
The implementation changes are relatively small; add package name
loading to scan(), cache that result, and allow callers to control what
directories are scanned so that it can skip GOROOT.
I also cleared out most of the stdlib from the unimported completion
test and added a simple external completion to it for safety's sake.
Change-Id: Id50fd4703b1126be35a000fe90719e19c3ab84bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199178
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This function was removed in CL 202298, but used in CL 200597.
Analysis diagnostics should be converted to the source.Error type in the
analysis runner as a complete fix, but this is fine for now.
Change-Id: Ie5f3f566719073d7df6ab4646f855c9f9ce22ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202539
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL adds support for "related information", which allows
associating additional source positions and messages with a
diagnostic.
Change-Id: Ifc0634f68c9f3724b6508dc6331c62c819a24f78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200597
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a source.Error type which is used to collect the error
information that comes out of the loading, parsing, and type checking
stages. We also add specific sources per-error, rather than having them
all be labeled as "LSP".
This change will enable follow-ups that do a better job of extracting
error ranges.
Change-Id: I3fbb5e42d66aa2c5bb1b2f41d1eadfc45f3a749b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202298
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
A package really should always be associated with its snapshot rather
than its view. This eliminates some extra parameters in a few utility
functions.
Change-Id: I60f9b7286e0072d3268602f6bd32052a3d2e5559
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202039
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There was a regression where gopls would not type-check any package with
a bad import. This change fixes the regression and adds a test to make
sure it doesn't happen again.
Change-Id: I3acf0917d46e9444c20135559f057f0ecd20e15b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201539
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We now continue deep completion search across function calls. The
function must take no arguments and return a single argument. For
example, when completing "fo<>" you might get candidates such as
"foo.bar().baz()".
Previously we would stop searching for deep completions when we hit a
function call. For example, we would stop at "foo.bar()", never
finding "foo.bar().baz()". At the time I was worried about the search
scope growing too large, but now that we dynamically limit the search
scope there isn't much left to worry about.
Change-Id: I48772c154400662876682503c1f58ef6e3dca688
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201222
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Previously we unconditionally qualified literal candidate types with
their package. For example:
var buf *bytes.Buffer
buf = &bytes.Bu<>
would complete to:
buf = &bytes.bytes.Buffer{}
Now we don't qualify the type if the cursor position is in the
selector of an *ast.SelectorExpr. We only generate literal candidates
for type names, so if we are in a selector then we can assume it is a
package qualified type (as opposed to an object field).
We also handle the insertion of "&" for literal pointers better. If you are in
the selector of an *ast.SelectorExpr, we prepend the "&" to the beginning of the
expression rather than the selector. For example, you will end up with
"&bytes.Buffer{}" instead of "bytes.&Buffer{}".
Updates golang/go#34872.
Change-Id: I812aa809cd4e649a429853386789f80033412814
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now we offer completion candidates for labels when completing "break",
"continue", and "goto" statements. We are reasonably smart about
filtering unusable labels, except we don't filter "goto" candidates
that jump across variable definitions.
Fixesgolang/go#33987.
Change-Id: If296a7579845aba5d86c7050ab195c35d4b147ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change moves to the approach of caching the analysis using the
memoize package. This means that we will do less work, as we no longer
need to recompute results that are unchanged. The cache key for an
analysis is simply the key of the CheckPackageHandle, along with the
name of the analyzer.
Change-Id: I0e589ccf088ff1de5670401b7207ffa77a254ceb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This will prevent us from panicking in cases with errors.
Fixesgolang/go#34824
Change-Id: I02c20655f6926ec00c1591a905ff5a107cc44192
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200300
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We used to return an error from textDocument/highlight if the cursor
wasn't over an identifier. Logging these errors is not useful, as the
cursor is often not on an identifier.
Change-Id: Ibb43908149315c72923a22bdca567aa2b3ee68d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This makes it so the diff package is depended on by all implementations, rather
than the diff package having to depend on the default myers implementation.
Change-Id: I04b9caee6ff1017fa8e5476a7434e4b0e17753c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198379
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change includes dependencies in the cache keys for
CheckPackageHandles. This should fix the issue with propagating results
to reverse dependencies.
Updates golang/go#34410
Change-Id: I025b7f0d6b0dcaa89c3461ebd94eadd35de4955f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In cases like "fmt.Pr<>int()" we previously would replace "Print" with
the new completion, yielding for example "fmt.Println()". Now we no
longer overwrite, yielding "fmt.Println()int()". There are some cases
where overwriting the suffix is what the user wants, but it is hard to
tell, so for now stick with the more expected behavior of not
overwriting.
Fixesgolang/go#34011.
Change-Id: I8c3ccd8948245c27b52408ad508d8e01dc163ef4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196119
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change does not complete the work to handle snapshots correctly,
but it does implement the behavior of re-building the snapshot on each
file invalidation.
It also moves to the approach of caching the FileHandles on the snapshot,
rather than in the goFile object, which is now not necessary.
Finally, this change shifts the logic of metadata invalidation into the
content invalidation step, so there is less logic to decide if we should
re-load a package or not.
Change-Id: I18387c385fb070da4db1302bf97035ce6328b5c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197799
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The loops are common to all the testing layers, so lift them.
This prepares for more test improvements, without any funcitonal changes.
Change-Id: Ib750c8a7bb4c424a185cb0bd841674a69db1385b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197717
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In CL 192137 deep fuzzy matching was enabled by default. We also have
options independent options "deepCompletion" and "fuzzyMatching" to
control this. When fuzzy matching is disabled, case insensitive prefix
matching is used.
Provide an option, "caseSensitiveCompletion", which allows for case
sensitive prefix matching when fuzzy matching is disabled.
Change-Id: I17c8fa310b2ef79e36cc2f7303e98870690b5903
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194757
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 missing imports detection to return diagnostics and
warning messages to the user.
Updates golang/go#34484
Change-Id: If7bb7e702b8bdbd7e1ad5e26f93acc50d16209b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now we always expect type names inside of *ast.FieldList. This expands
the previous func signature logic to also work for *ast.StructType
and *ast.InterfaceType. For example, we will now prefer type names in
cases like:
type myStruct struct { i i<> }
Also, fix a check for anonymous fields to make sure the field is
actually embedded. This fixes cases like this to properly have no
completions:
type myStruct struct { i<> i }
where this will still give type name completions:
type myStruct struct { i<> }
I introduced a new error type source.ErrIsDefinition so source_test.go
could avoid erroring out on tests that make sure definition
identifiers have no completions.
Fixesgolang/go#34412.
Change-Id: Ib56cb52af639f2e2b132274d1f04f8074c0d9353
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196560
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix objects defined in the function signature to only be completable
inside the function body. For example:
func (dog Dog) bark(d<>) { // Don't complete <> to "dog".
d<> // Do complete <> to "dog".
}
Change-Id: Ic9a2dc2ce6771212780f2d6af2221a67d203f35f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196559
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This prevents piping back to the file, a common pattern.
Multi file forms should use the unified diff.
Change-Id: I1ea140c59de24feb74a64b0cb41890536f23cd3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197157
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Race condition: I deleted the code that acquired the mutex when checking
if a file's imports have changed.
Merge conflict: I submitted a change without rebasing and re-running TryBots.
Change-Id: Iae351f2fff893cfd94db79d9f79578d9827a8c21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197297
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
source.DiagnosticSeverity and source.CompletionItemKind are duplicated
and not worth maintaining.
Change-Id: I8d6c8621a227855309c0977da59d8c9fa53617bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Normally you don't want literal candidates for basic types (e.g.
"int(0)") since you can type the literal value without the type name.
One exception is if you are creating a named basic type that
implements an interface. For example:
http.Handle("/", http.FileServer(<>))
will now give "http.Dir()" as a candidate since http.Dir is a named
string type that implements the required interface http.FileSystem.
Change-Id: Id2470c45e469ea25cd0f9849cfdad19ac0e784bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195838
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I was messing around with the new diagnostic tags feature and
had to update to get it to work.
Change-Id: I4294513b460ec4806d23af20bf908cee8673f7c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
This change moves from caching package information the file object to
caching in a map that gets invalidated when content changes.
This simplifies cache invalidation and reduces the number of fields
guarded by the (*goFile).mu lock.
Change-Id: I33fef6e0b18badb97e49052d9d6e3c15047c4b63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196984
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Instead of relying on the diagnostics cached on the package, use the
diagnostics sent by the code action when computing suggested fixes.
Change-Id: I77f7fd468b34b824c6c5000a51edbe0f8cc6f637
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197097
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our completion tests check for a lot of different behaviors. It may be
easier to develop if we have separate tests for things like deep
completion and completion snippets.
Change-Id: I7f4b0c0e52670f2a6c00247199933fd1ffa0096f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196021
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This will allow view configuration to modify the set of analyzers being applied, and also allow the main gopls to inject new analyzers
Change-Id: Ic2a76118c3e29b059e19b31bd1fb54b1d9e15e54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196320
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This changes adds basic support for running `go mod tidy` as a code
action when a user opens a go.mod file. When we have a command
available like `go mod tidy -check`, we will be able to return edits as
part of the codeAction. For now, we execute the command directly.
This change also required a few modifications to our handling of file
kinds so that we could distinguish between a Go file and a go.mod file.
Change-Id: I343079b8886724b67f90a314e45639545a34f21e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196322
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now we will offer function literal completions when we know the
expected type is a function. For example:
sort.Slice(someSlice, <>)
will offer the completion "func(...) {}" which if selected will
insert:
func(i, j int) bool {<>}
I opted to use an abbreviated label "func(...) {}" because function
signatures can be quite long/verbose with all the type names in there.
The only interesting challenge is how to handle signatures that don't
name the parameters. For example,
func HandleFunc(pattern string, handler func(ResponseWriter, *Request)) {
does not name the "ResponseWriter" and "Request" parameters. I went
with a minimal effort approach where we try abbreviating the type
names, so the literal completion item for "handler" would look like:
func(<rw> ResponseWriter, <r> *Request) {<>}
where <> denote placeholders. The user can tab through quickly if they
like the abbreviations, otherwise they can rename them.
For unnamed types or if the abbreviation would duplicate a previous
abbreviation, we fall back to "_" as the parameter name. The user will
have to rename the parameter before they can use it.
One side effect of this is that we cannot support function literal
completions with unnamed parameters unless the user has enabled
snippet placeholders.
Change-Id: Ic0ec81e85cd8de79bff73314e80e722f10f8c84c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193699
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add support for literal completion candidates such as "[]int{}" or
"make([]int, 0)". We support both named and unnamed types. I used the
existing type matching logic, so, for example, if the expected type is
an interface, we will suggest literal candidates that implement the
interface.
The literal candidates have a lower score than normal matching
candidates, so they shouldn't be disruptive in cases where you don't
want a literal candidate.
This commit adds support for slice, array, struct, map, and channel
literal candidates since they are pretty similar. Functions will be
supported in a subsequent commit.
I also added support for setting a snippet's final tab stop. This is
useful if you want the cursor to end up somewhere other than the
character after the snippet.
Change-Id: Id3b74260fff4d61703989b422267021b00cec005
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193698
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The sortslice analyzer warns the user when they're
using a non-slice parameter to sort.Slice. It can
also suggest fixes for 3 different cases of misuse:
1. The parameter is a pointer to a slice.
2. The parameter is an array
3. The parameter is a function with no arguments
that returns a slice.
Change-Id: If6327b19ab3c476cf17ca3c5fd75bc43bb8683e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195898
Run-TryBot: Johan Brandhorst <johan.brandhorst@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If we encounter `go list` errors when loading a user's package, we
should try to see if they've encountered any of our common error cases.
They are: 1) a user has GO111MODULE=off, but is outside of their GOPATH,
and 2) a user is in module mode but doesn't have a go.mod file.
Fortunately, go/packages does a great job handling edge cases so gopls
will work well for most of them. The main issue will be unresolved
imports. These show up in DepErrors in `go list`, so go/packages doesn't
propagate them through to the list of errors. This will require changes
to go/packages.
Updates golang/go#31668
Change-Id: Ibd5253b33b38caffeaad54a403c74c0b861fcc14
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Parse errors need to be treated separately from actual errors when
parsing a file. Parse errors are treated more like values, whereas
actual errors should not be propagated to the user. This enables us to
delete some of the special handling for context.Canceled errors.
Change-Id: I93a02f22b3f54beccbd6bcf26f04bb8da0202c25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195997
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We had too many options for functions to use to get type information for
a package. Now we stick with having one option to get the check package
handles, and then the caller can refine the results as needed.
Change-Id: I81f69a670e1539854ee23b6f364159a7de9b782f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This function incorrectly used cached packages to get ASTs and type
information that should have been directly found from the origin
package. Shift to using pkg.FindFile instead.
Change-Id: I9f73209bb1a1343f53b430150e78ffd180e14a44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Tweak a couple things to improve how we reduce our search scope based
on remaining time budget:
- Check our budget on the first candidate rather than waiting for the
1000th candidate. If type checking is slow you can be out of budget
before you even begin.
- Reduce our budget check interval from 1000 candidates to 100
candidates. This just helps us adjust our search scope faster.
The first tweak required me to raise the completion budget for tests
because 100ms is not always enough. I moved the budget into the
completion options so that tests can raise it.
Change-Id: I1aa7909d7baf9c998bc830c960dc579eb33db12a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195419
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Revert my previous change to include fuzzy matches with a score of
zero. Zero scorers have some characters that match, but they are
pretty poor overall. Pulling in all the extra junk candidates was
slowing things down in certain cases.
Change-Id: I560f46903281f21b0628c9b14848cddf1e3c0a38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195418
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change allows to remove some of the special handling for the
builtin package.
Change-Id: I105fcefd8812af2d42ff42edca954824c98db429
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195758
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
A mapper is always uniquely tied to a file at a specific version, so
just build it when we get a new *ast.File. We build the mapper using the
*token.File associated with the particular *ast.File, which is why there
is one per ParseGoHandle instead of FileHandle.
Change-Id: Ida40981ef91f6133cdd07e9793337fcd67510fba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
I wasn't sure if we should just manually construct the URI here or use
the URL unescaping function. Let me know which you think is best.
Updates golang/go#34270
Change-Id: Idb48fc2650d39f3e54cac141a70f356c31e303ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195618
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
protocol.Position doesn't implement String(), so don't use "%s"
formatting directive. We could implement String(), but it is generated
code, so its a bit iffy.
Change-Id: If21b5fee66c6e1bd59f19b520c7d36f0e648cb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195042
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We turned them off for the release, but let's keep them on at master so
that we can get more feedback.
Change-Id: Iaadf27d59ef925ba8ee2bc02acbb9c77c2935ce2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195059
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Previous changes to the config mechanism made the config options
per-view, not per-session. We should now make sure to obey config
changes per-view. This does not fix the configuration handling for
"watchChangedFile" however. This should be done in a future CL.
Change-Id: I73f6236386c36d2587fdb9c0601670833a4366c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194818
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This fixes the issue of config options not being applied.
Also, handle config errors and deprecation by showing a message to the
user.
Change-Id: I850d5303a7a1e301c97324060a87929710ee6700
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194682
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change temporarily disables fuzzy matching and deep completion by
default, leaving them opt-in for the tagged versions. These will be
re-enabled after the tag on master to continue iterating on them.
Also, update the hard-coded version string.
Change-Id: I0aa688ce067abfe8ae8ebe52a25c8514ec2c7e48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194777
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, handle *ast.StarExpr in the identifier code. This fixes a specific
case with deep completions and documentation.
Change-Id: I630ae4e8f1c123ba1fdea85e6862ae93396e2cd4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194564
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our original caching plan was to use only the file ParseGoHandles as
cache keys to define a given package. However, because of package test
variants, we cannot rely on files alone. A package may have the exact
same set of files, but be a test variant. Add the ID to the key to avoid
clobbering entries in the cache.
Also, remove the unused metadata ID cache.
Change-Id: I4b33de1f83f6c769d23441e98a2a7324ff0fa1b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194571
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now when a file is deleted we force the file's packages to refresh
go/packages metadata, and kick off diagnostics.
I made a couple other changes to watched file handling:
- Kick off diagnostics in a goroutine to match how DidChange works.
This will allow us to work through big sets of file changes faster,
and will save duplicated work once type checking can be canceled.
- Don't assume a watched file is only part of one view.
Two interesting cases we don't handle yet:
- If the deleted file was the only file in the package, we don't
currently update diagnostics for dependent packages. This requires
rejiggering how diagnostics are invoked a bit.
- If the deleted file is still open in the editor and then later
closed, we don't trigger metadata/diagnostics refresh on DidClose.
Updates golang/go#31553
Change-Id: I65768614c24d9800ffea149ccdbdbd3cb7b2f3d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193121
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We now wait to build views until we have the options for that view,
and pass the options in to the view constructor.
The environment and build flags are now part of the view options.
Change-Id: I303c8ba1eefd01b66962ba9cadb4847d3d2e1d3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In the case of documentation items for completion items, we should make
sure to use the ASTs and type information for the originating package.
To do this while avoiding race conditions, we have to do this by
breadth-first searching the top-level package and its dependencies.
Change-Id: Id657be969ca3e400bb2bbd769a82d88e91865764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
I moved the "usePlaceholders" config field on to CompletionOptions.
This way the completion code generates a single snippet with a little
conditional logic based on the "WantPlaceholders" option instead of
juggling the generation of two almost identical "plain" and
"placeholder" snippets at the same time. It also reduces the work done
generating completion candidates a little.
I also made a minor tweak to the snippet builder where empty
placeholders are now always represented as e.g "${1:}" instead of
"${1}" or "${1:}", depending on if you passed a callback to
WritePlaceholder() or not.
Change-Id: Ib84cc0cd729a11b9e13ad3ac4b6fd2d82460acd5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193697
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add a new @completePartial note that does not require you to specify
the full list of completions. This gets rid of a lot of noise when you
just want to test the relative order of some completion candidates but
don't care about all the other candidates in scope.
I changed one existing test to use @completePartial as an example.
Change-Id: I56005405477e562803f094c0cac05ef2b854ad1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192657
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Remove the wantSuggestedFixes flag, and run the flagged code
by default.
Add test cases for suggested fixes.
Generate a suggested fix to the assign analysis that suggests removing redundant assignments.
Fix the propagation of suggested fixes (using rstambler's code).
Change-Id: I342c8e0b75790518f228b00ebd2979d24338be3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193265
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
CL 193726 accidentally turned off deep completions and fuzzy matching by default.
Re-enabling them now.
Change-Id: Ia120766b3a72243efe9c398c0efd6d1a101d0e7f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194020
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Improve the existing fix-the-AST code to better identify the
expression following the "go" or "defer" keywords:
- Don't slurp the expression start outside the loop since the
expression might only have a single token.
- Set expression end to the position after the final token, not the
position of the final token.
- Track curly brace nesting to properly capture an entire "func() {}"
expression.
- Fix parent node detection to work when BadStmt isn't first statement
of block.
- Add special case to detect dangling period, e.g. "defer fmt.". We
insert phantom "_" selectors like go/parser does to prevent the
dangling "." from messing up the AST.
- Use reflect in offsetPositions so it updates positions in all node
types. This code shouldn't be called often, so I don't think
performance is a concern.
I also tweaked the function snippet code so it properly expands
"defer" and "go" expressions to function calls. It thought it didn't
have to expand since there was already a *ast.CallExpr, but the
CallExpr was faked by us and the source doesn't actually contain the
"()" calling parens.
Note that this does not work for nested go/defer statements. For
example, completions won't work properly in cases like this:
go func() {
defer fmt.<>
}
I think we can fix this as well with some more work.
Change-Id: I8f9753fda76909b0e3a83489cdea69ad04ee237a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193997
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Downranking builtins causes weird interplay with other completion
candidates due to fuzzy matching. For example:
notNil := 123
var foo *int = nil<>
ranks "notNil" before "nil" in the builtin list, which is counter
productive.
Change it to not downrank builtins. In my testing with this change,
builtins never were ranked above lexical items with similar names. I
think this is because the "natural" order of completion items puts
builtins last, and we stable sort items by score, so their relative
order is preserved.
Change-Id: Ifbad02be205e3cb26c1d4ce500b77690e7ac5b04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193897
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The next in the sequence of CLs to convert to using protocol positions.
Change-Id: Ib3421bfc73af1b546b60c328ca66528cb9031e19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193719
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This cl is the first in a set that change the configuration behaviour.
This one should have no behaviour differences, but makes a lot of preparatory changes.
The same options are set to the same values in the same places.
The options are now stored on the Session instead of the Server
The View supports options, but does not have any yet.
Change-Id: Ie966cceca6878861686a1766d63bb8a78021259b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193726
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Folding ranges need to be computed to present folding ranges that make
sense when lineFoldingOnly is true. This change computes the folding
ranges to include the lines that are contained within the start and end
parenthesis/braces when applicable.
Folding ranges are not returned when the contained nodes begin or end on
the same lines as the parenthesis/brace. This is to avoid misleading
folding ranges like the following in unformatted code:
if true {
fmt.Println("true") } else {
fmt.Println("false")
}
---folding "if true {}"--->
if true {
fmt.Println("false")
}
Change-Id: I2931d02837ad5f2dd96cc93da5ede59afd6bcdce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192678
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This feature has been in an experimental state for a long enough time
that I think we can enable it by default at master.
Change-Id: I9bbb8b41377719f0e97f608f6e5163a883a176b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192259
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
If the client registers with foldingRange.lineFoldingOnly = true, only
return folding ranges that span multiple lines. Do this as they are
computed, so that if other filtering is applied later, we do not include
ranges that would go unused by the client anyway.
Change-Id: I27ea24428d25f180e26892de0f6d16c211225bf7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192477
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
FoldingRanges may be nested. Test nested folding ranges by separating
out the folding ranges by nested level and checking each level.
Change-Id: I12c72daa3e6c6b9d4959209b3a41b27e2b59866f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192398
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The existing implementation did not suggest struct field names
when running completion from within a slice literal of
pointers. Now, struct field names are suggested in that case.
Fixesgolang/go#33211
Change-Id: I6028420a9a789846b070fcc6e45ec89dc4d898d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192277
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Invert "useDeepCompletions" config flag to "disableDeepCompletion" and
separate out "disableFuzzyMatching" which reverts to the previous
prefix matching behavior.
I separated fuzzy matching tests out to a separate file so they aren't
entangled with deep completion tests. In coming up with representative
test cases I found a couple issues which I fixed:
- We were treating a fuzzy matcher score of 0 as no match, but the
matcher returns 0 for candidates that match but have no bonuses. I
changed the matcher interface so that a score of 0 counts as a
match. For example, this was preventing a pattern of "o" from
matching "foo".
- When we lower a candidate's score based on its depth, we were
subtracting a static multiplier which could result in the score
going negative. A negative score messes up future score weighting
because multiplying it by a value in the range [0, 1) makes it
bigger instead of smaller. Fix by scaling a candidate's score based
on its depth rather than subtracting a constant factor.
Updates golang/go#32754
Change-Id: Ie6f9111f1696b0d067d08f7eed7b0a338ad9cd67
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Prepare rename gets the range of the identifier to rename. Returns an
error when there is no identifier to rename.
Change-Id: I5e5865bc9ff97e6a95ac4f0c48edddcfd0f9ed67
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191170
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Deep completions can take a long time (500ms+) if there are many
large, deeply nested structs in scope. To make sure we return
completion results in a timely manner we now notice if we have spent
"too long" searching for deep completions and reduce the search scope.
In particular, our overall completion budget is 100ms. This value is
often cited as the longest latency that still feels instantaneous to
most people. As we spend 25%, 50%, and 75% of our budget we limit our
deep completion candidate search depth to 4, 3, and 2,
respectively. If we hit 90% of our budget, we disable deep completions
entirely.
In my testing, limiting the search scope to 4 normally makes even
enormous searches finish in a few milliseconds. Of course, you can
have arbitrarily many objects in scope with arbitrarily many fields,
so to cover our bases we continue to dial down the search depth as
needed.
I replaced the "enabled" field with a "maxDepth" field that disables
deep search when set to 0.
Change-Id: I9b5a07de70709895c065503ae6082d1ea615d1af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190978
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now we register for and handle didChangeWatchedFiles "change"
events. We don't handle "create" or "delete" yet.
When a file changes on disk, there are two basic cases. If the editor
has the file open, we want to ignore the change since we need to
respect the file contents in the editor. If the file isn't open in the
editor then we need to re-type check (and re-diagnose) any packages it
belongs to.
We will need special handling of go.mod changes, but start with
just *.go files for now.
I'm putting the new behavior behind an initialization flag while it is
under development.
Updates golang/go#31553
Change-Id: I81a767ebe12f5f82657752dcdfb069c5820cbaa0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190857
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change allows renamings of the name of an import spec.
Since there is not always explicit identifier available to select and
rename, allow renaming packages from positions within the import spec.
Change-Id: I0a8aaa92c26e1795ddb9c31a1165b2f2ee89aa34
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191165
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When there is an explicit name for an import spec, treat it as its own
identifier, separate from the import path.
Example:
import h "hello"
The name h is defined in that import spec, not in the package hello
it contains its own references. If asked about a position within the
import path, continue treating that as referencing the imported package.
If the position is within the name, use the identifier there that is
local to that file.
This change allows for go to definition of the explicit name to point to
itself, find all references from the import spec, and rename the
explicit name from the import spec.
Change-Id: Ia1d927a26e73f2dc450d256d71909c006bdf4c37
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191164
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Since renaming an identifier within an import spec is not yet supported,
return an error when this is encountered. These idents from the import
spec have a nil declaration object.
Import paths that contain '.' or '/' are caught by the valid identifier check
avoiding the crash, but import paths such as "fmt" are not as fmt is a
valid identifier. This change checks if i.decl.obj is nil and returns an error
if it is to avoid the crash.
Fixesgolang/go#33768
Change-Id: I4e757b42bedffd648fc821590e4a383826200dc3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191163
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Optimize a few things to speed up deep completions:
- item() is slow, so don't call it unless the candidate's name matches
the input.
- We only end up returning the top 3 deep candidates, so skip deep
candidates early if they are not in the top 3 scores we have seen so
far. This greatly reduces calls to item(), but also avoids a
humongous sort in lsp/completion.go.
- Get rid of error return value from found(). Nothing checked for this
error, and we spent a lot of time allocating the only possible error
"this candidate is not accessible", which is not unexpected to begin
with.
- Cache the call to types.NewMethodSet in methodsAndFields(). This is
relatively expensive and can be called many times for the same type
when searching for deep completions.
- Avoid calling deepState.chainString() twice by calling it once and
storing the result on the candidate.
These optimizations sped up my slow completion from 1.5s to
0.5s. There were around 200k deep candidates examined for this one
completion. The remaining time is dominated by the fuzzy
matcher. Obviously 500ms is still unacceptable under any
circumstances, so there will be subsequent improvements to limit the
deep completion search scope to make sure we always return completions
in a reasonable amount of time.
I also made it so there is always a "matcher" set on the
completer. This makes the matching logic a bit simpler.
Change-Id: Id48ef7031ee1d4ea04515c828277384562b988a8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190522
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
this moves the actual diff algorithm into a different package and then provides hooks so it can be easily replaced with an alternate algorithm.
Change-Id: Ia0359f58878493599ea0e0fda8920f21100e16f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190898
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Test that having a comment at the start of a file allows imports
to be added correctly.
Updates golang/go#33721
Change-Id: Id1673c2509537413710b73261ad2a59afe06b93f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190800
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Insert imports into existing multiline import declarations when
possible.
Logic for choosing where to insert taken from
golang.org/x/tools/go/ast/astutil.
Change-Id: Ie5ad96d5e3d5db2e92a2c05a6104d14a4a192ed3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190598
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Unimported packages may be suggested as completion items. Since these
are not yet imported, they should be ranked lower than other candidates.
They also require an additional import statement to be valid, which is
provided as an AdditionalTextEdit.
Adding this import does not use astutil.AddNamedImport, to avoid
editing the current ast and work even if there are errors. Additionally,
it can be hard to determine what changes need to be made to the source
document from the ast, as astutil.AddNamedImport includes a merging
pass. Instead, the completion item simply adds another import
declaration.
Change-Id: Icbde226d843bd49ee3713cafcbd5299d51530695
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190338
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is the first in a series of many changes that will change the API
of the source package to use different types for positions. Using
token.Pos is particularly fragile, since the pos has to refer to the
specific *ast.File from which it was derived.
Change-Id: I70c9b806f7dd45b2e229954ebdcdd86e2cf3bbbb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190340
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Rename checks all of the packages that may be affected for conflicts. An
error in any of them leads to renaming error. Returning errors from
multiple packages may be confusing (for example, when there is a test
variant of a package and the same error appears twice). This change
stops after an error is found and returns that error instead of
continuing to search.
Change-Id: Ifba1feddbf8829d3aad30309154d578967e05a36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190416
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
this shuffles things so there a single exporter API rather than an observer
It also removes most of the globals.
per telemetry type.
Change-Id: Iaa82abe2ded1fff9df8e067ed4a55bcbd9d9591f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190405
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This change will just make it a bit easier to debug the context
cancellation errors.
Change-Id: I580751ac04e3357031678eb31914828029c96e4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190412
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is a straight move of some code with no changes.
It splits the part of the telemetry code that will become a standalone library from the bit that belongs in the lsp.
Change-Id: Icedb6bf1f3711da9251450531729984df6df7787
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190403
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This fix adds all packages to the renamer packages map.
Renaming performs checks on each package to make sure there are no
conflicts. If there are multiple packages, each package needs to be
checked. The packages were being incorrectly added to the map and were
all being put under a single key.
Fixesgolang/go#33664
Change-Id: I68466ce34ac49b700ce6d14ce0b53e2795926fa9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190399
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Make use of the existing fuzzy matcher to perform server side fuzzy
completion matching. Previously the server did exact prefix matching
for completion candidates and left fancy filtering to the
client. Having the server do fuzzy matching has two main benefits:
- Deep completions now update as you type. The completion candidates
returned to the client are marked "incomplete", causing the client
to refresh the candidates after every keystroke. This lets the
server pick the most relevant set of deep completion candidates.
- All editors get fuzzy matching for free. VSCode has fuzzy matching
out of the box, but some editors either don't provide it, or it can
be difficult to set up.
I modified the fuzzy matcher to allow matches where the input doesn't
match the final segment of the candidate. For example, previously "ab"
would not match "abc.def" because the "b" in "ab" did not match the
final segment "def". I can see how this is useful when the text
matching happens in a vacuum and candidate's final segment is the most
specific part. But, in our case, we have various other methods to
order candidates, so we don't want to exclude them just because the
final segment doesn't match. For example, if we know our candidate
needs to be type "context.Context" and "foo.ctx" is of the right type,
we want to suggest "foo.ctx" as soon as the user starts inputting
"foo", even though "foo" doesn't match "ctx" at all.
Note that fuzzy matching is behind the "useDeepCompletions" config
flag for the time being.
Change-Id: Ic7674f0cf885af770c30daef472f2e3c5ac4db78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change eliminates the need for the package cache map, and instead
stores package type information in the store. We still have to maintain
invalidation logic because the key is not computed correctly.
Change-Id: I1c2a7502b99491ef0ff68d68c9f439503d531ff1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
When it is certain we are completing a struct field name, we don't
want deep completions. The only possible completions are the remaining
field names.
I also silenced the log spam in tests by disabling the go/packages
logger and the lsp logger.
Fixesgolang/go#33614
Change-Id: Icec8d92112b1674fa7a6a21145ab710d054919b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190097
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change configures the ordering of documentation on hover. If the
user has requested full documentation, the signature appears at the top,
to avoid the user having to scroll. Otherwise, the signature appears at
the bottom, to minimize the distance between the triggering identifier
and the signature.
Updates golang/go#33352
Change-Id: I017baaabd0ee0c31cb13cb6abdda296782929823
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189943
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change fixes documentation for completion items by using cached
package and AST information to derive the documentation. We also add
testing for documentation in completion items.
Change-Id: I911fb80f5cef88640fc06a9fe474e5da403657e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189237
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We're still leaving open the possibility of changing this API,
but things have baked for a bit so I feel comfortable removing the
build tag.
Also add some documentation.
Change-Id: I3beb666b58177553fc406dc9670d569d5928fedd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189460
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is probably a better approach than showing an extra diagnostic,
since a user cannot dismiss a diagnostic.
Fixesgolang/go#33397
Change-Id: I92b9a00f51a463673993793abfd4cfb99ce69a91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/188766
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This relates to https://github.com/golang/go/issues/31374 and should switch all instances within `gopls` to use `x/errors` instead of `fmt` to create new errors.
Change-Id: I18339b75d12418d852e0dcc2ba0ed6c2970783b3
GitHub-Last-Rev: f4a55d9b79e7458ef1f1e06cb5eabbabd884f321
GitHub-Pull-Request: golang/tools#108
Reviewed-on: https://go-review.googlesource.com/c/tools/+/179880
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Get quick fixes for the diagnostics related to import errors. These
fixes add, remove, or rename exactly one import.
This change exposes the individual fixes found by the imports package,
and then applies each of them separately to the source. Since applying each
fix requires a new ast anyway, we pass in the source to be parsed each time.
Change-Id: Ibcbfa703d21b6983d774d2010716da8c25525d4f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/188059
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The results of running 'go list -m' are only valid as long as the
current module and the modules in its replace directives do not
change their go.mod files. Store the 'go.mod' versions that are
used in the imports call, and reinitialize the module resolver if
they change.
Change-Id: Idb73c92b9e4dc243a276885e5333fafd2315134d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186597
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Packages with errors may still contain files that can be formatted.
Try to format the source of the files in packages that have errors.
This change will still not format files with parse errors.
Updates golang/go#31291
Change-Id: Ia5168d7908948d201eac7f2ee28534022a2d4eb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/187757
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Save the packages found when scanning of the module cache.
The computed package may have a different import path due
to replace directives, so this needs to be updated
when the moduleResolver is initialized again.
Change-Id: Ib575fcc59b814ff263b431362df3698839a282f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186301
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds a Logf field to the packages.Config.
Change-Id: I144a9a1e1181585bbe621898c4a3e6a007a38322
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185993
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
The imports ProcessEnv contains cached module and filesystem state. This change
allows gopls to use the same ProcessEnv and resolver across multiple calls to the
internal/imports library.
A ProcessEnv belongs to a view, because the cached module state depends
on the module that is open in the workspace.
Since we do not yet track whether the 'go.mod' file has changed, we
conservatively reset the cached state in the module resolver before
every call to imports.Process.
Change-Id: I27c8e212cb0b477ff425d5ed98a544b27b7d92ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184921
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change removes the need for the ast and token fields on the *goFile
object. We switch to using source.ParseGoHandles on the package, which
means that we can easily access both the AST and token via the package,
which is already cached.
Change-Id: I5f78bbe09362f4d95eb15556617bdbd809a7a55d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185878
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Noticed this because I was accidentally running these tests with Go
1.11.
Change-Id: Ic35d71bd1da9078b4bde6aa2ed62d54a8b95b0e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186298
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
We recommend that gopls integrators apply the []TextEdit responses in
reverse order to get a correct resulting document. This strategy works
when the response is already sorted. Have gopls return sorted []TextEdit
for each file.
Fixesgolang/go#33123
Change-Id: Ib570881c9623695d2ae3194fa8a97b0a681a3250
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186258
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
And purge the loggers from the view and session.
Change-Id: I262958f340e9a5ac9cc9b3db9e9910381e457478
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185989
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change merely modifies session.DidOpen to accept the document's
language ID. It does not actually add any handling of the language ID.
Change-Id: I2582ae307d1ca062f37b4683907cdbcfdfc61809
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184160
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
also change the return type to be and end function and not an incomplete span
Change-Id: Icd99d93ac98a0f8088f33e905cf1ee3fe410c024
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185349
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Since 'IdentifierInfo' doesn't contain ast node of import spec,
gopls will construct an empty string under plaintext mode and
'```go\n\n```' under markdown mode for *ast.ImportSpec. For now,
the hovering result of import spec is the corresponding node
format.
Fixesgolang/go#33000
Change-Id: I4c25782ddb5bcc557ace82f46d480316b0b90509
GitHub-Last-Rev: 150728f401c5f9b161b557584ad3250f46e50869
GitHub-Pull-Request: golang/tools#134
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185357
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For all uses inside the lsp we use the detatch logic instead
For tests we build it in the test harness instead
This is in preparation for things on the context becomming important
Change-Id: I7e6910e0d3581b82abbeeb09f9c22a99efb73142
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185677
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Support renaming of identifiers in test packages. The packages for
all of the references must be checked and the changes need to be
deduped, since both a package and its test package contain some of the
same files.
Fixesgolang/go#32974
Change-Id: Ie51e19716faae77ce7e5254eeb3956faa42c2a09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185277
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
