Commit Graph

5637 Commits

Author SHA1 Message Date
Heschi Kreinick 94fce4dc28 internal/lsp/cache: remove stray debug logging
Change-Id: Iaf1ad157f45a7af894d03024beed7804fb6a1ae3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290729
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-02-09 20:31:31 +00:00
Dmitri Shuralyov 6140657873 godoc: show earliest version when identifier was added
CL 85396 implemented parsePackageAPIInfo with the idea that each
identifier shows up in exactly one of api/go*.txt files, when it
was added. We now know that it may show up more than once, when
the signature changes (generally in a compatible way, such as
when existing types are replaced with aliases to an equivalent
type).

Modify the algorithm to parse the api/go*.txt files in reverse
order, in order to find and display the earliest Go version when
each identifier was first added.

Fixes golang/go#44081.
Updates golang/go#5778.

Change-Id: I83171fd8c161d703f284011375d74b824c279d41
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289089
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-02-09 00:10:52 +00:00
Rob Findley 5ab06b02d6 internal/lsp/source: sort commands alphabetically
For stability and to ease navigation, sort commands alphabetically.

This will simplify the diff in later CLs, where command discovery is
refactored.

Change-Id: I346cbc2162b1b4dbac16572a653c4169b93cc0f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290390
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>
2021-02-08 16:45:41 +00:00
sanposhiho 5941677e6c go/analysis: fix path to example-test.go
Hello 👋

I found the wrong path in `go/ssa/ssautil/load.go`, so fixed it.

Change-Id: Id9233e8aa0875dd51060d5a1b4043ebb7582f059
GitHub-Last-Rev: c69ca973dca71db0ca1f41a759763ae3e4735eda
GitHub-Pull-Request: golang/tools#274
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290250
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-02-08 15:33:33 +00:00
Muir Manders ef80cdb6ec internal/lsp/completion: fix untyped int type inference
For index expressions, optional "make" args, and composite literal
slice/array keys, we were inferring an expected type of int instead of
untyped int. This caused candidate rankings to not be quite right in
general, and in particular, after support for automatic type
conversions was added, the issue manifested as:

    var foo []int
    var bar int32
    foo[ba<>] // completed to "int(bar)" instead of "bar"

Fixes golang/go#43375.

Change-Id: I6daef7d23b767f296bdbbc8f47f5b2c972ad9b80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289272
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
2021-02-05 20:20:24 +00:00
Muir Manders b30482dd32 internal/lsp/cache: allow fixing multiple syntax errors
Allow fixSrc to run multiple times on a file. For example:

    func main() {
    	var s S
    	if s.<>
    }

    type S struct {
    	i int
    }

To properly complete at <>, we need to perform two fixes. We must
insert a phantom underscore after "s." to deal with the dangling
selector, and then we must insert phantom "{}" for the "if" statement
to deal with the incomplete "if" block. Previously we stopped at one
fix, but now we allow for up to 10. I added a limit because I am
afraid there are cases where we could get stuck trying to apply the
same fix again and again.

Also, drive-by set isIncomplete=true when we return early in
lsp/completion.go. I have found my editor incorrectly caches this zero
result in certain cases because it thinks results are "complete".

Fixes golang/go#43471.

Change-Id: Idd34cc164d579fa12a27920dc3afb372799abf26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289271
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-02-05 19:47:01 +00:00
Heschi Kreinick 513be0a9d2 internal/lsp/cache: disable network for mod tidy diagnostics
The only thing that the mod tidy diagnostics use the network for is
adding dependencies, and we already have quick fixes for those. The one
exception is the case covered by TestBadlyVersionedModule, a dependency
that fails to declare one of its own dependencies and therefore requires
an indirect dependency in the workspace module. That only triggers an
error on the dependency's import statement, which the user will never
see.

Fortunately, the go command does expose these problems in the DepsErrors
field of the list response. Add an internal API to access that, and turn
it into diagnostics on both the file and the controlling go.mod.
Refactor the go get diagnostic generation so that it applies to both
modules and packages.

