1
0
mirror of https://github.com/golang/go synced 2024-11-06 07:36:13 -07:00
Commit Graph

4719 Commits

Author SHA1 Message Date
Ian Cottrell
a466788a31 internal/event: change event.At to be a private field
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>
2020-04-23 18:13:49 +00:00
Ian Cottrell
a82abb5396 internal/event: extract keys to their own package
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>
2020-04-23 18:13:43 +00:00
Ian Cottrell
c81623a0cb internal/event: move event/core.Tag to event/label.Label
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>
2020-04-23 18:13:33 +00:00
Ian Cottrell
7b212d60a1 internal/event: renaming the main event API functions
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>
2020-04-23 17:21:36 +00:00
Ian Cottrell
cf0cb92717 internal/telemetry: renaming to internal/event
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>
2020-04-23 17:20:48 +00:00
Rebecca Stambler
72e4a01eba internal/lsp: refactor code for formatting signatures
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>
2020-04-22 20:52:58 +00:00
Rob Findley
3d37a67796 internal/gocommand: clean up docstring typos and staticcheck errors
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>
2020-04-22 17:07:37 +00:00
Rebecca Stambler
3d57cf2e72 internal/lsp: add regtest for golang/go#37984
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.

Fixes golang/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>
2020-04-22 02:23:33 +00:00
Rebecca Stambler
4e4aced336 internal/lsp/regtest: add regtest for golang/go#38207
Fixes golang/go#38207

Change-Id: I78396814a2e6a3ccecc860953ac6892ffa1b68e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226960
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-22 01:51:11 +00:00
Rebecca Stambler
504fe87a53 internal/lsp: add regtest for golang/go#38211
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.

Fixes golang/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>
2020-04-22 01:32:14 +00:00
Rebecca Stambler
66008de356 internal/lsp/regtest: add regtest for golang/go#36951
Not sure when this got fixed, but confirmed manually and via a
regression test.

Fixes golang/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>
2020-04-21 18:57:00 +00:00
Ian Cottrell
7504fd2219 internal/lsp: fix broken lsp logs
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>
2020-04-21 17:00:05 +00:00
Rob Findley
26dd2a56eb internal/telemetry/export: guard against NPE when merging tag maps
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>
2020-04-21 14:47:19 +00:00
Ian Cottrell
4b6a508a9c internal/lsp: merge the in and out protocol logging functions
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>
2020-04-21 13:43:21 +00:00
Ian Cottrell
2dc4334630 internal/jsonrpc2: rewrite streams in terms of messages
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>
2020-04-21 13:42:58 +00:00
Paul Jolly
cfa8b22178 go/packages/packagestest: do not walk packages and their test variants
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>
2020-04-21 04:27:24 +00:00
Paul Jolly
b4d5569d26 internal/lsp/tests: provide SymbolInformation.Name in @symbol annotations
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>
2020-04-21 04:22:56 +00:00
Ian Cottrell
9f075f7bfe internal/lsp: switch the protocol logger to use the new jsonrpc2 message types
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>
2020-04-20 21:06:19 +00:00
Ian Cottrell
ca8a290ca5 gopls: convert replay to use the new message types
this save it from having to understand the json wire format.

Change-Id: I4261cb53a4d5fe2263d7d73ebf9e96e6be2e54a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228720
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
2020-04-20 21:06:04 +00:00
Ian Cottrell
434f7a8fef internal/jsonrpc2: add concrete types for Call, Notify and Response
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>
2020-04-20 21:05:32 +00:00
Ian Cottrell
98173f2f69 internal/jsonrpc2: clean up the tests
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>
2020-04-20 21:05:12 +00:00
Ian Cottrell
f0ebba1556 internal/telemetry: add support for using telemetry in tests
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>
2020-04-20 21:04:52 +00:00
smasher164
978e26b7c3 internal/imports: update stdlib index for 1.14
$ go run mkstdlib.go

Fixes golang/go#38464.

