1
0
mirror of https://github.com/golang/go synced 2024-10-01 01:18:32 -06:00
Commit Graph

1354 Commits

Author SHA1 Message Date
Rebecca Stambler
480da3ebd7 internal/lsp: return early in completion where possible
This a pure cut-and-paste.

Fixes golang/go#38868

Change-Id: I2ff07134a8b9f6186bab737caceab8a34a8e2f44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232677
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-07 20:50:54 +00:00
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
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
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
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
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
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
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
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
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
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
Ian Cottrell
c81623a0cb internal/event: move event/core.Tag to event/label.Label
Also moves core.Key to label.Key, but leaves the implementations
behind for now.
After using for a while, the word Tag conveys slightly the wrong
concept, tagging implies the entire set of information, label maps
better to a single named piece of information.
A label is just a named key/value pair, it is not really tied to the
event package, separating it makes it much easier to understand the
public symbols of the event and core packages, and allows us to also
move the key implementations somewhere else, which otherwise dominate
the API.

Change-Id: I46275d531cec91e28af6ab1e74a2713505d52533
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229239
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-23 18:13:33 +00:00
Ian Cottrell
7b212d60a1 internal/event: renaming the main event API functions
event.Log removed
event.Print -> event.Log
event.Record -> event.Metric
event.StartSpan -> event.Start

In order to support this core now exposes the MakeEvent and Export functions.

Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:21:36 +00:00
Ian Cottrell
cf0cb92717 internal/telemetry: renaming to internal/event
internal/telemetry/event was renamed to internal/event/core
Some things were partly moved from internal/telemetry/event straight to
internal/event to minimize churn in the following restructuring.

Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:20:48 +00:00
Rebecca Stambler
72e4a01eba internal/lsp: refactor code for formatting signatures
The code for formatting function signatures is fairly confusing.
Factoring out an unexported signature type simplifies things a bit.
Hopefully we'll be able to pull out more formatting logic from the other
features. Ideally, I'd like to return to the separation between
internal/lsp/source and internal/lsp so that a formatting package can be
pulled out and used in internal/lsp.

Change-Id: I7428db5004eab371e46402188e0dc6bb30f0c425
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229318
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-22 20:52:58 +00:00
Rebecca Stambler
3d57cf2e72 internal/lsp: add regtest for golang/go#37984
This change also required the addition of a new run configuration -
WithEnv, which adds extra environment variables to the configuration.
Please let me know if this is the wrong approach.

Fixes golang/go#37984

Change-Id: Ied2a53a443dc74c7ed723b91765eeddc1a7c1c00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229128
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-22 02:23:33 +00:00