1
0
mirror of https://github.com/golang/go synced 2024-11-18 13:04:46 -07:00
Commit Graph

1557 Commits

Author SHA1 Message Date
Heschi Kreinick
4d1d9acccf internal/lsp/cache: fix parseKey
The FileIdentity struct mixes information about the file itself
(filename, hash) with information about the LSP references to that file
(session ID, version). When we create a cache key using it, we only want
the former, as returned by the String method. Otherwise we split the
cache whenever those irrelevant fields are different.

We also use FileIdentity as an element of diagnosticsKey, but I believe
that use is appropriate.

Change-Id: I094e00d2700e05778da635effbb69d0ebcb6726e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244020
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:57 +00:00
Heschi Kreinick
72051f7961 internal/lsp: pass snapshot/view to memoize.Functions
Due to the runtime's inability to collect cycles involving finalizers,
we can't close over handles in memoize.Functions without causing memory
leaks. Up until now we've dealt with that by closing over all the bits
of the snapshot that we want, but it distorts the design of all the code
used in the Functions.

We can solve the problem another way: instead of closing over the
snapshot/view, we can force the caller to pass it in. This is somewhat
scary: there is no requirement that the argument matches the data that
we're working with. But the reality is that this is not a new problem:
the Function used to calculate a cache value is not necessarily the one
that the caller expects. As long as the cache key fully identifies all
the inputs to the Function, the output should be correct. And since the
caller used the snapshot/view to calculate that cache key, it should
always be safe to pass in that snapshot/view. If it's not, then we
already had a bug.

The Arg type in memoize is clumsy, but I thought it would be nice to
have at least a little bit of type safety. I'm open to suggestions.

Change-Id: I23f546638b0c66a4698620a986949087211f4762
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244019
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:46 +00:00
Heschi Kreinick
60da08ac03 internal/lsp/cache: remove files from the memoize.Store
The memoize cache buys us little for files: the cache value is not
really a function of the inputs, but rather the filesystem state. It's
pretty much just as easy to manage them explicitly, and it's a start at
simplifying our caching strategy.

We do lose one small feature: if we try to read the same file
concurrently, reads will not be deduplicated. I suspect that doesn't
matter.

Change-Id: I75e219467fb7a512d9cfdf87443d012c85f03df9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243197
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-28 17:34:34 +00:00
Heschi Kreinick
051e64e62c internal/lsp: minimize PackageHandle interface
The PackageHandle interface is fairly redundant with the Package
interface. As much as convenient, move users to Package and
weaken/remove methods from PackageHandle.

I would like to get rid of CompiledGoFiles too but
NarrowestPackageHandle is a little annoying. I think this is
unambiguously a step forward so I figured we can get it in and keep
iterating.

Change-Id: I6c5a3f462b1f19cbca6a267fedc36ce54613b6fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244018
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:24 +00:00
Rebecca Stambler
2ad651e9e2 internal/lsp: handle bad formatting with CRLF line endings
The importPrefix logic is complicated by Windows line endings, since
go/ast isn't aware of different line endings in comment text. I made a
few changes to the way that import prefixes are computed to handle this.

Specifically, for comments, we try to make sure the range ends on a full
line as much as possible, because that addresses the line ending issue.

Fixes golang/go#40355

Change-Id: I84c1cfa0d0bae532e52ed181e8a5383157feef24
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244897
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-28 16:05:17 +00:00
Josh Baum
3c048e20c6 internal/lsp: support return statements in extract function
Previously, users could not extract code that contained a return
statement. Now, users can extract code with return statements, as long
as the statements are nested within an if, case, or other control
flow statement.

Updates golang/go#37170

Change-Id: I2df52d0241517472decabce3666a32392ff257bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243650
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 16:04:03 +00:00
Peter Weinbergr
cd83430bb0 internal/lsp: update code for LSP protocol
1. Change code.ts so it generates []json.RawMessage in place of []interface{}
for Command-related arguments. As usual, vscode introduces a lot of
whitespace-only changes.
2. Generate code based on the July 28 version of vscode-languageserver.
The changes are mostly related to SemanticToken, and didn't require
any changes to gopls, other than in the generated code.

Change-Id: I673e29e2fbc097409683dfe7af911d8f66e25c5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245134
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-28 15:42:44 +00:00
Pontus Leitzler
55644ead90 internal/lsp: allow narrower scope for convenience CodeActions
Code actions that apply convenience fixes were filtered by the start
line so it wasn't possible to narrow the scope to a specific range.

This change allows clients to send a specific range (or cursor position)
to filter all fixes where the range doesn't intersect with the provided
range. It also widens the diagnostic returned by fillstruct analysis.

The idea is to provide a way to narrow the scope without breaking
clients that do want to ask for code actions using the entire line.

Updates golang/go#40438

Change-Id: Ifd984a092a4a3bf0b3a2a5426d3e65023ba4eebc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244519
Run-TryBot: Pontus Leitzler <leitzler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-27 23:36:28 +00:00
Rebecca Stambler
7b4c4ad3dc internal/lsp: correctly marshal arguments for upgrade code lens
I'm not sure how the regtest didn't catch this - is it possible that
it could unmarshal a single string a slice of string? Either way, I'd
like to get the fix in quicker - I'll try to add more regtests for this
later.

