Previously, workdone reporting was managed by the goroutine(s) that
compute diagnostics following a file change. The intention was to signal
when diagnostics resulting from file changes were complete.
This was buggy, in two ways:
+ When no snapshots are determined to require diagnosis, no work is
reported.
+ If multiple snapshots required diagnosis, we'd get multiple work IDs.
Fix this by lifting up the 'work' to the level of didModifyFiles.
Change-Id: I73cf8bf9946dba777650bba9d4d18dad6954620e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235738
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Exit now closes the connection rather than exiting the process.
This allows things to shutdown gracefully, and removes special
cases. It also allows the tests to call CloseEditor instead of
just Shutdown, which prevents goroutine leaks.
Change-Id: I26121ba5d393ef74ce0e912611c8b3817e3691ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231798
Reviewed-by: Robert Findley <rfindley@google.com>
This allows us to rely on higher level functionality like timeouts and
close cancelling pending reads cleanly.
Change-Id: I3a43d21ed35d3da1eb818ea22f8d02201488a1d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230464
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Also the ability to wait for them to correctly close.
Change-Id: I198c9e24a21c04d5c05bae7a4a0f503429ab0346
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231699
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This test partially reproduces some strange behavior with creating
new tests files. In particular, it creates a new x test in a package
that already has a test variant and adds content with a missing import.
In the test, the import is never added. However, in my own experience
debugging this in VS Code, I see the import get added but the diagnostic
never get removed. One thing at a time though...
Updates golang/go#39315
Change-Id: I724a145688b915d04abd1f21efc6f9a7506be043
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235581
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change handles the case when an empty x test is provided in an
overlay. Previously, the package name was being changed back to the name
of the rest of the files in the package, but in reality, we don't want
that behavior for x tests.
Change-Id: I4c7d938e9c26b92d4a688d3cfd42755b35e2c1c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233657
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
When a file with errors is removed outside the editor, sometimes its
errors are cleared by the editor and sometimes they are not. If the file
is still open in the editor gopls does not clear the errors, taking the
editor's version as the truth. Otherwise the errors are cleared.
(This behavior depends on the editor sending gopls a notification that
the workspace changed.)
There seems to be no good way yet to test that gopls takes no action after
receiving the didChangeWatchedFiles notification.
Updates golang/go#38878
Change-Id: Ie418dd786d4c5f827cf0665a31f0f9913f7cfdc0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235377
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Setting innerHTML is a more heavyweight operation that supports HTML.
What's being done here doesn't use HTML, so it's viable to use the more
lightweight textContent.
Change-Id: I46e9359ffe776001836cc09fea88b04364870db7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235577
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Now that we're not using build tags any more we can consolidate files.
Do so.
I tried a little to find good places for the moved code, but only a
little.
Change-Id: I6b66afb7cad870471d7d4a3d86c13fadb94a40e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235457
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Many tool features, particularly modules-related, require particular Go
versions. Build tags are unwieldy, requiring one-off test files which
break up test organization.
Add a suite of testenv functions that check what Go version is in use.
Note that this is the logical Go version, as denoted by the release
tags; it should be updated at the beginning of the release cycle per
issue golang/go#38704.
For ease of reviewing, I'll merge/delete files in a followup CL.
Change-Id: Id85ce0f83387b3c45d68465161cf88447325d4f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234882
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Cached packages are probably more relevant than uncached packages, but
we still need to go in relevance order, since we'll stop adding results
after we hit the cap.
Fixesgolang/go#38461. (Hopefully.)
Change-Id: I555dd5f7568baa8d69760ed5836341a474e94346
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231619
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I made a silly mistake and checked the prefix on the import path rather
than the package name, which obviously breaks everything other than
top-level stdlib packages.
Fix that, then tweak the ranking a bit. We now get deep completions, which
is nice, but filled up the results too fast. Now instead of 5 results of
any kind, we give up after 5 packages searched.
Change-Id: I15b293f68f17531077da9ffe791a38ccc0e129f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231617
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Also switched the internals of the stream implementations to using
net.Conn to enable asynchronous closing, not yet exposed int the API.
Change-Id: I57f1c36e7a46729d24f4339ba2fecc3f868e823f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231698
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
This adds the ability to make a net.Conn based on a
reader/writer pair.
This is primarily useful for pretending stdin/stdout are a network
connection, but can be used for any reader/writer pair.
Change-Id: Ib398e62e0fac423db04aed27cfc48070bda2b93b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231697
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This adds a package for dealing with stacks in tests.
The only function at this time is NoLeak which verifies that a test
does not leak any goroutines, and prints a stack summary if it does.
Change-Id: I284b846f422334b745bef0e96d84138295120a62
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232681
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This can be used either to directly parse runtime.Stack output or
process text that includes stack dumps, like test timeouts or panics.
It includes a binary, gostacks that processes stdin to stdout replacing
stack dumps in place.
Change-Id: Id7b1cfd69b8aea36c66f12ec0bdf38b68cba5afb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232658
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
%O is supported since Go 1.13. See golang.org/design/19308-number-literals for
the background.
Support for %O has been added by copying and adapting the %o implementation.
Updates golang/go#29986
Change-Id: Ic49d3cc8d9aefcc0ecbfcfe5ebf206e6f951d413
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235100
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
One line legal code like `package x; import "os"; func f() {}` was
being misformatted. In these cases the parse flag ImportsOnly loses
important parts of the code, while full parsing works. Presumably
all these cases are short enough that there is no appreciable penalty
from the extra parsing.
Fixes https://github.com/golang/go/issues/36824
Change-Id: I9a4581d67c590578f8fdea5ed2a2a58e0bc3c40b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234900
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This reverts commit 655248709e.
Reason for revert: We have gotten reports of users still relying on binary export data
Change-Id: I82b06e79495ee1372552fe687dadbcad0702d967
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235279
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Windows does actually support symlinks, but older versions of
Windows only support symlinks when running as an administrator.
Newer versions of Windows support symlinks for all users.
Instead of skipping based on GOOS, first try the Symlink operation.
If it succeeds, we can proceed with the test; otherwise, we can try to
write a regular file to determine whether the problem was the symlink
operation itself or the destination path.
For golang/go#38772
Change-Id: Idaa9592011473de7f514b889859e420a84db6d01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234537
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Switching to using a t.Skip means we are more likely to remember to
actually re-enable the test at some point.
Also picked up a staticcheck fix along the way.
Change-Id: I382eaa8d204bee74a7ff46e8a1b11dab567b83ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Assigning a slice to the appendage of itself is common and tedious
enough to warrant a special case completion candidate. We now offer
smarter "append()" candidates:
var foo []int
foo = app<> // offer "append(foo, <>)"
fo<> // offer "foo = append(foo, <>)"
The latter is only offered if the best completion candidate is a
slice. It is inserted as the second-best candidate because it seems
impossible to avoid annoying false positives if it is ranked first.
I added a new debug option to disable literal completions. This was to
clean up some test logic that was disabling snippets for all tests
just to defeat literal completions. My tests were failing mysteriously
due to having snippets disabled, and it was hard to figure out why.
Change-Id: I3e8313e00a1409840cb99d5d71c593435a7aeb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221777
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change introduces Module and ModuleError struct types to the
packages package with the same types as defined in the cmd/go
documentation for module information output by go list (with the
exception of the Module type's Versions and Update fields).
go/packages will fill the module struct with the module information
output by go list. Drivers that support modules can also provide
module information by filling the Module fields in the packages in
their driverResponses.
Fixesgolang/go#35921
Change-Id: Icbdf79869f09d26f6a01c3670146ace4f6ffa25e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234219
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
fake.Editor.Server was exported, but CL 233117 was rebased on top while
still using the unexported field.
Update the rebased code.
Change-Id: Ie8f1f2c3948a1122b9e30a843cc33c79ef623ea4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234494
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
The code was introducting syntax errors for some edge cases (example in
regtest/import_test.go), and I found it hard to follow.
The new code passes all the tests. There are new regtests to guarantee
no CodeActions are returned for some cases that vim testing noticed.
Change-Id: Ia09da667f74673c11bfe185e4f76a76c66940105
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233117
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The priorities for which comment to show should be 1) documentation
directly above the var/const, 2) documentation for the var/const block,
3) line comments.
See https://github.com/microsoft/vscode-go/issues/3240.
Change-Id: Ie136f0f25ac8208147070682bb1f3a663d6da25f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234101
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Something is making this test deterministically fail in some
environments, such as @bcmills' desktop.
Skip it while I build go at tip and debug.
Updates golang/go#39135
Change-Id: I1bf8c55c5cfca471a904de85936f504313094807
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234480
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Many tools test check for the ability to compile cgo programs.
Consolidate them all into testenv.NeedsTool("cgo").
Change-Id: I62c96e7b4dc72df34b8fdbf10326c7d19e0613e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234108
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now that we support authoring cgo packages better, we need a way to
regenerate the C definitions. Doing it automatically is very difficult;
in particular, referencing a new symbol from the C package may require
regeneration, but we don't want to do that for every typo.
For now, give the user a button and make them push it. We attach a
code lens to the import "C" line. This is vulnerable to the usual
user-didn't-save glitches.
Updates golang/go#35721.
Change-Id: Iaa3540a9a12bbd8705e7f0e43ad0be1b22e87067
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234103
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
An import path like "foo/bar.v1" is still a local path, not an external
package, and should be grouped as such.
Change-Id: I06be3c01076f616a3cdc8e23bc9c056643035ad1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234111
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The format of descriptions for the `-remote.*` flags were inconsistent.
Clean them up.
Also remove a TODO in lsprpc.go about adding a test for telemetry. That
is enough of a separate concern (and one that is rapidly changing) that
I no longer think this TODO makes sense.
Change-Id: Id14796000634ae4be4b480d0386b1da9069737c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234113
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Currently, our hover text by default links point to public documentation
sites (e.g. pkg.go.dev). This doesn't make sense for private repos, so
hide the hovertext link when the import path matches GOPRIVATE.
Implementing this was a little messy. To be optimal I had to thread
the value of goprivate through cache.view, and to be correct I had to
duplicate some code from cmd/go internal.
Regtest will follow after https://golang.org/cl/232983 is submitted.
Updates golang/go#36998
Change-Id: I1e556471bf919fea30132d9642426a08fdb7f434
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233524
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In CL 229779 I enabled Cgo type checking for go/packages, but we don't
actually type check there. We need to enable it in our own type checking
too.
No test updates because the negative effects are relatively subtle and
caught by an upcoming regtest.
Change-Id: I31691d69eb104cdabfd4fbe0a14b1f3c9741eabb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234102
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The Mode field is part of an RPC interface with gopackagesdriver
implementations, so renumbering the Need bits by adding a new value in
the middle is a compatibility break. Move it to the bottom.
Also update an incorrect comment.
Change-Id: Idad9e9e8b163f3b516ff9055d503af8749dd343c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234099
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When on 1.15+, enable TypecheckCgo. This improves cgo support
significantly, but we'll still have trouble with newly-referenced C
identifiers and changes to the magic comment.
Updates golang/go#35721.
Change-Id: I44dc95ce2d91d552e1e66e3722dc4230ab59fedd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229779
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TestBadGOPATH tests for a empty element in the GOPATH list using
"GOPATH=:/path/to/gopath", assuming that ':' is the path list
separator. On Windows the test fails, because Windows path list
separator is ';'. Therefore this test isn't built for Windows.
On Plan 9, os.PathListSeparator is '\000', so the test fails
there too, and should not be built for Plan 9.
Change-Id: Icdc2f8de098c1415103ec6124906ad6c578ad183
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233718
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
