1
0
mirror of https://github.com/golang/go synced 2024-11-18 19:44:46 -07:00
Commit Graph

4630 Commits

Author SHA1 Message Date
Rebecca Stambler
df82bb964a internal/lsp: add a test for internal imports handling
I'm still not sure if we need to handle any other non-standard package
path apart from "command-line-arguments".

Also, a couple of staticcheck fixes.

Change-Id: I0bb3e60f6ffe104ff9027dbebb628020caaa1af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-25 18:38:19 +00:00
Ian Cottrell
a0ec867d51 internal/telemetry: add a benchmark with a null writer for comparison
Make all the benchmarks cleanly pass around the context to remove a non logging
difference.
Rename the non logging calls benchmark to Baseline

Change-Id: I24a33b0a817df403fb43c53b5f097bc1e9418af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220520
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-02-25 02:20:59 +00:00
Rob Findley
daf22849ce internal/lsp/lsprpc: add a 1m timeout to auto-started gopls
It would be bad behavior if a gopls forwarder process started the gopls
daemon and it stuck around indefinitely. Add an idle timeout by default
for the gopls daemon.

Updates golang/go#34111

Change-Id: I0408a1e6a3b89d862803ae5c439d6aa0d8ed9494
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220521
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 23:36:36 +00:00
Rob Findley
63caf62cea internal/lsp/lsprpc: clean up client session on disconnection
Gopls behavior on disconnection is currently somewhat undefined, because
it hasn't mattered when there was a single gopls session per binary
invocation. With golang/go#34111, this changes.

Checks are added to ensure clients and sessions are cleaned up when an LSP
connection closes. Also, normal client disconnection is differentiated
with the jsonrpc2.ErrDisconnected value.

Updates golang/go#34111

Change-Id: I74d48ad6dcfc30d11f7f751dcffb20c18a4cbaa3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220519
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 23:23:05 +00:00
Jordan Christiansen
a6bebb6330 gopls/doc: Clear out fixed issues
I was checking on the status of the known issues, and I noticed that some are already fixed. I checked each one and culled the list.

Change-Id: I178cb5fdc3aa21e658c3ffd8bf558d5c48c5528f
GitHub-Last-Rev: b0135c2c6e95395026641ce8214f08a2aa623d92
GitHub-Pull-Request: golang/tools#211
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220681
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-24 23:14:51 +00:00
Rob Findley
9ffc0ab4ef internal/jsonrpc2: add an idle timeout for stream serving
When running gopls against an automatically started remote instance, we
want the lifecycle of the remote to be detached from that of its
clients, so that it doesn't shut down while clients are still connected.
On the other hand, a gopls process can consume significant resources, so
we don't want it to remain when there are no more connected clients.

The jsonrpc2 package is updated to support the concept of idle timeout:
a duration after which the server is shut down when there are no
connected clients. This is exposed in the gopls serve command via the
-listen.timeout flag.

Update golang/go#34111

Change-Id: Id62b3d4a2fa66de2c9306d130ca431717f01d1e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220281
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 23:06:35 +00:00
Rob Findley
a208025ccb internal/lsp/lsprpc: automatically resolve and start the remote gopls
Most users will not want to manage their own gopls instance, but may
still want to benefit from using a shared instance.

This CL adds support for an 'auto' network type that can be encoded in
the -remote flag similarly to UDS (i.e. -remote="auto;uniqueid"). In
this mode, the actual remote address will be resolved automatically
based on the executing environment and unique identifier, and the remote
server will be started if it isn't already running.

Updates golang/go#34111

Change-Id: Ib62159765a108f3645f57709b8ff079b39dd6727
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220137
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 22:51:55 +00:00
Rob Findley
20f46356b3 internal/lsp/lsprpc: add a handshake between forwarder and remote
In the ideal future, users will have one or more gopls instances, each
serving potentially many LSP clients. In order to have any hope of
navigating this web, clients and servers must know about eachother.

