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

1211 Commits

Author SHA1 Message Date
Muir Manders
ef1313dc6d internal/lsp/source: prefer unexported and non-func candidates
A common annoying mis-completion is as follows:

    type foo struct {
      field int
    }

    func (f foo) Field() int { return f.field }

    func (f foo) logic() {
      if f.f<>
    }

Now at <> we prefer "field" over "Field()". Similarly:

    type foo struct {
    }

    func (f foo) DoSomething() { }

    func (f foo) doSomething() { }

    func (f foo) logic() {
      f.d<>
    }

Now at <> we prefer "doSomething()" over "DoSomething()". All else
being equally, you normally want private objects over public objects
when the private objects are available.

The same logic is applied to deep completions so we prefer "c.foo.bar"
over "c.Foo().bar".

Change-Id: Ic91cba7721ddb1f2a30338037693ddcce8c621f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223877
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-23 21:07:25 +00:00
Rebecca Stambler
8849913b69 internal/lsp: be more careful about showing workspace misconfig message
We can re-enable this when we have a version of Go that includes the fix
for golang/go#37971. Then we can continue playing whack-a-mole with go
list corner cases :)

Fixes golang/go#37958

Change-Id: I2417731bf5c01fbde867fef2ef03b5025ae779c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224538
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-23 19:22:00 +00:00
Rebecca Stambler
18ea2c8f73 internal/lsp: fix nil pointer in renaming to "error"
Fixes golang/go#37963

Change-Id: I0a46cbcbd250729761b9a8323b942202bb1dbd75
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-23 16:43:54 +00:00
Ian Cottrell
8dcfad9e01 internal/telemetry: switch metrics to use the event system
Change-Id: If036530ffe47e7df925bcf6592f664d6020a3d65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223997
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:44:30 +00:00
Ian Cottrell
b378960d5b internal/telemetry: replace event.TagList with event.TagSet
This allows us to hide the implementation details of how tags are stored on a
context from the normal interface, to allow us to explore more efficient
mechanisms.
The current storage is not intended as the most efficient choice, this cl is
about isolating the API so we can experiment with benchmarks in the future.

Change-Id: Ib101416bccd8ecdee269cee636b1564d51e1da8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222854
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:43:48 +00:00
Rebecca Stambler
0d839f3cf2 internal/lsp: remove completion commands
The next version of VS Code Go has to be released before this can be
released.

Fixes golang/go#37966

Change-Id: I6b97f2a3ff9c8400ae85b815d8bf83a9b12b1069
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2020-03-21 22:47:14 +00:00
Daisuke Suzuki
521f4a0cd4 internal/lsp/cmd: fix definition test to run independently
definition.Run requires a nil check of opts before applying to avoid
panic, and test must be run with markdown enabled.
When running 'go test' without -run flag, connection.initialize() was
not called and there was no problem because the default value of
Options.PreferredContentFormat was Markdown.
In addition, currently using the same connection despite different
options, therefore make to use a different connection for different
options.

Change-Id: I6ef773c14cbdcae621da9295d4c618c3aff0beee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222497
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-19 21:04:07 +00:00
Rob Findley
9a0fabac01 internal/lsp: fix errors found by staticcheck
While experimenting with different static analysis on x/tools, I noticed
that there are many actionable diagnostics found by staticcheck. Fix the
ones that were not false positives.

Change-Id: I0b68cf1f636b57b557db879fad84fff9b7237a89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222248
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-19 19:20:54 +00:00
Ian Cottrell
540150da73 internal/telemetry: add type safe tag keys
This changes the way keys work, there is still a single internal key
implementation for performance reasons, but the public interface is a set of key
implementations that have type safe Of and Get methods.
This also hides the implemenation of Tag so that we can modify the storage form
and find a more efficient storage if needed.