Fixes golang/go#38462.

Change-Id: Ie2af940087654682a40de9ecfccd46f404a88b60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289309
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>
2021-02-05 19:41:14 +00:00
Muir Manders 8938cee73c internal/lsp/completion: fix invalid struct literal field snippet
In cases like:

    type foo struct { a int; b float64 }
    foo{b<>}

We were completing to "foo{int(b: <float64>)}" (the problem being the
nonsensical int() conversion).

The expected type at "<>" is int to allow completions to match "a".
When we pass the *types.Var representing "b" through the candidate
matching machinery, we say "Oh, a float64! I can convert that to my
expected type of int!".

Fix by bailing out of candidate matching early if the candidate is a
composite literal struct field name. Field names aren't really objects
you can do anything to.

Fixes golang/go#43789.

Change-Id: Ie4dab166973dfcdcb519f864532ead1f792d25a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289130
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
2021-02-05 19:39:40 +00:00
Muir Manders c3a8a1d828 internal/lsp/completion: fix untyped ints to match floats
In cases like:

    foo<> == 100

We weren't preferring floats at <>. Fix the basic type comparison
logic to know that an untyped int is always compatible with a float.

Fixes golang/go#44066.

Change-Id: I9cf9bac1632178db100c0a5447351be208b4a2af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289129
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
2021-02-05 19:39:34 +00:00
Rob Findley 842a9283d6 gopls/internal/regtest: only run in 'singleton' mode on certain GOOS
The gopls regtests have highly variable performance on certain operating
systems, we believe due to kernel contention. Flakes are a problem, and
it's better to have coverage of our default execution mode than no
coverage at all.

For golang/go#44099
For golang/go#42789

Change-Id: Ie9f3f93bb950930401aac3ce55cdffb889d0f0e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289692
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>
2021-02-04 18:06:13 +00:00
Rob Findley bd8508e405 gopls/internal/regtest/codelens: increase the timeout on TestGCDetails
Experiment with increasing the timeout for TestGCDetails, given how its
flakiness correlates with slow builders.

For golang/go#44099

Change-Id: I27a8732256a77c3d9ce2a1b1f75ce5d0fbc11e67
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289690
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>
2021-02-04 18:05:21 +00:00
Heschi Kreinick 6d19fbfa2b internal/lsp/cache: fix AllowModfileModifications on 1.16
We still need to pass -mod=mod when AllowModfileModifications is enabled
on 1.16.

Change-Id: I9cecd30914eb52c9faa877cece25d5722b36df79
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289695
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>
2021-02-04 17:47:36 +00:00
Koichi Shiraishi 6baea3f8f3 internal/jsonrpc2: remove unused invalidID constant
Change-Id: If00f4459be3176a44f7654bb304e03d9d6b393c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288273
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-02-04 15:49:51 +00:00
Rebecca Stambler ed97357570 all: upgrade all dependencies, if possible
Change-Id: Ifce11f6d94b1c6b4231abffd848cf45a10c962df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288733
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2021-02-03 19:28:29 +00:00
Heschi Kreinick e7dfe0279f internal/lsp: remove redundant fields/code after source.Error deletion
Collapse Diagnostic.Kind, Source, and Category into just Source. Remove
code that converted from Diagnostic to Diagnostic. Notes on the changes
I had to make along the way:

- We used to use Kind to determine Severity. Set Severity when the
Diagnostic is created instead.
- Use constants for Source as much as possible -- we still need to use
Analyzer.Name for analysis diagnostics. It would be nice to break that
dependency so that Source was totally opaque, but that's a separate
issue.
- Introduce a new Source for gc_details, "optimizer details". It was "go
compiler" previously.
- Some of the assignments are a little arbitrary. Is inconsistent
vendoring really a "go list" error?
- GetTypeCheckDiagnostics had code to cope with diagnostics that had no
URI associated with them. We now spread such diagnostics to all files
when they are generated.
- Analyze modifies Diagnostics by adding a Tag to them. That means it
has to own them, so I had it clone them. I would like to push that logic
down to the diagnostics, per the TODO, but that's another CL.