To allow for such an exchange of information, this CL adds an additional
handler layer to the serving configured in the lsprpc package. For now,
forwarders just use this layer to execute a handshake with the LSP
server, communicating the location of their logs and debug addresses.

Updates golang/go#34111

Change-Id: Ic7432062c01a8bbd52fb4a058a95bbf5dc26baa3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220081
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 22:51:04 +00:00
Rob Findley
e02f5847d1 internal/lsp/debug: move all debug state onto the Instance
For testability, and to support the exchange of debug information across
Forwarder and server, it is helpful to encapsulate all debug information
on the instance object.

This CL moves all state in the debug package into a new 'State' type,
that is added as a field on the debug.Instance. While doing so, common
functionality for object collections is factored out into the objset
helper type.

Also add two new debug object types: Client and Server. These aren't yet
used, but will be in a later CL (and frankly it was easier to leave them
in this CL than to more carefully rewrite history...).

Updates golang/go#34111

Change-Id: Ib809cd14cb957b41a9bcbd94a991f804531a76ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220078
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 22:50:47 +00:00
Muir Manders
023911ca70 internal/lsp/source: untangle completion type comparison
The code to check if a candidate object matches our candidate
inference had become complicated, messy, and in some cases incorrect.
The main source of the complexity is the "derived" expected and
candidate types. When considering a candidate object "foo", we also
consider "&foo", "foo()", and "*foo", as appropriate. On the expected
side of things, when completing the a variadic function parameter we
expect either the variadic slice type and the scalar element type.

The code had grown organically to handle the expanding concerns, but
that resulted in confused code that didn't handle the interplay
between the various facets of candidate inference.

For example, we were inappropriately invoking func candidates when
completing variadic args:

    func foo(...func())
    func bar() {}
    foo(bar<>) // oops - expanded to "bar()"

and we weren't type matching functions properly as builtin args:

    func myMap() map[string]int { ... }
    delete(myM<>) // we weren't preferring (or invoking) "myMap()"

We also had methods like "typeMatches" which took both a "candidate"
object and a "candType" type, which doesn't make sense because the
candidate contains the type already.

Now instead we explicitly iterate over all the derived candidate and
expected types so they are treated the same. There are still some
warts left but I think this is a step in the right direction.

Change-Id: If84a84b34a8fb771a32231f7ab64ca192f611b3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218877
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-24 18:12:40 +00:00
Robert Griesemer
e1da425f72 Revert "Revert "go/analysis: add pass to check for impossible interface-to-interface type assertions""
This reverts commit 11eff242d1.

Reason for revert: The 1.15 tree is now open for early submits.

The original CL was authored by Akhil Indurti (aindurti@gmail.com).

Change-Id: I8cfcd0253a6666a6392fa938bb19a1b426ba712d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220139
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Akhil Indurti <aindurti@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
2020-02-21 22:42:23 +00:00
Pontus Leitzler
57f3fb51f5 gopls: update Staticcheck to 2020.1.2
This change updates Staticcheck to the newly released 2020.1.2.

Change-Id: I80606b9c993de2f504c0ca3ee68f695ec8bd50e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220477
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-21 19:17:10 +00:00
Rohan Challa
9c32af924b internal/lsp: create more descriptive temp go.mod name
This change improves the temporary go.mod file name to include the base directory of the folder that is opened.

Change-Id: Ide759222e268bdc24f82b29625ac18f4f285aa64
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220361
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-21 19:09:30 +00:00
Rob Findley
fe62aff319 internal/lsp/regtest: skip flaky TestGoToStdlibDefinition
This test is flaking on the Trybots. Skip it until this is understood.

Updates golang/go#37318

