This extracts the printing code from the log writer so it can be re-used
and then changes the loggers in the lsp to use it.
This means that messages that used to look like
date:
message=text
now print as
date: text
which makes the logs a lot easier to read.
Change-Id: I9dfbae47cdc9aeb7d3ca3279e445f39f2e590827
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232989
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
As a follow-up to CL 232990, return in the default case so that the
compiler will complain if we fail to return.
Change-Id: Ib771dfcd1a67b33fd51508ac183b861c00417efb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232992
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A missing return in the SelectorExpr case meant that cloneExpr would
return the original node, resulting in AST corruption when the caller
modified it.
It might be nice to panic in the default case to prevent this from
happening again, but for now let's just fix it.
Fixesgolang/go#38927.
Change-Id: Ib99f2dadecf52007ac9319c480fbd2d636a0474a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232990
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In VS Code, a workspace symbol query with an empty query parameter
is issued as soon as users open the symbol search box. There are many
symbols in a reasonably sized project and the chance that a user finds
a result in the randomly chosen 100 items out of those many symbols is
low. Thus, this first query is often useless.
Ignore this query and return an empty result immediately.
Change-Id: Idc7703c8e460c9115ecbcf198907acc9c82add4d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232986
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TestVerifyUnified in internal/lsp/diff/difftest requires specific
behaviour of the 'diff' command which is known to be satisfied by
GNU diff. The plan9 'diff' command has no '-u' option, and the
illumos 'diff -u' produces output in a different format. Checking
specifically for the GNU version in the HasTool function ensures
the expected behaviour, and otherwise causes the test to be skipped.
Updates golang/go#38772
Change-Id: I5493fa8cfc48a112dc0b7356618c62d3ccb0366f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232479
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While writing the fake editor, I added some state tracking without using
it (log messages, events etc). We have since duplicated this logic in
the regtest package using client hooks.
Fix two messy aspects of this:
- remove the state tracking in the editor
- pass in the client hooks when connecting, so that they may be used
without locking, and so that we do not miss any hooks that may fire
during session initialization.
Change-Id: I24c17a28e9cfa4fca32b7ddd17c7bf01cbb12e0f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232744
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A couple string options were not using the asString helper. Update them,
and also add a setString helper to be consistent with setBool.
Change-Id: I38caef5b1595a3535b759e5bf1c8d350a8bf061e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232741
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Some code lenses may be undesirable for certain users or editors -- for
example a code lens that runs tests, when VSCode already supports this
functionality outside of the LSP. To handle such situations, support
configuring code lenses via a new 'codelens' gopls option.
Add support for code lens in regtests, and use this to test the new
configuration. To achieve this, thread through a new 'EditorConfig' type
that configures the fake editor's LSP session. It made sense to move the
test Env overlay onto this config object as well.
While looking at them, document some types in source.Options.
Change-Id: I961077422a273829c5cbd83c3b87fae29f77eeda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232680
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In (*snapshot).addPackage, we return early if the package handle is
already cached, but we continue building the dependency graph with a
handle passed into addPackage.
This seems fine since both handles should have the same cache key,
but if we clone the snapshot, we will end up dropping the handle that
had the type information on it. It will then have to be recomputed,
causing the skew in the types.Package.
Fixesgolang/go#38403
Change-Id: I0e360447a428123fcac444fbea3c2a3232ef941a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
It is useful to know whether the session has any unsaved files, for
example to warn/error when executing a command that interacts only with
files on disk.
Add a new UnsavedFiles method to the Session.
Change-Id: Iea4bf472e3ed6897979306b670eb974a2ee0d3bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232747
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Some x/tools tests use too much memory (virtual or real) or other
resources to run on "small" builders. Originally only linux-arm
was classified as small. This change addes plan9-arm to the list,
since it normally runs on Raspberry Pi boards with 1GB of RAM.
Partial workaround for golang/go#38772
Change-Id: I93307af10cccf7b1e26d57b9af914c85cf676d43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232478
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
LSP https://microsoft.github.io/language-server-protocol/specifications/specification-current/#cancelRequest
expects the server to send back the response even when the request is cancelled.
Gopls LSP protocol implements the cancellation using the context cancellation.
That is, upon a cancellation request from the client, the server calls the
corresponding canceller that cancels the context passed to the handler.
Reusing this cancelled context for the replyer is not safe because code in any layer
can decide to shortcircuit and skip sending the data back to the client.
E.g. https://cs.opensource.google/go/tools/+/master:internal/jsonrpc2/stream.go;l=63
This CL make sure to pass the detached context to the replier.
Alternative, or a better approach to avoid any unexpected side-effect of
using a detached context is to send out the response at the point of cancellation
with a separate context. But that requires more significant code change.
Testing is currently hard, but maybe doable once the current refactoring is
done. Test is left as a TODO.
Change-Id: I4611af00ad913e96b62c6b7180c6673b0465daf9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232300
Reviewed-by: Ian Cottrell <iancottrell@google.com>
LoadMode and ParseMode are currently hashed with a string(int)
conversion, but this conversion is now discouraged (see the vet check
'stringintconv' for more information). Since the hash uses the code
point, this change replaces these instances with string(rune(int)).
Updates golang/go#32479.
Change-Id: I0d8e91d073fc34ac9faafe75a0d86cf71a5327d4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232679
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Our fake.Workdir generates synthetic file events for "watched" files
when using its file API, but we have no such hooks for file changes
originating from an external process, in this case the go command.
Previously, we detected a file event by checking go command logs to see
if a go.mod file was created. This is bound to be fragile, so we replace
this with a helper function that walks the working directory looking for
changes.
Change-Id: I2c2176099b97f3a3b25f4aed2b18de13ae05544a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232637
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We were previously sending diagnostics for nonexistent files, and then
adding them to the snapshot in the process. Remove this behavior, and
add a regression test. Case insensitive filesystems were too confusing
to write a test for, but fortunately, Filippo reported another instance
of this bug, so I used that for the regression test.
Fixesgolang/go#38602
Change-Id: I4ef6b51944f3338e838875a5aafffd066e8392f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230315
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
NewStream implies the default stream type, which it is not.
NewHeaderStream is actually the default choice.
Change-Id: I1744d7e902d27c13393f3b367fe2d29e5d7dc283
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231618
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
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>
The current use of start time in the cache key prevents re-use of agents under
some circumstances, so we update it later in the exporter instead.
Change-Id: I2f6927d65a0841f77a0ee1b848b5a3b243936083
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231257
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 attempts to detect a Connect call with the same configuration and return
the same exporter.
This mostly affects tests where we end up starting a new ticking go-routine per
test, even though they all have the same configuration. It was noticable that
a goroutine dump of a test would have a very large number of go routines that
were just ticking exporters, making it very hard to see the real work.
Change-Id: Ie8b64b69fce736d2bf3dbadffc1681f7caee6dd8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230463
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@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>
It was not very useful, it basically renamed io.EOF under some limited
circumstances, and was only there for the befit of tests.
Instead, the test now checks io.EOF directly, but also checks for
io.ErrClosedPipe which was also happening.
We also make sure we wrap errors rather than replacing them.
This prevents some weird random test failures due to races in the way they were
closed.
Change-Id: I236b03ac5ba16bf763299b95d882cf58b1f74776
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230303
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
