This change adds support for goimports as a code action that can be run
on save. However, there do appear to be issues with the propagation of
the context.Only field of the CodeActionParams, so we treat every
codeAction as an organizeImports action - this should be fixed in the
next vscode-languageclient release
(https://github.com/Microsoft/vscode-languageserver-node/issues/442).
Change-Id: I64ca0034c393762248fde6521aba86ed9d41bf70
Reviewed-on: https://go-review.googlesource.com/c/154338
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Create helper functions for the exported URI functions to test
the logic that isn't OS-specific (filepath.{To,From}Slash is the OS-specific part).
Also add helpers to determine is a file or URI path is Windows-specific.
Change-Id: I6ba5119424ad5edcd59b946276e4268b2525505f
Reviewed-on: https://go-review.googlesource.com/c/153867
Reviewed-by: Ian Cottrell <iancottrell@google.com>
If several RPCs finish together the existing code may mix the log
messages, making reading them difficult.
Change-Id: I1de15373f739c97ccfaf210511b5f5443f7ef036
Reviewed-on: https://go-review.googlesource.com/c/154346
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Primarily for golang/go#29251 but also update the base Docker layer
from Go 1.9 to Go 1.11, update other deps, and fix some Kubernetes
config cleanups that happened prior without testing apparently.
Fixesgolang/go#29251
Change-Id: I0aafccdfedfc0d9ebb75d0c1a3b0819245ea5f19
Reviewed-on: https://go-review.googlesource.com/c/154181
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Import test fix https://golang.org/cl/153831 from std lib
version (skips issue29198.go test for older versions of gccgo).
Updates golang/go#29198.
Change-Id: Iddd903068a73a307b1d19d93a7c03b1d00365022
Reviewed-on: https://go-review.googlesource.com/c/153870
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When in module mode (go env GOMOD is nonempty), use go/packages to load
package information. This adds module support, without slowing down
non-module mode. Functionality should be the same in both cases.
Since we intend to use go/packages for everything eventually, the tests
run in three modes: GOPATH, using the old code paths; GOPATH_GoPackages,
using go/packages on a GOPATH layout; and Modules, using go/packages on
a module layout. They do take a little while to run.
The code path for go/packages is a lot more convoluted than it should be,
since it mixes in and out of the older path. I expect we'll clean that
up after deleting the old one.
Change-Id: I5b54bd470e091e6fb429597dfbfd1c8fa03f5b59
Reviewed-on: https://go-review.googlesource.com/c/152317
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Import changes from std lib version, specifically
https://golang.org/cl/153821
which fixes a bug (#29198) in reading of import data.
Change-Id: I26d4da015b88bbc8f4f7775b0850bef5dffadbec
Reviewed-on: https://go-review.googlesource.com/c/153868
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
URI should be started with file:/// always.
Change-Id: I123e577d421de3e85dfec00596fbdb63c2231938
Reviewed-on: https://go-review.googlesource.com/c/153618
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is the fix I should have made in CL 153440. What I get for being in
a rush.
Fixesgolang/go#29180
Change-Id: I7ee3e26173b86c70574b9710f84094e46db27a37
Reviewed-on: https://go-review.googlesource.com/c/153859
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
ReverseProxy doesn't re-set the Request's Host field, only
Request.URL.Host.
The HTTP/2 client prefers Request.Host over Request.URL.Host, so this
results in the request being sent back to the host that originally
accepted the request.
This results in an infinite redirect (and consumption of many connections to
itself).
See Issue golang/go#28168 for details.
Replace it with a simple proxy that drops all the headers (except
Content-Type).
I tried setting the proxy.Director, but it still didn't work. Could do
with some more investigation.
Fixesgolang/go#28134.
Change-Id: I5051ce72a379dcacfbe8484f58f8cf7d9385024d
Reviewed-on: https://go-review.googlesource.com/c/141718
Run-TryBot: Chris Broadfoot <cbro@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fix some logic errors around renamed sibling imports. The one in
findMissingImport was just a silly mistake, the one in addCandidate
was because I hadn't thought about renamed imports.
Change-Id: Iecd0b4e6151bff7b8cb6ad21065f017fb1245bfd
Reviewed-on: https://go-review.googlesource.com/c/153440
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We've been using https://github.com/stamblerre/vscode-test-golsp to test
the golsp. Ideally, we'd like to add integration tests with the major
editors, so we're moving this basic extension into x/tools, with the
intention of integration tests here. This is not meant to be used by any
users of golsp; it is only for testing.
Change-Id: I7c45e7c638311d29629ff6d3291332e68cf4621a
Reviewed-on: https://go-review.googlesource.com/c/153438
Reviewed-by: Ian Cottrell <iancottrell@google.com>
imports exposes a few function vars that can be overridden to customize
its behavior. Now that the pass type exists, it should be easier to use
that as a rough API, rather than replacing functions piecemeal.
Remove all the existing hooks and just allow for fixImports to be
overridden entirely.
Change-Id: I8400eb64dab7b7b1bfa0b85fe2599df5780ddf37
Reviewed-on: https://go-review.googlesource.com/c/153437
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some people put multiple packages' files in the same directory, most
commonly commands and code generators. Global variables from one package
should be ignored in other packages.
Change-Id: I9a5d27778570183dfe391dd3273dfa8277a29bf2
Reviewed-on: https://go-review.googlesource.com/c/153419
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add a default "auto" value to the "--logfile" flag that allows logs to be
written to a default location.
Change-Id: I1952ad2622b824795906c6b8183b58f88c35fb62
Reviewed-on: https://go-review.googlesource.com/c/153197
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is needed to write the more advanced test cases for editor integrated
tools.
The expectation system also uses the overlay rather than the file if it is
supplied.
Change-Id: I8fd21f4efe5ac5869fa6e25d3cd0d5096051e5e5
Reviewed-on: https://go-review.googlesource.com/c/153240
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A few minor fixes: check that the insert text has the expected prefix
before removing it, use a bytes.Buffer to build up snippets, and check
that functions have parentheses in their insert text before trimming
them.
Change-Id: Iaeaf6fda25d6b0a375460b975188486685be1061
Reviewed-on: https://go-review.googlesource.com/c/152978
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
we check length of the map, and a non empty map forces source mode.
Change-Id: Id314ad87b96d9154198122586fd8672a70c5ca95
Reviewed-on: https://go-review.googlesource.com/c/153198
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
files specified in file= queries will now also be checked for in overlay
packages.
Fixesgolang/go#29048
Change-Id: I096d02c3777e6c7decedfcb97c0d011ce9b47b28
Reviewed-on: https://go-review.googlesource.com/c/152797
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change separates a cache package out of the
golang.org/x/tools/internal/lsp/source package. The source package now
uses an interface instead a File struct, which will allow it be reused
more easily. The cache package contains the View and File structs now.
Change-Id: Ia2114e9dafc5214c8b21bceba3adae1c36b9799d
Reviewed-on: https://go-review.googlesource.com/c/152798
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change adds a very simple implementation of hovering. It doesn't
show any documentation, just the object string for the given object.
Also, this change sets the prefix for composite literals, making sure we
don't insert duplicate text.
Change-Id: Ib706ec821a9e459a6c61c10f5dd28d1798944fa3
Reviewed-on: https://go-review.googlesource.com/c/152599
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This CL updates the importer to match the original code in the std lib
but for the necessary changes to make the code work in x/tools and
with older versions of the std lib.
Notably, it brings over changes from:
https://go-review.googlesource.com/c/152378
Change-Id: Icaa05303a05a226c710b5923d8574f255651fe47
Reviewed-on: https://go-review.googlesource.com/c/152577
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The interface is now stable.
Change-Id: I7dc3feb70131cddb003f9320272a0fbd9b314048
Reviewed-on: https://go-review.googlesource.com/c/152597
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
importer.For does not populate the caller's token.FileSet
leading to spurious position information in diagnostics.
This change is the upstream fix for
https://go-review.googlesource.com/c/go/+/152258.
Fixesgolang/go#28995
Change-Id: I9307d4f1f25c2b0877558426d4d71b3f1df99505
Reviewed-on: https://go-review.googlesource.com/c/152578
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The first issue is that %b and %o can print pointers, but printVerbs
didn't reflect that:
The %b, %d, %o, %x and %X verbs also work with pointers,
formatting the value exactly as if it were an integer.
The second issue is that arrays can never be printed as pointers. This
was previously reported as part of #27672.
The third issue is that only %p can print all slices, maps, and
functions as if they were pointers. This differs from verbs like %b or
%o, which can't print these types as pointers.
Fix all of the issues above, and add extensive test cases covering all
the combinations. Verified all of them with an executed program. The
amount of test cases is perhaps overkill, but this is not the first time
we've gotten the printf pointer logic wrong.
Updates #27672.
Fixes#28858.
Change-Id: I62eb79d505fd1e250a16b90bda3c68b702f35a29
Reviewed-on: https://go-review.googlesource.com/c/149979
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Previously running "go tool vet" said
vet: invalid command: want .cfg file (this reduced version of vet is
intended to be run only by the 'go vet' command)
With this change it says:
vet: invoking "go tool vet" directly is unsupported; use "go vet"
Updates golang/go#28869
Change-Id: I603ab2f75bb52d860e5cd7466e89d051dfbf3f08
Reviewed-on: https://go-review.googlesource.com/c/152217
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
For clarity and performance reasons, we want the fast path of goimports
to be purely syntactic. Packages whose import paths don't match their
package names make this harder. Before this CL, we parsed each imported
package to get its real package name. Now, we make named imports for
such packages, and on subsequent runs we don't have to do the extra
work.
A package name matches its import path if the name is the last segment
of the path, or the next-to-last before a version suffix vNN. gopkg.in
style .vNN suffixes are considered mismatching.
The previous attempt (CL 145699) failed because I assumed we'd be able
to find all packages from scratch. That's not true if the import path is
completely unrelated to the package name. To avoid that problem, rather
than removing and re-adding mismatched imports, we just literally add
names to them.
Fixesgolang/go#28428
Change-Id: I93bd820f5956162ae9c99afa73bdcfc570a2e7b4
Reviewed-on: https://go-review.googlesource.com/c/152000
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Redesign fixImports to have a clearer workflow, and hopefully create
clear places to plug in go/packages. This change is mostly
performance/functionality neutral, but does clean up some corner cases.
The new flow centers around the pass type, which encapsulates the
process of loading information about the current code, adding possible
new imports, and trying to apply them. I'm hoping that it's easy to
understand what's happening just by reading fixImports, and that new
sources of information (e.g. a network service) fit well into that flow.
Where possible, I left the functions near where they were extracted in
hopes of making review easier, but it's probably not going to be easy.
Sorry. I might move them into a more reasonable order in a followup CL.
Notable modifications:
- The stdlib cache is restructured to match pass' internal storage.
- Sibling imports with conflicting names are considered.
- Package name lookups are batched, hopefully making it easier to plug
in go/packages.
Questions that might be worth answering:
- Should findImportGoPath really scan $GOROOT? Unless the user is working
on a development copy, it's totally redundant with the cache.
- What is the best way to combine candidates from multiple sources?
Right now the first one wins, and findStdlibCandidates relies on that
to get crypto/rand ahead of math/rand.
- In the third pass, should it assume sibling imports or should it
actually go load the exports? It didn't load them before, but that seems
arbitrary.
Change-Id: Ie4ad0b69bfbe9b16883f2b0517b1278575c9f540
Reviewed-on: https://go-review.googlesource.com/c/150339
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It used to rewrite golang.org/x/* imports to golang_org/x/*.
But https://golang.org/cl/147443 renamed golang_org/x to internal/x,
which broke the ability to run "go generate" in net/http, which runs
this command.
Given that net/http (and Go itself) is the only caller of cmd/bundle
or the -underscore flag, repurpose it to instead rewrite the
golang.org/x imports to internal/x, like CL 147443. But we keep its
name out of laziness and to minimize the number of cross-repo changes
needed.
Change-Id: I310ce8b45812a26c8b3522eaf407fffff138b1be
Reviewed-on: https://go-review.googlesource.com/c/152097
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This CL updates the importer to match the original code in the std lib
but for the necessary changes to make the code work in x/tools and
with older versions of the std lib.
Notably, it brings over changes from:
https://go-review.googlesource.com/c/152078https://go-review.googlesource.com/c/152077https://golang.org/cl/151997https://golang.org/cl/151557https://golang.org/cl/149957
including test fixes (we want tests to run when gccgo is available,
not just when all go tools are gccgo-based), bug fixes (primarily
related to aliases), performance enhancements, and new code to read
the V3 export data emitted by the most recent gccgo.
Change-Id: I2d34bace23769e62795599b93db8295169076594
Reviewed-on: https://go-review.googlesource.com/c/151717
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This support is not perfect, but should produce more accurate results than
the previous behavior, which is to silently drop those files.
Change-Id: Ia00c042bf303e9ec0fb1cbd579c0fccb29073de0
Reviewed-on: https://go-review.googlesource.com/c/151999
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The expectation langauge is LL(1) but the scanner does not support true
lookahead
This change adds a true LL(1) token stream and rewrites the parser in terms of
it.
Also clean up the error handling and use the behaviour to fix all the broken
edge cases, and then change the tests to cover the now correct behaviour.
Change-Id: If3d602cda490ed2f4732efce400eb8eabce8a8ec
Reviewed-on: https://go-review.googlesource.com/c/151998
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When I tried to use the Emacs instructions, I found there was no go-mode-autoloads available. Searching, I found: https://github.com/dominikh/go-mode.el/issues/222
Removing the (require) line solved my problem.
I don't know what the add-to-list invocation was supposed to do, so I propose removing it too.
Change-Id: I027654dd2c634f0747dcefee71f413866049a608
GitHub-Last-Rev: 7d43dabf6a46210eaaa849900c13cd52001878cb
GitHub-Pull-Request: golang/tools#57
Reviewed-on: https://go-review.googlesource.com/c/151680
Reviewed-by: Dominik Honnef <dominik@honnef.co>
This allows for multi-line comments where a new line is allowed between
expectations.
It also allows trailing new lines and commas in expectation lists.
Change-Id: I714f8ad483a0238281f26ceb74ce9e73d922af69
Reviewed-on: https://go-review.googlesource.com/c/151718
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Our tests compare import graphs from go/packages to expected graphs,
and one of the test cases imports math/bits. But in tip math/bits
picked up a dependency on unsafe, which means the expected graph
is different when run against a Go version >= go1.11. Remove that edge
before comparing against the expected graph to work around the breakage.
Change-Id: Ic586a75ba530741d251df9f87d0817a8e37d92ea
Reviewed-on: https://go-review.googlesource.com/c/151657
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Currently, stress logs are named "/tmp/go-stress-" plus a random
number. This makes it really annoying to tease apart failures from
different stress runs.
This CL changes the default prefix to
"/tmp/go-stress-<ISO 8601 date/time>-<random number>"
where the date/time part is when the stress command started. This
naming clusters logs by stress invocation, making it easy to tease
apart invocations, even if there are multiple stresses running
concurrently or they are run back-to-back.
This also provides a flag to override this prefix.
Change-Id: I043e5ee7168ba6db4e2355e39b147071edbc6864
Reviewed-on: https://go-review.googlesource.com/c/150047
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The LSP specification doesn't have a Score field, so we must provide
sortText to the protocol in order to maintain the correct order.
Change-Id: I075849f520c21a0465dfb2060c598d8bae5f876b
Reviewed-on: https://go-review.googlesource.com/c/151237
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This CL moves gorename TODOs from -help output
to a comment in the code.
Fixesgolang/go#25824
Change-Id: I60e843804ebf1e374882ea3424e4ab749d038c1f
Reviewed-on: https://go-review.googlesource.com/c/150937
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
When running go list for a file= query, the go/packages code previously ran "go list ." command in the directory of the provided file. This was necessary for compatibility with the fallback, but caused problems when running file= queries for files in the module cache. We now use absolute directory paths (supported in Go >=1.11) in the non-fallback case and retain the previous behavior in the fallback case.
Change-Id: I3a9feaaeda12e068ec3d510b5f78437482c65b4c
Reviewed-on: https://go-review.googlesource.com/c/148199
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is needed because testing.TB does not have a "Run" method for sub
tests, and the signature of the function it takes needs to match the
testing parameter.
Change-Id: I9201dd75891ed1bda300a4247fe7eac1e25bd30d
Reviewed-on: https://go-review.googlesource.com/c/150857
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Pass in parameters to the driver in stdin rather than through argv.
This allows us to more easily route parameters to the driver and add
more parameters without breaking users. The driver doesn't need to
have the same interface as go list, because it's unlikely the driver interface
will converge with the go list interface.
We still pass in the query "words" to the driver in the argv, because
that's as good a place as any, but there's no "list" command.
Question: should we add a "command" option to the driverRequest struct?
Change-Id: Ifdbb3f84b6bfd04259f5ab63e756341d7f69de9b
Reviewed-on: https://go-review.googlesource.com/c/150337
Reviewed-by: Ian Cottrell <iancottrell@google.com>