Change-Id: I6a39cc75c2824c6a92e52d59f16e82e876f16e9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223137
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:29:43 +00:00
Ian Cottrell
d7fc2cf50e internal/telemetry: delete the event.TagOf method
It encourages poor performing log lines, and also reduces the readability of
those lines.
Also delete the Key.With method which was unused, and should never be used
instead of the event.Label function anyway.

Change-Id: I9b55102864ee49a7d03e60af022a2002170c0fb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222851
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:23:15 +00:00
Ian Cottrell
04208b9e8a internal/lsp: move the telemetry package
Move the lsp specific telemetry package that just declares the labels under the
debug package and call it tag, to make all the usages much more readable.

Change-Id: Ic89b3408dd3b8b3d914cc69d81f41b8919aaf424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222850
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:49 +00:00
Ian Cottrell
cb106d260e internal/telemetry: allow ProcessEvent to modify the event
This allows early exporters to adjust the event for later ones.
This is used to lookup key values from the context if needed.
Also add a Query type event which is intended to perform all event
modifications but nothing else, and is used to lookup values from
the context. This cleans up a weirdness where the current lookup
presumes there will be an exporter with a matching mechanism.

Change-Id: I835d1e0b2511553c30f94b7becfe7b7b5462c111
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223657
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:01 +00:00
Ian Cottrell
9fceb114c3 internal/lsp: move the operation key to the telemetry package
Change-Id: If064d9bb396169fe78214b07f72a0536082bf374
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223748
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-18 13:08:08 +00:00
Muir Manders
11a475a590 internal/lsp/source: fix literal completions in variadic args
We now properly offer literal completions when completing the variadic
parameter. For example:

    func foo(...[]int) {}
    foo(<>) // now offers "[]int{}"

Updates golang/go#37656.

Change-Id: I91c47d455ae3f8051078c82a308f2b5ad4b3d4cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222199
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-18 05:47:22 +00:00
Muir Manders
613a0345a2 internal/lsp/source: optimize enumeration of a type's fields
When searching for deep completions, we can end up enumerating struct
types' fields a lot. Optimize fieldSelections to reduce work:

- Wait until we see an embedded field before we create the "seen"
  map.
- Use a callback style to iterate over the struct's fields rather than
  returning a slice of fields.
- Change "seen" checking strategy back to track struct types rather
  than each individual field.

Struct with 5 non-embedded fields:

name       old time/op    new time/op    delta
Fields-16     293ns ± 1%      20ns ± 2%   -93.13%  (p=0.008 n=5+5)

name       old alloc/op   new alloc/op   delta
Fields-16      120B ± 0%        0B       -100.00%  (p=0.008 n=5+5)

name       old allocs/op  new allocs/op  delta
Fields-16      4.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)

Same struct but add an embedded struct with 2 fields:

name       old time/op    new time/op    delta
Fields-16     389ns ± 1%     142ns ± 1%  -63.53%  (p=0.008 n=5+5)

name       old alloc/op   new alloc/op   delta
Fields-16      120B ± 0%      144B ± 0%  +20.00%  (p=0.008 n=5+5)

name       old allocs/op  new allocs/op  delta
Fields-16      4.00 ± 0%      2.00 ± 0%  -50.00%  (p=0.008 n=5+5)

I think the alloc/op went up because the "seen" map is no longer
allocated on the stack. There is more room for more optimization, but
it's probably not worth making things more complicated.

Change-Id: I6f9f2124334a8594ef9d6f9b5ac4b3a8aead5f49
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223419
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-18 05:47:12 +00:00
Marwan Sulaiman
cf1dd6fc34 x/tools/gopls: 'go generate' should use $/progress if available
This commit will make the 'go generate' CodeLens command check the client
capabilities to use $/progress to report the progress of the command or otherwise
it will fallback to showing the message box already in place. The $/progress will
give you a play by play update while the message box shows only the beginning and
the end. The gopls logs will have all the details in either case.

Note that the $/progress is an LSP 3.15+ feature and not yet supported in all clients.

Updates golang/go#37680