Change-Id: Ie4c1db47797b88d5eb201a73c4ddfb5481f362ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220360
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-21 15:21:58 +00:00
Muir Manders
8a925fa4c0 internal/lsp/source: improve completions at file scope
Add support for var/func/const/type/import keywords at the file scope.
I left out "package" because, currently, if you are completing
something that means you must already have a package declaration. The
main hurdle was that anything other than a decl keyword shows up as
an *ast.BadDecl at the file scope. To properly detect the prefix we
manually scan for the token containing the position.

I also made a couple small drive-by improvements:
 - Also suggest "goto" and "type" keywords in functions.
 - Allow completing directly before a comment, e.g. "foo<>//". I
   needed this for a test that would have been annoying to write
   otherwise.

Updates golang/go#34009.

Change-Id: I290e7bdda9e66a16f996cdc291985a54bf375231
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217500
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-20 22:48:06 +00:00
Rob Findley
947cbf1911 internal/lsp/regtest: increase test timeout to 60s
In golang.org/issues/37318, it appears that the regtests are
occasionally timing out on the builders. I'm not sure why they're
running so slowly, but as a temporary measure lets increase the test
timeout to hopefully eliminate flakes.

Updates golang/go#37318

Change-Id: Id9c854ea518c9dc3618ea2b521fe6133e71af8b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220280
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-20 15:52:24 +00:00
Dmitri Shuralyov
2086a0a691 go/internal/gcimporter: update TestImportedTypes for BImportData removal
BImportData was removed from the standard library in CL 143023.
Update TestImportedTypes test data to use FindPkg instead.

Updates golang/go#27999.

Change-Id: I8ddb061da31187cc4a3b1c65f2d82d379a6c1066
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220277
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-20 05:18:52 +00:00
Rohan Challa
88b1720de6 internal/lsp/tests: fix regexp for removing links that contain versions
This change fixes the regex that removes the versions for links so that tests will still run under GOPATH mode. It also removes a link for an import that needed to be downloaded.

Change-Id: I7ed4f500d1bd9d2136188d30952eedb8d8aee6e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220140
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-19 22:25:53 +00:00
smasher164
d3d48f09aa go/analysis: add pass to check string(int) conversions
Issue golang/go#3939 proposes to remove string(int) from the language on the
grounds that it produces non-obvious results that can't be statically checked.
An intermediate step is to have go vet check for these sorts of conversions.
This change adds an analysis pass to check for string(int) conversions. It
suggests a fix to replace string(int) with string(rune(int)).

Updates golang/go#32479.

Change-Id: Ifafd6d74f9bd4a903ce6b29ac3a3c7a15f8a1ad9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212919
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-19 21:13:14 +00:00
Brian Falk
77adbdfd2c internal/lsp: report use of disallowed internal packages
An error should be reported if an "internal" package is imported
into code that is outside of the tree rooted at the parent
of the "internal" directory.

Fixes #35937

Change-Id: If5ff3dd79b462087381d575dddb20b78c10f0a83
GitHub-Last-Rev: f5d19960046da7f9701325afc36b5bd0b9663ab6
GitHub-Pull-Request: golang/tools#207
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218977
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-19 20:30:42 +00:00
Rebecca Stambler
55b11c713e internal/lsp: clear diagnostics for deleted files
Diagnostics should be cleared for files which are (1) deleted on disk
and not open in the editor, and (2) closed and only open in the editor.

Enable the corresponding regression test, and fix a few issues raised by
staticcheck.

Fixes golang/go#37049

Change-Id: Iff736a7f6c3eaacda4237c2e4cf7926e9949dece
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220079
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-19 20:26:41 +00:00
Rohan Challa
7c4b6277d7 internal/lsp: add module versions from "go list" to pkg.go.dev links
This change appends to the pkg.go.dev link the version of the module that is being used. To get this functionality, go/packages.Package now contains a module field which gets populated from the "go list" call. This module field is then used to get the version of the module that we are linking to.

Updates golang/go#36501

