1
0
mirror of https://github.com/golang/go synced 2024-09-30 20:18:33 -06:00
Commit Graph

4767 Commits

Author SHA1 Message Date
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
Rebecca Stambler
59e73619c7 internal/lsp: correctly handle type aliases when formatting
This change improves our approach to handling type aliases. Previously,
we were not fully qualifying the names in the AST, making the code
inserted in completions incorrect at times. Now, we clone the relevant
AST expr and qualify it. We also add handling for the return values of a
function, instead of just the parameters.

Fixes golang/go#38230
Fixes golang/go#37283

Change-Id: Ib79f4636891c9b610ae848e9fa4dbae7c63db509
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229319
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-23 20:53:58 +00:00
Rob Findley
38a97e00a8 internal/lsp/regtest: track outstanding work using the progress API
In preparation for later changes, add support for tracking outstanding
work in the lsp regtests. This simply threads through progress
notifications and tracks their state in regtest.Env.state. A new
Expectation is added to assert that there is no outstanding work, but
this is as-yet unused.

A unit test is added for Env to check that we're handling work progress
reports correctly after Marshaling/Unmarshaling, since we're not yet
exercising this code path in actual regtests.

Change-Id: I104caf25cfd49340f13d086314f5aef2b8f3bd3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229320
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-23 20:44:50 +00:00
Rebecca Stambler
2723c5de0d go/packages/packagestest: dedupe based on note position
The "forTest" field is used for x_tests and for test variants, making
the deduplication too agressive. Instead, we deduplicate by the
position of the note.

Change-Id: I809364d6bba95be5ad95eced67fae645435f5592
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229597
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-23 20:11:57 +00:00
Heschi Kreinick
01e6875574 cmd/goimports: set correct permissions on Windows
As of Go 1.14, WriteFile on Windows will set read-only on existing files
if you pass 0 for perms. Pass the pre-existing permissions.

Updates golang/go#38225.

Change-Id: I3174469efd4dc4c7eacc8522386a0712cfa39d11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229297
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-23 18:57:08 +00:00
Ian Cottrell
a466788a31 internal/event: change event.At to be a private field
This was the last piece of Event that was public, and it was only public to
allow mutation in tests.
Adding CloneEvent allows tests to create an updated copy rather than
update the event in place.

Change-Id: I2215d1eb0317063948ef0fac955fa768a209564d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229241
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 18:13:49 +00:00
Ian Cottrell
a82abb5396 internal/event: extract keys to their own package
Now key types can be implemented outside the package that holds labels or events, they should be.
This prevents the large list of types from poluting the public interface of the core packages.

Change-Id: I927f31cb5e4e1d0c29619681015962f890623e5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229240
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 18:13:43 +00:00