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

1285 Commits

Author SHA1 Message Date
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
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
Rohan Challa
0abbfa5d31 internal/lsp: turn tempModfile flag on by default
This change turns the tempModfile flag on by default in the master branch.

Change-Id: If96369aee4e970421a80d5ff89385a38c7ccf0b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219225
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 15:33:58 +00:00
Muir Manders
49b8ac185c internal/lsp/cache: add file contents to ParseGoHandle
Currently there is no need for this because the file contents are part
of the file handle. This change is in preparation for an impending
improvement that tweaks the source code during the parse stage to fix
certain kind of terminal parse errors. Any code that wants to use
an *ast.File or *token.File in conjunction with the file contents
needs access to the doctored source code so things line up.

Change-Id: I59d83d3d6150aa1264761aa2c1f6c1269075a2ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218979
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-02-13 05:05:14 +00:00
Muir Manders
cbc0cc175f internal/lsp/source: fix completion crash in append()
We were crashing in cases like:

    var _ []byte = append([]byte{}, ""...<>)

We were type asserting the type of append's second param
to *types.Slice, but in this case it is a string (*types.Basic). Fix
by checking the type assert was successful.

Note that we still don't attempt to give string completions when
appending to a byte slice. We can add that special case later once
everyone is clamoring for it.

Change-Id: I1d2fbd7f538e580d33c2dab4ef127a88e16d7ced
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219144
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 02:33:03 +00:00
Rebecca Stambler
6dd6151793 internal/lsp: ignore irrelevant on-disk changes
Change-Id: Ibdceadbfc822a64066d9370eefa9ff5f7988d0a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219202
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-12 21:15:05 +00:00
Muir Manders
ea181f53ac internal/lsp/source: filter candidates when type name required
Now when we expect a type name at the cursor, we omit non-type name
completion candidates. For example:

inch := 1
var foo in<> // don't offer "inch"

I also added expected type name detection for value specs:

// Expect a type name at <>
var foo <>

Fixes golang/go#32806.