Change-Id: I9668a6da0fd3ec8f4cde017986419c8d28196765
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219079
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-19 19:55:21 +00:00
Rob Findley
c4d4ea9c79 internal/lsp/cache: return concrete types where possible
For testability, and to allow the exchange of debug information when
forwarding the LSP, it will be necessary to access debug information
stored in cache objects. This is cumbersome to do if our constructors
return source interfaces rather than concrete types.

This CL changes cache.New and (*Cache).NewSession to return concrete
types. This required removing NewSession from source.Cache. I would
argue that this makes sense from a philosophical perspective: everything
in the source package operates in a context where the Session and Cache
already exist.

Updates golang/go#34111

Change-Id: I01721db827d51117f9479f1544b15cedae0c5921
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220077
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-02-19 18:42:16 +00:00
Heschi Kreinick
33153249e7 internal/lsp: limit diagnostics concurrency
Diagnostics runs cannot be canceled until they finish a package. If a
user has a very expensive package, we may stack up diagnostics runs to
the point where the machine runs out of memory. We see hints of this in
various issues.

To avoid that, only allow one diagnostics run at a time. We can change
the limit later if we want.

Updates golang/go#37223.

Change-Id: Icd0eec4da936153306cf0a1f7175ae2b4b265272
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219958
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-19 18:35:52 +00:00
Rob Findley
66de428735 internal/jsonrpc2,internal/lsp/regtest: clean up some leaked tempfiles
Not all regtests resulted in LSP shutdown, which caused temp modfiles to
be leaked. After this fix I have confirmed that /tmp is clean after a
successful run of the regtests.

Also proactively clean up the unix socket file when serving jsonrpc2
over UDS.

Change-Id: I745fbd3d2adeeb165cadf7c54fd815d8df81d4e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220061
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-19 18:32:50 +00:00
Rob Findley
5fb17a1e7b internal/jsonrpc2: support serving over unix domain sockets
For tests (and perhaps later, for daemon discovery), unix domain sockets
offer advantages over TCP: we can know the exact socket address that will be
used when starting a server subprocess. They also offer performance and
security advantages over TCP, and were specifically requested on
golang.org/issues/34111.

This CL adds support for listening on UDS, and uses this to implement an
additional regtest environment mode that starts up an external process.
This mode is disabled by default, but may be enabled by the
-enable_gopls_subprocess_tests.

The regtest TestMain may be hijacked to instead run as gopls, if a
special environment variable is set. This allows the the test runner to
start a separate process by using os.Argv[0]. The -gopls_test_binary
flag may be used to point tests at a separate gopls binary.

Updates golang/go#36879
Updates golang/go#34111

Change-Id: I1cfdf55040e81ffa69a6726878a96529e5522e82
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218839
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-19 16:14:01 +00:00
Rob Findley
741f65b509 internal/lsp/lsprpc: add a forwarder handler
Add a forwarder handler that alters messages before forwarding, for now,
it just intercepts the "exit" message.

Also, make it easier to write regression tests for a shared gopls
instance, by adding a helper that instantiates two connected
environments, and only runs in the shared execution modes.

Updates golang/go#36879
Updates golang/go#34111

Change-Id: I7673f72ab71b5c7fd6ad65d274c15132a942e06a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218778
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-19 15:38:09 +00:00
Daisuke Suzuki
ca8c272a4d internal/lsp/tests: fix reset of golden files
internal/lsp/reset_golden.sh fails when golden file does not exist, so
skip loading the golden file on update.
Additionally, add the missing primarymod directory as the update
destination path so that golden files are placed under the primarymod
directory.
However, keep the location of summary.txt.golden in the same directory
as the primarymod directory.
As a result, some unnecessary data was deleted.

Change-Id: I98120c8b7d483174644600786fd30acdc2e4c52e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219577
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-02-19 14:42:13 +00:00
Muir Manders
753a1d49df internal/lsp/cache: kill unused func fixAccidentalDecl
I think I resurrected this accidentally when resolving a merge
conflict.