And some observations:
- It's obviously tempting to combine DiagnosticSource and
diagnosticSource, but they mean very different things. I'm open to a
better name for one or the other.

Change-Id: If2e861d6fe16bfd2e5ba216cf7e29cf338d0fd25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288215
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>
2021-02-02 23:37:55 +00:00
Heschi Kreinick 51ce8377eb internal/lsp: remove the source.Error type
source.Error and source.Diagnostic are almost identical types, used
arbitrarily in different parts of the code. This CL is the first step in
cleaning up that redundancy: it deletes the source.Error type.

To do that, I added the fields from source.Error to source.Diagnostic,
and made absolutely no other semantic code changes -- I just renamed
things that were named Error to Diagnostic. With only aesthetic concerns
in play, I hope this CL will be easy to review. The next CL will clean
up all the stupid-looking code that converts a Diagnostic to a
Diagnostic, etc.

Change-Id: I1298cc8144c686b8a37fc2cc106930105e511353
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288214
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-02-02 20:40:51 +00:00
pjw 5659e49346 internal/lsp/protocol/typescript: update LSP generating code
code.ts and util.ts for the latest version of the LSP protocol.

Change-Id: I911254ee7e66d91071e465ed83c456975e358ca4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288732
Trust: Peter Weinberger <pjw@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-02-02 17:43:53 +00:00
Rebecca Stambler ddc05f8a13 internal/lsp: enable semantic tokens as part of all experiments
Let's start testing semantic tokens in the Go Nightly extension.

Change-Id: I0c5b1f0891d1e55f876bde1bf8c81feca97fb57b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288652
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>
2021-02-02 16:03:30 +00:00
pjw c3402e3c42 internal/lsp: update to latest version of LSP protocol
The Typescript source is still at version 3.16, but there are new
requests, more detailed client capabilities, and an attempt to be
more specific about ranges of number in the Typescript code.

Vscode defines types integer and uinteger (32-bit signed and unsigned),
so the Go code now uses int32 and uint32.

They've changed the use of TextDocument, so version information is sometimes
missing. cache/session.go:625 was changed correspondingly.

This CL also make CodeAction.Disabled into a pointer.

New requests or notifications:
DidCreateFiles, DidRenameFiles, DidDeleteFiles (notifications)
ShowDocument, WillCreateFiles,WillRenameFiles, WillDeleteFiles (request)

It's a lot of code; I've probably missed something.

Change-Id: I8449ad8473ac00947d0344c5f6133f9bd73b9e10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286192
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
2021-02-02 11:32:59 +00:00
Heschi Kreinick 2ab23861a0 internal/lsp: stop using structured errors
Using structured errors in gopls has proven to be difficult to manage:
it's hard to know whether a given error return is expected to be
structured without any type information. We have mostly eliminated
them; finish the job.

I don't intend any semantic changes here.

I considered eliminating CriticalError altogether, but it does seem
useful to have a convenient bundle for return values. So I left it
alone for now.

Change-Id: I4b5f85a8de9712babffbc1383088151b596bd815
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287792
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>
2021-02-01 22:49:46 +00:00
Heschi Kreinick 9b8df07b91 internal/lsp: enable -mod=readonly in workspace module mode
Now that workspace module mode generates a combined go.sum there are
relatively few blockers to enabling -mod=readonly. Fix them and do it.

This CL is a bit of a grab bag, but the fixes are relatively separate. I
can split it into multiple CLs if desired.

