1
0
mirror of https://github.com/golang/go synced 2024-11-19 01:34:40 -07:00
Commit Graph

4773 Commits

Author SHA1 Message Date
Rebecca Stambler
6441d34c3f internal/lsp: fix caching issue with duplicate handles
In (*snapshot).addPackage, we return early if the package handle is
already cached, but we continue building the dependency graph with a
handle passed into addPackage.

This seems fine since both handles should have the same cache key,
but if we clone the snapshot, we will end up dropping the handle that
had the type information on it. It will then have to be recomputed,
causing the skew in the types.Package.

Fixes golang/go#38403

Change-Id: I0e360447a428123fcac444fbea3c2a3232ef941a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-07 19:23:25 +00:00
Rob Findley
08cbf656ce internal/lsp/cache: add an UnsavedFiles method to Session
It is useful to know whether the session has any unsaved files, for
example to warn/error when executing a command that interacts only with
files on disk.

Add a new UnsavedFiles method to the Session.

Change-Id: Iea4bf472e3ed6897979306b670eb974a2ee0d3bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232747
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-07 17:51:45 +00:00
Richard Miller
ff647943c7 internal/testenv: make ExitIfSmallMachine apply to plan9-arm builders
Some x/tools tests use too much memory (virtual or real) or other
resources to run on "small" builders. Originally only linux-arm
was classified as small. This change addes plan9-arm to the list,
since it normally runs on Raspberry Pi boards with 1GB of RAM.

Partial workaround for golang/go#38772

Change-Id: I93307af10cccf7b1e26d57b9af914c85cf676d43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232478
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-07 17:29:48 +00:00
Hana (Hyang-Ah) Kim
625332f3c5 internal/lsp/protocol: send responses for cancelled requests
LSP https://microsoft.github.io/language-server-protocol/specifications/specification-current/#cancelRequest
expects the server to send back the response even when the request is cancelled.

Gopls LSP protocol implements the cancellation using the context cancellation.
That is, upon a cancellation request from the client, the server calls the
corresponding canceller that cancels the context passed to the handler.
Reusing this cancelled context for the replyer is not safe because code in any layer
can decide to shortcircuit and skip sending the data back to the client.
E.g. https://cs.opensource.google/go/tools/+/master:internal/jsonrpc2/stream.go;l=63

This CL make sure to pass the detached context to the replier.

Alternative, or a better approach to avoid any unexpected side-effect of
using a detached context is to send out the response at the point of cancellation
with a separate context. But that requires more significant code change.

Testing is currently hard, but maybe doable once the current refactoring is
done. Test is left as a TODO.

Change-Id: I4611af00ad913e96b62c6b7180c6673b0465daf9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232300
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-05-07 15:26:07 +00:00
smasher164
a1532b81a2 internal/lsp/cache: avoid string(int) conversion
LoadMode and ParseMode are currently hashed with a string(int)
conversion, but this conversion is now discouraged (see the vet check
'stringintconv' for more information). Since the hash uses the code
point, this change replaces these instances with string(rune(int)).

Updates golang/go#32479.

Change-Id: I0d8e91d073fc34ac9faafe75a0d86cf71a5327d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232679
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-07 05:02:07 +00:00
Rob Findley
be0c89d0d3 internal/lsp/fake: check for file changes after running the Go command
Our fake.Workdir generates synthetic file events for "watched" files
when using its file API, but we have no such hooks for file changes
originating from an external process, in this case the go command.

Previously, we detected a file event by checking go command logs to see
if a go.mod file was created. This is bound to be fragile, so we replace
this with a helper function that walks the working directory looking for
changes.

Change-Id: I2c2176099b97f3a3b25f4aed2b18de13ae05544a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232637
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-07 03:33:52 +00:00
Rebecca Stambler
88e38c1d8d internal/lsp: make sure diagnostics only refer to existing files
We were previously sending diagnostics for nonexistent files, and then
adding them to the snapshot in the process. Remove this behavior, and
add a regression test. Case insensitive filesystems were too confusing
to write a test for, but fortunately, Filippo reported another instance
of this bug, so I used that for the regression test.