Change-Id: Ib871068902eeaaca9b95878f3e40ed5e53d6814f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220018
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-19 05:42:38 +00:00
Heschi Kreinick
f8e42dc477 internal/span: handle URI escaping better
We expected URIs to be canonical, but VS Code's URI escaping is
overeager and escapes things that don't need to be. Forcibly unescape
them, then re-escape them with Go's encoding, so that we know they'll
match URIs generated by URIFromPath.

Fixes golang/go#37231.

Change-Id: I4b163ae3c91e8846ab72e5b5e89bd6d018c3995e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219899
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-18 20:59:02 +00:00
Heschi Kreinick
8fd784fb38 internal/lsp/debug: drop memory debug threshold to 1GiB
Most people do not have giant 64-GiB workstations, so 10GiB of RAM (5
active heap, 5 inactive for GOGC=100) is a really high threshold. Drop
to 1GiB active to drop profiles.

Change-Id: If0ae418828377a648a93322e269f4610fd64ebb3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219937
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-18 20:58:54 +00:00
Heschi Kreinick
dc8ced655d internal/lsp: add scheme checks to CodeLens
I missed this newly-added function in CL 219483.

Change-Id: I4490f579236a7ed358fb7a7ed80276560f19b04e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219539
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-18 19:00:56 +00:00
Rob Findley
b320d3a0f5 internal/jsonrpc2/servertest: support both TCP and pipe connection
Update the servertest package to support connecting to a jsonrpc2 server
using either TCP or io.Pipes. The latter is provided so that regtests
can more accurately mimic the current gopls execution mode, where gopls
is run as a sidecar and communicated with via a pipe.

Updates golang/go#36879

Change-Id: I0e14ed0e628333ba2cc7b088009f1887fcaa82a5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218777
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-16 19:22:41 +00:00
Heschi Kreinick
5916a50871 internal/lsp: check for file URIs on LSP requests
In general, we expect all URIs to be file:// scheme. Silently ignore
requests that come in for other schemes. (In the command-line client we
panic since we should never see anything else.)

The calling convention for beginFileRequest is odd; see the function
comment.

Fixes golang/go#33699.

Change-Id: Ie721e9a85478f3a12975f6528cfbd28cc7910be8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219483
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-14 22:51:26 +00:00
Heschi Kreinick
f7b8cc7bd0 internal/span,lsp: disambiguate URIs, DocumentURIs, and paths
Create a real type for protocol.DocumentURIs. Remove span.NewURI in
favor of path/URI-specific constructors. Remove span.Parse's ability to
parse URI-based spans, which appears to be totally unused.

As a consequence, we no longer mangle non-file URIs to start with
file://, and crash all over the place when one is opened.

Updates golang/go#33699.

Change-Id: Ic7347c9768e38002b4ad9c84471329d0af7d2e05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219482
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-14 22:51:03 +00:00
Rohan Challa
548b770e2d internal/lsp/cache: parse go.mod file before running go mod tidy
This change reorders the logic within ModTidyHandle and ParseModHandle to parse the modfile first before we copy the contents to the temporary go.mod file. This was causing issues where a go.mod would be in a bad state and then we would try to run "go mod tidy" on the corrupted file.

Change-Id: I1df8fb70f5f3e2bcff306a58b16bc96c32debf2a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219480
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-14 20:11:35 +00:00
Rohan Challa
bc664416c6 internal/lsp: fix error handling when getting go.mod codelens
This change has a fix for mod/codelens: check if we get an error from ParseModHandles().Upgrades(). This change also only runs codelens on save.

Change-Id: I6dab7ddf3a08c650e4c670b039b1e99153ec8187
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219478
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-14 19:54:16 +00:00
Ayke van Laethem
9976379007 go/ssa: add DebugRef.Object
This function is in the documentation for *ssa.DebugRef but is not defined anywhere, so define it now.

