...since we do our own parsing.
Change-Id: Id762cca408692b9535b8bb36017d6719180e5bb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/449498
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Currently compilebench relies on installed .a files for std and
cmd, as it runs the compiler and linker directly. For the upcoming
Go 1.20, compiled .a files will not be installed. Don't rely on
them. Instead, build importcfg file and pass it to the compiler
and the linker.
Verified that this approach still works with previous versions of
Go (1.19 and 1.18).
For golang/go#47257.
Change-Id: Ie0eb9541fb995649e5b68d4481a5acfbdfe8f2a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448118
Reviewed-by: Michael Pratt <mpratt@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Update golang.org/x dependencies to their latest tagged versions.
Once this CL is submitted, and post-submit testing succeeds on all
first-class ports across all supported Go versions, this repository
will be tagged with its next minor version.
Change-Id: Ie52140f20343bd6dd2b73662fce64c8065f5a80b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/449096
Auto-Submit: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Gopher Robot <gobot@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The only explanation I can think of for the failure in
https://go.dev/issue/49387#issuecomment-1303979877 is that maybe the
idle timeout started as soon as conn1 was closed, without waiting for
conn2 to be closed. That might be possible if the connection returned
by Dial was still in the server's accept queue, but never actually
accepted.
To eliminate that possibility, we can send an RPC on that connection
and wait for a response, as we already do with conn1. Since the conn1
RPC succeeded, we know that the connection is non-idle, conn2 should
be accepted, and the request on conn2 should succeed unconditionally.
Fixesgolang/go#49387 (hopefully for real this time).
Change-Id: Ie3e74f91d322223d82c000fdf1f3a0ed08afd20d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448096
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Add [Lapce](https://lapce.dev) Go plugin to `gopls` documentation
Change-Id: I58ec42d69708b519cfba3de1cdee269ffecdbbc4
GitHub-Last-Rev: 37762df491
GitHub-Pull-Request: golang/tools#413
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448235
Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fix the test failure demonstrated in the following failed builder:
https://build.golang.org/log/d0511c583201e8701e72066985ebf950d9f5511d
It should be OK to set multiple keys in the validated map. Support this
by keeping track of seen and deletion clock time. There are still
potential problems with this analysis (specifically, if a map is
constructed via SetAll), but we ignore those problems for now.
Change-Id: I5940d25f18afe31e13bc71f74d4eea7d737d593d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448696
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
I felt a bit confused on my first reading of the docs for using `go work`.
It wasn't clear to me if the `tools` argument in `go work use tools tools/gopls` was an alias or a directory name, so I thought this might make it very clear to understand for first time users.
Change-Id: I9c5a04a8928207b53acfb36ce7add8ca5f033d46
GitHub-Last-Rev: 49e125d83e
GitHub-Pull-Request: golang/tools#409
Reviewed-on: https://go-review.googlesource.com/c/tools/+/441415
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
I meant to do this in the first CL, but was prevented by
a bug which I have since reported and linked to from the code.
Change-Id: I651e728c535cdeb0885eae4d510fda3c24518dcf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448376
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change will make it possible to do semantic token regtests.
Change-Id: I9963c60f61af30f973a2ee4cd32aaa5545bdc4ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448296
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Coverage.go computes the test coverage from running all the gopls tests.
This CL accounts for the changed source tree (internal/lsp is gone)
and new actions returned by go test -json ('pause' and 'cont').
Change-Id: I970b3ec107746ce02e3dcdcad9f8c19cffad8d11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448295
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This test is flaky, likely due to some race in the
experimentalWorkspaceModule logic. Since we're about to delete support
for that experimental feature, simply skip the test to stop the flakes.
Leave the test as an artifact, as it will be deleted as part of the
clean up of experimentalWorkspaceModule. No need to delete it before
then.
Updates golang/go#55331Fixesgolang/go#55923
Change-Id: Ic17485e42e335459df462af00a2088812ecfb5f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448116
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Findley <rfindley@google.com>
When awaiting diagnostics, we should almost always wrap expectations in
a OnceMet(precondition, ...), so that tests do not hang indefinitely if
the diagnostic pass completes and the expectations are still not met.
Before this change, the user must be careful to pass in the correct
precondition (or combination of preconditions), else they may be
susceptible to races.
This change adds an AllOf combinator, and uses it to define a new
DoneDiagnosingChanges expectation that waits for all anticipated
diagnostic passes to complete. This should fix the race in
TestUnknownRevision.
We should apply a similar transformation throughout the regression test
suites. To make this easier, add a shorter AfterChange helper that
implements the common pattern.
Fixesgolang/go#55070
Change-Id: Ie0e3c4701fba7b1d10de6b43d776562d198ffac9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448115
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The old code based on packages.Visit traversed in a
deterministic order, and didn't stop when it found its
target (the 'return false' only prunes that subtree).
This CL replaces it with a precomputation of the
PkgPath-to-*Package mapping.
The performance difference is small for this test but
it nearly dominates on a larger input (e.g. k8s).
Example code shouldn't steer users into asymptotic traps.
Change-Id: I19f4fc2c25da3d2ae00090704df30a54d8516bf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447958
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
While checking for changes to go.mod files in the importsState, there is
no reason for special handling based on workspace mode: we can simply
hash all active modfiles. This moves us towards an improved API for the
workspace: it should simply be responsible for tracking active modfiles.
This also incidentally avoids the panic reported in golang/go#55837.
Fixesgolang/go#55837
Change-Id: I8cb345d1689be12382683186afe3f9addb19d467
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447956
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Update selected dependencies following the v0.10.0 release, excluding
sergi/go-diff and x/vuln.
Gofumpt@v0.4.0 requires go1.18, so link it selectively following the
pattern of staticcheck. While at it, clean up some things related to the
wiring of staticcheck and gofumpt support. Notably, in VS Code error
messages do not support formatting such as newlines or tabs.
Add a test for the conditional Gofumpt support.
For golang/go#56211
Change-Id: Id09fdcc30ad83c0ace11b0dea9a5556a6461d552
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446736
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds an internal API for marshalling and
unmarshalling a types.Package to "shallow" export data,
which does not index packages other than the main one.
The import function accepts a function that loads symbols
on demand (e.g. by recursively reading export data for
indirect dependencies).
The CL includes a test that the entire standard
library can be type-checked using shallow data.
Also:
- break dependency on go/ast.
- narrow the name and type of qualifiedObject.
- add (test) dependency on errgroup, and tidy go.mod.
Change-Id: I92d31efd343cf5dd6fca6d7b918a23749e2d1e83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447737
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
We plan to add experimental features to this package for
use by gopls, but the directory structure makes this
tricky using the "internal directory" mechanism.
Change-Id: Ib842c0b100b167f6978c6ff783ea0e5d0704b4a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447955
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Clean up some broken links and stale documentation in gopls/README.md,
and add new documentation for the gopls release policy.
Fixesgolang/go#55267
Change-Id: I9c7ed1f1d3949025f3c02edb69b475cf34f214eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446863
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Remove the internal guard preventing the loopclosure analyzer from
checking parallel subtests.
Also, improve the accuracy of the parallel subtest check:
- only consider statements after the final labeled statement in the
subtest body
- verify that the *testing.T value for which T.Parallel() is invoked
matches the argument to the subtest literal
Fixesgolang/go#55972
Change-Id: Ia2d9e08dfa88b5e31a9151872025272560d4b5e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447256
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Prior to this CL we already shut down a jsonrpc2_v2.Conn when its
Reader breaks, which we expect to be the common shutdown path.
However, with certain kinds of connections (notably those over
stdin+stdout), it is possible for the Writer side to fail while
the Reader remains working.
If the Writer has failed, we have no way to return the required
Response messages for incoming calls, nor to write new Request
messages of our own. Since we have no way to return a response,
we will now mark those incoming calls as canceled.
However, even if the Writer has failed we may still be able to read
the responses for any outgoing calls that are already in flight. When
our in-flight calls complete, we could in theory even continue to
process Notification messages from the Reader; however, those are
unlikely to be useful with half the connection broken. It seems more
helpful — and less surprising — to go ahead and shut down the
connection completely when it becomes idle.
This is a redo of CL 446315, with additional fixes for bugs exposed on
the -race builders and some extra code cleanup from the process of
diagnosing those bugs.
Updates golang/go#46520.
Updates golang/go#49387.
Change-Id: I746409a7aa2c22d5651448ed0135b5ac21a9808e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447035
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
This CL includes some clarifications while trying to
understand the performance of the initial workspace load
and analysis. No significant behavior changes.
Server.diagnose:
- Factor the four copies of the logic for dealing
with diagnostics and errors.
- Make the ActivePackages blocking step explicit.
Previously mod.Diagnostics would do this implicitly,
making it look more expensive than it is.
Server.addFolders:
- eliminate TODO. The logic is not in fact fishy.
- use informative names and comments for WaitGroups.
- use a channel in place of a non-counting WaitGroup.
Also, give pkg a String method.
Change-Id: Ia3eff4e784fc04796b636a4635abdfe8ca4e7b5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445897
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Rename (*connection).AddFile to openFile, which is more accurate, and
fix a deadlock resulting from holding a Client lock while issuing a
Server request.
Fixesgolang/go#56450
Change-Id: Ie6f34613e1e10e3274c3e6728b12f77e3a523b89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446856
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Fix a panic during completion on variables of type *error. As a
predeclared type, the error type has nil package. Fix the crash
resulting from this oversight, as well as a related crash in the tests
analyzer, from which the new completion code was adapted.
Fixesgolang/go#56505
Change-Id: I0707924d0666b238821fd14b6fc34639cc7a9c53
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446815
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Simplify the api-diff implementation to use `go run` and cmp.Diff. The
latter is more maintainable and produces more readable output, due to
supporting line diffs for multi-line strings.
For golang/go#54459
Change-Id: I11c00e9728ce241aef8f9828f3840b4202294a9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444799
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Prior to this CL we already shut down a jsonrpc2_v2.Conn when its
Reader breaks, which we expect to be the common shutdown path.
However, with certain kinds of connections (notably those over
stdin+stdout), it is possible for the Writer side to fail while
the Reader remains working.
If the Writer has failed, we have no way to return the required
Response messages for incoming calls, nor to write new Request
messages of our own. Since we have no way to return a response,
we will now mark those incoming calls as canceled.
However, even if the Writer has failed we may still be able to read
the responses for any outgoing calls that are already in flight. When
our in-flight calls complete, we could in theory even continue to
process Notification messages from the Reader; however, those are
unlikely to be useful with half the connection broken. It seems more
helpful — and less surprising — to go ahead and shut down the
connection completely when it becomes idle.
Updates golang/go#46520.
Updates golang/go#49387.
Change-Id: I713f172ca7031f4211da321560fe7eae57960a48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446315
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Memory profiles show heavy allocation for the stack
and the function closure of FindDeclAndField.
This change moves both outside the loop, reducing
this function's fraction of allocation from 6.7%
before to 5.0% after, and reducing running time
by 3-7% on these benchmarks.
before
BenchmarkStructCompletion/completion-8 100 10432280 ns/op
BenchmarkImportCompletion/completion-8 1350 921785 ns/op
BenchmarkSliceCompletion/completion-8 100 10876852 ns/op
BenchmarkFuncDeepCompletion/completion-8 142 7136768 ns/op
BenchmarkCompletionFollowingEdit/completion-8 63 21267031 ns/op
After
BenchmarkStructCompletion/completion-8 100 10030458 ns/op
BenchmarkImportCompletion/completion-8 1311 918306 ns/op
BenchmarkSliceCompletion/completion-8 100 10179937 ns/op
BenchmarkFuncDeepCompletion/completion-8 150 6986303 ns/op
BenchmarkCompletionFollowingEdit/completion-8 63 20575987 ns/op
Change-Id: Ia459e41ecf20851ff4544f76ad7b415a24606cd1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446185
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Serve had a misleading name and signature: it did not actually block
on serving the connection, and never returned a non-nil error.
Updates golang/go#56281.
Change-Id: Ia6df0ba20066811b0551df3b3267dff2fffd7881
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443678
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Also make (*Connection).Close safe to call from Bind.
A jsonrpc2_v2.Server has no good way to report an error from Bind.
If the Server saves the error to return from its own Wait method, that
might not ever actually happen: Wait waits for in-flight connections
to complete, but if some existing connection stays up then Wait will
not return.
If the Server goes ahead with establishing the connection and installs
its own Handler, that Handler needs to decide whether to serve the
error from Bind or something more opaque, and at that point Bind may
as well return a handler that makes that choice more precisely.
If the Server merely logs the error and closes the Connection, then
the Bind method itself may as well do that directly too.
It seems to me that the only winning move is not to play. Only Bind is
in a position to decide how to handle its errors appropriately, so it
should not return them to the Server.
Updates golang/go#56281.
Change-Id: I07dc43ddf31253ce23da21a92d2b6c0f8d4b3afe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443677
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Also implement and use the Shutdown method, which was mentioned in a
doc comment in CL 292169 but not actually present at that time.
With proper synchronization, we don't need heuristics to determine
whether an error is due to a connection or listener being closed.
We know whether we have called Close (and why), and we can assume
that if we have called Close then that is probably the reason for
any returned error.
Fixesgolang/go#56281.
Change-Id: I5e0ed7db0f736ca8df8cd8cf556b674e7a863a69
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443675
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Prior to this CL, (*Server).run only filters out inactive connections
when it accepts a new connection. If existing connections complete,
their associated resources can't be garbage-collected until either the
next connection is accepted or the Listener is closed.
This change moves the open-connection accounting to an explicit hook
passed to newConnection, eliminating the need to call Wait entirely.
For golang/go#46047
Change-Id: I3732cb463fcea0c142f17f2b1510fdfd2dbc81da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/388774
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change fixes the semantics of Close to actually wait for
in-flight requests before closing the ReadWriteCloser. (Previously,
the Close method closed the ReadWriteCloser immediately, which I
suspect is what led to many of the failures observed in
golang/go#49387 and golang/go#46520.)
It achieves this by explicitly tracking the number of in-flight
requests, including requests with pending async responses, and
explicitly rejecting new Call requests (while keeping the read loop
open!) once Close has begun.
To make it easier for me to reason about the request lifetimes, I
reduced the number of long-lived goroutines from three to just one
(the Read loop), with an additional Handler goroutine that runs only
while the Handler queue is non-empty. Now, it is clearer (I hope!)
that the number of in-flight async requests strictly decreases
after Close has begun, even though the Read goroutine continues
to read requests (and, importantly, responses) and to forward
Notifications to the preempter.
For golang/go#49387
For golang/go#46520
Change-Id: Idf5960f848108a7ced78c5382099c8692e9b181e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/388134
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
In a type such as
type X interface { m() []*interface { X } }
the traversal never encounters the named type X in the result of m
since Interface.Methods expands it to a set of methods, one that
includes m, causing the traversal to get stuck.
This change uses an alternative, shallow hash function on the types
of interface methods to avoid the possibility of getting stuck in
such a cycle.
(An earlier draft used a stack of interface types to detect cycles,
but the logic of caching made this approach quite tricky.)
Fixesgolang/go#56048Fixesgolang/go#26863
Change-Id: I28a604e6affae5dfdd05a62e405d49a3efc8d709
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439117
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
When the default filter glob changed to **/node_modules,
the performance of BenchmarkWorkspaceSymbols was observed
to degrade by an astonishing 20%.
(CPU profiles of the benchmark reported that the Disallow
functions percentage had increased only slightly, but these
measures are misleading since the benchmark has a very
CPU-intensive set-up step, so all the percentages are
quotients of this figure, masking their relative importance
to the small region during which the benchmark timer is
running.)
This change removes the unnecessary ^.* prefix from the
generated regular expression. Really the regexp package
ought to do this.
Also, minor cleanups and tweaks to the surrounding code.
Change-Id: I806aad810ce2e7bbfb2c9b04009d8db752a3b10d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446177
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 445581 inadvertently removed suppression of the Go version error if
the Go version was undetected. Add it back, with a test.
Fixesgolang/go#56465
Change-Id: I352369096280c8d3423a7345123ec9309359fb58
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446175
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
VSCode Go Nightly uses `allExperiments` setting which triggers
calling this option. It doesn't make sense to add the settings
that are scheduled to be deleted.
Change-Id: I443d7b1722feafee04b6c63a06ff514a396c5d50
Reviewed-on: https://go-review.googlesource.com/c/tools/+/446095
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Improve the Go version deprecation message to be a warning at 1.13-15,
and provide better instructions for making it go away. Clarify support
in the gopls README.
Change-Id: I6b08e0bd698f5c085eee7a851a130c53affb8ab5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445581
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Previously, the disaster logic in the new diff implementation
would "encrypt" the before/after files using a monoalphabetic
substitution, which has been insecure since the 9th century.
Instead, save plain text, in file with mode 0600, and invite
the user to audit the file before sharing it with us.
Also, separate the two files using a NUL byte, not a newline,
which is highly ambiguous.
Also, in the JSON diff stats writer:
- print a warning if we can't create the log file.
(The previous code was subtle--it stored a nil *os.File in
an io.Writer, which caused Writes to fail with an error,
in effect, silently.)
- Don't hold the mutex around the write operation.
- Fix minor off-by-one error (re: 15)
- Crash if JSON encoding fails; it "can't happen".
Change-Id: I9b6a4145451afd77594f0ef9868143634a9c4561
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445580
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This cl updates go/internal/gcimporter the to anticiapte missing
targets and .a files the same way CL 442303 updates the two other
versions of gcimporter to do the same. It also adds a couple of
helpers to create importcfg files for the compiler and list the
locations of cached stdlib .a files in internal/goroot and
internal/testenv, the analogues of their import paths in the go
distribution.
Change-Id: Ie207882c13df0e886a51d31e7957a1e508331f10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445455
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This test ensures that all std and x/tools packages can
be re-typechecked using export data.
Change-Id: I189a8138d74cb38f69dfb51c613849e43b316322
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445096
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The previous version of bundle simply joins together with a space " ".
While this works for simple arguments this causes problems when you
need to pass special strings like an empty string or a string
containing a space.
The following example shows how the previous version handles an empty
string argument `-prefix ""`.
//go:generate bundle -o /dev/stdout -prefix example.com/mod
This change quotes the arguments with strconv.Quote, if needed, before
joining together with a space:
//go:generate bundle -o /dev/stdout -prefix "" example.com/mod
Change-Id: Ic706a3bd7916515ba91dbe5e0def956703ab2988
GitHub-Last-Rev: 8dc0c88fc1
GitHub-Pull-Request: golang/tools#411
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444956
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
In func f(x int) int {return x;} the second x used to be marked as a
variable, but now is marked as a parameter, visually tying it to its
definition.
Fixesgolang/go#56257
Change-Id: I8aa506b1ddff5ed9a3d2716d48c64521bdea0fd5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445095
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This is the first in a series of CLs implementing the new stub
generator. The code is intended to reproduce exactly the current
state of the generated code.
This CL has the final file layout, but primarily consists
of the parsing of the specification.
The LSP maintainers now provide a .json file describing the messages and
types used in the protocol. The new code in this CL, written in Go,
parses this file and generates Go definitions.
The tests need to be run by hand because the metaModel.json file is not
available to the presubmit tests.
Related golang/go#52969
Change-Id: Id2fc58c973a92c39ba98c936f2af03b1c40ada44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443055
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
When a module contains multiple vulnerabilities, previously gopls published
one diagnostic message for each vulnerability. This change modifies this
behavior to publish only one vuln diagnostic for each module. This will
make the PROBLEMS panel more concise and readable. However, the information
about each vulnerability finding is useful, so we supplement this diagnostics
by sending a hover message in the module require line. An added benefit
of this approach is that, unlike the Diagnostics, VS Code supports rich
text rendering for Hover messages. So we can use markdown to add links and
necessary highlighting.
Before this change, go.mod require hover messages (e.g. go mod why result)
were associated only with the module path part, excluding the version string
part. But for vulnerability information hover message, I think it is better
to be applied to the entire module require line (both module path & version)
because they are information about the specific module/version. Currently
LSP hover returns only one hover, so we cannot use this different range only
to the vulnerability information hover. Thus, one side effect of this change
is that the module info hover message will be also shown to the version
part of each require statement.
Change-Id: Iccacd19fdebadc4768abcad8a218bbae14f9d7e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444798
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change moves the facts package so that it can be reused by gopls.
It remains a tools-internal API.
A forthcoming reimplementation of gopls's analysis driver will
make use of the new packages and the features mentioned below.
Also:
- change parameter of read() callback from 'path string' to *Package.
- use NewDecoder().Decode() to amortize import-map computation across calls.
Change-Id: Id10cd02c0c241353524d568d5299d81457f571f8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444796
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
The iexport function was emitting the wrong implicit
qualifying package name for unexported struct fields.
The logic was deceptively similar to that in the compiler,
and that was the bug: the compiler records which package
a struct{...} syntax node appears in, and uses that (correctly)
as the qualifier for unexported fields; but go/types only
records a package in each Object, such as the field Vars,
which led the author of the previous code to use the
current package, not the struct's package, as the implicit
qualifier. The solution is to use the package of any
field; they should all be the same.
(Unlike interfaces, where embedding is flattened out,
leading to interface types in which method Func objects
may belong to different packages, struct embedding is
not flattened out, so all field Vars belong to the
same package in which the struct was declared.)
Also, a regression test.
Thanks to Rob Findley for identifying the cause.
I don't understand how this hasn't shown up sooner,
since the test case is far from obscure.
Change-Id: I0a6c58a566b87a148827fb0ab4655a020806c31a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445097
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Robert Findley
Alan Donovan
Cherry Mui
Gopher Robot
Bryan C. Mills
Arsen Musayelyan
Angus Dippenaar
pjw
Hana (Hyang-Ah) Kim
Michael Matloob
eNV25