Fixes golang/go#38602

Change-Id: I4ef6b51944f3338e838875a5aafffd066e8392f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230315
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 02:01:22 +00:00
Rebecca Stambler
002d754683 internal/lsp/regtest: add test for a GOPATH that's missing an element
Test the case described in
https://github.com/fatih/vim-go/issues/2673#issuecomment-622307211.

Change-Id: I55ff3b8719fc255ec0901cf3778e68b48630323d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232360
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 01:58:00 +00:00
Rob Findley
c20a87c16a internal/lsp/fake: split up and rename the Workspace type
The Workspace type has accumulated too much additional functionality of
late: managing the Env, GOPATH, and GOPROXY in addition to the working
directory. Additionally, the name 'Workspace' can easily be confused
with 'workspaceFolder' in the LSP spec, and they're not quite
equivalent.

Split off a Proxy type to be responsible for the fake module proxy, and
a Workdir type to be responsible for working with the temporary
directory. Rename what remains of 'Workspace' to a more appropriate name
for such a collection of resources: Sandbox.

This is mostly just moving things around, with one significant change in
functionality: previously our three temporary directories (workdir,
gopath, and goproxy) were in separate toplevel directories below
$TMPDIR. Now they are all below a new sandbox temp directory, so that
they are correlated in the filesystem and can be cleaned up with one
call to os.RemoveAll.

Change-Id: I1e160a31ae22f0132355117df941fe65822900eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230758
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-06 18:17:57 +00:00
Dominik Honnef
26f46d2f7e go/types/objectpath: cache result of call to scope.Names
Names is a somewhat expensive method, which sorts the returned slice.
Avoiding the extra call helps clients that use objectpath a lot.

Change-Id: I49a3445bb4f0056d1b915a2104e136925ee9dbf9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231518
Run-TryBot: Dominik Honnef <dominik@honnef.co>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-05-05 02:31:15 +00:00
Rob Findley
9f0e5ee6c7 internal/lsp/fake: skip TestWorkspace_CheckForFileChanges
This test is failing on darwin-amd64-10_12: skip it while I investigate.

Change-Id: I506f54355924e86cf4235b04fd8890f59b48ac33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232198
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-04 21:58:16 +00:00
Rob Findley
9bfbc38543 internal/lsp: fix incorrect format strings when calling event.Error
I noticed that in a couple places, event.Error was called with a
message containing formatting verb. This was my likely done out of
habit, but is an incorrect use of the API: err is not formatted in the
message but is rather applied as an event label.

Remove the unused formatting verbs.

Change-Id: I52f914da81e338013c7449066e5d9ffa40a0a4c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232137
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-04 19:35:31 +00:00
Rob Findley
1d9c21a7db internal/lsp/regtest: add support for testing generate commands
Our editor interaction for running `go generate` was untested. Add
support for triggering generate from the fake editor, and a simple test.

To enable this, some helpers were added to list Workspace files and
check for file state changes, to avoid having to synthetically create
file events. This workaround is not ideal as it results in a leaky
abstraction: in other cases the regtest may assume that FileEvents are
triggered by workspace interactions (e.g. ws.WriteFile), but in this
case it cannot. Unfortunately the only real solution for this would be
to make file watching more realistic, by polling file state on an
interval or using an actual file watching library. Neither of those
options seemed worthwhile just to keep the fake.Editor API pristine.

A new debugging option is added, SkipCleanup, to allow inspecting
regtest working directories after a test with minimal code change.

Change-Id: I64dceeb21a4eb9eff2b6936e44f80f4bd24b82da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230313
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 19:25:30 +00:00
Ian Cottrell
33427f1b03 internal/jsonrpc2: rename NewStream to NewRawStream
NewStream implies the default stream type, which it is not.
NewHeaderStream is actually the default choice.