Change-Id: I0a6c07ccac16851541a5fe337f0bba66608949cb
GitHub-Last-Rev: 30f39663b48acd419a9ae8a90ae34d9350aff218
GitHub-Pull-Request: golang/tools#201
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216477
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-14 19:16:34 +00:00
Muir Manders
ea829e2eb2 internal/lsp/source: handle completing into variadic params
For example:

func foo(string, ...int)

func a() string
func b() (string, int)
func c() (string, int, int)

// Prefer "a()" and "b()" and "c()". Previously we didn't prefer any of
// them.
foo(<>)

Fixes golang/go#36540.

Change-Id: I144b3f63942b7699d3034efcc9ad8535a7fa3165
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215538
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-14 18:12:26 +00:00
Muir Manders
c229649527 internal/lsp/cache: fix crash fixing curlies near EOF
We were crashing in cases like:

1:    func foo() {
2:     if b<> <EOF>

We were trying to get the line start position for line 3, but there is
no line 3. Fix by bailing out early if we are the last line in the
file because there is nothing to fix in that case.

Fixes golang/go#37226.

Change-Id: I4ad5746d7b55bdcc2de57c04e972c15a61084faa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219498
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-14 17:25:38 +00:00
Rob Findley
88be01311a internal/jsonrpc2: fix races in cancellation
We had a deadlock in cases where a request was cancelled (1) after being
written to the stream, but (2) before a response was received. This
resulted in the request ID being removed from the pending map while the
server has the request, after which point the server response would hang
in Conn.Run trying to send to a nil channel.

After fixing this nil send there was still a race: it was possible that
Conn.Run could get the pending request, and Conn.Call would select
ctx.Done before Conn.Run could send to the response channel, again
resulting in a blocking send. Fix this by adding a buffer to the
response channel.

The response channel management is also made less forgiving, because we
should be able to reason precisely about how many sends and receives
will occur:
 + Don't close the response channel after sending a response: there
   should only be one recipient.
 + Don't delete the ID from pending map twice: it should only be cleaned
   up by Conn.Call.

Cancellation tests in the lsprpc package are updated to exercise the
race conditions.

Fixes golang/go#37159

Change-Id: Ie3207442ea910f79247b18d8647fd52f39fb15db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219126
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-14 14:43:24 +00:00
Rob Findley
0fd2d649e6 internal/lsp/lsprpc: add an LSP forwarder and regtest environment
Add a new Forwarder type to the lsprpc package, which implements the
jsonrpc2.StreamServer interface. This will be used to establish some
parity in the implementation of shared and singleton gopls servers.

Much more testing is needed, as is handling for the many edge cases
around forwarding the LSP, but since this is functionally equivalent to
TCP forwarding (and the -remote flag was already broken), I went ahead
and used the Forwarder to replace the forward method in the serve
command. This means that we can now use the combination of -listen and
-remote to chain together gopls servers... not that there's any reason
to do this.

Also, wrap the new regression tests with a focus on expressiveness when
testing the happy path, as well as parameterizing them so that they can
be run against different client/server execution environments. This
started to be sizable enough to warrant moving them to a separate
regtest package. The lsprpc package tests will instead focus on unit
testing the client-server binding logic.

Updates golang/go#36879
Updates golang/go#34111

Change-Id: Ib98131a58aabc69299845d2ecefceccfc1199574
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218698
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-14 14:21:06 +00:00
Rohan Challa
98b3097d01 internal/lsp: add codelens for go.mod dependency upgrades
This change adds a code lens for go.mod files that will let a user know if a module can be upgraded, once it is clicked gopls will run a command to update that module.

Updates golang/go#36501

Change-Id: Id22b8097ede4972cf73bc029ec927544a71b7150
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218557
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-14 14:15:08 +00:00
Muir Manders
88e652f7a8 internal/lsp/cache: improve completion after dangling selector
Dangling selectors such as:

    func _() {
      x.
    }
    var x struct { i int }

tend to wreak havoc on the AST. In the above example you didn't used
to get completions because the declaration of "x" was missing from the
AST.

We now work around this issue by inserting a "_" into the source code
before parsing to make the selector valid:

    func _() {
      x._ // <-- insert "_" here
    }
    var x struct { i int }

This makes completion work as expected because the declaration of "x"
is present in the AST.

I had to change fixAST() to be called before fixSrc() because
otherwise this new workaround in fixSrc() breaks the "accidental
keyword" countermeasures in fixAST().

Fixes golang/go#31973.
Updates golang/go#31687.

Change-Id: Ia7ef6c045a9c71502d1b8b36f187ac9b8a85fe21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216484
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 22:46:42 +00:00
Daisuke Suzuki
885dec1b2d internal/lsp/source: make matchers selectable in WorkspaceSymbols
This change allows to use fuzzy or case-sensitive matchers in addition
to case-insensitive when searching for symbols.
Matcher is specified by UserOptions.Matcher just like Completion.

Updates golang/go#33844

Change-Id: I4000fb7984c75f0f41c38d740dbe164398032312
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218737
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-13 22:46:20 +00:00
Rohan Challa
695c81b9c6 internal/lsp: fix diagnostics not clearing when creating new files
This change fixes an issue where import errors would not disappear when creating new files in a module by removing the segment of code where we check for listErrors when fixing imports.

Updates golang/go#36960

Change-Id: Iefa17edeb0417cac7e33ffa88faf7c9a607e98b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219222
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 21:50:53 +00:00
Peter Weinberger
e3acd584e2 internal/lsp: two trivial cleanups from CL 219077
Change-Id: Ia55bfc4d8283589a7a70e2b7b2ed3265ead18d62
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219378
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-13 21:22:11 +00:00
Peter Weinberger
63d1300efe internal/lsp: change return type of PrepareRename to *Range
PrepareRename used to return interface{}, but now returns the more
precise result *Range.

Almost all the changes in code.ts come from vscode reformatting it.
The sole substantive change is in goUnionType, near line 685.

Change-Id: If575cb166f26962d24261e79f9d958006011402e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219077
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-13 20:00:52 +00:00
Muir Manders
9f716520f4 internal/lsp/cache: improve completion in init statements
In cases like:

  if i := foo<>

we get an *ast.BadExpr because the parser is expecting the condition
expression, but "i := foo" is not a valid expression. Now we move the
statement into the "init" field and add a dummy "cond" expression.

We also needed a slight tweak to our missing curly brace fix. Now we
insert an extra semicolon in cases like:

for i := 0; i < foo

yielding

for i := 0; i < foo;{}

The parser doesn't like having only two clauses in the three clause
"for" statement.

Updates golang/go#31687.

Change-Id: I12d51e0d8af03436741227753f8e71452a463b05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216483
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 18:11:06 +00:00
Muir Manders
2f0693cea3 internal/lsp/cache: improve completion when missing opening curly
In cases like:

func _() {
  if fo<>
}

func foo() {}

Completing at "<>" does not include "foo" because the missing "if"
opening curly brace renders the rest of the file unparseable. "foo"
doesn't exist in the AST, so as far as gopls is concerned it doesn't
exist at all.

To fix this, we detect when a block is missing opening "{" and we
insert stub "{}" to make things parse better. This is a different kind
of fix than our previous *ast.BadExpr handling because we must reparse
the file after tweaking the source. After reparsing we maintain the
original syntax error so the user sees consistent error messages. This
also avoids having the "{}" spring into existence when the user
formats the file.

Fixes golang/go#31907.
Updates golang/go#31687.

Change-Id: I95ba60a11f7dd23dc484c063b4fd7ad77daa4e08
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216482
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 18:10:51 +00:00