Change-Id: I32477cb286d2050bac5ccc767f8a608124fa5acd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216400
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-12 15:05:39 +00:00
Rebecca Stambler
6822f1ada4 internal/lsp: disable nilness analyzer, unless staticcheck enabled
Users with staticcheck enabled will be more tolerant of increased RAM
usage. We might be able to disable this check entirely once the next
version of staticcheck is released with a similar analysis (https://staticcheck.io/docs/checks#SA5011).

Change-Id: I1a844cc226e34e0f62222f12912797a6cc9d06e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-11 20:49:38 +00:00
Rebecca Stambler
e2a38c8363 internal/lsp: propagate file invalidations to all views
We were previously only invalidating files if they were contained within
a subdirectory of a view, but we should really be invalidating all files
that are known to a view.

Fixes golang/go#34955

Change-Id: I2eb1476e6b5f74a64dbb6d47459f4009648c720d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218859
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 18:37:05 +00:00
Rob Findley
f41547ceaf internal/lsp/debug: fix early closure of logfile
As of https://golang.org/cl/218457, logs are not being captured because
the logfile is prematurely closed due to the scope of the deferred
closure changing.

Change-Id: I1754e5555025c7b2a5da58f621184d6740fd03cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219080
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 18:05:03 +00:00
Muir Manders
2de505fc53 internal/lsp: support multi-dereferencing completion candidates
Now we keep a count of how many times to dereference a candidate. For
example:

    var foo ***int
    var _ int = f<> // Now we offer "***foo" instead of "*foo".

Change-Id: I14edc40aeec6884399eceb3dd3b4f85dc74a773c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218580
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 04:52:51 +00:00
Muir Manders
1e13d9580f internal/lsp/source: fix type modifier detection in composite literals
When completing a composite literal value, we were returning from
candidate inference before we recorded type modifiers such as prefix
"&" or "*". This was causing funny completions like:

type myStruct struct { s *myStruct }
myStruct{s: &mySt<> // completed to "&&myStruct{}"

Now we properly pick up on the "&" prefix so we know our literal
"myStruct{}" candidate does not need a "&".

Change-Id: I908936698cfedfef81bc0c1cbcd93e14dc00e3a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218377
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 03:24:02 +00:00
Tomohiro Kusumoto
8feddd8b6a internal/lsp/tests: fix missing period
Change-Id: Idb0b6405aefec1dcef84958482325a68afec5348
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218957
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 03:20:47 +00:00
Muir Manders
0bc66720f3 internal/lsp/source: limit to 5 unimported package name candidates
Currently we show up to ~100 unimported package names matching the
completion prefix. This isn't useful since, assuming the user even
wants an unimported package, they will just type more to narrow it
down rather than scroll through 100 options. Having so many candidates
also slows things down due to per-candidate overhead in gopls and in
the LSP client. Now we instead limit to 5 unimported package names.

Unimported package members, on the other hand, make sense to list
many. The user may want to scroll through because they don't remember
the name of what they are looking for. I left the max value at 100.

Change-Id: I00e11fa0420758f8db6c7049f80fa156773a5ee6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218879
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 00:32:09 +00:00
Heschi Kreinick
b0390335f4 internal/lsp/debug: write debug info for large goplses
We've had various reports of high memory usage in gopls. Catching it in
the act can be difficult, so check every second and write relevant
profiles to os.TempDir every time the heap reaches a new watermark.

Note that the logging in this package is broken. I didn't fix it, so
nobody will know this is happening until we tell them. So it goes.

Change-Id: I972d7ccfe5308658f86dde717465f0e0151b396d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218858
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 22:37:59 +00:00
Jay Conrod
1ace956b0e internal/imports: import packages from x/mod instead of internal copy
This CL deletes ./internal/module and ./internal/semver, which are
copies of packages in golang.org/x/mod. The copies were created before
x/mod was a separate module.

./internal/imports is updated to use the packages in x/mod.

Change-Id: Id434f5f0a240de97d18505cb7c925c2e062f6231
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218897
Run-TryBot: Jay Conrod <jayconrod@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-10 19:23:13 +00:00
Rohan Challa
3868802a52 internal/imports: change processEnv to use buildflags
This change adds a buildFlags variable to the processEnv struct rather than appending them to the GOFLAGS by using a space separator.

Fixes golang/go#37108

Change-Id: I4331066c30fa51f0133504d723132527b00ce74a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218857
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-10 18:52:02 +00:00
Ian Cottrell
bdd8440908 internal/lsp: move all debugging support to the debug package
We now carry around a debug instance in the app, and it manages the log file and
all telemetry

Change-Id: If4a51a36c38ef301b1b2bbda8e4c0a8d9bfc3c04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218457
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-10 18:44:17 +00:00
Muir Manders
2ef0387721 internal/lsp/source: fix unimported member completion ranking
When completing members on a type checked, unimported package, you get
fully typed members. That means you get deep completions. Because we
downrank the initial unimported package members so much, any deep
completions were dominating the rankings. For example

    context.Back<>

yielded "context.Background().Err" ranked above "context.Background".
Fix by scoring context.Background in this example as
stdScore+tinyRelevanceScore instead of just tinyRelevanceScore. I also
changed untyped candidate scores in the same way so they stay
competitive when you have both imported and unimported candidates.

The other option was to propagate the score penalty into deep
candidates, but that wasn't easy. In general I think you are better off
avoiding big score penalties because they complicate the interplay
between different kinds of candidates. Scoring needs an overhaul, but
at least we are building up our test suite in the meantime.

Change-Id: Ia5d32c057b04174229686cec6ac0542c30e186e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218378
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-10 18:17:33 +00:00
Rob Findley
9fbd0ccf67 internal/lsp/lsprpc: add test for definition outside of workspace
Add regression tests for GoToDefinition. In particular, exercise the
panic from golang/go#37045.

Updates golang/go#37045
Updates golang/go#36879

Change-Id: I67b562acd293f47907de0435c14b62c1a22cf2ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218322
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 17:53:45 +00:00
Rob Findley
babff93c04 internal/lsp/lsprpc: add test for empty diagnostics in deleted files
Add a test for the bug reported in golang/go#37049: we are missing empty
diagnostics for deleted files. Doing this involved added a missing
RemoveFile method on the fake.Watcher type.

Skip the test for now, as it is failing.

Updates golang/go#37049
Updates golang/go#36879

Change-Id: Ib3b6907455cc44a2e6af00c2254aa444e9480749
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218278
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 17:44:40 +00:00
Rob Findley
f9587291b6 internal/lsp/fake: add fakes for testing editor interaction
A lot of bug reports originating from LSP clients are related to either
the timing or sequence of editor interactions with gopls (or at least
they're originally reported this way). For example: "when I open a
package and then create a new file, I lose diagnostics for existing
files".  These conditions are often hard to reproduce, and to isolate as
either a gopls bug or a bug in the editor.

Right now we're relying on govim integration tests to catch these
regressions, but it's important to also have a testing framework that
can exercise this functionality in-process.  As a starting point this CL
adds test fakes that implement a high level API for scripting editor
interactions. A fake workspace can be used to sandbox file operations; a
fake editor provides an interface for text editing operations; a fake
LSP client can be used to connect the fake editor to a gopls instance.
Some tests are added to the lsprpc package to demonstrate the API.

The primary goal of these fakes should be to simulate an client that
complies to the LSP spec. Put another way: if we have a bug report that
we can't reproduce with our regression tests, it should either be a bug
in our test fakes or a bug in the LSP client originating the report.

I did my best to comply with the spec in this implementation, but it
will certainly develop as we write more tests. We will also need to add
to the editor API in the future for testing more language features.

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

Change-Id: Ib81188683a7066184b8a254275ed5525191a2d68
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217598
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-10 17:44:19 +00:00
Muir Manders
61798d64f0 internal/lsp: fix crash completing recursive pointer types
We were recursing infinitely evaluating objects of recursive pointer
types such as "type foo *foo". Now we track named pointer types we
have already seen to avoid trying to dereference such objects forever.
I lazily initialized the "seen" map to avoid the allocation in the
normal case when you aren't dealing with named pointer types.

Fixes golang/go#37104.

Change-Id: I5f294cfc5a641e7b5fd24e1d9dc55520726ea560
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218579
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 22:44:06 +00:00
Rebecca Stambler
6dcdf1db2c internal/lsp/cache: refactor functions that return PackageHandles
Now that we build PackageHandles in load, we can simplify a lot of our
logic for rebuilding them and reloading metadata.

The only possibly-significant change is the missing imports logic. Now
that we always create package handles in load, we don't have to do the
extra "sameSet" logic. If the package handle hasn't been invalidated
since we last built it, we will keep the benefits of caching it. Otherwise,
it will be rebuilt in load.

Updates golang/go#36918

Change-Id: Ieb45898d248501e3cebdb95c8b518149ba7498e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217157
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 22:39:43 +00:00
Heschi Kreinick
009580c43b internal/lsp/source: export FindFileInpackage
And delete a copy of it.

Change-Id: Ice7b932327dbfe5e00f1d084fc6669f1e4059afe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218320
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 21:55:11 +00:00
Rebecca Stambler
8bb641bf89 internal/lsp/source: default to full documentation for hover
I'm starting to think that it might make more sense to show all
documentation on hover as a default. A number of people have requested
this behavior, and I think it would help ensure a more consistent
experience for users. We had originally defaulted to the synopsis
because VS Code Go had this behavior, but I see no reason to follow that
as a guideline.

Change-Id: I67aa530d253422550f59b5583e4c4a90ebd48f5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217727
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 21:32:44 +00:00
emahiro
b6336cbc8d internal/lsp/tests: move workspace symbol test helpers functions into util.go
Change-Id: I747c7a0ca9b41e82e33a39fc91ddc2afaf458be6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218139
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 21:23:39 +00:00
Rebecca Stambler
7cfd24942e internal/lsp/cache: hardcode parse modes instead of guessing them
This change largely reverts CL 217139, which attempted to guess a
package's parse mode based on whether or not it was in the user's
workspace. This ignored the fact that a user may jump to the definition
of a file outside of their workspace.

Fixes golang/go#37045

Change-Id: Icb6b9d055bd1f260013227db1a6a34873c45b680
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218499
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-07 20:00:15 +00:00
Tomohiro Kusumoto
112b90105c internal/lsp/source: eliminate setKind
Eliminate unnecessary function

Change-Id: I1b590961c3b2042f244eeb4c11c34fbf20b8b74a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218138
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 19:42:34 +00:00
Rebecca Stambler
3d51b05cfb internal/lsp: don't use overlays from the session in the snapshot
Hold the session's overlay mutex the whole time we compute new overlays,
and then pass these overlays directly into clone. This avoids us calling
s.session.GetFile, which can return overlays that the snapshot doesn't
yet "know" about.

Change-Id: I1a10c78e26f8fec64550bfe0a97b5975ea8f976b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 19:21:16 +00:00
Rebecca Stambler
b753a1ba74 internal/lsp: build overlays through the snapshot
This is the first in a series of changes to move overlay handling to the
snapshot instead of the session. We may not be able to fully get away
from managing overlays on the session, but we should be able to only use
overlays when they are known to the snapshot.

Change-Id: I88b125117cd2cfbd0ff9ef16a944a34297c81b10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218324
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 18:37:49 +00:00
Rob Findley
58fec203a2 internal/lsp/cache: don't type check types.Unsafe
Recent test runs exposed that we are racing to types.Unsafe, as a result
of parallel type checking. Example:
https://storage.googleapis.com/go-build-log/494dd1dd/linux-amd64-race_c978961e.log

We shouldn't have to type check unsafe, so we can return early instead.

Change-Id: I20143dbfb07925d85d7342b93360bdda1c45e4aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218497
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 18:36:39 +00:00
Rohan Challa
a9bd9c230f internal/lsp/cache: improve ModTidyHandle cache key
This change alters the key that is used to cache go.mod diagnostic changes, in particular it replaces using the snapshot ID with a string of all the imports used in the module and the hashed contents of the go.mod file. This reduces the number of times that we run "go mod tidy" to only when we detect import changes or the go.mod file is changed.

Change-Id: Icf49db34f44a4ae4772fff6dfb8b9a6955a8e2d6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218238
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-07 18:07:39 +00:00
Ian Cottrell
59bd84bb0c internal/lsp: change debug instance to a struct
We now build an instance and invoke methods on it.
This is a step towards removing some global state,
allowing multiple instances in one process and cleaning
up some telemetry handling.

Change-Id: I067469fed39c96653fffe96945c79193e86458d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218157
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-07 11:26:02 +00:00
Rohan Challa
6fdc5776f4 internal/lsp: add quickfixes for missing dependencies in go.mod
This change adds quick fixes for diagnostics in .go files, specifically for diagnostics that deal with imported packages that are not declared in the go.mod file. These quick fixes will automatically add the dependency in the go.mod file and format the file if there are any issues.

Updates golang/go#31999

Change-Id: Iab151ce96194fae4b1995859aec416c5473da6e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215898
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-07 00:16:14 +00:00
Rob Findley
c29062fe1d internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).

This change reorganizes Server instantiation as follows:

 + The lsp.Server is now purely an implementation of the protocol.Server
   interface. It is no longer responsible for installing itself into the
   jsonrpc2 Stream, nor for running itself.

 + A new package 'lsprpc' is added, to implement the logic of binding an
   incoming connection to an LSP server session. This is put in a
   separate package for lack of a clear home: it didn't really
   philosophically belong in any of the lsp, cmd, or protocol packages.
   We can perhaps move it to cmd in the future, but I'd like to keep it
   as a separate package while I develop request forwarding.

   simplified import graph:

    jsonrpc2 ⭠ lsprpc ⭠ cmd
               ⭩           ⭦
            lsp           (t.b.d. client tests)
           ⭩   ⭨
     protocol  source

 + The jsonrpc2 package is extended to have a minimal API for running a
   'StreamServer': something analogous to an HTTP server that listens
   for new connections and delegates to a handler (but we couldn't use
   the word 'Handler' for this delegate as it was already taken).

After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.

This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.

To test this change, the following improvements are made:

 + A servertest package is added to make it easier to run a test against
   an in-process jsonrpc2 server. For now, this uses TCP but it could
   easily be modified to use io.Pipe.

 + cmd tests are updated to use the servertest package. Unfortunately it
   wasn't yet possible to eliminate the concept of `remote=internal` in
   favor of just using multiple sessions, because view initialization
   involves calling both `go env` and `packages.Load`, which slow down
   session startup significantly. See also golang.org/issue/35968.

   Instead, the syntax for `-remote=internal` is modified to be
   `-remote=internal@127.0.0.1:12345`.

 + An additional test for request cancellation is added for the
   sessionserver package. This test uncovered a bug: when calling
   Canceller.Cancel, we were using id rather than &id, which resulted in
   incorrect json serialization (as only the pointer receiver implements
   the json.Marshaller interface).

Updates golang/go#34111

Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
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-02-06 23:12:37 +00:00
Rebecca Stambler
37215997d4 internal/lsp: move all of the test helpers functions into one file
Each of these files had a couple of functions that were very similar in
nature. There's no need to have separate files for all of these.

Change-Id: I4ca648d1b7e90539f274871d45b7c97a8111631f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218319
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 20:47:26 +00:00
emahiro
f8c9a4c3ab internal/lsp: export DiffSymbols to avoid duplication
Change-Id: I9002d19067011e0fcff22782eebc593a124594c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2020-02-06 20:09:13 +00:00
Rebecca Stambler
222e5d4b1b internal/lsp: don't return references for builtins
https://github.com/fatih/vim-go/issues/2701 contains a report of a panic
when finding references on a builtin. Avoid this by simply not returning
any references.

Change-Id: I195fcb4502634201888f0c65022c9d16169cc1f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 19:49:06 +00:00
Rebecca Stambler
6224300ba8 internal/lsp: remove unnecessary source.SignatureInformation type
We should just use the protocol.SignatureInformation type, as it's
essentially the same thing. Refactor tests a bit to make use of the
shared type.

Change-Id: I169949f6e23757ce0a6f54de36560c4c8e0479ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217731
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-06 19:05:38 +00:00
Rohan Challa
64a0f23fc3 go/packages/packagestest: do not overwrite existing go.mod file
This change adds support for testing go.mod files within packagestest. Primarily, if there are markers in the go.mod file, this will copy the contents to a temporary file, build the modcache, then set the contents back.

Updates golang/go#36091

Change-Id: Icb707906eb7fc9e4a06fe043f94f34d9223d84c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216839
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 15:23:23 +00:00
Rohan Challa
2529d2857a internal/lsp/tests: standardize testdata folder format
This change standardizes the folder structure for testdata that are used for testing the lsp. In particular, it uses the following format:
- dir
  - primarymod
    - .go files
    - packages
    - go.mod (optional)
  - modules
    - repoa
      - mod1
        - .go files
        -  packages
        - go.mod (optional)

As we can see, any folder inside of testdata should be of this format, where the primary test files with the markers are all located inside the primarymod folder. The modules folder is used to hold any potential dependencies that are used for testing.

A consequence of this change is that we can have one directory separated by folders, where each folder is it's own module, this allows us to use internal/lsp/tests with go.mod files. Now, tests.Load() will return an array of Data objects, where each object corresponds to one of the directories structured above.

Updates golang/go#36091

Change-Id: I437cc2a2a9fc1bac93779845737aa74383fbf9c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217541
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 14:14:23 +00:00
Muir Manders
dd0d5d4851 internal/lsp: filter keyword completions in tests
Filter keywords out of completion results in tests similar to
builtins. You don't care about keyword completions unless you are
explicitly testing keyword completion.

Change-Id: I0caaaef8b0f5b08c4b15ba3ada1a963f35a14028
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217499
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:08:30 +00:00
Muir Manders
f66ef90017 internal/lsp/source: improve completion after accidental keywords
Completion often fails when the completion prefix happens to be a
keyword. We previously tried to fix this with AST surgery, but
often the accidental keyword is not apparent looking at the AST.
For example:

    chan<>
    foo()

parses as CallExpr{Fun: ChanType{Value: Ident{"foo"}}} with very few
hints that something is wrong, and:

    default
    foo()

is completely omitted from the AST.

Rather than look in the AST, we now instead manually look for a
keyword token that contains the completion position. If we find one,
we treat that as our surrounding identifier.

Updates golang/go#34332.

Change-Id: I68ed0dd905848c0eae61f39ecb8b73adb1e72746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216961
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:08:22 +00:00
Muir Manders
50d618665b internal/lsp/source: improve completion involving multiple return values
For example:

// Prefer functions that return one or two values. Previously
// we had no preference.
foo, bar := <>

// Prefer functions that return "(int)" or "(int, ??)". Previously we
// only preferred the former.
var foo int
foo, bar := <>

// Prefer functions that return "(int)" or "(int, int)". Previously we
// only preferred the former.
var foo func(int, int)
foo(<>)

In the above example, we don't handle "foo" being variadic yet.

I also took the liberty to break up matchingCandidate() into separate
functions since it was getting rather long.

Updates golang/go#36540.

Change-Id: I9140dd989dfde1ddcfcd9d2a14198045c02587f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215537
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:07:08 +00:00
Rebecca Stambler
403f1254bd internal/lsp: stop returning metadata from (*snapshot).load
This metadata was hardly being used, and it's not really necessary now
that we are creating package handles in load. There are still a number
of cases that can simplified because of this fact, but those will be
done in follow-ups.

Also, fix a stray staticcheck warning.

Change-Id: I12d1b4d568a8fd145d08397a926e7ba6f3428604
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 00:15:21 +00:00
Heschi Kreinick
9f574694fd internal/imports: prevent self-imports in the stdlib
goimports has a fast path for stdlib imports that was not checking for
import cycles. Add the check.

Fixes golang/go#37063.

Change-Id: I46c98c317d8f06f83018fef9ef7edf9222e6b3f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217958
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 00:08:43 +00:00
Koichi Shiraishi
aa017ee804 internal/jsonrpc2: fix invalid invocations of Fatalf in goroutines
Found by running the go vet pass 'testinggoroutine'.

Change-Id: I38f17877e2a97ffb823bb97850d21107743271d7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217179
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-05 19:51:38 +00:00
Daisuke Suzuki
6e8b36d2c7 internal/lsp: add support for workspace symbol
This change adds support for the LSP workspace/symbol. Unlike
documentSymbol, the target is symbols that exist not only in a specific
file, but also in the current or imported packages. It returns symbols
whose name contains the query string of the request(case-insensitive),
or all symbols if the query string is empty.

However, the following is not implemented:
- Setting of deprecated and containerName fields in SymbolInformation
- Consideration of WorkspaceClientCapabilities
- Progress support
- CLI support

Updates golang/go#33844

Change-Id: Id2a8d3c468084b9d44228cc6ed2ad37c4b52c405
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213317
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-05 19:03:17 +00:00
Rohan Challa
9e60d4e5b9 internal/lsp: continue diagnostics if diagnosing go.mod fails
This change ensures that diagnostics for .go files get calculated and published regardless of whether or not the go.mod file diagnostics fail. This change also updates lsp_test.SuggestedFix to make use of the snapshot.Diagnose function vs the source.FileDiagnostics.

Change-Id: I54c35106b1701b54cca9a413edfb253f3c4c5ab7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217839
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-05 19:02:14 +00:00
Rebecca Stambler
67a4523381 internal/lsp: determine parse mode based on workspace packages
Our current invariant is that all workspace packages are parsed in full
mode, and all dependencies are parsed in exported mode. We can rely on
this, as well as the fact that workspace packages are set during
metadata loads, to reduce the amount of plumbing the mode requires.W

Change-Id: Ib9406ca3c0dc2c81c7ee3158407f28022924d4d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217139
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-04 23:03:16 +00:00
Rohan Challa
7124308813 internal/lsp/tests: add severity parameter to @diag markers
This change will add another parameter to the diagnostic markers (@diag).

Change-Id: I69f4672e7d5417099c11d1a46d9bc45a4d1e5ecc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217719
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-04 19:24:00 +00:00
Rohan Verma
1cc6d1ef6c tools/gopls: add cmd support for prepare_rename
This change adds command line support for prepare_rename.

Updates golang/go#32875

Change-Id: I7f155b9c8329c0faa26a320abab162730a7916ad
GitHub-Last-Rev: 118e846420d7c56fbf0e4f416c8fd6a52e9638a8
GitHub-Pull-Request: golang/tools#188
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-04 07:42:04 +00:00
Rebecca Stambler
59e246924e internal/lsp/cache: use snapshot ID and view folder in ModTidyHandle key
Just to be safe, the snapshot ID and view's folder should be enough to
uniquely identify the ModTidyHandle.

Change-Id: Ie3a0dc64bf16bcc8e4eb75af107481c07de81967
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217657
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-04 06:02:07 +00:00
Heschi Kreinick
cf5ba95194 internal/imports: stop leaking listeners
(*dirInfoCache).ScanAndListen saves a listener when it's called. That
listener is a chain of callbacks that leads all the way back up to the
original caller, i.e. the gopls completion code. It needs to unregister
that listener, but in cases where the context was cancelled before it
finished its initial walk of the cache, it failed to. In gopls' case,
that means retaining the entire state of the workspace as of completion
triggering.

Return a real stop function on all paths.

Change-Id: Iff3046e627b1fa89f576371c4742fee6fdca7589
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217677
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-04 05:47:48 +00:00
Rebecca Stambler
35ac94b00d internal/span, internal/lsp: fix URI escaping
We had previously worked around a VS Code URI bug by unescaping URIs.
This is incorrect, so stop doing it and then add a specific workaround
just for that one bug.

Fixes golang/go#36999

Change-Id: I92f1a5f71749af7a6b1020eee1272586515f7084
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217599
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-04 00:54:49 +00:00
Heschi Kreinick
49e540660c internal/lsp/debug: serve cache entry counts
We seem to be leaking cache entries. A simple status page will help us
confirm that.

Change-Id: I485bfff6ebfb5d30655554487583e15a3f49f9a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217597
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-03 22:21:18 +00:00
Rebecca Stambler
ab391d50b5 internal/lsp: don't show links in hover for test functions
source.Identifier previously was used for references and rename, so it
needed to take a package policy. Now, it's only used for definition and
hover, so it should always be the narrowest package handle. We can use
this fact to determine if the identifier is located in its declaring
package, and if that package is a test variant, we don't link to the
documentation on pkg.go.dev, since it doesn't exist.

Change-Id: I5686828858a3feafb8ff2e4c5964b562f66db9fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-03 21:56:10 +00:00
Rohan Challa
33212cd6a0 internal/lsp: aggregate diagnostics before publishing them
This change aggregates all the diagnostics before publishing them to the client. This also updates the lsp_test to use lsp.Diagnose vs source.FileDiagnostics, unless lsp.Diagnose doesn't return any results.

Change-Id: I39959b50af3f65fce5d8e15fb138ccc019ce8073
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217338
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-03 19:35:11 +00:00
Rohan Challa
a014e0aa6a internal/lsp: skip packages load for auxilary go.mod changes
This change will skip packages.load calls when a go.mod file changes if that go.mod file is not the go.mod file associated with the view.

Change-Id: I23a214a89203dd58417f3e2f69725ce3b669a5ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217238
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-03 17:58:37 +00:00
Rohan Challa
c4cbd3b08f internal/lsp: add diagnostics in .go files for missing deps in go.mod
This change adds diagnostics to imports in .go files that are missing the dependency in the go.mod file. The quick fix that adds the appropriate require statement is coming in a follow up CL.

Updates golang/go#31999

Change-Id: I314cdbe8e3dd27da744ca0391e7a6e5dc1ebaa77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216277
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-03 17:57:15 +00:00
Rebecca Stambler
2de6fe5e3e internal/lsp: return error if there is no builtin package
When we stopped returning an error from awaitInitialized, we didn't
handle this case in LookupBuiltin.

Fixes golang/go#36975.

Change-Id: I9668a7148f60ca1f9cad953d46caf6ec5500541e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217399
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-03 17:52:18 +00:00
Peter Weinberger
6682176fbb internal/lsp: remove useless check
err must be nil, as the code checks in line 117.

Change-Id: I23d87bdc9489c9ed249bfa1ac61b1ed722121c6a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217402
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-03 17:13:28 +00:00
Muir Manders
6f24f261da internal/lsp/cache: refactor a few small things
In preparation for some meaningful changes, rework a few things:

- rename "fix" to "fixAST"
- separate "parseExpr" into "parseStmt" and "parseExpr"
- pull out "walkASTWithParent" function

Change-Id: If6c8a249441feda95704f37bc9bde3ef2b64cbd2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216481
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-03 02:30:11 +00:00
Muir Manders
bc0b458b10 internal/lsp/source: improve completion for "make()" args
Fix type inference to expect a type name for the first "make()"
parameter and an integer for later parameters. For example:

   // Previously we expected "[]int{}", now we expect "[]int".
   var _ []int = make(<>)

Note that we don't currently support actually completing to unnamed
type names like "[]int", but this improvement at least eliminates
nonsensical completion suggestions.

   // Previously we had no expectation, now we expect an int.
   var _ []int = make([]int, <>)

Change-Id: Ifd349767662ab6902d3a3ea9e52de7df70cb37c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217310
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-02 23:20:50 +00:00
Rob Findley
ecb101ed65 internal/lsp/debug: support dropping sessions and views
These functions were unimplemented.

Change-Id: Ie95ee4e99511e1e00689eefe6d97df0780b72d2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217090
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-31 21:12:09 +00:00
Rob Findley
1baf5b43f6 internal/lsp/cache: add Env to debugView
Env is used in the debug template, but not available.

Change-Id: I8e270464aecd927bc553bfbf9e7c85a477833890
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217085
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-31 20:48:24 +00:00
Daisuke Suzuki
5dffee2a89 internal/lsp: change to helper.go to output a formatted file
Use format.Source before output.

Change-Id: I8504fc80defa77aeaab748b9dc65d6382da500a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217177
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-31 20:35:38 +00:00
Rohan Challa
d6d1b0d853 internal/lsp/cache: add context cancellation check inside importerFunc
This change adds a check inside of the types.ImporterFunc to see if the context has been cancelled.

Updates golang/go#34683

Change-Id: I0f12da0f8158ecda0eec00150ed6ff772c2f89c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217257
Run-TryBot: Rohan Challa <rohan@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-31 18:31:50 +00:00
Rob Findley
b4fe758a9b internal/lsp/source: don't allow mutating DefaultOptions
DefaultOptions was a value type, but held map values. This CL changes it
to a function that returns an Options value that has new instances of
all reference types. It would be better if this function returned a
pointer, but that change ended up being too large. I will need to
refactor handling of options later anyway, in order to support sessions
with differing options for golang.org/issues/34111.

This fixes a race in internal/lsp/tests: internal/hooks/analysis.go
mutates the Analyzers map.

See for example the trybots result at:
https://storage.googleapis.com/go-build-log/0d34f5f0/linux-amd64-race_4ecdf9c8.log

Change-Id: I41be450b590a3f3104ac9a1cb9cb312ea3ff7ff4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217077
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-01-31 15:48:08 +00:00
Daisuke Suzuki
097c1f2eed internal/lsp: fix typo
Change-Id: I2480725dc20f67bb0427c71dab83a0cbe72832a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217178
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
2020-01-31 14:37:46 +00:00
Heschi Kreinick
b4207ef493 internal/lsp: use span.URI constructor everywhere
We should never be converting to a URI directly, in case it's encoded or
needs Windows-specific fixes.

Change-Id: I8af272f878e2e4faf428bd7b74b7cba4f65093a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217086
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-31 00:08:51 +00:00
Rebecca Stambler
0725381040 internal/span: always uppercase the drive letter for Windows
Drive letters are always case-insensitive, so we should standardize them
by always keeping them uppercase.

Updates golang/go#36904

Change-Id: I8de25b175790b01627f947600c1511edf38c316c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217080
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-30 22:30:17 +00:00
Rebecca Stambler
4e65565728 internal/lsp: set workspace packages during (*snapshot).load
We can assume that any package we load directly will be a package in the
workspace, so it's reasonable to set workspace packages at that point.
We're guaranteed not to directly load dependencies, or we may end up
with indirect dependencies in the go.mod file.

Change-Id: I5d406e54da2bc3278b139c75b436d111b5564418
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216726
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-30 00:17:45 +00:00
Rohan Challa
c8253cffe2 internal/lsp/mod: fix test that checks for unchanged go.mod
The test that checks to make sure a user's go.mod file does not change if tempModfile=true is not doing anything. At some point, the go.mod file to test added a go directive. This change removes the go directive from the go.mod file.

Change-Id: I1c4b6b326e99d8fec5c21ac779dcdcd73610f48c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216840
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-29 23:28:05 +00:00
Rebecca Stambler
8a05c59e79 internal/lsp/cache: move shouldLoad function to snapshot.go
Some housekeeping. The shouldLoad function doesn't actually make sense
in load.go.

Change-Id: Idcfade57ab853cdae809c688fa21026a0a432c60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216727
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-29 22:51:48 +00:00
Heschi Kreinick
716555639d internal/lsp/cache: use telemetry logging for imports refresh
Any context that came in through the lsp protocol package will work, live or dead.

Change-Id: I7566ec07b09e1c8e54a5255ebda8553843cfe974
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216846
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-29 22:22:23 +00:00
Heschi Kreinick
ec2e11d257 internal/lsp/source: check cancellation before analysis
We check cancellation pretty early on in analysis, but not before we
read Options. Checking earlier avoids a race in the command tests.

Fixes golang/go#36699.

Change-Id: I557c0441d70c80e122655be00a03b47080ddb7f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216844
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-29 21:19:39 +00:00
Rebecca Stambler
11f6c2ac6d internal/lsp/cache: delete a few unused functions
Change-Id: I723ae6d2676bdbb4cfc0fec339e87604a032e807
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216725
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-29 21:03:00 +00:00
Rebecca Stambler
207d3de1fa all: fix some staticcheck errors
Updates golang/go#35718

Change-Id: I10bfd5421cd44bb58b8bcaa6e9205040c25f51be
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208257
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-29 04:53:41 +00:00
Heschi Kreinick
e1fd5825ff internal/lsp/cache: handle invalid analysis Pos
The nilness analysis gives us diagnostics with invalid start Pos. We can
just ignore those and log them.

Add a missing error check while I'm in here.

Change-Id: I4a205f253a9e47ec1513ff6299479f52e414a48c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216724
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-29 01:28:57 +00:00
Rebecca Stambler
ae42f3cd5c internal/lsp: recover from a view initialization failure
If an orphaned file is used to recover a workspace package, we should
remove the initialization error and treat the view as correctly
initialized.

Also, stop caching metadata for packages with no files. We have no way
to invalidate it, and it's useless, so just re-load those files as
needed.

Fixes golang/go#36795.
Fixes golang/go#36671.
Fixes golang/go#36772.

Change-Id: I0aee5a43401517b6073d27136cca533160effef2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-28 22:02:46 +00:00