Change-Id: I97cbd0abf6b9d874739d236429aba8ef271c48ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228884
Run-TryBot: Akhil Indurti <aindurti@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-20 00:18:25 +00:00
Rebecca Stambler
c07e33ef32 internal/lsp/source: enable type error analyzers
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>
2020-04-17 14:00:56 +00:00
Rebecca Stambler
e33929705b internal/lsp/regtest: enable test for golang/go#37195
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.

Fixes golang/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>
2020-04-17 14:00:42 +00:00
Hana (Hyang-Ah) Kim
fc959738d6 internal/lsp: fill in ServerInfo in the initialize response
Fixes golang/go#38459

Change-Id: I98476ddc35c4ab2c43145083149454875cb4b041
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228398
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-16 21:44:02 +00:00
Rob Findley
f038785680 internal/lsp/regtest: generalize expectations beyond diagnostic messages
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>
2020-04-16 21:39:01 +00:00
Rob Findley
92fa1ff4b1 internal/lsp/regtest: add support for custom test proxy data
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>
2020-04-16 19:38:27 +00:00
Rob Findley
405595e0b5 internal/lsp/fake: be more careful when closing the workspace
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>
2020-04-16 19:25:41 +00:00
pjw
ea51688515 go/packages: fix misbehavior when an overlay changes the package name
The new code checks for inconsistencies between the package name on disk
and the package name in overlays (as might happen when editing).

This Cl is meant to fix https://github.com/golang/go/issues/38328

Change-Id: If56b542775c216b4e0ed8ce91d6721c672bff792
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228236
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-04-16 18:54:13 +00:00
Rebecca Stambler
5744cfde56 internal/lsp: fix imports issue with duplicate package decl
We shouldn't apply formatting fixes when we are
organizing imports.

Fixes golang/go#38412

Change-Id: I082f4d9a7d1d1dd571304733c287b0d5e90ea448
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228264
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-16 06:17:24 +00:00
Ian Cottrell
9c19d0db36 internal/telemetry: delete the unit package
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>
2020-04-16 04:05:50 +00:00
Ian Cottrell
18395615f2 internal/telemetry: pack strings efficiently into tags
TagOfString() deconstructs a string into length and data pointer using the
unsafe package, and then stores the length into Tag.packed and the pointer into
Tag.untyped, which it can do without allocations.
Tag.UnpackString can reconstruct a string on access also using the unsafe
package to rebuild the string header.

This makes tag significantly smaller, which in turn makes things a faster

name                old time/op    new time/op    delta
/Baseline-8            160ns ± 6%     158ns ± 8%     ~
/StdLog-8             7.50µs ± 8%    7.47µs ±20%     ~
/LogNoExporter-8      1.13µs ± 7%    1.04µs ± 2%   -8.08%
/TraceNoExporter-8    3.61µs ±15%    2.96µs ± 5%  -18.08%
/StatsNoExporter-8    1.65µs ± 7%    1.39µs ± 7%  -16.14%
/LogNoop-8            4.43µs ±14%    4.05µs ± 7%     ~
/TraceNoop-8          10.9µs ± 5%    10.1µs ± 8%     ~
/StatsNoop-8          8.08µs ± 3%    7.42µs ±13%     ~
/Log-8                16.2µs ±14%    13.4µs ± 3%  -17.10%
/Trace-8              61.7µs ±22%    53.6µs ± 7%     ~
/Stats-8              11.3µs ±10%     9.5µs ± 4%  -15.56%

name                old alloc/op   new alloc/op   delta
/Baseline-8            0.00B          0.00B          ~
/StdLog-8               552B ± 0%      552B ± 0%     ~
/LogNoExporter-8       0.00B          0.00B          ~
/TraceNoExporter-8    3.58kB ± 0%    2.82kB ± 0%  -21.43%
/StatsNoExporter-8     0.00B          0.00B          ~
/LogNoop-8            3.58kB ± 0%    2.82kB ± 0%  -21.43%
/TraceNoop-8          11.5kB ± 0%     9.2kB ± 0%  -20.00%
/StatsNoop-8          7.17kB ± 0%    5.63kB ± 0%  -21.43%
/Log-8                3.58kB ± 0%    2.82kB ± 0%  -21.43%
/Trace-8              27.9kB ± 0%    23.6kB ± 0%  -15.60%
/Stats-8              7.17kB ± 0%    5.63kB ± 0%  -21.43%

