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>
Previously we would type check all package handles as we fetched them.
This meant if you only cared about a file's primary package you would
still have to wait for all its packages to be type checked. For
example, when completing in foo.go, you would wait for [foo.go] and
[foo.go, foo_test.go] to be checked (the latter being the test
variant).
Now we don't type check packages as we put together the package
handles.
Change-Id: Ibca0c6b34cf4f0a07bcdeb62959d60158f4ccf17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
Changes go.ts to check that the commit hash of the vscode is the one that it
is expecting. README.md now contains more explanation.
Change-Id: Ia5a947c6d5d026c2b7d9ab18877c320e8a7f45d2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195438
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>
These versions of go.ts and requests.ts generate tsprotocol.go,
tsserver.go, and tsclient.go. README.md now gives the version of the
vscode source used, showing that the typescript code and tsprotocol.go
are matched to the same git commit of vscode.
Many of the diffs are just whitespace from vscode's formatting.
Fixesgolang/go#34225
Change-Id: Ib66dad9476b452f332a4c0e990faf2c6060a588e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195297
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>
This was causing tests to report that they required a binary
called "off".
Change-Id: I592109029c86af20d48d2ee9989d4a25fdb11b3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194978
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
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>
Go 1.13 introduced a module in GOROOT/src. That triggered goimports to
think that it was an invalid module, usable only as a replace target,
because its declared module path "std" didn't match its apparent path
"src". The stdlib is always in scope, so skip the needs-replace check
for GOROOT.
Fixesgolang/go#34199
Change-Id: I1199378b940cfedc04e9a4e943c46b9ffdf18446
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194570
Run-TryBot: Heschi Kreinick <heschi@google.com>
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>
In api/*.txt, interface declarations are represented with lines like:
pkg container/heap, type Interface interface { Len, Less, Pop, Push, Swap }
or, when they have no exported methods:
pkg go/ast, type Expr interface, unexported methods
The latter form confuses mkstdlib into thinking that it's a method
because of the extra comma, and then it skips the interface entirely.
Running this program is a matter of seconds once per release, so rather
than trying to fix the optimization, just remove it. The parsing logic
doesn't care about the extra lines.
And the corresponding change to the copy in lsp/testdata/unimported.
Updates golang/go#34199
Change-Id: Ic34b8a47537608401e4ef6683617d797f9f50f8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194568
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
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>
And the corresponding change to the copy in lsp/testdata/unimported.
Change-Id: I604ae6d5217356e19bb18f7cbe69a8dd71e5f23e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194567
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Now when trying to fix *ast.BadStmt, we parse the manually extracted
expression using parser.ParseFile instead of parser.ParseExpr.
ParseFile will yield *ast.BadStmt nodes for any bad statements nested
in our first bad statement, allowing us to fix them recursively.
To turn our expression into a "valid" file we can pass to
parser.ParseFile, I wrapped it thusly:
package fake
func _() {
<our expression>
}
Change-Id: I0d4fd4ebce6450021da8e03caa11d0ae5152ea8d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194342
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
This replaces the definition of ApplyEdits to be more like that in
go vet -fix, so that we can apply the results of suggested fixes.
Change-Id: Ib5724139464954e3790bc51ed1edc3ce4b2115ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193959
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The latest version of the LSP protocol introduces a number of changes.
It is now possible to indicate partial results and progress. request.ts
had to construct some new types (at the end of tsclient.go and tsserver,go)
to avoid using a struct for a formal parameter type. Also,
instead of using the same type for many RPCs, most RPCs now have their own
types.
Change-Id: I095a3e872f42a9f851c01ca4e3c6ac6e32446042
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194177
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
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>
The existing code uses maps to associate requests with responses. This
change adds locking to avoid simultaneous and illegal reads and writes.
Change-Id: I7bfb21cad6b37ac25e4f6946cb660d82f23c2b80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193058
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
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>
In shouldRunGopackages we would reset a goFile's metadata and pkgs in
advance of re-running go/packages. However, if we did not end up
running go/packages for whatever reason (read: we got canceled), the
goFile gets stuck in the unfortunate state of not belonging to any
packages because "pkgs" is empty. I think this leads to "no
CheckPackageHandle" errors, at least in relation to GetCachedPackage()
calls.
Fix by deferring the reset of goFile's metadata and pkgs until after
the go/packages call has succeeded.
Change-Id: I95aace85c026e1232b42cadee9e7772951c817d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193601
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
