1
0
mirror of https://github.com/golang/go synced 2024-10-01 03:38:32 -06:00
Commit Graph

574 Commits

Author SHA1 Message Date
Rebecca Stambler
db1d4edb46 internal/lsp: make sure that deps are only checked in trimmed mode
This change encodes an invariant that, a dependency package will only
ever be parsed with trimmed ASTs.

Updates golang/go#34410

Change-Id: I2ceab3672c0bae0b98cec2a8e60b92a0c01a900f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-19 22:30:14 +00:00
hartzell
5adc211bf7 tools/internal/tool: refactor tool.Main() for testabilty
In it previous implementation, tool.Main was meant to be called from
an applications main().  If it encountered an error it would print the
error to standard error and exit with a non-zero status (2).

It is also called recursively and in various test functions.

Exiting on an error makes testing difficult, unnecessarily.

This change breaks the functionality into to parts: an outer
tool.MustMain() that is intended to be called by main() functions and
an inner Main that is used by MustMain() and by test functions.

None of the existing test functions use Main()'s error value, but the
failure case tests for the command line invocation of rename (#194878)
require it.

Fixes #34291

Change-Id: Id0d80fc4670d56c87398b86b1ad9fdf7a676c95b
GitHub-Last-Rev: cd64995c91c94b997754c8d8b1004afc488bf8b7
GitHub-Pull-Request: golang/tools#159
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195338
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-19 22:27:22 +00:00
Rebecca Stambler
928b73f71f internal/lsp: check the file's parse mode before formatting
Change-Id: I9ac3c273f305c41aed065cfcfdf96e59a828d71f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196317
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-19 18:00:25 +00:00
Muir Manders
f68e2b6f23 internal/lsp: fix infinite recursion while fixing AST
We were recursing infinitely in cases like this:

switch true {
case true:
	go foo.F<>
}

There were three things that came together to cause this:
1. We recently starting recursively fixing broken go/defer statements.
2. In this case we were failing to swap in the correct ast.Node in for
   the *ast.BadStmt because we were only looking
   for *ast.BlockStmt (and *ast.CaseStmt has no block).
3. After 2), we weren't returning an error so the fix() code thought
   it should recurse.

Fix 2) by using reflection to swap AST nodes in a generic way. Perhaps
a bit overkill in this case, but I happened to have already written
this for an upcoming change, so I just pulled it in to fix this bug.

Fix 3) by returning an error if we fail to swap the AST nodes.

Fixes golang/go#34353.

Change-Id: I17ff1afd52ae165c0ba9de5820dcec4cb7d756cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-09-19 16:27:28 +00:00
Muir Manders
7baacfbe02 internal/lsp: support function literal completions
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>
2019-09-18 23:49:17 +00:00
Rebecca Stambler
b8f1ca6a92 internal/lsp: fix bug in diagnostics for watched changed files
I accidentally copy-pasted code in CL 196019.

Change-Id: I092bfd745182f1c35e0540f080f139c4ced15776
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196277
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-18 22:02:41 +00:00
Rebecca Stambler
9098376d50 internal/lsp: change names used by per-folder configs
This was causing microsoft/vscode-go#2749, which was a result of users
having folders named "go", and VSCode-Go returning the "go"
configuration settings (which are all of its settings, not those of
gopls).

Change-Id: Ifbde3e32ad2de79265ed6adea53588c730ecd716
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-18 20:53:03 +00:00
Rebecca Stambler
2c18af7e64 internal/lsp: ensure that an AST cannot be nil without an error
Fixes golang/go#34366

Change-Id: I2f5269cd0b270aef5accc69adeced8be4523cb65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196142
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-18 18:10:22 +00:00
Muir Manders
3d643c64ae internal/lsp: add literal completion candidates
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>
2019-09-18 17:13:17 +00:00
Johan Brandhorst
f45d5df211 internal/lsp/source: add sortslice analyzer
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>
2019-09-18 16:24:38 +00:00
Rebecca Stambler
905c8ffbfa internal/lsp: fix diagnostics to report for all available files
Change-Id: Ie613161eb8444b5884733f0b6916b2f1971a4743
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196025
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-17 21:50:24 +00:00
Rebecca Stambler
11affa06ff internal/lsp: show errors when the user is in the wrong directory
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>
2019-09-17 21:41:55 +00:00
Rebecca Stambler
1cc9451822 internal/lsp: distinguish parse errors from actual errors
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>
2019-09-17 21:21:32 +00:00
Rebecca Stambler
d997db1b28 internal/lsp: return errors from diagnostics
Also, we were repeating the same code for diagnostics in 4 places.
Extract into a function.

Change-Id: I18d00e512a67679b915347a61126970cf02a7a4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196019
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-17 21:08:37 +00:00
Rebecca Stambler
3b4f30a44f internal/lsp: remove helpers for getting packages
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>
2019-09-17 16:23:42 +00:00
Rebecca Stambler
c52e9e2de4 internal/lsp: fix vet errors
Change-Id: I2435140e7b2b419df704e994ec45ad75932421b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195877
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-17 16:21:16 +00:00
Ian Cottrell
11bbd741f5 internal/memoize: changes to only one handle per key
This is to remove the confusion around having only handles that have had Get
called pin the value into memory.
Instead now there is a single handle per key, and it is the handle that is
weakly held not the value.

