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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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.
Fixesgolang/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>
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.
Fixesgolang/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>
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>
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>
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().
Fixesgolang/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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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 <>
Fixesgolang/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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
This change adds a buildFlags variable to the processEnv struct rather than appending them to the GOFLAGS by using a space separator.
Fixesgolang/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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
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>
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>
When we stopped returning an error from awaitInitialized, we didn't
handle this case in LookupBuiltin.
Fixesgolang/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>
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>
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>
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>
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>
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>
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>
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>
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>