Change-Id: I1744d7e902d27c13393f3b367fe2d29e5d7dc283
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231618
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
2020-05-04 15:25:39 +00:00
Ian Cottrell
535e1470ec internal/lsp: use %w in error wrappers
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.

Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 14:52:14 +00:00
Clint J Edwards
6b6965ac5d internal/lsp: add comment completions for remaining exported symbols
This should provide simple name completions for comments
above exported vars, constants, functions, and types.

Can be activated with `ctrl+space` within a comment.

Also fixes a panic introduced in the previous commit when completing comments that occur at the end of a file.

Fixes #34010
Fixes #38793

Demo: https://i.imgur.com/qN82CVA.mp4

Change-Id: If9aaec7ce03a3e085361144bce5c7a66535127d1
GitHub-Last-Rev: b9ac874c7ff3c9a164ba698d0d561141a59e2435
GitHub-Pull-Request: golang/tools#224
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230215
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 02:29:51 +00:00
pjw
ed308ab3e7 internal/lsp: avoid showing no-GOPATH-nor-module message so much
If the user's workspace is neither in GOPATH nor a module, and there
are errors in their code, send a message (with ShowMessage) only on
the first load, or when the configuration changes. The previous
behavior sent the message more frequently.

There is a regtest, and two new Expectations for when the fake
editor sees (or does not see) a ShowMessage notification.

Fixes golang/go#37279