Also, validate the upgrade results more thoroughly.

Change-Id: I812a3fecd9f0642a1408c0a9c0376bb98d50b397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245065
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-27 21:52:59 +00:00
Rebecca Stambler
59c6fc0b54 internal/lsp: correctly invalidate metadata for batched changes
I finally spent the time to understand why branch changes were causing
unexpected errors. There may be other bugs, but this is the first I
spotted. For batched invalidations, we were overriding the value of
invalidateMetadata for each file, so the results depended on the order
of files in the didChangeWatchedFiles notification.

Change-Id: Id3ca7a758af0115c46dcd74ede590a0be3f8307d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244606
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-27 19:55:46 +00:00
Rebecca Stambler
9267083701 internal/lsp: support refactor.extract through commands
The logic for extracting a function is quite signficant, and the code
is expensive enough that we should only call it when requested by the
user. This means that we should support extracting through a command
rather than text edits in the code action.

To that end, we create a new struct for commands. Features like extract
variable and extract function can supply functions to determine if they
are relevant to the given range, and if so, to generate their text
edits. source.Analyzers now point to Commands, rather than
SuggestedFixFuncs. The "canExtractVariable" and "canExtractFunction"
functions still need improvements, but I think that can be done in a
follow-up.

Change-Id: I9ec894c5abdbb28505a0f84ad7c76aa50977827a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244598
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-27 19:25:51 +00:00
Rebecca Stambler
eaaaedc6af internal/lsp: fix hover link for embedded fields and methods
Our logic to generate documentation links did not account for embedded
fields and methods. The types.Info.ObjectOf an embedded field returns
the *types.Var created for the field, not its types.TypeName, so we have
to navigate back to the actual definition of the field. This requires
traversing through all of the named types in the top-level type.

Fixes golang/go#40294

