The definition command-line interface doesn't match the rest of the
commands, because I think we originally wanted to make them all
subcommands of "gopls query". Remove this, since it's no longer in use.
Change-Id: Iee923db6328774f787d539931001e438d1a2ae6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
It was not very useful, it basically renamed io.EOF under some limited
circumstances, and was only there for the befit of tests.
Instead, the test now checks io.EOF directly, but also checks for
io.ErrClosedPipe which was also happening.
We also make sure we wrap errors rather than replacing them.
This prevents some weird random test failures due to races in the way they were
closed.
Change-Id: I236b03ac5ba16bf763299b95d882cf58b1f74776
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230303
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Instead of tagging events with their type, instead we infer the type from
the label pattern.
The standard event creators all have a matching test that returns true
if the the labels pattern matches the ones that would be built by the
creator.
Spans and logs already have a unique label pattern, other event types
required a special label marker.
This makes the system much more extensible, and also cleans up some
the API.
Change-Id: I1fbc9ec07aa84ead6c12bbd5ca65b13b605bfa4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229242
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
When jsonrpc2.Serve times out or is cancelled, we leak the goroutine
that is accepting connections, because it is stuck trying to write its
error back to the doneListening channel.
Fix this by adding a context cancellation for the serve func, and
selecting on this context when writing the error.
Change-Id: I3383535f58b44616983816e8b257a975e3c337a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229978
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Formatting keys in labels can panic when a label is constructed with a nil
error. Avoid that by not passing the defective label to event.Log.
Change-Id: I3099a7ac48c5830af1072141f2b619d0e0fbcf5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229985
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In cases like:
var v interface{}
fmt.Println(<>)
Completing to "v" would insert "v..." instead of "v". This was due to
a mixup where we were checking if the variadic type "[]interface{}"
was assignable to the candidate type "interface{}" instead of the
other way around.
Fixesgolang/go#38652.
Change-Id: I27c0b50bbf4b895924c8ed2c0c9dd6785e98cbe1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229921
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In order to experiment with adding more progress reporting to gopls, add
a new experimental configuration for verbose work done reporting.
Also, pass configuration in InitializationOptions when initializing the
editor.
Change-Id: Id2336790719d9c0b6d663997e1aef672aec43d28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229458
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change improves our approach to handling type aliases. Previously,
we were not fully qualifying the names in the AST, making the code
inserted in completions incorrect at times. Now, we clone the relevant
AST expr and qualify it. We also add handling for the return values of a
function, instead of just the parameters.
Fixesgolang/go#38230Fixesgolang/go#37283
Change-Id: Ib79f4636891c9b610ae848e9fa4dbae7c63db509
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229319
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In preparation for later changes, add support for tracking outstanding
work in the lsp regtests. This simply threads through progress
notifications and tracks their state in regtest.Env.state. A new
Expectation is added to assert that there is no outstanding work, but
this is as-yet unused.
A unit test is added for Env to check that we're handling work progress
reports correctly after Marshaling/Unmarshaling, since we're not yet
exercising this code path in actual regtests.
Change-Id: I104caf25cfd49340f13d086314f5aef2b8f3bd3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229320
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This was the last piece of Event that was public, and it was only public to
allow mutation in tests.
Adding CloneEvent allows tests to create an updated copy rather than
update the event in place.
Change-Id: I2215d1eb0317063948ef0fac955fa768a209564d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229241
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Now key types can be implemented outside the package that holds labels or events, they should be.
This prevents the large list of types from poluting the public interface of the core packages.
Change-Id: I927f31cb5e4e1d0c29619681015962f890623e5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229240
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Also moves core.Key to label.Key, but leaves the implementations
behind for now.
After using for a while, the word Tag conveys slightly the wrong
concept, tagging implies the entire set of information, label maps
better to a single named piece of information.
A label is just a named key/value pair, it is not really tied to the
event package, separating it makes it much easier to understand the
public symbols of the event and core packages, and allows us to also
move the key implementations somewhere else, which otherwise dominate
the API.
Change-Id: I46275d531cec91e28af6ab1e74a2713505d52533
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229239
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
event.Log removed
event.Print -> event.Log
event.Record -> event.Metric
event.StartSpan -> event.Start
In order to support this core now exposes the MakeEvent and Export functions.
Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238
Reviewed-by: Robert Findley <rfindley@google.com>
internal/telemetry/event was renamed to internal/event/core
Some things were partly moved from internal/telemetry/event straight to
internal/event to minimize churn in the following restructuring.
Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The code for formatting function signatures is fairly confusing.
Factoring out an unexported signature type simplifies things a bit.
Hopefully we'll be able to pull out more formatting logic from the other
features. Ideally, I'd like to return to the separation between
internal/lsp/source and internal/lsp so that a formatting package can be
pulled out and used in internal/lsp.
Change-Id: I7428db5004eab371e46402188e0dc6bb30f0c425
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229318
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Fix a couple typos in the RunRaw docstring, as well as some superficial
staticcheck errors.
Change-Id: I557d7ee63366b2b3a82d590d69ee2907d4e3ac59
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229417
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change also required the addition of a new run configuration -
WithEnv, which adds extra environment variables to the configuration.
Please let me know if this is the wrong approach.
Fixesgolang/go#37984
Change-Id: Ied2a53a443dc74c7ed723b91765eeddc1a7c1c00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229128
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
A few more updates to the regtest framework - add support for all
codeActions, not just organize imports. Also, return diagnostics from
env.Await to pass into code actions. Not sure if that's the correct way
to do it, so please let me know if there's a better way.
The test is for 1.14 only, since -modfile is only supported after 1.14.
Fixesgolang/go#38211
Change-Id: I5cdd35e2ec06e2b2487d1f3491197030008be9c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226958
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Not sure when this got fixed, but confirmed manually and via a
regression test.
Fixesgolang/go#36951
Change-Id: I7833a5d1119c34fd3fb45ea658e954eadbee750c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229129
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ID's are now by value not pointer, which caused it to not use the Format
method, resulting in broken id strings
The id maps need to be crossover (set and get go to different maps for a given direction of message)
Change-Id: Ide2b63ec1b009ae3587ee10e8bce018732ea342c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229244
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
In a separate CL, I was getting a nil pointer exception looking up a tag
value in a tag map chain where one map was nil. To avoid this, check
that maps are non-nil when merging them into a chain.
Change-Id: Ie2ec4865e1ed62f91b9b146d2f2b56f48c156043
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229130
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The special cases were all removed by the previous changes to jsonrpc2 messages.
The only difference now is the "Recieved" vs "Sending" text prefix, so we just paramaterize the common function with that instead.
Change-Id: If6f8de39dca38041cf68a6b5506ff0482af3cb8e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228890
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
messages are the atomic unit of communication, changing streams
to read and write whole messages makes the code clearer.
It also avoids the confusion about what should be an atomic
operation or when a stream should flush.
Change-Id: I4b731c9518ad7c2be92fc92211c33f32d809f38b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228722
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Currently, where a package has a test variant, we end up walking the
non-_test.go files twice: once when walking the package itself, the
second time when walking the test variant. In the gopls tests, this
means we end up double-counting the number of symbols (via the @symbol
annotation) in a package.
Extend the existing expect_test.go to demonstrate the problem is fixed
(a test which fails when the fix is not in place)
Change-Id: Ifd68b3d86e24f19d3f8efc3af71494b7d28b0acb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228761
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In preparation for a later change where we alter the implementation of
the workspace Symbol method, we now specify the Name that should be used
when constructing a SymbolInformation value from a @symbol annocation.
There is no change in the test expectations.
Change-Id: I4ee5f714d6060aab2ee33ef18339504f443cecdc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228757
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This saves it from having to know the wire format and understand the decoding
tricks.
Change-Id: I1f3ef3345ffee736a9d104f8ebfb436404d737c0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228721
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The previous implementation was exposing the details of the wire format
and resulted in non idomatic go, detecting the presence of absence of
values in fields to deterimine the message type.
Now the messages are distinct types and we use type switches instead.
Request still exists as an interface to expose the shared behaviour of
Call and Notification, as this is the type accepted by handlers.
The set of messages is deliberately closed by using a private methods on the
interfaces.
Change-Id: I2cf15ee3923ef4688670c62896f81f760c77fe04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228719
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This runs the tests as sub tests, and makes sure they clean up
correctly.
It also installs the telemetry debug handlers.
Change-Id: I75b4f7a19be378603d97875fc31091be316949e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228718
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For now this just logs to the testing.TB, it should be possible to also use it
to log metrics and timings in the future.
Also adds event.Debugf as a temporary debugging print to the telemetry
system, which is very helpful when debugging tests.
Change-Id: Ib2e919998919491e227885e2ee6eeea9e2fdc996
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228717
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
I had intended to enable these after gopls/v0.4.0 was released so that
people who test at master can try these out. Also, mark "fillreturns" as
high confidence so users can get it applied on save, much like
goreturns.
Change-Id: I71aa0b723b2e9b69474ceed7cb1d7da7c929d65d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228724
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
I believe that other changes in gopls/v0.4.0 also fixed the issue that
this regression test was written for. Re-enable it, and change it to
assert that diagnostics are cleared.
Fixesgolang/go#37195
Change-Id: I4538186ad288d9c6f70cc450f948b62f3868941f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228723
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Due to the asynchronous and non-transactional nature of the LSP, writing
regtests requires waiting for certain conditions to be met in the client
editor. To this point, the only type of condition for which we supported
waiting was the presence of diagnostic messages, but we can in principle
wait for anything that is triggered by a server notification.
This change generalizes the notion of expectations to also encompass log
messages. Doing this required expanding the value returned from checking
expectations to include a new "Unmeetable" verdict, to account for cases
where we know that a condition will never be met (for example if it is a
negative assertion). This may be useful for diagnostics as well.
A test is added to demonstrate these new expectations, where the initial
workspace load fails due to unfetchable dependencies.
Additionally, some helper flags are added to help debug regtests without
a code change, by allowing changing the test timeout, printing logs, and
printing goroutine profiles.
Updates golang/go#36879
Change-Id: I4cc194e10a4f181ad36a1a7abbb08ff41954b642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228399
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Certain regtests require referencing external data. To support this, add
the ability to use a file-based proxy populated with testdata.
To expose this configuration, augment the regtest runner with variadic
options. Also use this to replace the Runner.RunInMode function.
Add a simple regtest that uses this functionality.
Updates golang/go#36879
Change-Id: I7e6314430abcd127dbb7bca12574ef9935bf1f83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228235
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Closing the workspace has frequently been failing on Windows, due to
file locks held by the go command.
This change makes several tests more careful to check errors when
closing resources, and defers closing the regtest workspaces until the
entire test suite completes, at which point it is much more likely that
closing the workspace will succeed.
If this change results in test flakes on Windows, we should temporarily
demote errors in regtest.Runner.Close to a t.Log.
Updates golang/go#38490
Change-Id: Ibd2f7dd0e0e2faecfa0ca8c60237fc72e64f6719
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228231
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
It is not currently used at all, and is probably not the right approach.
Now we can easily add type safe keys we can add ones for types that know
their unit rather than storing it separately.
Change-Id: If393ad1df76033cff571c46f98cde8decb722b9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228265
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This means that format functions don't need an exhaustive list of the
key types to work correctly.
Change-Id: Iee17b225a0ecf82eb3f50d6341baf677655efc0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228233
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
These allow tags to be constructed and used from outside
the event package.
This makes it easy for users of these APIs to write their own
implementations of Key.
Change-Id: Ic3320a80f297bbe1d4cd6d9beafbe13ebbace398
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228232
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
The fix is just a missing return.
Also clean up a staticcheck thing, regenerate the golden files.
Fixesgolang/go#38417
Change-Id: I290b63df9d97211c59d6399fda7a273bc700fbdb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228297
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
It does not need to expose wire level fields, especially not by embedding.
It also no longer needs to know what Conn it came from.
Change-Id: I1aede2baaa2daa40da4e7a1278b2eaada25b2310
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227919
Reviewed-by: Jonathan Amsterdam <jba@google.com>
The same behavior can now be achieved by wrapping the Replier as we traverse the
handler stack instead.
Request is no longer mutated during handler invocation.
Change-Id: I2213de500f39e048f3f80161e234f3ae30464d70
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227918
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
While trying to be pragmatic, I simply locked the fake.Editor while
calling textDocument/formatting. This did indeed manifest later on as a
deadlock, so instead we release the lock but fail if the buffer version
changes while awaiting the formatting call.
Change-Id: I0ca24a502f3118e76de306a016449bbe665e70e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228230
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Configuration was not being set correctly, because the configuration
response was not honoring the configuration section order.
Change-Id: I8418535b45e6a24fd8f0605d58cb370e22664f17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228257
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In https://golang.org/cl/225157, I removed the redundant T and Context
parameters from the regtest func signature in an effort to reduce
confusion. In hindsight, removing the testing.T did not reduce
confusion, because there is a T in the test signature anyway (and now
it's the wrong T!).
Change-Id: I2e84009e9cb33cd51055012cfef9fe4987e1b9bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228229
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