Change-Id: I076bd95105359b9310dcf97019e3559159271356
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230897
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-02 20:28:11 +00:00
Robert Findley
542909fd99 internal/lsp/cmd: partially revert "add a flag to disable telemetry"
This reverts commit 17a19b5fe7. The revert
is partial because that change also added the -short flag when running
govim tests, which we preserve as without this the tests often time-out
(and I don't want to increase our test timeout right now).

Reason for revert: telemetry races have been fixed in https://golang.org/cl/226317

Change-Id: I5fcf034c1fe6e2db48994e2f06b73a593c779e54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231637
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-01 20:57:27 +00:00
pjw
2658dc0cad internal/lsp: add a regtest for formatting one-line files
Issue https://github.com/golang/go/issues/36824 complained about
legal go code (e.g., 'package a; func f() {}') that was mishandled
(by being rewritten just as 'package a'). This bug seems to have been
partially fixed, as certified by the new regtests. The comment on
OneLineImports36824 says that the bug would be fixed if gopls
formatted the file before fixing the imports, but it doesn't.

Change-Id: If27fa738e54d9434d5b2f17ed4e52d555cb7c499
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229303
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-01 15:50:19 +00:00
Rebecca Stambler
ab2804fb9c internal/lsp: don't offer suggested fixes for generated files
As suggested on Slack, a better fix for golang/go#38467 would be to hide
suggested fixes on generated files. This way, the diagnostics are still
visible but files are not unintentionally modified.

Also, deleted the SuggestedFixes field on source.Diagnostic, since it's
entirely unused.

Change-Id: I10756471e0f913465b1cccd7f222eea0f4de77fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230999
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-01 06:56:59 +00:00
Ian Cottrell
d351ea090f internal/event: fix the agent start time
The current use of start time in the cache key prevents re-use of agents under
some circumstances, so we update it later in the exporter instead.

Change-Id: I2f6927d65a0841f77a0ee1b848b5a3b243936083
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231257
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-01 00:59:04 +00:00
Muir Manders
f26c0dd982 internal/lsp/source: improve unimported package member ranking
Untyped members from unimported packages are scored the same as typed
members from unimported packages. We depended on the unimported
package relevance to rank the probably-more-relevant typed members
higher. However, there are some unrelated score penalties that can
only be applied to typed candidates, so the untyped candidates ended
up being ranked higher. Fix by increasing the relevance coefficient so
the relevance score overpowers other less important scoring
adjustments.

Fixes golang/go#38104.

Change-Id: Ie43f769a41511f9cc3747ce6936130be7a29cd31
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231238
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-30 22:11:53 +00:00
Rebecca Stambler
2840dafb9e Revert "internal/lsp: hide analysis diagnostics from generated files"
This reverts commit e4881b2459.

Reason for revert: <CL 230999 has a different approach to this>

Change-Id: I9ec47d858e7db2a66ec8a93063ab950b8553e45b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231042
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-30 19:28:56 +00:00
Ian Cottrell
4b814e0613 internal/event: re-use ocagent exporters
This attempts to detect a Connect call with the same configuration and return
the same exporter.
This mostly affects tests where we end up starting a new ticking go-routine per
test, even though they all have the same configuration. It was noticable that
a goroutine dump of a test would have a very large number of go routines that
were just ticking exporters, making it very hard to see the real work.

Change-Id: Ie8b64b69fce736d2bf3dbadffc1681f7caee6dd8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230463
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-30 04:03:29 +00:00
Ian Cottrell
28a3422bd6 internal/jsonrpc2: move the lock into logCommon
This means the lock is no longer held when writing to the underlying
stream.

Change-Id: I4f066ff593e35d771aa989c762712e4dd83a3f81
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231040
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-30 04:03:07 +00:00
Rob Findley
127c98bd79 internal/lsp/regtest: rearrange runner.go for readability
This is purely moving code: the getRemoteSocket, getTestServer, and
AddCloser funcs were above the more important RunOptions and runner.Run.
Move them closer to their usage.

Change-Id: I6d5b5577ca8ba5cbd5226b195541696967433dec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230998
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-29 21:33:35 +00:00
Heschi Kreinick
290e0af1a2 internal/lsp/regtest: cosmetic fixes
Add issue comments to tests for issues, move a couple constants closer
to their use.

Change-Id: I34fa2643195ae81e463a070952a1a1a4af2c6132
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230997
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-29 21:16:28 +00:00
Rebecca Stambler
b8428586f4 internal/lsp: handle hover documentation for package declarations
We had previously not been generating documentation on hover for
package declarations or import specs. We do this by adding a few special
cases, since package declarations don't appear in type information.

Throughout, we make the assumption that only one file in a package will
have the documentation for the package. go/doc just appends
documentation as it sees it. We may be able to do better by checking for
a "Package ..." but that still is not guaranteed. Not sure what the
right approach is, so this assumption may be the best option.

Fixes golang/go#38526

Change-Id: Ibc515f8729e1aba0d11090be62371be6b18d0cfa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-29 20:57:41 +00:00
Rebecca Stambler
e4881b2459 internal/lsp: hide analysis diagnostics from generated files
Don't show non-vet analyses when they appear in generated files. Vet
analyzers will give useful reports even in generated files.

Fixes golang/go#38467

Change-Id: I0e628760b386553932de4cf1f5ba39784a205b53
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-29 20:34:34 +00:00
Rob Findley
0c9eba77bc internal/lsp/regtest: add a OnceMet combinator for Expectations
A common problem when writing regtests is that if you have an error in
your expectations, you must wait until the regtest times out to see what
went wrong.

With the integration of additional progress reporting in the LSP server,
we know when diagnostic work should have been completed, and we should
be able to fail tests early once we know that our diagnostic
expectations will never be met.

This CL adds a new OnceMet Expectation, which combines two expecations:
the first is a precondition that must be met before checking the second.
The second is an arbitrary expectation, but is translated as follows:
once the precondition is met, the second condition is checked and any
Unmet verdicts are translated into Unmeetable.

Change-Id: Ie8c677229a347c624e2659a3ef9104304b175243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229977
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 21:14:28 +00:00
Rob Findley
dbf5ce1eac internal/lsp/regtest: add expository package documentation
Update the regtest docstring to further explain how the package works,
and give a sense for why it exists.

Fixes golang/go#36879

Change-Id: I4df4a286dc835ae56386b6d92128f7ea6c77ffb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229782
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 21:10:48 +00:00
Rob Findley
12a1c85843 internal/lsp/regtest: rename and document some symbols
Minor cleanup for the regtest package:
 - EnvMode is renamed to Mode, because it's really a server mode and not
   directly related to the Env type.
 - Modes are better documented.

Change-Id: Ia3aedfc70b665ea75a66731a72e4e87ae79db298
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229781
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 21:10:38 +00:00
Rob Findley
317da45f2f internal/lsp/regtest: extract Runner to a new file
The file length of env.go is getting hard to manage, so factor out the
test Runner to a new file.

Also move Runner.Close to the bottom of the file to have a more logical
progression.

Change-Id: Ifebea3277d826e9456aa02507a8349746386eb03
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229780
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 20:47:08 +00:00
Rob Findley
46dc332f25 internal/lsp: instrument work done reporting to use in regtests
In order for regtests to wait until file diagnostics are complete,
instrument diagnostics with verbose WorkDone reporting. In order for
this to be granular enough for use, the modification source needed to be
threaded through to the didModifyFiles function (which is where the
diagnostic goroutine is spun off).

A new expectation is added: CompletedWork, to allow specifying that a
specific work item has been completed. The problem with using
NoOutstandingWork was that it required a continuous chain of work to
prevent the regtest from succeeding when the bug was present, meaning
that by the time we have sent the didChange notification successfully
the server must have started work on its behalf. This was inherently
racy, and too tricky to get right.

Additionally, a couple bugs are fixed:
 - EmptyDiagnostics is corrected to account for the case where we have
   received zero diagnostics for a given file.
 - A deadlock is fixed in Await when expectations are immediately met.

Updates golang/go#36879
Fixes golang/go#32149

Change-Id: I49ee011860351eed96a3b4f6795804b57a10dc60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229777
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 20:46:32 +00:00
Rob Findley
9938a07982 internal/lsp: factor out progress reporting to a new WorkDone handle
Our current usage of WorkDone progress reporting (new in v3.15 of the
LSP spec) is in reporting progress on `go generate` commands. In
preparation for using this API more widely, factor out the reporting API
from the current io.WriteCloser wrapper (workDoneWriter).

Change-Id: Ib528093d81d4fc065528df90e100859e850b10df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229459
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 20:46:18 +00:00
Heschi Kreinick
e9a00ec821 internal/lsp/cache: correctly split env vars
We were using strings.Split on env vars, which did bad stuff when the
var contained an =, e.g. GOFLAGS=-tags=foo. Only split on the first =.

Irritatingly, this breaks only `go mod` commands, so almost nothing in
gopls failed, just organize imports and the `go.mod` code lens stuff.

Fixes golang/go#38669

Change-Id: I8d28c806b77a8df92100af1fa4fbcca5edf97cff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230560
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 18:55:08 +00:00
Ian Cottrell
006b16f6cf internal/lsp: share common command line test functionality
This moves the common code from the cmd and gopls tests to the shared cmdtest package, they were starting to drift apart.
This change was extracted from another larger cl where I was trying to work out why it broke in one but not the other.

Change-Id: I554ce364f4152e6b61f989da8162d968426d4ae5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230301
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 14:04:16 +00:00
Rob Findley
7ae4988eb4 internal/lsp/fake: clean up Workspace.proxydir on close
This change was unfortunately lost while rebasing, resulting in a lot of
unremoved directories.

Change-Id: Icc1b667e31ac85e617b1c3a8d7c58f78e999d151
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230314
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-28 02:10:58 +00:00
Dominik Honnef
4697a2867c go/analysis/analysistest: support testing mutually exclusive suggested fixes
Fixes golang/go#38431.

Change-Id: Ib2e389a7a657b8ff80995972cd3bcbcb937aeabb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229257
Run-TryBot: Dominik Honnef <dominik@honnef.co>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-04-27 21:46:58 +00:00
Rebecca Stambler
655248709e go/internal/gcimporter: remove support for binary export data
There should no longer be any clients of the binary export data format,
and it has been deleted from the standard library, so we can now delete
it from x/tools. This change deletes bimport.go, bexport.go, and
bexport_test.go, transferring any shared code to the corresponding
"i" files.

Fixes golang/go#37312

Change-Id: I25d8a36e89c33e10de68b586245722794c4d52c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229987
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-04-27 21:30:09 +00:00
Rebecca Stambler
352a5409fa internal/lsp: handle different package names in signature help
There were a few cases where we were not properly qualifying package
names, particularly if the original package had a named import. Now,
we map between these names correctly - handling the case of multiple
packages that need to be qualified. This requires applying edits to
*ast.SelectorExprs, as well as *ast.Idents.

We still do not fully qualify unimported packages, and likely won't,
unless that's an issue for many users.

Updates golang/go#38591

Change-Id: I966a4d1f936f37ede89362d03da3ff98d8952a06
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229979
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-27 20:59:12 +00:00
Rebecca Stambler
e56429333a internal/lsp/cmd: fix the command line query for definition
The definition command-line interface doesn't match the rest of the
commands, because I think we originally wanted to make them all
subcommands of "gopls query". Remove this, since it's no longer in use.

Change-Id: Iee923db6328774f787d539931001e438d1a2ae6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-27 20:15:23 +00:00
Rebecca Stambler
e0d5eebdf8 internal/lsp: fix docs on hover for ungrouped package variables
Fixes golang/go#38525

Change-Id: I8833c925663b67b2c82ad4cbf580d1c6f3c7a81d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230305
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-27 18:59:06 +00:00
Ian Cottrell
9ea0146da6 internal/jsonrpc2: remove jsonrpc2.ErrDisconnected
It was not very useful, it basically renamed io.EOF under some limited
circumstances, and was only there for the befit of tests.
Instead, the test now checks io.EOF directly, but also checks for
io.ErrClosedPipe which was also happening.
We also make sure we wrap errors rather than replacing them.
This prevents some weird random test failures due to races in the way they were
closed.

Change-Id: I236b03ac5ba16bf763299b95d882cf58b1f74776
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230303
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-27 16:39:59 +00:00
Ian Cottrell
a90b7300be internal/event: remove the event.eventType type
Instead of tagging events with their type, instead we infer the type from
the label pattern.
The standard event creators all have a matching test that returns true
if the the labels pattern matches the ones that would be built by the
creator.
Spans and logs already have a unique label pattern, other event types
required a special label marker.
This makes the system much more extensible, and also cleans up some
the API.

Change-Id: I1fbc9ec07aa84ead6c12bbd5ca65b13b605bfa4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229242
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-04-27 15:30:19 +00:00
Ian Cottrell
bed5418863 internal/event: don't print invalid labels
Change-Id: I781dfbd37aefe352c74d0df0af9da52bbf09fdcc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229989
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-27 14:36:10 +00:00
Rob Findley
479cc23432 internal/jsonrpc2: fix goroutine leak when listener is closed
When jsonrpc2.Serve times out or is cancelled, we leak the goroutine
that is accepting connections, because it is stuck trying to write its
error back to the doneListening channel.

Fix this by adding a context cancellation for the serve func, and
selecting on this context when writing the error.

Change-Id: I3383535f58b44616983816e8b257a975e3c337a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229978
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-04-27 12:55:06 +00:00
pjw
f3a5411a4c internal/lsp: fix panic when trying to log an event
Formatting keys in labels can panic when a label is constructed with a nil
error. Avoid that by not passing the defective label to event.Log.

Change-Id: I3099a7ac48c5830af1072141f2b619d0e0fbcf5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229985
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-04-26 10:28:38 +00:00
Muir Manders
8463f397d0 internal/lsp/source: fix false positive "..." in completions
In cases like:

    var v interface{}
    fmt.Println(<>)

Completing to "v" would insert "v..." instead of "v". This was due to
a mixup where we were checking if the variadic type "[]interface{}"
was assignable to the candidate type "interface{}" instead of the
other way around.

Fixes golang/go#38652.

Change-Id: I27c0b50bbf4b895924c8ed2c0c9dd6785e98cbe1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229921
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-25 04:34:58 +00:00
Rob Findley
3585060312 internal/lsp/source: add option for verbose work progress reporting
In order to experiment with adding more progress reporting to gopls, add
a new experimental configuration for verbose work done reporting.

Also, pass configuration in InitializationOptions when initializing the
editor.

Change-Id: Id2336790719d9c0b6d663997e1aef672aec43d28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229458
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-24 19:57:22 +00:00