Change-Id: Ia6573aebe66b7f60e2d6861a381cd7b07e7d7eaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244178
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-27 15:46:26 +00:00
Rebecca Stambler
102e7d3570 internal/lsp: fix GlobPattern for watching files
I noticed this wasn't working correctly when debugging issues with file
changes on disk. I think I misunderstood how to correctly match on
zero or more path segments. Confirmed that this is working as expected
with VS Code, but we really need regression tests for this in VS Code
(it seems to be based directly on VS Code's API). Filed
golang/vscode-go#404.

Change-Id: Ib906f73a97317dfa9bd30099877c90d4072651cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244605
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-25 20:09:36 +00:00
Rebecca Stambler
b5fc9d354d internal/lsp: add parse errors as diagnostics even when parsing fails
When a package consists of files that only fail to parse, we fail to
associate parse errors with the package, and therefore return no
diagnostics. To address this, we need to associate the errors with the
package. This involves adding a *token.File to the parseGoData so that
error messages and positions can be properly extracted, even when there
is no associated AST.

Fixes golang/go#39763

Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-24 17:29:32 +00:00
Rebecca Stambler
7017fd6b13 internal/lsp: fix hover for implicit type switch variable declarations
There was a bug in the hover for type switch variables. For example:

var x interface{}
switch y := x.(type) {
    case string:
    case int:
}

Hovering over y would previously show "var y string", because y's object
would be mapped to the first types.Object in the type switch. Now we
show the hover for y as "var y interface{}", since it's not yet in the
cases.

Change-Id: Ia9bd0afc4ddbb9d33bbd0c78fa32ffa75836a326
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-24 02:27:22 +00:00
Rebecca Stambler
cbb3c69a37 internal/lsp: add errors for missing modules that don't map to an import
Some modules may need to be added to the go.mod without being associated
with an import statement. In such cases, we show the missing module
diagnostic on the whole go.mod file. This isn't ideal, but mapping to
the full require statement isn't that simple, and this is an easy enough
starting point. The code in mod_tidy.go is becoming more unwieldy - I
think I will clean it up in a follow-up.

Fixes golang/go#39784

Change-Id: Ib32ec1fd74c455ce42ba778ea6cba0a475cf245a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243218
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-24 00:18:24 +00:00
Rebecca Stambler
37a045f3b9 internal/lsp: move undeclaredname suggested fix out of analysis
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>
2020-07-23 23:54:27 +00:00
Rob Findley
0e1f6f5c09 internal/lsp/regtest: fix WithoutWorkspaceFolders option
This option was previous a no-op.

Change-Id: Id28f6802c8f5b020fb3ae71f5b941c0517ca1725
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244548
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-23 23:38:05 +00:00
Rob Findley
93b64502c3 internal/lsp/regtest: add a simple stress test
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>
2020-07-23 23:37:47 +00:00
Rob Findley
84d0e3d1cc internal/lsp/regtest: add run options to support stress testing
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>
2020-07-23 23:37:37 +00:00
Rob Findley
a5e28d8dab internal/lsp/fake: move to a struct for configuring the sandbox
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>
2020-07-23 23:36:26 +00:00
Rob Findley
1ff154e68b internal/lsp: lift env out of the sandbox into the editor
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>
2020-07-23 21:58:45 +00:00
Rebecca Stambler
d2c1b4b270 internal/lsp: show errors in indirect dependencies as diagnostics
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>
2020-07-23 21:53:52 +00:00
Rebecca Stambler
0cdb17d11b internal/lsp: treat the module root as the workspace root, if available
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>
2020-07-23 21:33:49 +00:00
Rebecca Stambler
b3d141ddef internal/lsp/cache: copy *packages.Config environment completely
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>
2020-07-23 19:57:34 +00:00
Rebecca Stambler
80cb79702c internal/lsp: don't publish duplicate diagnostics in one report
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>
2020-07-23 19:34:40 +00:00
Brayden Cloud
bd1e9de8d8 internal/lsp: separate test and benchmark codelens
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>
2020-07-22 18:17:40 +00:00
Rebecca Stambler
6123e77877 internal/lsp: support go mod tidy on save without diagnostics
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>
2020-07-21 22:32:18 +00:00
Rebecca Stambler
b42efcd11c internal/lsp: try to parse diagnostics out of go list errors
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.

Fixes golang/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>
2020-07-21 22:03:47 +00:00
Rebecca Stambler
0a5cd10191 internal/lsp: handle unknown revision in go.mod file
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.

Fixes golang/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>
2020-07-21 21:40:18 +00:00
Rebecca Stambler
acdb8c158a internal/lsp: handle on-disk file deletions for opened files
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>
2020-07-21 21:30:00 +00:00
Rebecca Stambler
5ea363182e internal/lsp: change the way that we pass arguments to command
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>
2020-07-21 16:30:27 +00:00
Pontus Leitzler
b8e13e1a4d internal/lsp: return the actual range from convenience code actions
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.

Fixes golang/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>
2020-07-21 15:44:06 +00:00
Rebecca Stambler
77f530d86f internal/lsp: handle deletion of file content
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.

Fixes golang/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>
2020-07-21 03:22:37 +00:00
Rebecca Stambler
4025ed8474 internal/lsp: move fillstruct suggested fixes out of analysis
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>
2020-07-20 20:42:44 +00:00
Josh Baum
6d307edf52 internal/lsp: support extract function
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>
2020-07-20 18:55:41 +00:00
Rebecca Stambler
9cbb971a18 internal/lsp: split only on the first = for Go environment variables
Follow-up from CL 243538.

Change-Id: I15ac8ba738fa5326b18f7b179bed8623f4c57cfd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-20 16:24:43 +00:00
Rebecca Stambler
e6549b8dc8 internal/lsp: don't re-run go env for every call to WriteEnv
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>
2020-07-20 15:02:21 +00:00
Rebecca Stambler
6ddee64345 internal/lsp: add a configuration to enable/disable links in hover
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>
2020-07-17 02:43:01 +00:00
Rebecca Stambler
d518495ee8 internal/lsp: log errors for compute fix edits instead of returning
Fixes golang/go#40260

Change-Id: I69032b8cd6b32a262ecd3bb746ef6d1722966a51
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243238
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-07-17 02:25:49 +00:00
Rebecca Stambler
58eba7e750 internal/lsp: refactor go.mod diagnostics to simplify the API
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>
2020-07-16 22:50:03 +00:00
Rob Findley
f2c07d7d8e internal/lsp/lsprpc: improvements to daemon logging
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.

Fixes golang/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>
2020-07-16 19:33:22 +00:00
Rob Findley
cf799caead internal/lsp/regtest: simpler way to invert options
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>
2020-07-16 18:20:50 +00:00
Rebecca Stambler
43ed94694c internal/lsp: don't keep track of closed overlays
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.

Fixes golang/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>
2020-07-16 17:49:00 +00:00
Rebecca Stambler
e66011cbf8 internal/lsp: fix error in CL 242457
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>
2020-07-16 17:41:35 +00:00
Rebecca Stambler
a8f9df4c95 internal/lsp: clean up the code lens code
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>
2020-07-16 13:43:26 +00:00
Rebecca Stambler
c80dc571df internal/lsp: refactor generate code lens code
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>
2020-07-16 13:43:19 +00:00
Rebecca Stambler
ddb87c8c46 internal/lsp: add an importShortcut configuration
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.

Fixes golang/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>
2020-07-15 23:00:36 +00:00
Rebecca Stambler
4cea89713c internal/lsp: handle nil pointer in PackageStats
Fixes golang/go#40224

Change-Id: I9f3f194b1c115ff0242cbce89a36e20d9e21a41f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-15 19:57:38 +00:00
Rebecca Stambler
b42590c1b1 internal/lsp/source: handle nil pointer in newBuiltinSignature
Fixes golang/go#40230

Change-Id: I457dd3ef009c9df9293f66871acdd90dcc6e1dcc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242798
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-15 19:52:07 +00:00