Change-Id: I5ba37448be8e388f728394795e1bb5f1d50cc30d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223739
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-17 19:18:43 +00:00
Rohan Challa
d05f4d6edb internal/lsp: fix breadcrumbs when inside of a method
This change fixes breadcrumbs when the cursor is inside of a method by making the method a top level symbol and removing it from the struct's hierarchy.

Fixes golang/go#36949

Change-Id: I8da5f453a5c78ade1e7688fc4eeebf79ce96f1e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221819
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-17 19:04:34 +00:00
pjw
827390e901 internal/lsp: fix code.ts to generate progress and *TextEdit
This CL gets code.ts to generate code for $/progress and
window/workDoneProgres/create messages. $/progress uses a
ProgressParams type which contains one of three new data types,
WorkDoneProgressBegin, WorkDonProgressEnd, WorkDoneProgressReport.

In addition, a *TextEdit is now generated for CompletionItem.TextEdit.

The substantive differences in code.ts are around line 451 and line
682. Everything else is whitespace caused by vscode formatting typescript
differently on different OSes.

Change-Id: Ide441e6e0029cbc8401d6476f6a939216cc89634
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223743
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-17 18:47:13 +00:00
Marwan Sulaiman
63da46f303 x/tools/gopls: run go generate through CodeLens
This change adds support for recognizing a //go:generate directive
and offering a CodeLens that will then send a "generate" command to
the server to run "go generate" or "go generate ./...". Because
"go generate" can only be executed per package, there is no need to show
the CodeLens on top of every //go:generate comment. Therefore, only the
top directive will be considered.

The stdout/stderr of the go generate command will be piped to the logger
while stderr will also be sent to the editor as a window/showMessage

The user will only know when the process starts and when it ends so that they wouldn't
get bogged with a large number of message windows popping up. However, they can
check the logs for all the details.

If a user wants to cancel the "go generate" command, they will be able
to do so with a "Cancel" ActionItem that the server will offer to the client

Fixes golang/go#37680

Change-Id: I89a9617521eab20859cb2215db133f34fda856c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222247
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-17 04:34:34 +00:00
Marwan Sulaiman
3e76bee198 x/tools/gopls: add support for $/progress functionality
This CL adds support for sending progress notifications through $/progress
calls as well as being able to cancel them through window/workDoneProgress/cancel.
This feature is only supported in clients running LSP 3.15 and therefore the initialize
request will check for client capabilities for its progress support.

Updates golang/go#37680

Change-Id: Iff8c016694746a9dd553e5cc49444df7afcc21f5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222981
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-16 21:25:24 +00:00
aca
df010c5017 internal/lsp/cache: fix typo
Change-Id: Ida5ed631652d9d4e35018300ab098ebb24b75856
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223498
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-16 18:21:19 +00:00
Rob Findley
c312e98713 gopls/doc: update daemon.md and remove 'experimental' caveats
Now that the feature-set is relatively complete for running gopls in
daemon mode, daemon.md is updated to reflect the new features and to
remove some of the experimental warnings. Similarly the `-remote` flag
is made more welcoming.

The feature still needs more usage, but it shouldn't be considered
experimental anymore.

Fixes golang/go#34111

Change-Id: I994fc8f9f84bf856f24e1eadabd73c503267e804
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222717
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-12 19:44:00 +00:00
Rob Findley
c1c69b635f internal/lsp: clean up on Shutdown even if not initialized
Previously it was an error to call shutdown while the server was not yet
initialized. This can result in session leak for very short-lived
sessions.

Instead, allow shutdown to proceed and simply log the error. On the
other hand, for consistency make it an error to call initialized without
first calling initialize.

Updates golang/go#34111

