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>
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.
Fixesgolang/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>
`go tools vet -shadow` ignored the case like this form
```golang
func shadowBlock() {
var a int
{
var a = 3
_ = a
}
_ = a
}
```
This commit fix it on "idiomaticRedecl" func, and add the code above in testcase.
Change-Id: I007f8287766f59cd7ded86072ba6bf6743c392be
GitHub-Last-Rev: b8b302b2048d709a39fb17496ba80917f9f4c889
GitHub-Pull-Request: golang/tools#143
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189158
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
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>
This change handles the case when an import gets added in an overlay a
package with a test variant. Previously, we would only add that
dependency to the test variant of the package.
Fixesgolang/go#34379
Change-Id: I82c3d72d7c2d0b970fb27c1aea5be71783b83764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196259
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Files named go.mod define a module boundary and punch a hole in the
repository when the module is fetched with go get. We had a couple in
testdata. Get rid of them.
In one case the changes I made to produce a module cache in packagestest
were enough. In the other, the test needs multiple minor/patch versions
of the same module, which we have no provision for. Rename them to
"definitelynot_go.mod" in the repo and fix it up in the test.
Updates golang/go#34352
Change-Id: I284578b3aebb0f1fec3ddb4bef0df24f050d0636
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196258
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
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>
When running in GOROOT/src, `go list -m all` shows the std package (or
cmd package) as the main module. This confuses goimports into adding
std/ or cmd/ at the beginning of import paths. Skip canonicalization for
paths under GOROOT.
Fixesgolang/go#31814
Change-Id: Iff5cc7e2a2053e4cc87c1a579a4c47d856cd0a2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195063
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
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>
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>
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 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>
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>
By default, github.com/natebosch/vim-lsc shows messages from stderr in
vim's :messages, which may cause problems and is generally annoying.
Suggest disabling that by setting suppress_stderr to v:true.
Change-Id: If997b8f8fd036a1ac08ba74a3886f33ff71413c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194137
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>
The sort.Slice method accepts an empty interface as its first
argument, but a slice type is the only valid use of the method.
This analyzer adds a diagnostic if the user uses the sort.Slice
method with anything other than a slice type as the first argument.
Change-Id: I3b54873faba2e9c2e832223a3cdab15a0b534650
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191598
Run-TryBot: Johan Brandhorst <johan.brandhorst@gmail.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@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>
If an absolute directory path being listed is outside any modules,
go list -e returns a non-zero exit status and non-empty stderr, but
should suppress the error. This was causing a weird bug when golang.org/cl/186337
was submitted because that changed the conditions when -export was passed,
which in turn affected how we suppressed the go list -e error (because
-export causes a compile it overtriggers errors, so we explicitly
suppress errors in that case). The way the error was being suppressed,
no error was generated, and no fake package was generated (which go list
is supposed to do), so the contains query fallback code wasn't run.
Fixesgolang/go#34265
Updates golang/go#34273
Change-Id: I1213cff0e03a62c6976e50db5b2d805aa3ddbb7a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195065
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
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>
When a user asks Syntax or TypesInfo, we need to typecheck from sources.
If the sources are missing fall back to trying to load from export data,
but still report an error.
This will help in some cases where the driver has incomplete data.
Change-Id: I3b23e90a5cd865c5f729e50f09f5fadff2d32994
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194683
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fixes this problem when trying to use the documented installation command:
```
$ GO111MODULE=on go get golang.org/x/tools gopls@latest
go: finding golang.org/x/tools latest
go: downloading golang.org/x/tools v0.0.0-20190911230505-6bfd74cf029c
go: extracting golang.org/x/tools v0.0.0-20190911230505-6bfd74cf029c
go get gopls@latest: malformed module path "gopls": missing dot in first path element
```
Change-Id: Ifdd13ec85eca5a069c17ae89486efe1abf4016f4
GitHub-Last-Rev: a4ddc02e3b8e6ccce12dfa711a8e7af5edb1302c
GitHub-Pull-Request: golang/tools#158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195037
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>
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.
Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.
Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".
Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.
Fixesgolang/go#31752, fixesgolang/go#33077, fixesgolang/go#32814,
fixesgolang/go#31699, fixesgolang/go#31930
Change-Id: I416e3c1035d555d67039e45a4fdd1deb9b2184ef
GitHub-Last-Rev: 2e3a46e6572d100ace486af33dcde45288cced46
GitHub-Pull-Request: golang/tools#139
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186337
Reviewed-by: Michael Matloob <matloob@golang.org>