- If module A depends on module B at v1.0.0, the go command will want to
upgrade the workspace module from v0.0.0-goplsworkspace to v1.0.0. To
prevent that, use vN.999999.0 as the base pseudoversion, adjusting v0 to
v1 where appropriate. A few test cases needed updating as a result.
- For old Go versions, sort the generated workspace module and
synthesize a go statement from the maximum go version declared in the
workspace.
- Some regtests need go.sum files created.
- matchErrorToModule created incorrect quick fixes: it would try to
download the top-level module mentioned in the error message, not the
one that actually caused the problem. Now it issues quick fixes for the
lowest-level module.
- TestMultiModuleModDiagnostics accidentally included the same module
in the workspace twice. Fix it, and make that an error.

Fixes golang/go#43346.

Change-Id: I605f762a4d23bedd914241525e64c1b3ecc42150
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287032
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-02-01 17:36:10 +00:00
Heschi Kreinick f1f686b0d0 internal/lsp: re-enable upgrades for individual dependencies
In CL 271297, I disabled the constantly-running upgrade check, which
removed the upgrade commands for individual dependencies. This seems to
have been a relatively popular feature. Re-introduce it, but requiring
explicit user interaction.

We now run an upgrade check when the user clicks "Check for upgrades".
Those results are stored on the View and used to show diagnostics on
any requires they apply to. Right now we only check the go.mod the user
has open; in multi-module workspaces it might make sense to check all of
them, but I'm not sure.

Fixes golang/go#42969.

Change-Id: I65205dc99a4fa9daafdb83145b0294b6f3be5336
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286474
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>
2021-02-01 17:10:16 +00:00
Heschi Kreinick d8a2a07971 go/packages: improve go invocation errors
Rather than coming up with an ad-hoc error message, use the friendly
errors from the gocommand package. I noticed this because the latter
is missing the verb.

I *think* this is generally an improvement; instead of seeing a massive
command line, users just see the actual error from the go command. Even
if it's not, I'd rather improve the single source of the error than play
whack-a-mole.

Change-Id: Ib110eaac6c3984e617339cf14cffe3b59f33591b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287033
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>
2021-02-01 17:07:00 +00:00
Heschi Kreinick 19db92ec3b internal/lsp/cache: remove mod upgrade code
At this point I'm fairly sure we don't want it.

Change-Id: Ib0657e8954463df751ab740aa5f582e496ee035b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286475
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>
2021-02-01 16:52:01 +00:00
Rebecca Stambler 0cef57b5b5 internal/lsp/protocol: use a pointer for code action's disabled field
We were always sending an empty reason for "disabled" in code actions,
which leads clients to see all code actions as disabled.

Change-Id: I855fb622a52557cc56ce91cf90fdc971df099a90
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287796
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-01-29 18:11:47 +00:00
Rob Findley db4c57db23 gopls/internal/regtest: split regtests up into multiple packages
Regtests have gotten large, and started timing out on some slow
builders. They also can't yet be run in parallel, due to some
process-level shared state (e.g. gc_details). Furthermore, there seems
to be some per-process resource leak that we don't yet fully understand
causing slowdown as the tests progress.

Address these problems by splitting the regtests up into multiple
packages. This also makes it easier to run only relevant tests during
development.

For golang/go#42789
For golang/go#39384

Change-Id: I1a74e4c379f3a650f4c434db44f9368e527aa459
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287572
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-01-28 15:45:56 +00:00
Rob Findley f871472f1b internal/lsp/cache: lock in snapshot.knownFilesInDir
We were not locking while iterating s.files in snapshot.knownFilesInDir.
All other accesses of s.files appear to be locked, so this should fix
golang/go#43972.

Fixes golang/go#43972

Change-Id: I01184c3992c91f8beb4a3239f70cc4487a528ec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287573
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-01-28 15:26:31 +00:00
Rob Findley c2bea79de9 internal/lsp/source: make it an error to rename embedded fields
Field embedding links two objects (a TypeName and a Var) by name,
requiring special handling during renaming. In CL 282932, renaming of
types was made to propagate to uses of their embeddings. However, no
such propagation in the reverse direction was added, meaning that
renaming an embedded field would not rename the corresponding type, and
code could still be left in a non-compiling state.