Change-Id: I0330d81323ddda6beec0f6ed9eb065f8b717dea0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222671
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-12 19:43:25 +00:00
Rob Findley
30fd94b347 internal/lsp/lsprpc: expose configuration for auto-started daemon
Three new flags are added to the serve command, and threaded through to
the LSP forwarder:
 -remote.listen.timeout: -listen.timeout for the auto-started daemon
 -remote.debug: -debug for the auto-started daemon
 -remote.logfile: -logfile for the auto-started daemon

As part of this change, no longer enable debugging the daemon by
default.

Notably none of this configuration affects serving, so modifying this
configuration has been chosen not to change the path to the automatic
daemon. In other words, this configuration has effect only for the
forwarder process that starts the daemon: all others will connect to the
daemon and inherit whatever configuration it had at startup. This should
be OK, because in the common case this configuration should be static
across all clients (e.g., many Vim sessions all sharing the same
.vimrc).

Exposing this configuration made the signature of lsprpc.NewForwarder
a bit hard to understand, so I decided to go ahead and switch to a
variadic options pattern for initializing both the Forwarder and
StreamServer, the latter just for consistency with the Forwarder.

Updates golang/go#34111

Change-Id: Iefb71e337befe08b23e451477d19fd57e69f36c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222670
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-12 19:43:16 +00:00
Rob Findley
bd435c612c internal/lsp/cmd: add an inspect verb
Add a new toplevel `inspect` verb to the gopls command, to expose the
internal state of a running gopls server. For now, this verb exposes a
single subcommand `sessions`, which lists some information about current
server sessions on a gopls daemon.

This can be used even if the debug server is not running, which will
become the default in a later CL.

Updates golang/go#34111

Change-Id: Ib7d654a659fa47280584f9a7301b952cbccc565a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222669
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-12 17:21:45 +00:00
Muir Manders
11d5b4c81c internal/lsp/source: offer loop keyword completions in range stmt
Be sure to offer "continue" and "break" keyword completions in
RangeStmt in addition to ForStmt.

Fixes golang/go#34009.

Change-Id: I01ea0c3d8d110e322cb6582fda32aeb060116bb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223120
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-12 04:57:24 +00:00
Ian Cottrell
eff45d17df internal/telemetry: convert key from string to struct
This makes keys a pointer, which reduces the allocations during logging, and has
a significant improvement in memory and cpu cost when there is no exporter.
Packing a string into an interface requires a 16 byte allocation, packing a pointer
does not.

name                old time/op    new time/op    delta
/LogNoExporter-8      4.39µs ± 2%    3.83µs ± 2%  -12.74%  (p=0.000 n=18+20)
/Log-8                23.5µs ± 2%    22.6µs ± 1%   -4.20%  (p=0.000 n=19+18)

name                old alloc/op   new alloc/op   delta
/LogNoExporter-8      1.70kB ± 0%    1.38kB ± 0%  -18.78%  (p=0.000 n=20+20)
/Log-8                4.91kB ± 0%    4.91kB ± 0%     ~     (p=1.000 n=20+20)

name                old allocs/op  new allocs/op  delta
/LogNoExporter-8        83.0 ± 0%      63.0 ± 0%  -24.10%  (p=0.000 n=20+20)
/Log-8                   163 ± 0%       163 ± 0%     ~     (all equal)

Change-Id: Iec127f1bff8d5c8f4bd0a6d9f6d8fc4b8bc740b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222599
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 04:17:52 +00:00
Ian Cottrell
206ec5b82a internal/lsp: migrate telemetry to using the event package
Change-Id: Idc662385ed08bd62593ccd1d54afd3fa8c1a7d29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 03:59:16 +00:00
Ian Cottrell
d780ff7bdd internal/telemetry: unify the event handling to an event package
This is now the only package that is exposed to normal use, and should
be the only thing to appear in libraries.

Change-Id: I90ee47c6519f30db16ff5d5d2910be86e91e5df2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222557
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 03:58:56 +00:00
Muir Manders
3dc7fec788 internal/lsp/cache: remove parseGo semaphore
The original intention was to limit the number of open file handles,
but the parseLimit semaphore was limiting parser.ParseFile calls that
don't access files, so there was no benefit. The file limiting is
already handled generically by another semaphore in the lsp FileSystem
abstraction.

