The Workspace type has accumulated too much additional functionality of
late: managing the Env, GOPATH, and GOPROXY in addition to the working
directory. Additionally, the name 'Workspace' can easily be confused
with 'workspaceFolder' in the LSP spec, and they're not quite
equivalent.
Split off a Proxy type to be responsible for the fake module proxy, and
a Workdir type to be responsible for working with the temporary
directory. Rename what remains of 'Workspace' to a more appropriate name
for such a collection of resources: Sandbox.
This is mostly just moving things around, with one significant change in
functionality: previously our three temporary directories (workdir,
gopath, and goproxy) were in separate toplevel directories below
$TMPDIR. Now they are all below a new sandbox temp directory, so that
they are correlated in the filesystem and can be cleaned up with one
call to os.RemoveAll.
Change-Id: I1e160a31ae22f0132355117df941fe65822900eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230758
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This test is failing on darwin-amd64-10_12: skip it while I investigate.
Change-Id: I506f54355924e86cf4235b04fd8890f59b48ac33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232198
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I noticed that in a couple places, event.Error was called with a
message containing formatting verb. This was my likely done out of
habit, but is an incorrect use of the API: err is not formatted in the
message but is rather applied as an event label.
Remove the unused formatting verbs.
Change-Id: I52f914da81e338013c7449066e5d9ffa40a0a4c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232137
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Our editor interaction for running `go generate` was untested. Add
support for triggering generate from the fake editor, and a simple test.
To enable this, some helpers were added to list Workspace files and
check for file state changes, to avoid having to synthetically create
file events. This workaround is not ideal as it results in a leaky
abstraction: in other cases the regtest may assume that FileEvents are
triggered by workspace interactions (e.g. ws.WriteFile), but in this
case it cannot. Unfortunately the only real solution for this would be
to make file watching more realistic, by polling file state on an
interval or using an actual file watching library. Neither of those
options seemed worthwhile just to keep the fake.Editor API pristine.
A new debugging option is added, SkipCleanup, to allow inspecting
regtest working directories after a test with minimal code change.
Change-Id: I64dceeb21a4eb9eff2b6936e44f80f4bd24b82da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230313
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.
Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This should provide simple name completions for comments
above exported vars, constants, functions, and types.
Can be activated with `ctrl+space` within a comment.
Also fixes a panic introduced in the previous commit when completing comments that occur at the end of a file.
Fixes#34010Fixes#38793
Demo: https://i.imgur.com/qN82CVA.mp4
Change-Id: If9aaec7ce03a3e085361144bce5c7a66535127d1
GitHub-Last-Rev: b9ac874c7ff3c9a164ba698d0d561141a59e2435
GitHub-Pull-Request: golang/tools#224
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230215
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If the user's workspace is neither in GOPATH nor a module, and there
are errors in their code, send a message (with ShowMessage) only on
the first load, or when the configuration changes. The previous
behavior sent the message more frequently.
There is a regtest, and two new Expectations for when the fake
editor sees (or does not see) a ShowMessage notification.
Fixesgolang/go#37279
Change-Id: I076bd95105359b9310dcf97019e3559159271356
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230897
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This reverts commit 17a19b5fe7. The revert
is partial because that change also added the -short flag when running
govim tests, which we preserve as without this the tests often time-out
(and I don't want to increase our test timeout right now).
Reason for revert: telemetry races have been fixed in https://golang.org/cl/226317
Change-Id: I5fcf034c1fe6e2db48994e2f06b73a593c779e54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231637
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Issue https://github.com/golang/go/issues/36824 complained about
legal go code (e.g., 'package a; func f() {}') that was mishandled
(by being rewritten just as 'package a'). This bug seems to have been
partially fixed, as certified by the new regtests. The comment on
OneLineImports36824 says that the bug would be fixed if gopls
formatted the file before fixing the imports, but it doesn't.
Change-Id: If27fa738e54d9434d5b2f17ed4e52d555cb7c499
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229303
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
As suggested on Slack, a better fix for golang/go#38467 would be to hide
suggested fixes on generated files. This way, the diagnostics are still
visible but files are not unintentionally modified.
Also, deleted the SuggestedFixes field on source.Diagnostic, since it's
entirely unused.
Change-Id: I10756471e0f913465b1cccd7f222eea0f4de77fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230999
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Untyped members from unimported packages are scored the same as typed
members from unimported packages. We depended on the unimported
package relevance to rank the probably-more-relevant typed members
higher. However, there are some unrelated score penalties that can
only be applied to typed candidates, so the untyped candidates ended
up being ranked higher. Fix by increasing the relevance coefficient so
the relevance score overpowers other less important scoring
adjustments.
Fixesgolang/go#38104.
Change-Id: Ie43f769a41511f9cc3747ce6936130be7a29cd31
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231238
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This means the lock is no longer held when writing to the underlying
stream.
Change-Id: I4f066ff593e35d771aa989c762712e4dd83a3f81
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231040
Reviewed-by: Robert Findley <rfindley@google.com>
This is purely moving code: the getRemoteSocket, getTestServer, and
AddCloser funcs were above the more important RunOptions and runner.Run.
Move them closer to their usage.
Change-Id: I6d5b5577ca8ba5cbd5226b195541696967433dec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230998
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add issue comments to tests for issues, move a couple constants closer
to their use.
Change-Id: I34fa2643195ae81e463a070952a1a1a4af2c6132
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230997
Reviewed-by: Robert Findley <rfindley@google.com>
We had previously not been generating documentation on hover for
package declarations or import specs. We do this by adding a few special
cases, since package declarations don't appear in type information.
Throughout, we make the assumption that only one file in a package will
have the documentation for the package. go/doc just appends
documentation as it sees it. We may be able to do better by checking for
a "Package ..." but that still is not guaranteed. Not sure what the
right approach is, so this assumption may be the best option.
Fixesgolang/go#38526
Change-Id: Ibc515f8729e1aba0d11090be62371be6b18d0cfa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Don't show non-vet analyses when they appear in generated files. Vet
analyzers will give useful reports even in generated files.
Fixesgolang/go#38467
Change-Id: I0e628760b386553932de4cf1f5ba39784a205b53
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A common problem when writing regtests is that if you have an error in
your expectations, you must wait until the regtest times out to see what
went wrong.
With the integration of additional progress reporting in the LSP server,
we know when diagnostic work should have been completed, and we should
be able to fail tests early once we know that our diagnostic
expectations will never be met.
This CL adds a new OnceMet Expectation, which combines two expecations:
the first is a precondition that must be met before checking the second.
The second is an arbitrary expectation, but is translated as follows:
once the precondition is met, the second condition is checked and any
Unmet verdicts are translated into Unmeetable.
Change-Id: Ie8c677229a347c624e2659a3ef9104304b175243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229977
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Update the regtest docstring to further explain how the package works,
and give a sense for why it exists.
Fixesgolang/go#36879
Change-Id: I4df4a286dc835ae56386b6d92128f7ea6c77ffb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229782
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Minor cleanup for the regtest package:
- EnvMode is renamed to Mode, because it's really a server mode and not
directly related to the Env type.
- Modes are better documented.
Change-Id: Ia3aedfc70b665ea75a66731a72e4e87ae79db298
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229781
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The file length of env.go is getting hard to manage, so factor out the
test Runner to a new file.
Also move Runner.Close to the bottom of the file to have a more logical
progression.
Change-Id: Ifebea3277d826e9456aa02507a8349746386eb03
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229780
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In order for regtests to wait until file diagnostics are complete,
instrument diagnostics with verbose WorkDone reporting. In order for
this to be granular enough for use, the modification source needed to be
threaded through to the didModifyFiles function (which is where the
diagnostic goroutine is spun off).
A new expectation is added: CompletedWork, to allow specifying that a
specific work item has been completed. The problem with using
NoOutstandingWork was that it required a continuous chain of work to
prevent the regtest from succeeding when the bug was present, meaning
that by the time we have sent the didChange notification successfully
the server must have started work on its behalf. This was inherently
racy, and too tricky to get right.
Additionally, a couple bugs are fixed:
- EmptyDiagnostics is corrected to account for the case where we have
received zero diagnostics for a given file.
- A deadlock is fixed in Await when expectations are immediately met.
Updates golang/go#36879Fixesgolang/go#32149
Change-Id: I49ee011860351eed96a3b4f6795804b57a10dc60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229777
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Our current usage of WorkDone progress reporting (new in v3.15 of the
LSP spec) is in reporting progress on `go generate` commands. In
preparation for using this API more widely, factor out the reporting API
from the current io.WriteCloser wrapper (workDoneWriter).
Change-Id: Ib528093d81d4fc065528df90e100859e850b10df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229459
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We were using strings.Split on env vars, which did bad stuff when the
var contained an =, e.g. GOFLAGS=-tags=foo. Only split on the first =.
Irritatingly, this breaks only `go mod` commands, so almost nothing in
gopls failed, just organize imports and the `go.mod` code lens stuff.
Fixesgolang/go#38669
Change-Id: I8d28c806b77a8df92100af1fa4fbcca5edf97cff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230560
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This moves the common code from the cmd and gopls tests to the shared cmdtest package, they were starting to drift apart.
This change was extracted from another larger cl where I was trying to work out why it broke in one but not the other.
Change-Id: I554ce364f4152e6b61f989da8162d968426d4ae5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230301
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change was unfortunately lost while rebasing, resulting in a lot of
unremoved directories.
Change-Id: Icc1b667e31ac85e617b1c3a8d7c58f78e999d151
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230314
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There were a few cases where we were not properly qualifying package
names, particularly if the original package had a named import. Now,
we map between these names correctly - handling the case of multiple
packages that need to be qualified. This requires applying edits to
*ast.SelectorExprs, as well as *ast.Idents.
We still do not fully qualify unimported packages, and likely won't,
unless that's an issue for many users.
Updates golang/go#38591
Change-Id: I966a4d1f936f37ede89362d03da3ff98d8952a06
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229979
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
Fixesgolang/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>
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>
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>
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>
Change the tag iteration api to something less flexible
that allows for event iteration without allocation.
Change-Id: I212d45ebceea0183d1a61e6b611e0558649be60a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227301
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This change attempts to fix a concurrency error that would cause
textDocument/CodeLens, textDocument/Formatting, textDocument/DocumentLink,
and textDocument/Hover from failing on go.mod files.
The issue was that the go command would return a potential concurrency
error since the ModHandle and the ModTidyHandle are both using the
temporary go.mod file.
Updates golang/go#37824
Change-Id: I6cd63c1f75817c7308e033aec473966536a2a3bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224917
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change is the first step in handling golang/go#38136. Instead of
creating multiple diagnostic reports for type error analyzers, we add
suggested fixes to the existing reports. To match the analyzers for
FindAnalysisError, we add an ErrorMatch function to source.Analyzer.
This is not an ideal solution, but it was the best one I could come up
with without modifying the go/analysis API. analysisinternal could be
used for this purpose, but it seemed to complicated to be worth it, and
this is fairly simple. I think that go/analysis itself might need to be
extended for type error analyzers, but these temporary measures will
help us understand the kinds of features we need for type error
analyzers.
A follow-up CL might be to not add reports for type error analyzers
until the end of source.Diagnostic, which would remove the need for the
look-up.
Fixesgolang/go#38136
Change-Id: I25bc6396b09d49facecd918bf5591d2d5bdf1b3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Turns out that these were already disabled in the
golang.org/x/tools/internal/lsp tests, which is why we only ever save
the failures in golang.org/x/tools/internal/lsp/source tests. Discussing
the reasons that this is necessary on the issue, but I think completion
scoring in general needs to be centralized and documented, so hopefully
we can handle this case as part of that larger fix.
Fixesgolang/go#38269
Change-Id: Ie1b615315e417ab8211a3580cf8f27cbdc3b74e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The code for `gofmt -s` directly modifies the AST, since the ASTs are
not long-lived. Some of this code made it into our analysis
implementations, causing very strange bugs to manifest. Added a
regression test for this specific case.
Fixesgolang/go#38267
Change-Id: I235620adcbf2bbc7027c6d83ff2c7fe74729062e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Makes Event implement TagMap directly, instead of
having to build an entirely new object.
Change-Id: I0c1e8638de3dc3347f60fd93af3df6b7f8387751
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227300
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
I originally made this change to see if it would help with the timeouts.
Based on the TryBot results, it doesn't -- but I still think it's more
correct to have the contexts this way. It was my mistake to put the
context on the completer in the first place, I think.
Change-Id: Ib77c8f0ac0b0d0922b82db4120820fb96cb664f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227303
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now it only uses the telemetry messages directly rather than the metric system.
This is much faster and more direct, and removes a blocker on improving the
metrics support.
Also fixes the fact that recieced and sent were the wrong way round before,
probably as an artifact of the old protocol logging code, and also removes
the bytes histogram which was a lie (it was a histogram of write sizes that
was presented as a histogram of message sizes)
fixesgolang/go#38168
Change-Id: Ib1c3459c0ff1cf0c6087a828981e80c1ce1c5c1b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227139
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This pushes the handler construction out to the user, allowing flexability of
use, and is the final stage of the switch to the new handler API.
Change-Id: Id2e61813a817df0d6e4d20dd47ce8c92b0ae87db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227024
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
We can do cancelling at the top level handler now, it can drop the cancel
messages themselves before they enter the queue stage, and also track
all the events as they flow through it.
The ugly part is the OnCancelled interface, which is a bit clunky.
Change-Id: I3fa972198625fb3517fdecc740d1a3fdb19a188a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226959
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
It is now a programatic error to have a handler registered to a connection that
does not call reply for all messages, including notifications.
This normalizes the flow making the code easier to understand and fixes a
couple of long standing hard to find bugs.
Change-Id: If41c39ece70e3bc64420abefac75ec647a8f8b37
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226838
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Handler is now a function type that mapps to what used to be the Deliver method.
The only handler that used other methods was Canceller, for now that still
exists as LegacyHooks. Once the handlers are fully cleaned up we should be able
to re-implement canceller as handler middleware.
Each connection is now only allowed one handler, and it is passed to the Run
method, but handlers are composable.
Change-Id: I370e0459df851bb9c9c2a679b99cff073b94489e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226479
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Instead of checking the context, check the error. This may expose some
errors that are not wrapped correctly. Replaced all uses of errors
with golang.org/x/xerrors.
Change-Id: Ia40160f8ea352e02618765f2a9415a4ece0dcd94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227036
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The modifications in CL 226559 introduced a new call to
RunProcessEnvFunc that was not using the context with a timeout, so deep
completion tests were hitting the timeout early (I think?).
Also, fix a staticcheck warning that's been bugging me today.
Change-Id: Iac894f630ebfa1cbc5588ae3a5490a93fa53aba2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227057
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A couple small improvements for regtests:
- module names fixed to include a '.'. This was fixed for diagnostic
tests already.
- For regtests that were written to exercise specifig github issues,
include the issue number in the test name.
Change-Id: I67da6d78dc166e854d2543cf63dac382202f9dbc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226844
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We need to search all transitive dependencies of a package, not just its
immediate imports.
Fixesgolang/go#38100
Change-Id: I15b4dbe226ba851691ca0c95460c3648ede32f04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227030
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
One of the tricky things about asserting on conditions in regtests is
the asynchronous nature of LSP. For example, as the LSP client we cannot
be sure when we've received all diagnostics for a given file.
Currently, regtests are implemented by awaiting specific diagnostic
expectations. This means that if gopls generates diagnostics that do
not match those expectations, we can only time out the test.
Ideally, we would want to know that gopls is done generating all diagnostics
for the current file state. This is not possible without knowing the
status of diagnostics for. Barring this, we would want to know that
diagnostics are done for the current file version. Unfortunately, that
also is not possible, because a new version of file B can affect
diagnostics in file A.
So in lieu of this information, this CL exposes a few tools that can be
used to improve the experience of writing new regtests.
- A new expectation is added: AnyDiagnosticAtCurrentVersion, that is
satisfied if any diagnostics have been received for the current
buffer version.
- ExpectDiagnostics is added to Env, to help check whether the current
diagnostics matches expectations.
Updates golang/go#38113
Change-Id: I48d2c3db87c13ac3ab424d01d9444cbc285af9e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226842
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When proposing unimported package members, we start with packages loaded
in-memory first, for three reasons: we know that they're fast, they're
fully typed, and they're probably pretty relevant. I hadn't bothered
doing that for package names, because the first two reasons aren't very
relevant. But the third still is -- loaded packages are a pretty good
approximation for in module scope, etc.
With this change we do package names the same as members, so relevance
should be about as good. Not perfect, but nobody's complained much yet.
Fair bit of copy-and-paste, but I don't want to extend the
getAllCandidates abstraction outside of the imports package yet.
Updates #38104.
Change-Id: Ia479181607dff898baee3cd6aa84d1ab61715d19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226559
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If we release gopls/v0.4.0 soon, we should keep these new analyzers off
by default. They were just merged, so they haven't been used enough to
be enabled, I think. We'll turn them on by default for gopls/v0.5.0.
Also, ended up creating a helper function to check if analysis has been
abled (which fixed a small bug in FindAnalysisError), and another helper
function to enable all analyses for testing purposes.
Updates golang/go#38212
Change-Id: I5ee94b3582dfc0863978650fc6ce51bfa0606c13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226962
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
The options can be modified without the view being recreated, so we need
a mutex there.
Change-Id: I87e881835622a941fce98e4a1062aa41acd84fcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227022
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This change ports the functionality of https://github.com/sqs/goreturns
to be used as code actions on diagnostics that have missing
return values. It improves on the original goreturns functionality by:
- filling out empty return statements
- trying to match existing return values to the required return
values and then filling in missing parameters
Fixesgolang/go#37091
Change-Id: Ifaf9bf571c3bc3c61e672b0a2f725d8d734d432d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224960
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