It should be an invariant that renaming does not change program
behavior. To enforce with field embeddings this we'd need to also rename
the corresponding type, but this seems problematic. If I'm hovering over
the field selector x.T, and rename T, it is surprising that this would
end up renaming a type.

For lack of a better solution, make it an error to rename embedded
fields, but try to provide a helpful error message.

Also handle the blank identifier, for which renaming was giving a
message to "please file a bug".

Marker tests are added for the new errors in rename, but not for
prepareRename. The prepareRename tests were not set up for asserting on
errors -- perhaps that would be a good project for a later CL where we
clean up errors.

Fixes golang/go#43616

Change-Id: I66c2dd5e531dd102431d1edd443d553687d9ca7e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284312
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: 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>
2021-01-26 13:52:46 +00:00
Heschi Kreinick 514964b755 gopls/internal/hooks: improve license file test
Now that we're only including licenses for the things gopls actually
depends on, we don't need to access the network on the trybots, and we
can get rid of the go.sum check that adds a step to the release process.

Change-Id: I3d38334ea3a9c904dfa125157c7b0468a0699b54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286436
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-01-26 00:28:59 +00:00
Ajanthan Balachandran 68bf78a686 internal/lsp/cmd: improve help output of gopls subcommands
This cleans up empty flag documentation of gopls subcommands with no flags.

Fixes #43447

Change-Id: I4d6f689062af24db4e6c652ec9ce0bc1a4a42a8c
GitHub-Last-Rev: d917bdb6ea20117f28b93d72d706874b0c1fcfeb
GitHub-Pull-Request: golang/tools#269
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284215
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-01-25 18:56:18 +00:00
Pontus Leitzler 4922717de7 go/analysis/passes/fieldalignment: delete doc style comments in fix
CL 278872 fixed the fact that field comments messed up the suggested
edits by removing comments entirely.

This change also remove documentation style comments since they also
produce broken suggestions in the same way.

Change-Id: I18d9c42fbf1453dcd5019b2a4aa3f4be4a58f7c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284732
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Pontus Leitzler <leitzler@gmail.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-01-25 18:11:30 +00:00
Rob Findley 917f61dfb7 gopls/internal/regtest: automate counting of editor notifications to await
Using awaiting a certain number of work items in the regtest is a source
of flakes and latent bugs. This CL replaces all such assertions with
assertions based on the number of notifications sent by the fake.Editor.

While implementing this, I discovered several tests that had incorrect
counting, so this may fix some flakes.

Implementing this required pushing the asynchronous processing of file
events into the Editor, rather than the Workdir.

Change-Id: I9c3639409f2beed4a76295cbd53180c6e2ace126
Reviewed-on: https://go-review.googlesource.com/c/tools/+/285612
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: Rebecca Stambler <rstambler@golang.org>
2021-01-25 18:11:24 +00:00
Rebecca Stambler 2972602ec4 internal/lsp: correct links provided in critical error pop-ups
Fixes golang/go#43600

Change-Id: Ib36b832652d20b542a3065544d41fee1f2ae3172
Reviewed-on: https://go-review.googlesource.com/c/tools/+/285515
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>
2021-01-22 20:33:18 +00:00
pjw e13398c87d internal/lsp: display current diagnostics in the debug server
The change displays outstanding diagnostics on the Client page of
the debug server.

Very occasionally gopls displays incorrect diagnostics or diagnositics
that won't go away.  It is possible that providing more complete
information will help us find the causes, at least when the debug
server is running.

Change-Id: I01f2bbbfeac86e7296f57f4a9aad8e1e658babfa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/285252
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-01-22 19:37:57 +00:00
Rebecca Stambler cf1022a4b0 gopls: factor out advanced documentation from the README
The README still felt a little long and contained information that I
don't expect most users need. I pulled some of it out into an "advanced
users" document. Let me know if you think another word might be better--
the word advanced has lost all meaning to me.

