I noticed this wasn't working correctly when debugging issues with file
changes on disk. I think I misunderstood how to correctly match on
zero or more path segments. Confirmed that this is working as expected
with VS Code, but we really need regression tests for this in VS Code
(it seems to be based directly on VS Code's API). Filed
golang/vscode-go#404.
Change-Id: Ib906f73a97317dfa9bd30099877c90d4072651cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244605
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When a package consists of files that only fail to parse, we fail to
associate parse errors with the package, and therefore return no
diagnostics. To address this, we need to associate the errors with the
package. This involves adding a *token.File to the parseGoData so that
error messages and positions can be properly extracted, even when there
is no associated AST.
Fixesgolang/go#39763
Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
There was a bug in the hover for type switch variables. For example:
var x interface{}
switch y := x.(type) {
case string:
case int:
}
Hovering over y would previously show "var y string", because y's object
would be mapped to the first types.Object in the type switch. Now we
show the hover for y as "var y interface{}", since it's not yet in the
cases.
Change-Id: Ia9bd0afc4ddbb9d33bbd0c78fa32ffa75836a326
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Some modules may need to be added to the go.mod without being associated
with an import statement. In such cases, we show the missing module
diagnostic on the whole go.mod file. This isn't ideal, but mapping to
the full require statement isn't that simple, and this is an easy enough
starting point. The code in mod_tidy.go is becoming more unwieldy - I
think I will clean it up in a follow-up.
Fixesgolang/go#39784
Change-Id: Ib32ec1fd74c455ce42ba778ea6cba0a475cf245a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243218
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This CL is a follow-up from CL 241983. I didn't realize that the
undeclaredname analysis was also using the go/printer.Fprint trick,
which we decided was both incorrect and inefficient. This CL does
approximately the same things as CL 241983, with a few changes to make
the approach more general.
source.Analyzer now has a field to indicate if its suggested fix needs
to be computed separately, and that is used to determine which
code actions get commands. We also make helper functions to map
analyses to their commands.
I figured out a neater way to test suggested fixes in this CL, so I
reversed the move to source_test back to lsp_test (which was the right
place all along).
Change-Id: I505bf4790481d887edda8b82897e541ec73fb427
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242366
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Using the new run options to configure a testing environment suitable
for long-running tests, a trivial stress test is added to type
arbitrarily in a known problematic repository
(github.com/pilosa/pilosa).
Change-Id: I2c8237843111f17ff5a096515cb4704c62513ed0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244441
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
A bunch of options are added to enable long-running performance-oriented
tests in existing directories. They will be used in a later CL to
implement a simple stress test, as an example of what is possible.
Change-Id: I531b201b415362ea135978238b3d64b903226359
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The Sandbox constructor was getting out of control, and this allows
binding regtest options directly to the sandbox configuration struct.
Change-Id: I18da4ddf43c86b3b652516c3ddca7726cd35e3b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244439
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This removes some unncessary mangling of strings, and allows the editor
to fully own its configuration.
Change-Id: I0fde206824964124182c9138ee06fb670461d486
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244360
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change is a follow-up from CL 242477. We now also surface errors
in indirect dependencies as diagnostics in the go.mod file. Any errors
we cannot match are surfaced as diagnostics on the entire go.mod file.
This isn't the most user-friendly way, but it seems simplest.
Change-Id: Ife92c68c09b74a0b53de29d95b6c4c2a9c78d3b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change expands the scope of a workspace to the whole module, if the
user is in module mode. This means that diagnostics will appear and will
be updated for the whole module, even if the user only opens a
subdirectory. Similarly, references and other such queries will always
return consistent results, no matter which directory the user opens.
A new "root" field is added to the view. This is either the view's
folder or its module root. Almost all uses of view.folder have been
changed to view.root.
Updates golang/go#32394
Change-Id: I46f401f7c44b1b8429505aa032e0c15e88c4e2ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I noticed a race in these logs:
https://build.golang.org/log/ec2915e97319de219284ed022338f2ebc549aff6.
We need to copy the environment and build flags for each config, since
we're treating the config as single use.
Change-Id: I9e717e688def088cb60f2b23b71d731e2b20b259
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244118
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 242579 changed the mechanism of reporting diagnostics so that we now
combine reports from multiple sources. Previously, we overwrote existing
diagnostics if new ones were reported. This was fine because
source.Diagnostics generated all of the diagnostics for Go files, and
mod.Diagnostics generated all of the diagnostics for go.mod files.
Now, we combine diagnostics from both sources -- mod.Diagnostics can
generate reports for Go files. We may get duplicate reports for packages
with test variants, so now, we check for those and dedupe.
Change-Id: I42e98079b4eead380058dd029a3a0c72a1796ebb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL adds benchmarking by using the "-bench" flag for the test
command.
Change-Id: Idf714de1c6c3350d26a6874e7b4278927d0336fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243159
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This change adds support for `go mod tidy` on save when users opt into
import organization on save. Previously, we supported this with the
go mod tidy command, but there's no need to do this when we already
have a ModTidyHandle available.
Change-Id: Ibb47b6a7611fd823dac2a3b4f3a43b9fbb3289b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change attempts to parse diagnostics out of `go list` error
messages so that we can present them in a better way to the user. This
approach is definitely tailored to the unknown revision error described
in golang/go#38232, but we can modify it to handle other cases as well.
Fixesgolang/go#38232
Change-Id: I0b0a8c39a189a127dc36894a25614535c804a3f0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242477
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change ensures that, when the initial workspace load fails, we
re-run it if the go.mod file changes. Previously, if a user opened a
workspace with a corrupt go.mod file, we never recovered.
To reinitialize the workspace on-demand, we use the initializeOnce field
as an indicator of whether or not we should reinitialize. Every call to
awaitInitialized (which is called by all functions that need the IWL),
passes through the initialization code. If a retry isn't necessary,
this is a no-op, but if it is, we will call the initialization logic.
Only the first attempt uses a detached context; subsequent attempts can
be canceled by their contexts.
To indicate that we should reinitialize, we call maybeReinitialize.
Right now, we only call this when the go.mod file changes. In the
future, we may need it in other cases.
Fixesgolang/go#38232
Change-Id: I77eefebb0ac38fbd0fe2c7da09c864eba45b075f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242159
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously, we only updated the opened file's overlay, but not the
snapshot. This meant that the snapshot was still operating with stale
data. Invalidating the snapshot creates a new snapshot with the correct
set of overlays.
The test is skipped because it will flake until we have a better caching
strategy for `go mod tidy` results.
Updates golang/go#40269
Change-Id: Ia8d1ae75127a1d18d8877923e7a5b25b7bd965ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Our approach to commands and their arguments has been ad-hoc until this
point. This CL creates a standard way of defining and passing the
arguments to different commands. The arguments to a command are now
json.RawMessages, so that we don't have to double encode. This also
allows us to check the expected number of arguments without defining
a struct for every command.
Change-Id: Ic765c9b059e8ec3e1985046d13bf321be21f16ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Convenience code actions operates at line-by-line, but a line can
contain more than one fix.
To be able to tell them apart each response now return the actual
range as reported by the analyzer.
Fixesgolang/go#40328
Change-Id: I9ffe4737653d2a5a6dafc3574e74975a71c71937
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243877
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We had previously been treating file changes with no content in the same
way as the deletion of content. Now, we distinguish between the two.
Fixesgolang/go#38424.
Change-Id: I44b338006af0c13a8a3f842844521789ea378470
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243577
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change moves the suggested fixes logic for fillstruct out of the
analysis and into internal/lsp/source. This logic is then used as part
of a new fillstruct command. This command is returned along with the
code action results, to be executed only when the user accepts the code
action.
This led to a number of changes to testing. The suggested fix tests in
internal/lsp doesn't support executing commands, so we skip them. The
suggested fix tests in internal/lsp/source are changed to call
fillstruct directly. A new regtest is added to check the command
execution, which led to a few regtest changes.
Also, remove the `go mod tidy` code action, as it's made redundant by
the existence of the suggested fixes coming from internal/lsp/mod.
Change-Id: I35ca0aff1ace8f0097fe7cb57232997facb516a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241983
Reviewed-by: Heschi Kreinick <heschi@google.com>
Extract function is a code action, similar to extract variable. After
highlighting a selection, if valid, the lightbulb appears to trigger
extraction. The current implementation does not allow users to
extract selections with a return statement.
Updates golang/go#37170
Change-Id: I5fc3b19cf7dbca4407ecf0cc37017661223614d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241957
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We still don't have a good way to make sure that we don't break 1.12
support, but this is an easy enough fix to start with.
Updates golang/go#39146
Change-Id: I14fe997fa1f3d60320d77e664208e25d97ae6f4f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243578
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This is unnecessary. It also saves a lot of time for the regtests.
Change-Id: Ifc12d4c8e0c215f336e958316d415bc7f30e5344
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243538
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
I had previously suggested that users set LinkTarget to "" to avoid
links in the hover text. However, this work-around isn't perfect because
it also disables the documentLink behavior in other cases.
Change-Id: I3df948e2a2e4d2312998de65ccea8dfb404768ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243239
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Currently, diagnostics for modules that are missing from the go.mod
appear in the Go files in which those modules are imported. As a result,
the diagnostics were previously calculated as part of the Go file
diagnostic calculations. This is convoluted and required passing around
an extra map.
This CL puts that logic in the ModTidyHandle where it belongs.
The diagnostics for the Go files are combined from the multiple sources.
Also, added a skipped test for golang/go#39784, since this CL was
originally intended to be a fix for that issue...
Change-Id: Ic0f9aa235dcd56ea131a2339de9801346f715415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The gopls daemon had different default logging behavior than the sidecar
gopls: by default, the daemon was started with -logfile=auto.
Additionally, because most logs are reflected back to the forwarder, the
actual daemon logs have very little (if any) information.
This means that if you simply start gopls with -remote=auto, you'll get
a single logfile named /tmp/gopls-<pid>.log, which is mostly empty. This
is not a delightful experience.
Fix this via several improvements:
+ Log lifecycle events in the Daemon, to give the log a purpose.
+ Give the daemon a new default log location:
/tmp/gopls-daemon-<pid>.log.
+ Don't pass -logfile=auto to the daemon by default.
Fixesgolang/go#40105
Change-Id: I5e91ea98b4968c512bce76a596bbae441f461a66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is an alternative to CL 241739, preserving the exiting variadic
options pattern and just adding a helper method to put the options up
front.
All things considered, I think this is better. CL 241739 was too
complicated, and being able to reuse "curried" runners isn't actually
important.
Updates golang/go#39384
Change-Id: I7ecd80d310cac879520b2d1feebcf5bd5e96e89b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243057
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
getFile still returns a FileHandle, even if the file doesn't exist on
disk. Work-around this by checking if the file exists before adding
the file handle to the map.
Fixesgolang/go#38498
Change-Id: Ie02679068b37bf4f3d19966c6cfcf2361086b1de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242924
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I didn't properly understand Heschi's comment, and missed a case where
we failed to return early.
Change-Id: I692c26678b2e659c6b748085bb4b16090e04472d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change refactors lens funcs to use only a FileHandle - each
code lens should manually compute a ParseGoHandle if it's needed. The
issue was that, if the code lens needed to also get a package, the
already parsed *ast.File was not necessarily the file used in
type-checking that package. I noticed that the code lens wasn't always
coming up.
Change-Id: Ic5a2c2b3d1010555acaa64ef8c6aa2484b8fbc9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242920
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This change moves the "generic" go generate code lens code into
progress.go. Fix up a few generate-specific things in the progress
writers.
The remaining generate code isn't much, and is moved into command.go.
Change-Id: I2b7b6279da2442c0b92758b9b3e259f25787fabc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242919
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This configuration deals with the fact that VS Code has the same
shortcut for go-to-definition and clicking a link. Many users don't like
the behavior of this shortcut triggering both requests, so we allow
users to choose between each behavior.
Fixesgolang/go#39065
Change-Id: I760c99c1fade4d157515d3b0fd647daf0c8314eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
importPrefix in format.go computes the end of the package statement
including a trailing newline. This causes a panic if the user has
not typed the newline. The code now checks for that case.
Fixesgolang/go#40208
Change-Id: I4c5118a5de78027e6c5e6ed91dfddca81e38f7e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242638
Reviewed-by: Heschi Kreinick <heschi@google.com>
The current behavior is inconsistent across different configurations.
The signature is the top line for FullDocumentation, but the last line
for synopsis documentation.
Change-Id: I2fe1e1c7bfa38ae46fda06b0ecd4c7712b0e74af
GitHub-Last-Rev: d3b768558f21f518d1296b6fc776cf3974f441d9
GitHub-Pull-Request: golang/tools#241
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242104
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change reworks CL 240118 to apply gofumpt directly as a formatter,
not an analyzer. Depending on how gofumpt changes, we may be able to use
it as an analyzer in the future, but for now it's just easier to add it
as a formatting hook.
Fixesgolang/go#39805
Change-Id: I227fde4b1916d8a82557f30dfca88e155136dff5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
I wanted to add a regtest for this, but then immediately got stumped on
how to do it well. The best I've got is to add a helper that wraps
the client's call to executeCommand, but it's used in so many places that
an easier fix was to put it in executeCommand itself.
Fixesgolang/go#40101
Change-Id: Iafb804fb77e57e6ac01e3de1d115058b21ac1954
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241984
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Not sure how this hasn't come up earlier. We aren't noticing file
changes on-disk for go.mod/go.sum files.
Fixesgolang/vscode-go#297
Change-Id: I4532a252f330404515efec244a9c266a765e6bcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242160
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This test exposes a lot of fundamental issues with `go list` overlays.
Now that we have the regression test framework, we can copy it over and
make any flakes completely reproducible by waiting for each change to
complete.
The issue here ended up being that initial workspace load returns
workspace packages with no associated files due to golang/go#39986.
Working around this allows invalidation to proceed as usual.
In the process of debugging this, I also noticed that packages can get
loaded as command-line-arguments even when we can get the correct
package path for them. It's pretty complicated to fix this, so
restructured the code a tiny bit and left a TODO. I'd like to come back
to this at some point, but it's not pressing since I was able to fix
this bug.
Fixesgolang/go#39646.
Updates golang/go#39986.
Change-Id: Id6b08a5e92d28eddc731feb0ef3fd3b3fc69e64b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change addresses an underlying issue with the go.mod code, which is
that it was modifying go.mod files without cloning them. This could've
resulted in some ugly race conditions.
We also handle the fact that new dependencies weren't being added
cleanly to files that already had unused dependencies.
Fixesgolang/go#39041
Change-Id: I96ee0052d8d29a25e24f0bda9688e780a0fa7442
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241443
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Replacing the text in the comment line-by-line prevents issues to do
with CRLF/LF line endings.
No test, because txtar expects LF line endings, and I didn't think it
was worth more invasive changes to handle this.
Fixesgolang/go#39364.
Change-Id: Ia26b311a851396e4dde1954ebfc1b40c0a3c04fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The default regtest timeout was increased to 60s a while ago to try to
avoid flakes.
We think we've figured out the flakes, so drop it back down. Start with
20s, since TestGOMODCACHE (which had overridden its timeout to 5s)
occasionally fails on freebsd:
https://build.golang.org/log/6c607c7fd3e886cf23127c958ae17637440fbc63
Change-Id: I10e6927985b8fb1d149070e204bd8ca120309c7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240060
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
For easier debugging (and less cruft if regtests are ctrl-c'ed), root
all regtest sandboxes in a common directory.
This also tries one last time to clean up the directory, and fails on an
error. This might be flaky on windows, but hasn't been so far...
Also give regtest sandboxes names derived from their test name.
Updates golang/go#39384
Updates golang/go#38490
Change-Id: Iae53c29e75f5eb2b8d938d205fbeb463ffc82eb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Apparently 5 seconds is too short for the freebsd-race builder and we
care about that.
Fixesgolang/go#40090.
Change-Id: I6dad57402ed18e7c9e01f695ada7cc8b0cf35fb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241521
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If the config changes in non-module mode, don't look at the nonexistant mod file.
I actually caught this case after review but didn't finish the job.
Fixesgolang/go#40121.
Change-Id: Ic888f7eab5f882e6ca79b046b5a3fb37b3b19e5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241520
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change makes these tests a little more resilient by making sure
that the initial workspace load is completed before we run them.
Change-Id: I24735da31cc045cb78dfb280e88136b5835351a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241276
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The discussion on golang/vscode-go#299 has uncovered the fact that
go/printer is very expensive, and we're calling it frequently on file
changes. Analyzers whose suggested fixes require the original file
content are generally posing issues; this is being discussed on
golang/go#40110.
Updates golang/vscode-go#299
Change-Id: I5cb370c9cb508203e463e7243fc781e75876fe30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Changing build flags (-modfile) while work is happening in the
background causes races. Explicitly detect relevant configuration
changes and only modify the ProcessEnv then, when the resolver is
inactive after the call to ClearForNewMod.
This still leaves a very small window for a race: if refreshProcessEnv
has already captured env but not yet started priming the cache, it may
race with the modification. But I don't expect it to be a problem in
practice.
Fixesgolang/go#39865.
Change-Id: I31c79f39be55975fee14aa0e548b060c46cdd882
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241317
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I add a code action that triggers upon request of the user. A variable
name is generated manually for the extracted code because the LSP does
not support a user's ability to provide a name.
Change-Id: Id1ec19b49562b7cfbc2cd416378bec9bd021d82f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240182
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
CL 239754 eagerly initialized the environment. This turns out to be a
problem for gopls, which calls ApplyFixes with no ProcessEnv.
Reinitializing it every time seriously harmed the performance of
unimported completions. Move back to lazy initialization.
Working with invalid options has caused a lot of confusion; this is only
the most recent. We have to maintain backwards compatibility in the
externally visible API, but everywhere else we can require fully
populated options. That includes the source byte slice and the options.
LocalPrefix is really more of an Option than an attribute of the
ProcessEnv, and it is needed in ApplyFixes where we really don't want to
have to pass a ProcessEnv. Move it up to Options.
Change-Id: Ib9466c375a640a521721da4587091bf93bbdaa3c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241159
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The previous implementation did not support filling anonymous structs.
This is distinct from filling a struct that has an anonymous
struct as a field. That field will still not be filled.
golang/go#39929 has been opened to address this limitation.
Fixes: golang/go#39803
Change-Id: Ia7ba9fda2bdee092d182e0a23829b9a6d9a4b986
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240463
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
CL 226639 changed the positions of the go.mod diagnostics (in a
negligible way), but the tests are quite brittle and can't handle the
different position. Rewrite this test as a regression test to handle it.
The special // indirect marker can be removed from the go/expect package
as a result, since it was only used in this one place.
Change-Id: I7d9a62e32e53d477838e65673635ed231c07b659
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240691
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
c.pkg.GetTypeInfo().ObjectOf(node.Name) will sometimes return nil.
Check for that and a few other things that c.found also checks for.
Fixesgolang/go#40043
Change-Id: I4a2b40adbbd740323e10b3460f025b29cff74130
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241019
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The current URL regexes (including xurls) match links in any part of a
string. However, this leads to odd underlining within words which just
so happen to have valid domain names as substrings. For example, a
comment which contains "reflect.DeepEqual" will match the URL regex as
"reflect.de" is a domain name.
Ensure that the URLs only match full regex words by adding \b around the
expressions. For xurls, this is done by pulling the expression out of
Relaxed() and recreating the Regexp.
While I'm here, make the non-gopls module expression prefer the longest
match, as xurls does.
Change-Id: I403db970fa1661c443b0693c03f8dee114f8eaff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240738
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change permits starting gopls without a root URI or any workspace
folders. If no view is found for an opened file, we try to create a new
view based on the module root of that file. In GOPATH mode, we just
use the directory containing the file.
I wrote a regtest for this by adding a new configuration that gets
propagated to the sandbox. I'm not sure if this is the best way to do
that, so I'll let Rob advise.
Fixesgolang/go#34160
Change-Id: I3deca3ac1b86b69eba416891a1c28fd35658a2ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
There are still many more cases to check, but this is a good starting
point. A few tests have skips in them because I encountered bugs, which
I plan to go back and fix.
Change-Id: I0b7bbeb632d38c09d6bdb1f4866d81a1690d6ca7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238917
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Completion could be slow due to calls to astutil.PathEnclosingInterval
for every candidate during formatting. There were two reasons we
called PEI:
1. To properly render type alias names, we must refer to the AST
because the alias name is not available in the typed world.
Previously we would call PEI to find the *type.Var's
corresponding *ast.Field, but now we have a PosToField cache that
lets us jump straight from the types.Object's token.Pos to the
corresponding *ast.Field.
2. To display an object's documentation we must refer to the AST. We
need the object's declaring node and any containing ast.Decl. We
now maintain a special PosToDecl cache so we can avoid the PEI call
in this case as well.
We can't use a single cache for both because the *ast.Field's position
is present in both caches (but points to different nodes). The caches
are memoized to defer generation until they are needed and to save
work creating them if the *ast.Files haven't changed.
These changes speed up completing the fields of
github.com/aws/aws-sdk-go/service/ec2 from 18.5s to 45ms on my laptop.
Fixesgolang/go#37450.
Change-Id: I25cc5ea39551db728a2348f346342ebebeddd049
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221021
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This hack was previously being used to correct the rankings for VS Code
Go users. This logic has since been moved into the VS Code Go plugin.
Change-Id: Ia6bff53eb2c6dbe479faa1fd0667d763837eea78
GitHub-Last-Rev: e7fed76b3f85b0db3b705cbb6945cecd855d2c7b
GitHub-Pull-Request: golang/tools#225
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231098
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The previous implementation missed a nil check which caused a panic
when the package of a type was nil.
Fixes: golang/go#39899
Change-Id: I2dfb50d6b79f52df367e093e5d857cd70b7cef27
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240537
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add a symbolStyle configuration option, and use it to parameterize the
following behavior when computing workspace symbols:
+ package (default): include package name in the workspace symbol.
+ full: fully qualify the symbol by import path
+ dynamic: use as the symbol the shortest suffix of the full path that
contains the match.
To implement this, expose package name in the source.Package interface.
To be consistent with other handling in the cache package, define a new
cache.packageName named string type, to avoid confusion with packageID
or packagePath (if confusing those two identifiers was a problem, surely
it is a potential problem for package name as well).
Change-Id: Ic8ed6ba5473b0523b97e677878e5e6bddfff10a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236842
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Fully qualify (using a package's import path) symbols when matching
them. This is a minimal change in preparation for later changes to add
more advanced query functionality to the workspace Symbol method.
Add more comprehensive regtest tests (covering case sensitive, case
insensitive and fuzzy matchers) in addition to updating lsp tests.
Change-Id: I675cde2f7b492158988cf9c3206a0a55fe29622a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228123
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the past, changes to GOPACKAGESDRIVER have led to some confusing
regtest failures. Explicitly set it off.
Updates golang/go#39384
Change-Id: I303a58380a5e46e6621c19b2edc40d43199bb343
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240058
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, our file watching only considered the root directory of the
view, which may not include the entire module or its replaced
dependencies. Now we expand our watching to include the whole module.
As part of testing this, I noticed that VS Code's file watcher actually
only sends updates for files in the workspace, even if we request
notifications for all files in the module. I filed an issue to ask about
this: https://github.com/microsoft/vscode-languageserver-node/issues/641.
Change-Id: I9499d31aff273f69e9c117511e7985ff58b7fdc4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239198
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL got away from me a little.
For a number of reasons, the existing goimports API of passing in values
for various GO* values was not working. For one, the number of necessary
variables kept growing. For another, we tried to avoid calling `go env`
in GOPATH mode by using `build.Default`, but that turns out to be buggy;
see golang/go#39838. And finally, it created massive confusion about
whether the values were intended to be read from the OS environment, or
fully evaluated by the `go` command.
There are only two users of the internal imports API, so there really
shouldn't need to be more than two modes. For the command line tool, we
have to call `go env` to deal with the `go/build` bug. So we just do it.
Tests use that same path, but can augment the enviroment to set
themselves up. In contrast, `gopls` needs to fully specify the
environment. It can simply pass in the fully evaluated GO* values.
Finally, make the change I was actually here to make: propagate
GOMODCACHE and use it where appropriate.
Fixesgolang/go#39761.
Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Correct file URLs should have three slashes: two for the protocol
(file://) and one for the beginning of the path. Perhaps unsurprisingly,
VS Code is sending us URLs with only two. Add the third.
Fixesgolang/go#39789
Change-Id: I922b3adf1d5980991a43229d952d77fecaf1366b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239743
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The spec states error data may be omitted. It is currently always encoded
as null, despite having no usage.
Omit the field if empty, and add a test to prove the behaviour.
Fixesgolang/go#39736
Change-Id: Icdb39409010f3a42f84d2372c2061e4bc7cc198e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239059
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Getting the right indentation for the text in the AST proves to be a
little complicated. The most reasonable approach seems to be writing
out the AST, getting the lines with the struct definition on it,
and trimming whitespace to get the current indent. Then, we add this
indent to the struct fields in the new text.
Change-Id: I1cc3421d95edae61cfb662254ff3fb759b5c487f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239199
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change separates out different functions of mod handles.
Previously, we had ModHandle and ModTidyHandle. ModHandle was used to
parse go.mod files and get the results of `go mod why` and possible
dependency upgrades.
Now, we factor this out into 4 handles: ParseModHandle, ModWhyHandle,
ModUpgradeHandle, and ModTidyHandle. This allows each handle to be
specific to its own functionality. It also simplifies the code a bit,
as the handles can be written in terms of ParseModHandles instead of
FileHandles.
I may have some follow-up CLs to refactor the `go mod tidy` logic out of
the cache package, though I'm no longer certain that that's a good
choice.
Change-Id: I8e12299dfdda7bb61b05903d9aa474461d7f4836
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
These are commands whose changes should be reflected in the existing
go.mod file, as they do not provide edits. Add a third way of running
the go command, explicitly without -modfile. Update the regression test
accordingly.
Change-Id: I866b5db83b504fae190e58c306c01a7a4296672d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The return of IgnoreFile. We continue using go list's ignore rules,
but only apply them to the part of the path underneath the source root.
This should be fixed to include replace targets once we have them.
Change-Id: I054fee8c12fc860a279b0d0c1fd670f44d78a63f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239288
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Marshalling an empty raw message produces a null rather than omitting the field
Instead we do not set the field unless there is no error.
Includes a test that reproduced the issue before the fix.
Fixesgolang/go#39719
Change-Id: I683b8ea55d3425613fc956993280ff51202f49fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239097
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The current implementation writes strings to the diagnostic instead
of creating new AST nodes.
Change-Id: Ibb37a93a3c43fb74ec5dc687091601e055c6e1b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238198
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Piped go commands cannot run concurrently, as a user may be looking at
the stdout/stderr, and we don't want to show any load concurrency
errors. We use a semaphore with a maximum number of in-flight go
commands to make sure that all running go commands have completed before
starting a piped command.
Change-Id: Ie027d9ff704fb7dd9640da06569345e89ed7a012
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238059
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The per-package stats have proven pretty useful, and I don't want to
have to teach users how to save them. Create zip files and add them in.
Since some users may be sensitive about revealing any information about
the code, generate two variants: one with package names, and one
without.
Change-Id: Icc5631b4cebbbabfdd2fcea4a4cdf4f205dbcab9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239037
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
I've forgotten to specify scheme a few times, and gopls is quiet
about invalid ocagent addresses.
Add an example to the CLI help output aid the users with the format
implicitly.
Change-Id: I89ba8a16c18dabff2b4b87b2235755c22ae71117
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239057
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Placing the cursor on a switch statement or corresponding break and call
document.Highlight will now highlight them.
Fixesgolang/go#39275
Change-Id: Ib7e3ba0c6e78141ed3dd37cfd3b72b567b857247
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238478
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The current implementation has a number of helper functions that have to
thread through context, errors, converters, etc. Simplify the code by
factoring out the conversion to a protocol range into a final step.
There are probably further improvements that can be made too.
As part of debugging, I noticed that the cmdtest server does not get
shutdown once tests finish, which leads to excessive logging. Is closing
the test server connection enough to trigger shutdowns? It doesn't seem
to be.
Change-Id: Ia67666f6b8debccd120795bb24bd089620c0a87b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238377
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Several fixes related to GOPRIVATE handling and links:
+ In Go source, fix links matching GOPRIVATE for external modules.
Previously, in these cases we'd try to match <mod>@v1.2.3/<suffix>,
which wasn't the correct input into the GOPRIVATE matching algorithm.
+ Similarly check GOPRIVATE for go.mod require statement hovers.
+ Likewise, for documentLink requests (both mod and source).
+ Move the existing hover regtest to link_test.go, and expand to cover
all these cases.
Along the way, I encountered a couple apparent bugs, which I fixed:
+ Correctly handle the case where there is only one require in a go.mod
file. This was exercised by the regtest, so took some debugging.
+ Only format links [like](this) if the requested format is actually
markdown.
Fixesgolang/go#36998
Change-Id: I92011821f646f2a7449dcca619483f83bdeb54b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238029
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
closerList was already unnecessarily abstract, and tracking connections
will allow us to also wait for all connections to finish.
Share functionality by embedding this type in PipeServer, TCPServer.
Change-Id: Ib2cb2157c1477f904bc278aa91902f5c633afe13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238547
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
To prevent misleading errors from outstanding go command invocations at
test completion, properly shutdown the LSP connection before cleaning up
exported files.
Change-Id: I9ad175060fefc5b914e544c5f58b9b6658405edc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238546
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While working in this code, I was confused by the fact that modHandle
and modData were used for both ModHandle and ModTidyHandle. Separate out
the two so it's a little more clear. This is mostly a copy-paste, with a
few fields renamed to simplify them.
Change-Id: I20ea11c32a624fd250eabf901d09ebb05fbad062
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238337
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Refactor internal/lsp/cache to use a new temporary go.mod file for each
go command invocation. This cleans up the abstraction in the source
package, as we no longer are aware of temporary go.mod files.
This will also fix the raciness of reusing the same temporary go.mod
file for each invocation.
Updates golang/go#37318.
Fixesgolang/go#39504.
Change-Id: I90bc17a678b5df222ab598c8f7dbf6c6fdd393f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Revert https://golang.org/cl/234480, which was unnecessary, and move it
to a more appropriate file.
Change-Id: I3f5a3eccaf0ffe324fee8e27945a2e5ece2ff12c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238597
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When a workspace package is deleted, we need to stop trying to load it.
Check that every workspace package still has at least one .go file when
we copy them between snapshots.
I think this is correct, but even if it's not, orphaned file loading
should patch it up.
Fixesgolang/go#38977.
Change-Id: I0b11010a40aac09f619f54b5ba02e2467b15a36c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238028
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Bad go/packages results were being cached, causing unexpected behavior
when new x tests are created. This fix also allows us to enable a
regression test that had been previously disabled.
Change-Id: I93c05df2f4d32c6c8a43e9f5aaeeb30bc4a32f3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238058
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In GOPATH mode, you can import example.com/foo even if it has a go.mod
that says its path is example.com/foo/v2. Adjust our import path
resolution to support that case.
Fixesgolang/go#39560.
Change-Id: I758a2220c579c4374084365c3a78a3a2bbd14b01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238260
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In a case where the type info for an ast.CompositeLit isn't found in
forEachLexicalRef(...) gopls will panic.
e.g. trying to rename "foo" where it is declared in this case:
func fn() {
var foo bool
make(map[string]bool
if foo {
}
}
Note the missing ')' after make.
This change will skip ast.CompositeLits if the type can't be found, and
that fixes the panic. But it also do rename the identifier as long as it
is possible, and I'm not convinced that we should allow rename at all if
the source can't be compiled.
Updates golang/go#39614
Change-Id: Ibb50b15ce4b31056f2f1da52a4dcab7b8b91a320
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238042
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Not sure how these got submitted, I thought I fixed them.
Change-Id: I2ffd4b838d37a110d3b2d9b44c31ef7aa3056a91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238030
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Return an actual error when closing the test server, and close at the
jsonrpc2.Conn level, rather than at lower levels.
Change-Id: I0bc153f40ced5dfee9ac26d3ffd666e446b812e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238197
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Add a regtest to verify that GOPRIVATE identifiers are not given a link
to pkg.go.dev. For efficiency, as well as to exercise dynamic
configuration, do all this in a single regtest.
Updates golang/go#36998
Change-Id: I9102a11312db5c334fdbd30cce9ca2d2e19e9ac2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237938
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The gopls workspace environment defaults to the process environment in
which gopls was started. This means that when switching environments,
gopls can potentially get a different environment when connecting as an
editor sidecar from when forwarding requests to the daemon.
To (hopefully mostly) mitigate this pain point, inject the Go
environment when forwarding the 'initialize' request, which contains
InitializationOptions containing the 'env' configuration. We could go
further and send the entire os.Environ(), but that seems problematic
both in its unbounded nature, and because in many cases the user may not
actually want to send their process env over the wire. Gopls behavior
should *mostly* be parameterized by gopls binary and Go env, and after
this change these should match for forwarder and daemon.
For go1.15, Explicitly set the GOMODCACHE environment variable in the
regtest sandbox. Without this, regtests were failing in the forwarded
environment because they implicitly shared a module cache.
Fixesgolang/go#37830
Change-Id: Ic1b335506f8b481505eac9f74c0df6293dc07158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234109
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Limiting the Config to the view seems reasonable, considering that it is
only used to run the `go` command. I prefer just having the cache run
go commands, so that source doesn't have to deal with the environment.
This also enables CL 237517.
Change-Id: I639c082592de30e9682dc25cdd12c7751ddb4f97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We should always go through the runner to avoid load serialization
errors. This change also updates the comments and uses pointers to
Invocations consistently. This causes a few updates throughout x/tools.
This change also results in a few modifications to the go:generate code
lenses, as they operate on directories rather than file URIs.
Change-Id: I6306e761e68dfdd23f1b410e44aab0ffa85d234c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237685
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Now that fillstruct is an analyzer, we can simplify the code that calls
it in code_action.go. We introduce a new class of analyzer --
convenience analyzers, which are closer to commands. These represent
suggestions that won't necessarily improve the quality or correctness of
your code, but they offer small helper functions for the user.
This CL also combines the refactor rewrite tests with the suggested fix
tests, since they are effectively the same.
For now, we only support convenience analyzers when a code action was
requested on the same line as the fix. I'm not sure how to otherwise
handle this without bothering the user with unnecessary diagnostics.
Change-Id: I7f0aa198b5ee9964a907d709bae6380093d4ef21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237687
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were marking normal tests as workspace packages, but pruning x_tests.
Don't do that.
Fixesgolang/go#39578.
Change-Id: Ia8eceea38e63ac17e40f50d6837de9c7a668469b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237943
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: zikaeroh <zikaeroh@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The go command ignores go files in specially-named directories like
testdata. We probably still want to try to offer services like
autocomplete and jump-to-definition there, but diagnostics are less
likely to be helpful. As a compromise, just squash diagnostics.
Note that the rules we use are slightly wrong; see the comment on
ignoreFiles for details.
Fixesgolang/go#39563.
Change-Id: I0bc00639e68bd71ea55d867af36e07ef4ec780a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237638
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A user that's editing packages in their workspace may introduce parse
errors (missing package statement), but we still want to be able to add
imports for them. Continue past parse errors.
Fixesgolang/go#39315.
Change-Id: I3bbf428e7b9ef32a87258af2dafbe0d7b86b7348
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237686
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The current implementation of the fill struct tool is not a part of
the analysis framework. This commit moves the functionality from the
source directory to the analysis directory.
Change-Id: Ibe37b57f3e6680c8729932dbbe010a4642600e4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237258
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Calculate and display very crude memory usage statistics. This is
complicated by various levels of sharing and indirection, so the numbers
should be taken with *large* grains of salt and interpreted mostly by
experts.
Still, the results are interesting and helpful.
Change-Id: Ia9aff974c7d5fddd63df0cfd5cecc08ead33cf84
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236163
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
ModHandle races with the initial workspace load if the go.mod file does
not yet exist. We should await for the initial workspace load to
complete before proceeding with update codelenses, etc.
Part of trying to figure out the flakes in golang/go#39504.
Also a few staticcheck fixes, and fix the Windows line endings in
fill_struct.go, because `git gofmt` complains.
Change-Id: Ide21a47137390792d1afb924740cff0bb6f0b764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The error messages from view cancellation clutter up the logs when
testing, especially if you're running a single subtest.
A few quick staticcheck fixes in the CL also.
Change-Id: Ia1ed5360ac754023c589ed526ec0ed3e94a79b2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The cache does not know the sessions associated with it, so the
debug template cannot display them.
Change-Id: I9b043592351ddce097f4e74f3e9aa7a6ad24e613
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237618
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Ignore ignored the builtin package and files that start with _. The
latter should already be ignored by "go list". The former seems
like too much effort to me. People shouldn't edit random parts of the
stdlib, and ignoring changes to (e.g.) the Error interface seems like
the least of the trouble they can get themselves into.
Remove it for now. If we get complains I'll re-add it, probably by
rejecting the write entirely somewhere.
We incidentally relied on this in the identifier functions; change those
to treat the builtin package slightly more specially.
Change-Id: I005b02a66b1a987c50a3074d53a2d28ff07d3324
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237597
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When you write a test in (say) the fmt package, you get a test variant
augmented with the test files. In many cases you also get test variants
of the things the fmt package depends on. The primary test variant,
(fmt [fmt.test]) is interesting to us, because it contains the tests.
But the intermediate variants (testing [fmt.test]) aren't -- the user
can only get to them indirectly. We certainly don't need to fully parse
them.
Treat intermediate test variants as non-workspace packages. This doesn't
accomplish much yet but paves the way for later optimizations.
Updates golang/go#36943.
Change-Id: I1a20abcd2d67767f07132a75a20f098be6f19a76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236397
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We use file identities pervasively throughout gopls. Prior to this
change, the identity is the modification date of an unopened file, or
the hash of an opened file. That means that opening a file changes its
identity, which causes unnecessary churn in the cache.
Unfortunately, there isn't an easy way to fix this. Changing the
cache key to something else, such as the modification time, means that
we won't unify cache entries if a change is made and then undone. The
approach here is to read files eagerly in GetFile, so that we know their
hashes immediately. That resolves the churn, but means that we do a ton
of file IO at startup.
Incidental changes:
Remove the FileSystem interface; there was only one implementation and
it added a fair amount of cruft. We have many other places that assume
os.Stat and such work.
Add direct accessors to FileHandle for URI, Kind, and Version. Most uses
of (FileHandle).Identity were for stuff that we derive solely from the
URI, and this helped me disentangle them. It is a *ton* of churn,
though. I can revert it if you want.
Change-Id: Ia2133bc527f71daf81c9d674951726a232ca5bc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237037
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The shared server was executing on ctx.Background(), which meant it
didn't have a debug.Instance. This resulted in logs being printed to
stderr, due to the fallback behavior of the global exporter.
Fixesgolang/go#39130
Change-Id: Ibb968f78d69752452bec71a7abeff808b1cccb04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237583
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Even on successful shutdown, test logs contain a lot of "failed reading
header line: EOF". This can be distracting, or worse, misleading. Do our
best to suppress these error logs.
Updates golang/go#39130
Change-Id: I6ebe61100501f69c7490b418f53871b4e9704a00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237582
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Use the -v flag to control whether RPC logs are always printed, rather
than a regtest specific flag.
Updates golang/go#39130
Change-Id: Ie6293815adee4b59defd80cfc015838cfbf2b3e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235920
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
The only substantial change is data types for CallHeirarchy.
util.ts has changed the hash, and adapted to a new source layout, plus
the usual pointless whitespace changes. code.ts has learned a little more
about typescript ASTs.
Change-Id: I9cb3a9a9034d46f4a479123779da3bb3474e4a42
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237377
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The existing implementation looks for matching types in the declaration
and the leftmost value in the return statement. Instead, the analyzer
now searches across all the values in the return statement to find one
that matches the type in the declaration. Additionally, if a value in
the return statement does not match any type in the declaration, it is
not overriden.
Change-Id: I4d4aed0ef67e59bfd886b44055d523a8c478255c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236962
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This uses log messages to convey information to the debug system, which
has the benefit of logging even if the debug pages are not active and
also not requiring systems to reach into the debug system or require
extra lifetime tracking Not all things are decoupled yet as there are a
couple of places (notably the handshaker) that read information out of
the debug system.
Change-Id: Iec1f81c34ab3b11b3e3d6e6eb39b98ee5ed0d849
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236337
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This function will skip tests in test environments where
the go command can't be used to build and run binaries.
This will be used by the test in golang.org/cl/236758
Change-Id: Ie61e8890084179b0e999dd377148693395043191
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236920
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When document.Highlight is called with the cursor on a loop statement or
branch statement, gopls doesn't look for labels. Placing the cursor at
the break statement below highlights the inner for loop:
Outer:
for {
for {
break Outer
}
}
By making highlight label aware, and ensure that unlabeled "break" in
"switch"/"select" doesn't highlight the outer loop, this change fixes
loop highlighting.
Adding support for highlight of "switch" and "select" will be handled in
a separate CL.
Updates golang/go#39275
Change-Id: I7014aa7b0dfb1da871863ced611623be995f3944
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236524
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This moves as much code outside the protocol generator
as possible making it easier to maintain both the code
and the generator.
Change-Id: I7fe932a58facece5bb0bd5a9c158e5cc7d5a277b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236838
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
regtests can use Await to wait for diagnostic expectations. But sometimes
it is useful (or more robust) to then look at the specific diagnostics.
This change introduces env.DiagnosticsFor, which returns the current
diagnostics for a file.
Change-Id: Iea35d28f6679289795bc853f156aae351279b205
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236837
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This removes the interfaces and the debug structs in the lsprpc package
in favour of just having the debug structs in the debug package.
Change-Id: I67541688444f4ef367007740c5446dcd7be6575f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236198
Reviewed-by: Robert Findley <rfindley@google.com>
This removes all the cache/session/view hooks from the lsp and instead
uses normal introspection from the client to find all the entities
rather than keeping shadow copies of the object graph in the debug page
handler.
This required the addition only of the ability to view the sessions open
on a cache and exposing the unique identifier for all the objects, both
of which are useful and reasonable things to have in the API anyway.
Change-Id: Ic19903e7b19832ca79290954ec373d4177789742
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236197
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This removes a TODO in the tests and has them check for shutdown
correctly.
This also required adding a Close to editor that does the Shutdown/Exit
sequence rather than just Shutdown.
Change-Id: I6433b76eb2dd1fe40b2332685fdf25ebc4fd4577
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236717
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The existing implementation contains strict equality rules for
comparing the return type in the function declaration against
the return types in the function body. This causes the code in the
return statement to be incorrectly overriden with its corresponding
zero value type by the fillreturns analyzer.
Fixesgolang/go#38707
Change-Id: I07522aaf85f2a5f811a86cbc0951ae61035ff55e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236617
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In addition to adding a `go mod vendor` command option, which can be
exposed via an editor client frontend, we show a suggestion to users who
experience the "inconsistent vendoring" error message.
The main change made here is that we save the view initialization error,
and we return it if the view has absolutely no metadata. This seems
reasonable enough, but my fear is that it may lead to us showing
outdated error messages. I will spend some time improving the handling
of initialization errors in follow-up CLs.
Updates golang/go#39100
Change-Id: Iba21fb3fbfa4bca956fdf63736b397c47fc7ae44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235619
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
It was directly generating messages and sending them on the conn, now it
just uses an editor method like all the other tests.
It was also broken because it never opened the file it was hovering in, so I am
not sure it was testing anything useful before.
Change-Id: I7a1b444015c95c82a0a137d3bb1da661ed9331af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232983
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Really the name is wrong now, but this is just a stepping stone towards removing
it entirely in favour of a new listener/dialer/server/client pattern, so I am
minimizing the churn by leaving the names alone for now.
Change-Id: I771d117490763ebe05ed2a8c52d490deeb4d5333
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232878
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This will allow varying implementations and wrappers, and more
closely matches the concepts used in the net library.
Change-Id: I4be4c6efb3def0eda2693f482cbb0c6f776e5642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232877
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When testing Highlight the highlight count is checked against expected
number of highlights. If it doesn't match t.Errorf(...) is called and
the test continues.
A few lines below the test ranges over results using the index for both
result and expected result leading to a panic if there are less then
expected highlights.
This change fails fast with t.Fatalf(...) instead to avoid the panic.
Change-Id: I33d0973f3145c307d9084d037ffbb73b244a3acb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236099
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, workdone reporting was managed by the goroutine(s) that
compute diagnostics following a file change. The intention was to signal
when diagnostics resulting from file changes were complete.
This was buggy, in two ways:
+ When no snapshots are determined to require diagnosis, no work is
reported.
+ If multiple snapshots required diagnosis, we'd get multiple work IDs.
Fix this by lifting up the 'work' to the level of didModifyFiles.
Change-Id: I73cf8bf9946dba777650bba9d4d18dad6954620e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235738
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Exit now closes the connection rather than exiting the process.
This allows things to shutdown gracefully, and removes special
cases. It also allows the tests to call CloseEditor instead of
just Shutdown, which prevents goroutine leaks.
Change-Id: I26121ba5d393ef74ce0e912611c8b3817e3691ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231798
Reviewed-by: Robert Findley <rfindley@google.com>
This allows us to rely on higher level functionality like timeouts and
close cancelling pending reads cleanly.
Change-Id: I3a43d21ed35d3da1eb818ea22f8d02201488a1d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230464
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Also the ability to wait for them to correctly close.
Change-Id: I198c9e24a21c04d5c05bae7a4a0f503429ab0346
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231699
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>
This test partially reproduces some strange behavior with creating
new tests files. In particular, it creates a new x test in a package
that already has a test variant and adds content with a missing import.
In the test, the import is never added. However, in my own experience
debugging this in VS Code, I see the import get added but the diagnostic
never get removed. One thing at a time though...
Updates golang/go#39315
Change-Id: I724a145688b915d04abd1f21efc6f9a7506be043
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235581
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When a file with errors is removed outside the editor, sometimes its
errors are cleared by the editor and sometimes they are not. If the file
is still open in the editor gopls does not clear the errors, taking the
editor's version as the truth. Otherwise the errors are cleared.
(This behavior depends on the editor sending gopls a notification that
the workspace changed.)
There seems to be no good way yet to test that gopls takes no action after
receiving the didChangeWatchedFiles notification.
Updates golang/go#38878
Change-Id: Ie418dd786d4c5f827cf0665a31f0f9913f7cfdc0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235377
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Now that we're not using build tags any more we can consolidate files.
Do so.
I tried a little to find good places for the moved code, but only a
little.
Change-Id: I6b66afb7cad870471d7d4a3d86c13fadb94a40e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235457
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Many tool features, particularly modules-related, require particular Go
versions. Build tags are unwieldy, requiring one-off test files which
break up test organization.
Add a suite of testenv functions that check what Go version is in use.
Note that this is the logical Go version, as denoted by the release
tags; it should be updated at the beginning of the release cycle per
issue golang/go#38704.
For ease of reviewing, I'll merge/delete files in a followup CL.
Change-Id: Id85ce0f83387b3c45d68465161cf88447325d4f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234882
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Cached packages are probably more relevant than uncached packages, but
we still need to go in relevance order, since we'll stop adding results
after we hit the cap.
Fixesgolang/go#38461. (Hopefully.)
Change-Id: I555dd5f7568baa8d69760ed5836341a474e94346
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231619
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I made a silly mistake and checked the prefix on the import path rather
than the package name, which obviously breaks everything other than
top-level stdlib packages.
Fix that, then tweak the ranking a bit. We now get deep completions, which
is nice, but filled up the results too fast. Now instead of 5 results of
any kind, we give up after 5 packages searched.
Change-Id: I15b293f68f17531077da9ffe791a38ccc0e129f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231617
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Also switched the internals of the stream implementations to using
net.Conn to enable asynchronous closing, not yet exposed int the API.
Change-Id: I57f1c36e7a46729d24f4339ba2fecc3f868e823f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231698
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
This adds the ability to make a net.Conn based on a
reader/writer pair.
This is primarily useful for pretending stdin/stdout are a network
connection, but can be used for any reader/writer pair.
Change-Id: Ib398e62e0fac423db04aed27cfc48070bda2b93b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231697
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This adds a package for dealing with stacks in tests.
The only function at this time is NoLeak which verifies that a test
does not leak any goroutines, and prints a stack summary if it does.
Change-Id: I284b846f422334b745bef0e96d84138295120a62
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232681
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This can be used either to directly parse runtime.Stack output or
process text that includes stack dumps, like test timeouts or panics.
It includes a binary, gostacks that processes stdin to stdout replacing
stack dumps in place.
Change-Id: Id7b1cfd69b8aea36c66f12ec0bdf38b68cba5afb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232658
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
One line legal code like `package x; import "os"; func f() {}` was
being misformatted. In these cases the parse flag ImportsOnly loses
important parts of the code, while full parsing works. Presumably
all these cases are short enough that there is no appreciable penalty
from the extra parsing.
Fixes https://github.com/golang/go/issues/36824
Change-Id: I9a4581d67c590578f8fdea5ed2a2a58e0bc3c40b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234900
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Windows does actually support symlinks, but older versions of
Windows only support symlinks when running as an administrator.
Newer versions of Windows support symlinks for all users.
Instead of skipping based on GOOS, first try the Symlink operation.
If it succeeds, we can proceed with the test; otherwise, we can try to
write a regular file to determine whether the problem was the symlink
operation itself or the destination path.
For golang/go#38772
Change-Id: Idaa9592011473de7f514b889859e420a84db6d01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234537
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Switching to using a t.Skip means we are more likely to remember to
actually re-enable the test at some point.
Also picked up a staticcheck fix along the way.
Change-Id: I382eaa8d204bee74a7ff46e8a1b11dab567b83ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Assigning a slice to the appendage of itself is common and tedious
enough to warrant a special case completion candidate. We now offer
smarter "append()" candidates:
var foo []int
foo = app<> // offer "append(foo, <>)"
fo<> // offer "foo = append(foo, <>)"
The latter is only offered if the best completion candidate is a
slice. It is inserted as the second-best candidate because it seems
impossible to avoid annoying false positives if it is ranked first.
I added a new debug option to disable literal completions. This was to
clean up some test logic that was disabling snippets for all tests
just to defeat literal completions. My tests were failing mysteriously
due to having snippets disabled, and it was hard to figure out why.
Change-Id: I3e8313e00a1409840cb99d5d71c593435a7aeb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221777
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change introduces Module and ModuleError struct types to the
packages package with the same types as defined in the cmd/go
documentation for module information output by go list (with the
exception of the Module type's Versions and Update fields).
go/packages will fill the module struct with the module information
output by go list. Drivers that support modules can also provide
module information by filling the Module fields in the packages in
their driverResponses.
Fixesgolang/go#35921
Change-Id: Icbdf79869f09d26f6a01c3670146ace4f6ffa25e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234219
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
fake.Editor.Server was exported, but CL 233117 was rebased on top while
still using the unexported field.
Update the rebased code.
Change-Id: Ie8f1f2c3948a1122b9e30a843cc33c79ef623ea4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234494
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
The code was introducting syntax errors for some edge cases (example in
regtest/import_test.go), and I found it hard to follow.
The new code passes all the tests. There are new regtests to guarantee
no CodeActions are returned for some cases that vim testing noticed.
Change-Id: Ia09da667f74673c11bfe185e4f76a76c66940105
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233117
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The priorities for which comment to show should be 1) documentation
directly above the var/const, 2) documentation for the var/const block,
3) line comments.
See https://github.com/microsoft/vscode-go/issues/3240.
Change-Id: Ie136f0f25ac8208147070682bb1f3a663d6da25f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234101
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Something is making this test deterministically fail in some
environments, such as @bcmills' desktop.
Skip it while I build go at tip and debug.
Updates golang/go#39135
Change-Id: I1bf8c55c5cfca471a904de85936f504313094807
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234480
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Many tools test check for the ability to compile cgo programs.
Consolidate them all into testenv.NeedsTool("cgo").
Change-Id: I62c96e7b4dc72df34b8fdbf10326c7d19e0613e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234108
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now that we support authoring cgo packages better, we need a way to
regenerate the C definitions. Doing it automatically is very difficult;
in particular, referencing a new symbol from the C package may require
regeneration, but we don't want to do that for every typo.
For now, give the user a button and make them push it. We attach a
code lens to the import "C" line. This is vulnerable to the usual
user-didn't-save glitches.
Updates golang/go#35721.
Change-Id: Iaa3540a9a12bbd8705e7f0e43ad0be1b22e87067
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234103
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
An import path like "foo/bar.v1" is still a local path, not an external
package, and should be grouped as such.
Change-Id: I06be3c01076f616a3cdc8e23bc9c056643035ad1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234111
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The format of descriptions for the `-remote.*` flags were inconsistent.
Clean them up.
Also remove a TODO in lsprpc.go about adding a test for telemetry. That
is enough of a separate concern (and one that is rapidly changing) that
I no longer think this TODO makes sense.
Change-Id: Id14796000634ae4be4b480d0386b1da9069737c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234113
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Currently, our hover text by default links point to public documentation
sites (e.g. pkg.go.dev). This doesn't make sense for private repos, so
hide the hovertext link when the import path matches GOPRIVATE.
Implementing this was a little messy. To be optimal I had to thread
the value of goprivate through cache.view, and to be correct I had to
duplicate some code from cmd/go internal.
Regtest will follow after https://golang.org/cl/232983 is submitted.
Updates golang/go#36998
Change-Id: I1e556471bf919fea30132d9642426a08fdb7f434
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233524
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In CL 229779 I enabled Cgo type checking for go/packages, but we don't
actually type check there. We need to enable it in our own type checking
too.
No test updates because the negative effects are relatively subtle and
caught by an upcoming regtest.
Change-Id: I31691d69eb104cdabfd4fbe0a14b1f3c9741eabb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234102
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When on 1.15+, enable TypecheckCgo. This improves cgo support
significantly, but we'll still have trouble with newly-referenced C
identifiers and changes to the magic comment.
Updates golang/go#35721.
Change-Id: I44dc95ce2d91d552e1e66e3722dc4230ab59fedd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229779
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TestBadGOPATH tests for a empty element in the GOPATH list using
"GOPATH=:/path/to/gopath", assuming that ':' is the path list
separator. On Windows the test fails, because Windows path list
separator is ';'. Therefore this test isn't built for Windows.
On Plan 9, os.PathListSeparator is '\000', so the test fails
there too, and should not be built for Plan 9.
Change-Id: Icdc2f8de098c1415103ec6124906ad6c578ad183
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233718
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's in the build tagged tests because the fix is only in 1.14.
Fixesgolang/go#36960
Change-Id: I606dfb4f73735c6db8c48f67f19720d340be8361
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232991
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When run in CLI, "DocumentSymbol()" returns "[]protocol.DocumentSymbol"
or "[]protocol.SymbolInformation", so need to handle that as well.
Change-Id: I7885d3c53899103553df57f7f0ceceb2a33ec021
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232557
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In a later CL we include the fully qualified path to a symbol in the
Name field of SymbolInformation. This means that we end up with
matches like:
golang.org/x/tools/internal/lsp/workspacesymbol/b.WorkspaceSymbolVariableB
A fuzzy match against this name using the query "wsym" would match the
"workspacesymbol" of the import path as well as the
"WorkspaceSymbolVariableB" that is the symbol name itself.
Therefore we rename the symbols in the:
internal/lsp/testdata/lsp/primarymod/workspacesymbol/...
from WorkspaceSymbol* to RandomGopher*, which allows our fuzzy matches
to be more precise.
Change-Id: Idbeb663f5750cae4835b0fdaa77531e30353fb89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228759
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
In preparation for later changes to the implementation of the workspace
Symbol method, we add the Symbol method to fake.Editor. This requires
the definition of a number of associated fake types (editor-friendly,
byte-offset-based versions of protocol UTF16-based types) for example
fake.SymbolInformation and the types it references.
We also implement a basic regtest for the Symbol method, exposing Symbol
on regtest.Env like other LSP server methods. To aid with the writing of
Symbol result assertions, we provide some helper functions to simplify
the process of defining matches that are evaluated against the result
set.
Change-Id: If73b493e1e791c8201423a303af8041f5a15ccfc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228121
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
In preparation for later changes to the workspace Symbol method, we add
a separate configuration option keyed by "symbolMatcher" that specifies
the type of matcher to use for workspace symbol requests. We also define
a new type SymbolMatcher, the type of this new option. We require
SymbolMatcher to be a separate type from Matcher because a later CL adds
a type of symbol matcher that does not make sense in the context of
other uses of Matcher, e.g. completion.
Change-Id: Icde7d270b9efb64444f35675a8d0b48ad3b8b3dd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228122
Reviewed-by: Robert Findley <rfindley@google.com>