name                old allocs/op  new allocs/op  delta
/Baseline-8             0.00           0.00          ~
/StdLog-8               30.0 ± 0%      30.0 ± 0%     ~
/LogNoExporter-8        0.00           0.00          ~
/TraceNoExporter-8      16.0 ± 0%      16.0 ± 0%     ~
/StatsNoExporter-8      0.00           0.00          ~
/LogNoop-8              16.0 ± 0%      16.0 ± 0%     ~
/TraceNoop-8            64.0 ± 0%      64.0 ± 0%     ~
/StatsNoop-8            32.0 ± 0%      32.0 ± 0%     ~
/Log-8                  16.0 ± 0%      16.0 ± 0%     ~
/Trace-8                 384 ± 0%       384 ± 0%     ~
/Stats-8                32.0 ± 0%      32.0 ± 0%     ~

Change-Id: If5c369f138b60435f1aa74120aa3c1b68baae402
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228234
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-04-16 04:05:44 +00:00
Ian Cottrell
be55493b88 internal/telemetry: add tag value formatting to the Key
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>
2020-04-16 04:05:32 +00:00
Ian Cottrell
bd061c738e internal/telemetry: expose the TagOf* and Unpack* methods
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>
2020-04-16 04:05:21 +00:00
Rebecca Stambler
5d8e1897c7 internal/lsp: fix erroneous documentation for struct fields
The fix is just a missing return.
Also clean up a staticcheck thing, regenerate the golden files.

Fixes golang/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>
2020-04-15 03:45:06 +00:00
Dominik Honnef
92398ad77b go/analysis/analysistest: print unified diff for test failures
Instead of printing the golden file and actual output using %#v, print
a unified diff. That way, instead of a possibly long, hard to decipher
error like this

        analysistest.go:134: suggested fixes failed for /home/dominikh/prj/src/honnef.co/go/tools/simple/testdata/src/CheckDeclareAssign/LintDeclareAssign.go, expected:
            "package pkg\n\nfunc fn() {\n\tvar x int = 1\n\t_ = x\n\n\tvar y interface{} = 1\n\t_ = y\n\n\tif true {\n\t\tvar x string = \"a\"\n\t\t_ = x\n\t}\n\n\tvar z []string\n\tz = append(z, \"\")\n\t_ = z\n\n\tvar f func()\n\tf = func() { f() }\n\t_ = f\n\n\tvar a int\n\ta = 1\n\ta = 2\n\t_ = a\n\n\tvar b int\n\tb = 1\n\t// do stuff\n\tb = 2\n\t_ = b\n}\n"
            got:
            "package pkg\n\nfunc fn() {\n\tvar x int = 1\n\t_ = x\n\n\tvar y interface{} = 1\n\t_ = y\n\n\tif true {\n\t\tvar x string = \"\"\n\t\t_ = x\n\t}\n\n\tvar z []string\n\tz = append(z, \"\")\n\t_ = z\n\n\tvar f func()\n\tf = func() { f() }\n\t_ = f\n\n\tvar a int\n\ta = 1\n\ta = 2\n\t_ = a\n\n\tvar b int\n\tb = 1\n\t// do stuff\n\tb = 2\n\t_ = b\n}\n"

we get a much more concise and readable diff:

        analysistest.go:133: suggested fixes failed for /home/dominikh/prj/src/honnef.co/go/tools/simple/testdata/src/CheckDeclareAssign/LintDeclareAssign.go:
            --- /home/dominikh/prj/src/honnef.co/go/tools/simple/testdata/src/CheckDeclareAssign/LintDeclareAssign.go.golden
            +++ actual
            @@ -8,7 +8,7 @@
             	_ = y

             	if true {
            -		var x string = "a"
            +		var x string = ""
             		_ = x
             	}

One downside of this approach is that unprintable characters won't be
visible in the diff. However, the vast majority of Go code does not
contain unprintable characters, and an even smaller amount of
suggested fixes affect unprintable characters. It is worth optimizing
readability for the common case.

