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>
When SWIG is used in a package, the returned "(*package.Package)" from
"packages.Load" contains C++ sources in "CompiledGoFiles". This adds a
skipped failing test case that reproduces this issue.
Updates golang/go#37098.
Change-Id: I56a875c6ad9d18508413b00c34d6b6ac0b116fda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228637
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
In the case of an invalid package name (the package name is a keyword),
`go list` doesn't report any filenames associated with the package.
As we have done previously, we can parse these out of the error message.
However, it seems like, in this case, the filename is in the error
string itself, not the error position.
Updates golang/go#39986
Updates golang/go#39763
Change-Id: Id9c68a93ac4f545e4e2152540ca85b92f1df4640
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244028
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
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>
Unmarshal package checks whether `Unmarshal` and(or) `Decode` call of `encoding/json`, `encoding/xml` and `encoding/gob` package is valid or not. However, it didn't check `encoding/asn1` package's one.
This change makes it check whether `asn1.Unmarshal` call is valid or not as well.
Change-Id: If32d243f3a82ebb5ca3ca80657f4616e5daced2f
GitHub-Last-Rev: e61a96941ee64d7d141e635b374fe07104751283
GitHub-Pull-Request: golang/tools#243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243397
Reviewed-by: Michael Matloob <matloob@golang.org>
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>