Change-Id: Idc7f2507af5287e983a7edf3e38a848d26770bbe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223119
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-12 03:18:52 +00:00
Daisuke Suzuki
0d653b92c5 internal/lsp/tests: fix WorkspaceSymbols tests
The tests will not run because there is no workspacesymbol marker.
Therefore, restore testdata used in WorkspaceSymbols tests, and
initialize data so that the tests run even if the target of the marker
is 0.

Change-Id: I051b842183b7e23bb746cc282fc3921a90ca8df1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221617
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-11 18:46:36 +00:00
Muir Manders
43e3193a9b internal/lsp/source: suppress keyword completions in "case" statement
Now we no longer offer keyword completions inside "case" statements:

    select {
    case fo<>: // don't offer "for"
    }

Updates golang/go#34009.

Change-Id: I908eda32af01fd1912fb587ce59efef1798b6741
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220580
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-11 03:54:31 +00:00
Muir Manders
7bb885034f internal/lsp/source: fix completion in empty switch statements
Now we properly offer "case" and "default" keyword completion in cases
like:

    switch {
      <>
    }

First I had to add an AST fix for empty switch statements to move the
closing brace down. For example, say the user is completing:

    switch {
    ca<>
    }

This gets parsed as:

    switch {
    }

Even if we manually recover the "ca" token, "<>" is not positioned
inside the switch statement anymore, so we don't know to offer "case"
and "default" candidates. To work around this, we move the closing
brace down one line yielding:

    switch {

    }

Second I had to add logic to manually extract the completion prefix
inside empty switch statements, and finally some logic to offer (only)
"case" and "default" candidates in empty switch statements.

Updates golang/go#34009.

Change-Id: I624f17da1c5e73faf91fe5f69e872d86f1cf5482
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220579
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-11 03:54:16 +00:00
Muir Manders
0983143fce internal/lsp/source: avoid type checking in AllImportsFixes
Change AllImportsFixes to get the ParseGoHandle directly rather than
using getParsedFile. The latter also type checks the containing
package, which is slow in general and can be really slow in certain
circumstances. This should speed up the organizeImports code action.

Change-Id: Ic505873db41d5d76c398cee42f0323e390d9e034
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222780
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-10 23:10:53 +00:00
Peter Weinbergr
3e041465cc internal/lsp: change the type of CompletionItem.TextEdit back
The recent LPS protocol change made the definition of textEdit into
a union type, so the generated code made TextEdit an interface{},
which broke govim. This CL reverts it to a *TextEdit. The code
generator will be fixed in another CL.

We can revisit this when  gopls wants to start using the new
InsertReplaceEdit type.

Change-Id: I4d7a14b3746b747f34b0907f72ecbc3593706a05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222765
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-10 21:12:04 +00:00
Rebecca Stambler
b84001dd64 internal/lsp: suggest filing an issue if a warning is incorrect
It seems that people are running into this warning for reasons other
than ad-hoc packages. Suggest filing an issue, since it may be that they
are opening the wrong directory or something.

Change-Id: I88036d2db9477bbea5099910f4cd8e5ba55e0624
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-10 21:11:10 +00:00
Muir Manders
138c814f66 internal/lsp/source: offer completion "if err != nil { return err }"
Now we offer an error-check-and-return completion candidate when
appropriate:

    func myFunc() (int, error) {
      f, err := os.Open("foo")
      <>
    }

offers the candidate:

    if err != nil {
      return 0, <err>
    }

where <> denotes a placeholder so you can easily alter "err".

The completion will only be offered when:
1. The position is in a function that returns an error as final result
   value, and
2. The statement preceding position is an assignment whose final LHS
   value is an error.

The completion will contain zero values for the non-error return values
as necessary.