I also rearranged some of the heading levels--does this make more sense
or is the old way better?

Change-Id: I34bcbe91ec52107cf2875c33f224837b3c7fb0d3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/285514
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>
2021-01-22 17:08:14 +00:00
Rebecca Stambler 87bc10f2be gopls: mention workspaces and build systems in the README
As discussed, mention workspaces.md in the README. I figured build
systems like Bazel might be also worth mentioning, though IDK if that
fits well under configuration...

Change-Id: I8dd3c88d2e6f1096468eb08351676a99b91023f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/285512
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>
2021-01-22 00:09:48 +00:00
Rebecca Stambler ce34e26943 internal/lsp: don't show context cancellation in the progress bar
As part of investigating golang/go#43023, I noticed that it's possible
for us to show context cancellation as a critical error--we should not
do this.

Change-Id: I1f7cc13a151e0a161d5a4ea4d5b55fba15270b32
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284937
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>
2021-01-21 23:49:53 +00:00
Rebecca Stambler bec622c317 gopls: merge README and user.md
These two pages were serving largely the same function, so it makes more
sense to merge them. Mostly deleted all of the information about
configuration--its really verbose and not likely to be helpful to users.

Change-Id: Id5eb6cd2acc19cc2d863943cb399cfefb6bdcee8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283644
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>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-01-21 18:28:06 +00:00
Rob Findley 7e51fbd4c4 gopls/internal/regtest: re-enable android builder
Remove the exception for the Android amd64 builder, now that flakes are
fixed (though the exception didn't actually work anyway due to
golang/go#39460).

Also, accept the import reorganization applied by gopls.

Fixes golang/go#43554

Change-Id: I9a7cce35998cfa673699d74a487111e4daecf7ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284935
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>
2021-01-20 16:51:16 +00:00
Roland Shoemaker fe37c9e135 all: replace all usages of os/exec with golang.org/x/sys/execabs
This change ensures that packages using exec.LookPath or
exec.Command to find or run binaries do not accidentally run
programs from the current directory when they mean to run programs
from the system PATH instead.

Change-Id: I5907aa630ff64012395a7eb472967a477d90f12e
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/949438
Reviewed-by: Katie Hockman <katiehockman@google.com>
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284773
Run-TryBot: Roland Shoemaker <roland@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2021-01-19 22:25:03 +00:00
Rebecca Stambler a46736d9d9 internal/lsp/source: handle possible nil pointer in rename check
Change-Id: I92cc4015361d40e8a10d05fa6857bee2b302cec4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284583
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>
2021-01-19 19:51:17 +00:00
Heschi Kreinick e0d201561e gopls/doc: rewrite troubleshooting.md
Rewrite the troubleshooting doc to be (hopefully) more helpful. I tried
to start from the assumption that the user had made a mistake rather
than that there was a bug in gopls. In particular, they're now
instructed to check for mistakes before updating gopls, and to try
Gophers Slack before filing a bug.

In general, I tried to adopt a more conversational voice as the gopls
authors, which I hope will make it more readable.

Once this looks good I'll do the VS Code version.

Change-Id: Ie36ad47af9e96b734475f8abe157ab3ad9c8cf09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283934
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-01-15 20:22:50 +00:00
Rebecca Stambler d78b04bdf9 gopls/doc: clean up and slightly reorganize documentation
This is a first CL to clean up the gopls documentation. The main changes
are cosmetic (to correct warnings found by the MD Lint VS Code
extension), though some parts are reworded or moved around. Ultimately,
more work needs to be done, particularly on the user guide and features
overview.

My current thinking is that we should rename settings.md to
configuration and move the configuration piece out of the user guide,
and also reorganize the user guide to be more "getting started"
oriented.

I'm still not sure where miscellaneous items should go (e.g., working on
the Go distribution itself)--I deleted the FAQ because it seemed useless
and is probably not the most discoverable, but it's the only place that
comes to mind so far.