Change-Id: I9e813a0dfe2adf4cb651af9b5cfc8878fa71c041
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186839
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-09-17 14:40:27 +00:00
Rebecca Stambler
2dc213d980 internal/lsp: remove cachedFileToMapper function
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>
2019-09-17 03:27:47 +00:00
Muir Manders
5999de1043 internal/lsp: tighten up completion budget check
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>
2019-09-17 02:32:08 +00:00
Muir Manders
bb199b9d33 internal/lsp: reduce completion candidate volume
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>
2019-09-17 02:23:53 +00:00
Rebecca Stambler
fff8d94173 internal/lsp: use ParseGoHandles for the builtin package
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>
2019-09-16 23:04:25 +00:00
Rebecca Stambler
5edc6aefed internal/lsp: reduce usage of column mapper
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>
2019-09-16 22:00:07 +00:00
Muir Manders
b31ee645dd internal/lsp: avoid unnecessary type checking
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>
2019-09-16 20:14:40 +00:00
Rebecca Stambler
cb62a53de3 internal/lsp/debug: bump version number for release
Change-Id: I6626e807cb947292d82ffba6c93c8f0c55247328
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195621
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-16 17:20:13 +00:00
Rebecca Stambler
64a767472a internal/span: handle escaping file URIs
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>
2019-09-16 17:14:20 +00:00
pjw
e45ffcd953 internal/lsp.protocol: identify the version of the LSP that code is generated for
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>
2019-09-16 13:03:36 +00:00
Rebecca Stambler
92af9d69ef internal/lsp: handle potential nil pointer
Change-Id: I345b0d22053248f875683d87515d9e7c5e1d16c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195518
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2019-09-16 03:47:16 +00:00
Muir Manders
f3d05f808b internal/lsp: fix error formatting directive
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>
2019-09-16 01:55:42 +00:00
Muir Manders
1d8cfc4bd2 internal/lsp: omit "iota" completion outside const decls
Add a special check to skip builtin "iota" candidate outside of const
declarations.

Change-Id: I767c012585dfc51b4c07cf5847d3b4083a4a2a7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195044
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-09-15 20:16:06 +00:00
Peter Weinberger
0240832f5c internal/lsp/protocol: bring the typescript code up to date
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.

Fixes golang/go#34225

Change-Id: Ib66dad9476b452f332a4c0e990faf2c6060a588e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195297
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-09-13 18:13:37 +00:00
Rebecca Stambler
87d9f09c5d internal/lsp: turn fuzzy matching and deep completions back on
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>
2019-09-12 18:56:36 +00:00
Rebecca Stambler
c7d52e45e2 internal/lsp: use the view options, not the session options
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>
2019-09-11 22:59:40 +00:00
Rebecca Stambler
b13fa046aa internal/lsp: merge session and view options into one
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>
2019-09-11 19:36:49 +00:00
Rebecca Stambler
4f2ddba30a internal/lsp: prepare for tagged version
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>
2019-09-11 17:42:33 +00:00
Ainar Garipov
feee8acb39 all: fix more typos
Change-Id: I978ad5e1800ebfceb78aaced438331a8341715d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194697
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-11 15:13:14 +00:00
Ian Cottrell
16c5e0f7d1 internal/lsp: process configuration options more thoroughly
Change-Id: Ic3e2f948f857c697564fa6ab02008444dd9392c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194458
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-09-11 02:21:29 +00:00
Rebecca Stambler
ec76318e79 internal/lsp: treat completion documentation errors as actual errors
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>
2019-09-10 23:05:03 +00:00
Rebecca Stambler
3e6e7c4239 internal/lsp: add ID to the package cache key
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>
2019-09-10 22:12:34 +00:00
Heschi Kreinick
d0542c01b0 internal/imports: add all interfaces in mkstdlib
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>
2019-09-10 20:25:02 +00:00
Muir Manders
5e3480f0e0 internal/lsp: start handling watched file deletes
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>
2019-09-10 20:08:16 +00:00
Heschi Kreinick
df93a1b922 internal/imports: fix mkstdlib, run for go1.13
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>
2019-09-10 19:51:17 +00:00
Muir Manders
3cd124fa3e internal/lsp: fix completion for nested *ast.BadStmt
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>
2019-09-10 16:55:22 +00:00
Ian Cottrell
3d22a3cfff internal/lsp: only build a view when we have its configuration
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>
2019-09-10 14:40:41 +00:00
Rebecca Stambler
238129aa63 internal/lsp: derive ASTs from type information
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>
2019-09-10 13:53:09 +00:00
Muir Manders
dd2b5c81c5 internal/lsp: simplify snippet config/generation
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>
2019-09-10 04:45:52 +00:00
Muir Manders
5797d2e298 internal/lsp: add more flexible completion tests
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>
2019-09-10 04:40:04 +00:00
Michael Matloob
75be6cdcda internal/lsp: enable suggested fixes by default
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>
2019-09-09 19:40:07 +00:00
Rebecca Stambler
dab579b762 internal/lsp: re-enable deep completions and fuzzy matching
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>
2019-09-09 19:15:43 +00:00
Muir Manders
dc339cc7c5 internal/lsp: improve completions in go and defer statements
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>
2019-09-09 19:09:43 +00:00
Rebecca Stambler
cdebb59945 internal/lsp: remove the GetToken and GetAST functions
Change-Id: Iddbdde5f47a31da9baab6539cd2b5bd858e7f811
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194057
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-09-09 18:10:35 +00:00