1
0
mirror of https://github.com/golang/go synced 2024-11-19 02:14:43 -07:00
Commit Graph

5108 Commits

Author SHA1 Message Date
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
Ian Cottrell
2595e72532 internal/jsonrpc2: replace NewErrorf with fmt.Errorf
We utilize error wrapping to recover the error codes when needed.
The code constants are also replaced by fully declared errors with
human readable messages.

Change-Id: I8edeb05f5028e99966e4ca28151f644f008d4e7d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227837
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:14 +00:00
Rebecca Stambler
79a7a3126e internal/lsp: add an extra bounds check to avoid nil pointers
A nil pointer was reported to the golang-tools group (see
https://groups.google.com/g/golang-tools/c/JrNTz8I6ifo/m/tcJRpek-AAAJ).
I think this bounds check should address it.

Updates golang/go#34433

Change-Id: I87352c269c65c844c86ebe9ee3fd2d041cc49ee9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227770
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-10 19:49:07 +00:00
Joe Tsai
e3f0bd94ad go/ast/inspector: impose maximum capacity
Consider a pathological file that contains a single string that
is 1GiB large. The current heuristic for allocating the
events would create a slice that is 330M elements large, when
only a small number of events would ever be needed. This may easily
cause the analysis to OOM on the pathological file.

Alleviate the consequences of a pathological file by imposing
a maximum number of events.

Change-Id: I937bb8a3f479866631c4895ac1e729458f6e5e34
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227861
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
2020-04-10 18:16:43 +00:00
Ian Cottrell
ae9902aceb internal/lsp: remove the CallCanceller
This required changing the jsonrpc.Conn.Call signature to also return the
request ID so it can be cancelled.
The protocol package now declares the Call function which wrapps up
Conn.Call and then sends a cancel message if the context was
cancelled during the call.
There is a small chance that a context can be cancelled on a
request that has already completed, but it is safe to do so.

Change-Id: Ic8040c193e1dd4ef376ad21194b1d0ea82145976
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-10 13:26:12 +00:00
Ian Cottrell
c92aeb7438 internal/lsp: improve ID formatting
Replace the String method with a Format method so we can use it for extra
formats.
Add some tests to make sure it is all correct

Change-Id: I39f361ffba036fad99c93f8c0944164f7cf199ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227486
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-10 13:25:36 +00:00
Rebecca Stambler
3bd20875a2 internal/lsp/cache: hide type errors if we fix up the AST
I was curious about why were logging errors during type-checking in
tests, and the answer turned out to be a bit more sinister than I
expected. We were getting type error messages without filepaths, so I
tried to reproduce it in the playground and wasn't able to. I realized
that these errors were coming from were coming from the "fixed" version
of the AST that we pass to the type checker.

Adding fake positions to our fake Cond statements trivially fixes the
logging issue, but it does nothing to handle the fact that the error
makes no sense to the user - because it applies to something that's not
in the source code. I figured we have two options: (1) skip type errors
for all packages with "fixed" ASTs, or (2) add something to the error
messages to indicate that the source code may not match. Starting with
(1) here, and if it becomes a problem, we can move to 2. All ASTs that
we fix have *ast.BadExpr in them, meaning that, by definition they have
parse errors which we will preferentially show those errors to users in
diagnostics (so I'm not sure how to test this).

Change-Id: I17733968aa15f989cdd3e4e7261c4f4fe9b97495
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227557
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-10 04:07:51 +00:00
Rebecca Stambler
c08edad6b3 internal/lsp: linkify IP addresses in textDocument/documentLink
Found golang/go#18824, which helped me understand what to do to fix this
issue. Also, changed some of the structure of the code to propagate
errors up to the top-level functions. I think this will help us deal
with actual errors vs. ignorable errors.

Didn't add tests because of golang/go#35880. Just prioritized that issue
for gopls/v0.5.0 - it's been biting us a lot lately.

Fixes golang/go#38285

Change-Id: I6962462818becb1bcedde4a5be3a2060e71c9389
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227559
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-10 03:56:59 +00:00
pjw
700752c244 internal/lsp/regtest: add test for issue 32149 (wrong package)
Gopls gets confused about the package if the package name is edited.
(See golang.org/issues/32149) This skipped test will succeed as long
as the bug is present.

Change-Id: Ic99ceda133f92f306baf94e2f8ad0381ed565814
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227760
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-04-09 21:04:53 +00:00
Rob Findley
bc09a2cac9 internal/lsp/regtest: output gopls logs on test failure
Wrap the regtest test servers to capture jsonrpc2 logs, so that they can
be printed on test failure.

There was a little bit of complication to implement this in the 'Shared'
execution mode, and since we're not really using this mode I just
deleted it.

Updates golang/go#36897
Updates golang/go#37318

Change-Id: Ic0107c3f317850ae3beb760fc94ae474e647cb78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226957
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-04-09 19:58:30 +00:00
Rob Findley
97c4fbe514 internal/lsp/protocol: make loggingStream log writes concurrency-safe
Per the documentation for jsonrpc2.Stream Write must be safe for
concurrent use, but this isn't the case for the loggingStream.

Guard it with a mutex.

Change-Id: I384892b90cef950d518089421d05cf8040c6b233
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227487
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-04-09 19:31:31 +00:00
Dmitri Shuralyov
77362c5149 go/packages: replace darwin with netbsd in TestSizes
The darwin/386 port has been removed per golang.org/issue/37610.
TestSizes needs an operating system that has both amd64 and 386 ports,
so darwin no longer qualifies. NetBSD has had its 386 port restored
recently in golang/go#31726, so it can be used instead.

Fixes golang/go#38319.
For golang/go#37610.

Change-Id: I37ce6d86ca3ddad43e9294e0de66f36091cba54a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227552
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
2020-04-09 17:04:54 +00:00