Change-Id: I3689362067672f7ad8d5e8fd97ca9c7c45cfc8c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280595
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2021-01-14 06:55:38 +00:00
Rebecca Stambler 1bdb73f5bd gopls/doc: move contents of golang/go#36899 to documentation
This change adds a workspace.md file that explains the different ways
of configuring your workspace with gopls. Ultimately, I think we should
move this to the user.md documentation, but I didn't want to deal with
merge conflicts from my other CL, so moving it here to start.

Fixes golang/go#36899

Change-Id: I9217dc85a10cc6d0cbb4471509a186405d2e2088
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283513
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>
2021-01-14 06:03:00 +00:00
Rob Findley f96436850f gopls/internal/regtest: fix synchronization for TestUseGoplsMod
Jumping to definition in a regtest can indirectly lead to a didOpen
call, so the awaits added to TestUseGoplsMod to synchronize metadata
were ineffectual. On Android, this can lead to the race described in
golang/go#43652.

But in any case, all this bookkeeping of notifications is fragile. Avoid
it entirely by having the fake editor keep track of notification
statistics. In the future, we should use this to clean up many existing
regtests.

For golang/go#43554
For golang/go#39384

Change-Id: Icd1619bd5cdd2f646d1a0050f5beaf2ab1c27f37
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283512
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-01-13 18:03:00 +00:00
Hyang-Ah Hana Kim 8b4aab62c0 gopls/doc/vscode.md: update vscode setting example
Update the vscode settings example based on the new settings organization.

Change-Id: I6a52cc525a038caa4f35716f6a8c2be145b5238e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283394
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2021-01-12 23:06:58 +00:00
Rob Findley 9a6582cd13 gopls/internal/regtest: avoid flake in TestGoModInvalidesOnSave
Investigating flakes in TestGoModInvalidatesOnSave highlighted a couple
bugs:
 + A didOpen on a go.mod file invalidates the workspace, since it is
   treated as a saved change. This, in itself, is probably not such a
   big deal, except that...
 + When metadata is deleted on this didOpen, we break the workspace
   before it can be recomputed.

Fix this by awaiting diagnostics from the didOpen, but it would be
better to have a more robust fix (e.g. CL 271477).

For golang/go#43554

Change-Id: I75d49b818ae2f3730a48ac6a473c24c664227523
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283352
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: 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>
2021-01-12 21:35:07 +00:00
Rob Findley f618651c09 internal/lsp/cache: compare file size when invalidating file cache
Our filesystem caching layer uses file modification time to invalidate
file contents. This is an imperfect heuristic, and on certain operating
systems with low resolution filesystem clocks (such as WSL), this can be
broken in practice.

A proper fix would be to just read the file contents directly and rely
on the snapshot to optimize file access, but we don't know that this is
a safe change. Instead, try to reduce the likelihood of false cache hits
by also checking the file size reported by Stat.

For golang/go#43554

Change-Id: I1af384db532725e84fa6f3a2e5469d10b43fee92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283053
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>
2021-01-12 21:35:00 +00:00
Rob Findley 7646fae968 internal/lsp/fake: use hash rather than mtime to identify workdir files
On builders with low resolution clocks (e.g. WSL), it's possible for us
to miss file events that occur within a single 'tick'.

Fix this by instead tracking full file identity. Since we should have
only a few relatively small files in tests, the additional overhead
should be small.

For golang/go#43554

Change-Id: I05a48567f83007ff2278145638547c6ebb2523fd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283052
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>
2021-01-12 21:27:48 +00:00
Rob Findley 45115c1c4d internal/lsp/source: rename uses of embedded fields
When renaming a type name, also rename indirect uses of the name as an
embedded field. This is conservatively isolated to just renames for now;
it's not clear to me that users would also want to see uses of embedded
fields as references.

Fixes golang/go#43616

Change-Id: I41913d037fedb8c27a448cd922eeaf11a02d01f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/282932
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
2021-01-12 21:27:35 +00:00