Progress reporting has three different kinds, "begin", "report" and
"end". Each of them comes with a text message attached.
The "report" kind is sometimes raw output, for example from go test, so
the output could end with a newline.
This change trims "report" messages to ensure consistency with "begin"
and "end" that doesn't end with a newline.
Change-Id: I779dc41fdf3f6281d9d44f64e9cee9bbcde0b4cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266078
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Daniel Martí <mvdan@mvdan.cc>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The "go test" command invoked via code action/code lens performs a
ShowMessage callback to the client when the test is done.
Previously it did set severity to "Error" when the test failed, but a
failing test isn't a error condition per se. This changes the result to
be of severity Info for both successful and failed test invocations.
Change-Id: Ib76558d98a434c706823617b9901a88e53864319
Reviewed-on: https://go-review.googlesource.com/c/tools/+/269257
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Running `staticcheck ./internal/lsp/...` turned up a few instances of
dead code. Remove them.
Change-Id: Ic53db59cd0959fad960b086158eb550a624a4d91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268580
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In practice, we only support the go command, and now that we rely on the
private packagesinternal APIs to set -mod and -modfile, GOPACKAGESDRIVER
is actively broken. Forcibly disable it.
Change-Id: I91f8c0d29fada2fe87ad9fdfec6ba8c5504c80cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268977
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
moduleLoadScope triggers a load of many packages, just like
viewLoadScope, and should not log each loaded package.
Change-Id: Ifd59b41a496da8eb57421f948b9327519e82d2ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268538
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add a regtest run option NestWorkdir, which causes the working directory
to be nested in the editor workspace, and use it in some modfile tests
to exercise the new workspace logic.
For now we only run the nested tests while using experimental workspace
modules. In a later CL, the 'legacy' mode will be updated to find a
solitary nested module, at which point we should be able to run nested
in all modes.
Fixesgolang/go#42111
Change-Id: I0bd3b31969684bc1ba1935633cbb9a3f26de1587
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257138
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
This CL adds a copy of the internal error codes added to go/types in
https://golang.org/cl/264179. In a subsequent CL, we will use these to
extract the unexported error code from go/types errors via reflection.
A generated String method is added to provide a human-readable code for
the user.
For golang/go#42290
Change-Id: I280c9b111598426dce4eef38b9979919ed590068
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267939
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This fixes an issue where we were reloading packages in the vendor
directory when they changed on-disk. We should only do this if the
packages are part of the workspace or the files are opened.
Change-Id: Iefbd690ec0c096d9a40c62ce567c18335024ea15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268038
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The server reports its version to the client--add some structure to this
report so that the client can parse and use it.
Fixesgolang/go#42171
Change-Id: I00bff3615391cbeede89e4be6b0d028fed67989e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266198
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
In CL 259998 we added the ability to find calls to methods through
interfaces. That included very common interfaces like Stringer, which we
judged unfortunate but possibly acceptable. We didn't consider the
behavior when searching for references to a type. When searching for
references to a type that happens to be a Stringer, the user almost
certainly doesn't want to see all uses of the Stringer interface. Don't
consider interface references to types.
No tests; I don't think this is worth a regtest and the marker tests
can't check a negative AFAIK.
Fixesgolang/go#42350.
Change-Id: I0b929d8743f7f0b4e7543e8d35921a7cf3784bf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268462
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
If an open buffer is unmodified from the on-disk version, and that
on-disk content changes, VS Code will update the buffer with the change.
This is relevant for our quick fixes that use the go command to make
changes to go.mod/sum -- we want tests to pick up the changes without
needing to close/reopen the buffer.
For whatever reason, VS Code doesn't do this for deletions, and it made
the implementation easier, so I followed suit. I also mimicked its
behavior of sending both in-memory and on-disk change notifications.
Change-Id: I838a64b2f48f3cbe1a86035293923951b53aecf3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267577
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Also, change withAnalysis to includesAnalysis to make the name a bit
more accurate.
Fixesgolang/go#42363
Change-Id: If50f7d36b953e6627ed9ba049357a2b86dc3f676
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267817
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
In CL 267717, the error message for invalid assignment of untyped values
is being corrected from 'cannot convert ...' to 'cannot use ... as ...
int ...'. This breaks the go/loader tests due to an assertion on the
former message.
Since the Go repo TryBots run x/tools tests, this assertion must be
loosened to allow merging the CL above.
Change-Id: I8a371397c9df58109090b7fd2eaa5b4a600776ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267686
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Per our discussion, it's too slow for a save hook.
Fixesgolang/go#38209. (for real this time?)
Change-Id: I264c6d1590a374eff09b09cb1a9162e8e5ff2dc7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267682
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When adding a require, we should add an `// indirect` comment if that's
what go mod tidy would do.
It's possible I should split Add out from Update and Remove, but this
was quick and easy and I'm not too worried about it for now.
Also minimize the test that covered this case, which was way more
complicated than it needed to be AFAICT.
Fixesgolang/go#38914.
Change-Id: I89c44f8573873227c4c9e637d1d31d8c1a6530aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267578
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Editors need a way to run commands in the same workspace that gopls
sees. Longer term, we need a good solution for this that supports
multiple workspace folders, but for now just write the first folder's
workspace to a deterministic location:
$TMPDIR/gopls-<client PID>.workspace.
Using the client-provided PID allows this mechanism to work even for
multi-session daemons.
Along the way, simplify the snapshot reinitialization logic a bit.
Fixesgolang/go#42126
Change-Id: I5b9f454fcf1a1a8fa49a4b0a122e55e762d398b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/264618
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
The logic that checked if a file was already being watched by the
default glob pattern was incorrect. Fix it, and use the newly added
InDir function.
Change-Id: Ia7e3851ab5b9fa1fa7590cae3b370676201a9141
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267119
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This avoid an embedded URI defaulting to ""
(https://github.com/golang/go/issues/42314)
Fixes:Fixes golang/go#42314
Change-Id: I614171d4ec80d139f5511b953d30f8385c2c7d5e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267106
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Peter Weinberger <pjw@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Our calling convention requires BP to be preserved (it is
callee-save). But that only happened with the introduction of
framepointers in 1.5. Furthermore, nothing checks it, so there is
assembly from before and since which violates the calling
convention. The frame pointer is not used by Go, but only by kernel
traceback code (for sampled profiling), so no one noticed. Also, the
frame pointer tends to "fix itself", since after it is clobbered by an
assembly function f, the calling function fixes the frame pointer up
when that function returns.
I've fixed the stdlib, CLs 248260, 248261, 248262.
This CL is a simple check, intended to be used in vet, to catch
assembly that violates the calling convention by overwriting the
frame pointer without saving/restoring it. It is not intended to
catch all violations, just ones that are easy to find.
We look for a write to the frame pointer before any use of the frame
pointer or any control flow instruction. In such a situation, there's
no way the code could restore the value of BP before returning. This
analysis is very conservative; it gives up in lots of cases. False
positive should be very rare, though.
Possible false positives:
// looks like a write to BP, but isn't.
CMPQ BP, BP
// BP actually isn't clobbered, just swapped with AX.
XORQ AX, BP
XORQ BP, AX
XORQ AX, BP
The first is unlikely, as it is using the contents of an incoming
register, which is junk. The second is a general problem of op=
assembly operations that are indistiguishable in syntax from =
operations. But anything other than the swap above also depends on
the incoming junk value in BP. The swap itself is pointless (XCHQ
works better).
Change-Id: Ie9d91ab3396409486f7022380ad46ac76c3fbed4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248686
Trust: Keith Randall <khr@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Keith Randall <khr@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change copies the logic from the go command's inDir function
(https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/search/search.go;drc=3931cc113f3f3e7d484842d6e4f53b7a78311e8e;l=570)
to replace gopls's "isSubdirectory" function. This function resolves
symlinks, which isSubdirectory did not previously do.
The only adjustments are to flip the arguments to match the previous
signature of isSubdirectory and to return a boolean instead of a string.
Fixesgolang/go#38558
Change-Id: I9c64604222ac277eae81a4111eef432ead887e9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266200
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We've recently noticed multiple instances of `go list` hanging
indefinitely during an initial workspace load. Heschi suggested setting
a 5 minute deadline on the IWL, but it seems reasonable to set this
deadline on all calls to packages.Load since that calls `go list`.
I also started with a 15 minute deadline to be a little more careful.
Do you think this is risky enough to merit an experimental setting?
Fixesgolang/go#42132
Change-Id: I0a38741f3d99b6a38c46c3e663daf61f2cb45581
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266478
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The majority of the initialization logic is already based on the
snapshot, not the view, so it makes sense to move it there. The
initialization status of the snapshot is copied and invalidated in
clone.
Fixesgolang/go#41764
Change-Id: I93234b394318964e7af4696e5ebd465088a05728
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266700
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This CL uses the approach of a source.ErrorList for the `go mod vendor`
error--rather than a ShowMessageRequest. The suggested fix is a command
that runs `go mod vendor`, so I added a CommandFix to the source.Error
type.
I'm not sure if a diagnostic is better than a ShowMessageRequest--
perhaps it should be both?
Fixesgolang/go#41819Fixesgolang/go#42152
Change-Id: I4ba2d7af0233f13583395e871166caed83454d6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261737
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Consistent whitespace makes godoc render the snippet without
extraneous leading indentation.
Change-Id: If7b6910cd431ada0db0f59b7aecc9e5bd011af23
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266877
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
Keep a workspace module dir around for running the go command against a
snapshot, bound to the contents of the workspace modfile.
This uses the cache's resource model to share the workspace module dir
across snapshots if it is not invalidated, and to delete it when it is
no longer in-use by a snapshot. Of course, the go command will still
only see files on the filesystem, but using this immutable model was
most consistent with the immutable workspace.
For golang/go#41836
Change-Id: Iaec544283b2f545071e5cab1d0ff2a66e6d24dff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263938
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Amazingly the builders are occasionally so slow that they can't send the
next event within 100ms. Debounce longer to give them a bit more time.
Change-Id: Ib06bef77099c569060dd32c5b964bf394103485a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266698
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
With its memoization and refcounting, the cache is well suited to the
sharing of other expensive resources, specifically those that interact
with the file system. However, it provides no means to clean up those
resources when they are no longer needed.
Add an additional argument to Bind to clean up any values produced by
the bound function when they are no longer referenced.
For golang/go#41836
Change-Id: Icb2b12949de06f2ec7daf868f12a9c699540fa5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263937
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
gopls.mod files should take precedence over go.mod files when finding a
workspace root. To allow this, implement our own algorithm for expanding
the workspace, rather than using the go command.
For golang/go#41837
Change-Id: I943c08bdbdbdd164f108e44bae95d2c659a6e21e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263897
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
When incorporating the gopls.mod file, the invalidation logic for
workspace module information becomes quite complicated. For example:
+ if a modfile changes we should only invalidate if it is in the set of
active modules
+ the set of active modules can be provided by either the filesystem or
gopls.mod
+ if gopls.mod changes, we may gain or lose active modules in the
workspace
+ if gopls.mod is *deleted*, we may need to walk the filesystem to
actually find all active modules
Doing this with only concrete changes to the snapshot proved
prohibitively complex (at least for me), so a new workspace type is
introduced to manage the module state. This new abstraction is
responsible for tracking the set of active modules, the workspace
modfile, and the set of workspace directories. Its invalidation logic is
factored out of snapshot.clone, so that it can be tested and to
alleviate some of the growing complexity of snapshot.clone.
The workspace type is idempotent, allowing it to be shared across
snapshots without needing to use the cache. There is little benefit to
the cache in this case, since workspace module computation should be
infrequent, and the type itself consumes little memory.
This is made possible because the workspace type itself depends only on
file state, and therefore may be invalidated independently of the
snapshot. The new source.FileState interface is used in testing, and so
that the workspace module may be computed based on both the session file
state as well as the snapshot file state.
As a result of this change, the following improvements in functionality
are possible:
+ in the presence of a gopls.mod file, we avoid walking the filesystem
to detect modules. This could be helpful for working in large
monorepos or in GOPATH, when discovering all modules would be
expensive.
+ The set of active modules should always be consistent with the
gopls.mod file, likely fixing some bugs (for example, computing
diagnostics for modfiles that shouldn't be loaded)
For golang/go#41837
Change-Id: I2da888c097748b659ee892ca2d6b3fbe29c1942e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261237
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
The majority of the initialization logic is already based on the
snapshot, not the view, so it makes sense to move it there. The
initialization status of the snapshot is copied and invalidated in
clone.
Fixesgolang/go#41764
Change-Id: Icebcf8f4dc750ae4d31e19aa93f9a4c0a57beb4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266343
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
It felt a little redundant to have completion duplicated in the file
names. Happy to revert if you prefer it the original way.
Change-Id: Ibf14a739688d48ecce0b86154b3cbe4807b2f8b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266477
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
These commands link to self-documentation hosted on godoc.org. Given
that this traffic will eventually be redirected to pkg.go.dev
(https://blog.golang.org/pkg.go.dev-2020), we should proactively update
them to pkg.go.dev.
For golang/go#42251
Change-Id: Iff1e4c6958d4e1ddff98150fae6b811e7764816a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/264999
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
Trust: Robert Findley <rfindley@google.com>
Currently, fully qualified symbol style does a simple match against a
fully qualified symbol. However, this doesn't really match the user's
intent. Much like the dynamic style, the match should be done "from the
right" (per the logic of bestMatch) with the results being fully
qualified for presentational reasons.
We therefore reuse the logic from the dynamic symbol style, but instead
return a matched symbol as the fully qualified value.
Change-Id: I5c8c6b647c0710c5054bc8e27dbb29cb0fac14d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266557
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Paul Jolly <paul@myitcv.org.uk>
Leave the legacy names to handle old versions of the VS Code language
client.
Updates golang/go#42148
Change-Id: Ia3eeef9e792e502c5c8018698b9c0ea3a9b0dfe5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266479
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
When gc_details is invoked on a file, it can add diagnostics for files
not in that file's package. Some of these will not go away when
gc_details is toggled off. The fix is not emitting compiler details
diagnostics about files not in the package.
Fixes: https://github.com/golang/go/issues/42198Fixesgolang/go#42198
Change-Id: Ib69cdd8b7be96d73ee417711f7241f56fb529836
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266342
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I had originally wanted to create a shared code path for hover in all
cases, but hover has a lot more differences from the documentation in
signature help and completion than I expected. You can't use markdown,
and you probably don't want links--it would take a bigger refactor to
extract something that worked for each feature.
Handling the Structured and SingleLine hover setting also doesn't seem
necessary--those settings are really specific to the way the client
presents the hover, which isn't related to signature help or completion.
For completion, all we need is an extra check on the hover kind for the
NoDocumentation option.
Fixesgolang/go#38577
Change-Id: Ib2037906c13f5be26813fcd2c20989e4d1b6c9bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266139
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
These were added in https://golang.org/cl/237419 and I don't think they
actually fixed anything. I'd like to remove them and see if anything
goes south.
Change-Id: I2f3a48d0e8e40b7a79b963a9319d426cbc6f5d9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266341
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In https://golang.org/cl/265237 I accidentally lost the os.Environ()
call, so we weren't using the OS environment as the defaults any more.
Change-Id: Id36e3c04f3c13d512b0b71b8d73ed086d9ee9f1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266337
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
RET f(SB) is a tail call. It is okay to not write the results,
as it has not returned to the caller at this point.
Change-Id: I670486d02285c3a346cbc93e91be3b9e61ab77bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/264319
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Modifying go.mod files directly leaves go.sum unchanged, and therefore
in need of updates later. Leaving work for the users to clean up isn't
ideal, so it'd be better to use the go command to make modifications.
Unfortunately, the go command has something of a mind of its own. The
most obvious problem is that using go get to add a new require adds a
// indirect comment to that new require, and there's no way to prevent
it. The only thing we can do is add the require first, then use go get
to do nothing but update the go.sum file.
The other inherent problem is that the go command operates on files as
they exist on disk, not the in-memory versions. As discussed, we issue
an error for this case. The alternative would be to work on temporary
files based on the in-memory contents, but that would be much larger
change, so I'd rather not at least right now.
To support Commands for quick fixes, add a new Command field to
source.SuggestedFix, and use it when forming the CodeAction response.
Fixesgolang/go#38209.
Change-Id: I0c13ea39199368623e7494e222ba38587323d417
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265981
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We have two parallel code paths for Load and other go command
invocations. Unify them by introducing a mode argument to the various
functions that run the go command, indicating the purpose of the
invocation. That purpose can be used to infer what features should be
enabled.
In the future, I hope we can use the mode to decide whether mod
file updates and network access should be allowed.
Change-Id: I49c67fcefc9141287b78c56e9812ee6a8ac8378a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265238
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The workspace symbols docstring was indenting quoted text using '>', but
this is incompatible with convention of simply using indentation to
cause pre-formatted text.
Change-Id: Ifbaf88e19ac09a29b83f1812c576de6ea96a0793
Reviewed-on: https://go-review.googlesource.com/c/tools/+/264636
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We pass around verb/args/wd in many places. Bundle them up as an
Invocation instead. goCommandInvocation now updates and returns an input
Invocation.
packages.Config is conceptually an extra layer of parsing and
type-checking on top of a go list invocation. It doesn't make sense for
us to construct the latter using the former. A later CL will construct
the Config in terms of the Invocation; for now duplicate a bit of logic.
Use the environment in the cache key for various module operations
rather than the Config hash. I'm not sure either is fully correct but I
think the environment captures everything that's likely to matter. This
way we don't need Configs in those functions at all.
Change-Id: Iebee2705e63638ab365b3ee18b23f8c3e8ca30ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265237
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Pontus Leitzler
Rob Findley
Heschi Kreinick
Rebecca Stambler
Peter Weinbergr
Keith Randall
Dmitri Shuralyov
Paul Jolly
Cherry Zhang