Change-Id: I857aa6b6ee719f0fb018d5007eb162882e79cc25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228118
Run-TryBot: Dominik Honnef <dominik@honnef.co>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-04-15 00:09:39 +00:00
Ian Cottrell
33e937220d internal/jsonrpc2: simplify request even futher
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>
2020-04-14 21:18:25 +00:00
Ian Cottrell
ce2627f346 internal/jsonrpc2: remove the OnReply callbacks
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>
2020-04-14 21:18:03 +00:00
Rob Findley
6a72e3782c internal/lsp/fake: don't lock while calling formatting
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>
2020-04-14 20:55:25 +00:00
Rob Findley
3bb7580c69 internal/lsp/fake: correctly configure the fake editor
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>
2020-04-14 20:27:33 +00:00
Rob Findley
912958979a internal/lsp/regtest: put testing.T back in the test func signature
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>
2020-04-14 18:42:46 +00:00
Yasuhiro Matsumoto
0037cb7812 all: Fix spelling of Marshaling.
The single "l" form is used throughout the adjacent tools package
and the Go standard library.

Change-Id: I88c3530ef9d3b1354895d342e39403fa20ccd4d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228237
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-04-14 13:15:30 +00:00
Dominik Honnef
332987a829 gopls/internal/hooks: don't run staticcheck's SA5011
SA5011 relies on facts from dependencies to avoid false positives.
However, gopls currently only loads export data for dependencies, it
does not compute facts.

SA5011 is unlike other analyzers in staticcheck, which may produce
false negatives if facts are missing, but no false positives.

Change-Id: I5063b701bbedca7b09d1894997f8c574fa497939
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228119
Run-TryBot: Dominik Honnef <dominik@honnef.co>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-14 03:22:29 +00:00
Rob Findley
a4a177c7d7 internal/lsp/diff/difftest: ignore for GOOS=illumos
illumos has a different copy of the `diff` executable, causing this test
to fail. Ignore it for this GOOS.

Updates golang/go#38414

Change-Id: I45ec5fcd9fd332977349bb6ba33d9ed09417a023
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228199
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-14 02:36:50 +00:00
Dominik Honnef
ae52e4b557 go/analysis/analysistest: give up if we can't find golden file
Change-Id: I6b3b5aa573edc4bcab50a262306b47cf0665f957
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228117
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-04-14 00:10:08 +00:00
Rob Findley
07bb9fb2f9 internal/proxydir: add an internal package for file-based proxies
Both packagestest and the gopls regtests need to write module data to
the filesystem in proxy structure. Since this seems like a common and
self-contained concerned, factor this out into a shared package.

Change-Id: I5275dbc0cd7b13290061e8bb559d6dd287fbb275
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227841
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-13 22:35:07 +00:00
Michael Matloob
250b2131eb go/loader: remove check on ioutil.TestTempFile in TestStdlib
The check was there to test that the loader worked with a cycle between the
three augmented packages compress/bzip2, io/ioutil, and regexp, because of
dependencies between each of the packages' tests and the next package.

The test in io/ioutil that had the dependency that created the cycle no longer
exists in that package (it's been moved out into the ioutil_test xtest).
Remove the check for that package.

Unfortunately this means that the cycle that was being checked for before is no
longer being checked. That could be fixed in a future change by creating three
fake packages in testdata that have this relationship.

Fixes golang/go#38318
Updates golang/go#19152

Change-Id: I8ce88102a5505d8edf8d54d2098c85a8d3cd622f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227772
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2020-04-13 16:19:37 +00:00
Ian Cottrell
1f08ef6002 internal/jsonrpc2: split reply from request
reply is now passed to handlers separately from request, which allows it to be
substituted by handlers.
This also makes the handler signature much closer to http (which has
ResponseWriter)

Change-Id: I12be2e8e8b9bd508982ba43c9092709429284eaf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227839
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-13 01:58:12 +00:00
Ian Cottrell
b854406a88 internal/jsonrpc2: make the wire structures private
The wire structures do not need to be public, and making them so might make it
harder to keep the package correct without breaking changes if the protocol
changes in the future.

Change-Id: I03a5618c63c9f7691183d4285f88a177ccdd3b35
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227838
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-13 01:57:29 +00:00