Using the above example, the completion will be offered after the user
has typed:

    i
    if
    if err

Basically the candidate will be offered after every keystroke as the
user types "if err".

I call this new type of completion a statement completion - perfect
for when you want to make a statement!

Change-Id: I0a330e1c1fa81a2757d3afc84c24e853f46f26b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221613
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-10 21:06:53 +00:00
Peter Weinberger
20ab64c0d9 internal/lsp: bring generated protocol code up to date; stop corrupting logs
1. logging at debug/rpc.go:105 was corrupting logs.
2. DocumentSymbol (a Server method) now returns []interface{}.
3. The latest version of the LSP changed CompletionItem.TextEdit to
possibly be of the new type InsertReplaceEdit, so the generated Go
code now has TextEdit an interface{} rather than a *TextEdit. This
required a change to tests/util.go.
4. The latest version also introduced several other new types,
and new members in some structs.

Tests pass and the I've use the new gopls a little.

Change-Id: Ic44d46f0c923882c9076c2754c1c85e09fbcaa2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222668
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-09 20:21:50 +00:00
Muir Manders
92d517f97b internal/lsp/source: suppress completions in ellipsis
Editors typically trigger completion automatically after ".". This
pops up annoying, useless completions after "..." variadic param, such
as "foo(bar...<>)". We now suppress completions in or directly after
the ellipsis.

Fixes golang/go#37358.

Change-Id: I9fc94fbdf69429bd787bcb2c643f4f730e24154d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222200
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-09 18:42:16 +00:00
Rob Findley
aa4048aca1 internal/lsp/lsprpc: don't connect to sockets owned by different users
When running gopls as a forwarder it attempts to forward the LSP to a
remote daemon. On posix systems, by default this uses a unix domain
socket at a predictable filesystem location.

As an extra precaution, attempt to verify that the remote socket is in
fact owned by the current user.

Also, change the default TCP listen address used on windows to bind to
localhost.

Updates golang/go#34111

Change-Id: Ib24886d290089a773851c5439586c3ddc9eb797d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222246
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-09 18:08:59 +00:00
Danish Prakash
c94e1fe145 internal/lsp/source: return location(s) for imported packages
Fixes golang/go#32993

Change-Id: Iff2f7ab8e34eea1ce7c1ab99fb2e51589dce95c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-09 16:25:02 +00:00
Muir Manders
11ec41452d internal/lsp/source: support completing "range" keyword
We now offer "range" keyword in the for loop init statement:

for i := r<> // offer "range" completion candidate

Updates golang/go#34009.

Change-Id: I2e4c1db11c37127406c78191681c39b9dd3439f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220504
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-08 01:35:34 +00:00
Muir Manders
2047c2d578 internal/lsp/source: complete keywords as types
Offer "struct", "interface", "map", "chan", and "func" keywords when
we expect a type. For example "var foo i<>" will offer "interface".

Because "struct" and "interface" are more often used when declaring
named types, they get a higher score in type declarations. Otherwise,
"map", "chan" and "func" get a higher score.

I also got rid of the special keyword scoring. Now keywords just use
stdScore and highScore. This makes the interplay with other types of
candidates more predictable. Keywords are offered in pretty limited
contexts, so I don't think they will be annoying.

Finally, keyword candidate score is now to be scaled properly based on
how well they match the prefix. Previously they weren't penalized for
not matching well, so there were probably some situations where
keywords were ranked too high.

Updates golang/go#34009.

Change-Id: I0b659c00a8503cd72da28853dfe54fcb67f734ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220503
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-08 01:33:38 +00:00
Rohan Challa
51e69f7192 internal/lsp/cache/mod: return errors only within ModHandle and ModTidyHandle
This change makes ModHandle and ModTidyHandle return a ModData structure that only contains an error if something goes wrong when parsing the .mod files.

Change-Id: I89661cfefeff666bdf13cf99f4c56137e120de82
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222242
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-06 19:16:17 +00:00
Rohan Challa
e4e22f76d0 internal/lsp: support when hierarchicalDocumentSymbolSupport is false
This change adds support when hierarchicalDocumentSymbolSupport is false, this can happen with editors who have not supported textDocument/DocumentSymbol. As a result, these older lsp clients need to recieve []protocol.SymbolInformation rather than []protocol.DocumentSymbol. This change required some changes to internal/lsp/cmd to handle not knowing which type it is receiving, this required manual parsing inside of cmd/symbols.go.

Fixes golang/go#34893

Change-Id: I944ae24302f155b561047227f65bee34b160def1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221823
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-06 18:17:37 +00:00
Rob Findley
8287d625a0 internal/lsp/debug: guard rpc stats with a Mutex
This type was previously guarded implicitly by the global exporter
mutex. With that gone, we've started seeing panics due to races in
(*rpcs).Metric.

Guard it with a mutex.

Change-Id: I2a8c670ecfbaee9422e1f1d282b1fedb86b6a5e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222300
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-03-06 16:15:13 +00:00
Rohan Challa
a0897bacdd internal/lsp/cmd: improve flexibility of suggested fixes
This change adds support for passing a span to cmd/suggested_fix.go, originally it would just take the filename and apply all the fixes for that file. Now, it can also take just a span within that file and only apply the fixes relevant to that span.

The //@suggestedfix marker now contains an extra parameter to specify which type of codeaction is expected.

Change-Id: I4e94b8dad719f990dc2d0ef3c50816f70f59f737
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222137
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-06 14:31:35 +00:00
Rohan Challa
136c3617b6 internal/lsp: support textDocument/formatting for .mod extension
This change implements support for textDocument/formatting when it comes to go.mod files. It also adds formatting on save ability.

To enable formatting on save for go.mod files, you need to include the following settings inside of your VSCode settings.json:

...
"[go.mod]": {
	"editor.codeActionsOnSave": {
		"source.organizeImports": true,
	},
	"editor.formatOnSave": true,
},
...

Updates golang/go#36501

Change-Id: I60ac14d0e99b2b086fe9a8581770771aafc2173c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221223
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-06 13:51:27 +00:00
Rob Findley
de023d59a5 internal/lsp/protocol: unmarshal to pointers when dispatching requests
With #34111, we are forwarding the LSP from one gopls instance to
another. This exposed an asymmetry in our LSP dispatching: for both
ClientDispatcher and ServerDispatcher, we unmarshal to non-nil response
structs. This means that when forwarding the LSP, we translate empty
JSON responses (corresponding to nil values) into the non-nil zero
value.

This causes problems for some editors, as reported in #37570. Fix it by
instead unmarshaling to a pointer.

This is, of course, a somewhat dangerous change. I fixed the one NPE
that occurred in tests, and have done some mild manual testing.  I
wouldn't be surprised if we discover more NPEs later on, but I still
think this is the right change to make.

Updates golang/go#34111
Fixes golang/go#37570

Change-Id: Ie69e92d2821c829cdfc4f4ab303679a725f1f859
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222058
Reviewed-by: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-05 22:45:36 +00:00
Rob Findley
d1d1f200c6 internal/lsp/lsprpc: use Setsid on POSIX GOOSes, to avoid SIGTERMs
When the gopls daemon is automatically managed (-remote=auto), it will
be started by one of the forwarder gopls processes that was in turn
started by the editor. By default, this puts it in the same process
group as the forwarder gopls.

Some editors (at least Vim) send SIGTERM to the process groups of
sidecar processes when exiting. This can cause the gopls daemon to
terminate, thereby losing state.

Rather than ignore SIGTERM (which is bound to be editor dependent
anyway), let's just put the gopls daemon in a separate session.

Updates golang/go#34111

Change-Id: I71386fb54b8c2efe1c565f59763f46693a7d48b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221220
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-05 21:44:44 +00:00