1
0
mirror of https://github.com/golang/go synced 2024-11-05 18:26:10 -07:00
Commit Graph

141 Commits

Author SHA1 Message Date
Heschi Kreinick
3366bbee47 internal/lsp/regtest: reproduce empty xtest bug
packages.Load returns bad data for an empty xtest in GOPATH mode.

Updates golang/go#40825.

Change-Id: Ibab8dcbeafc22a71a2a4c3be0008b728c13c4c80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249703
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-21 19:26:10 +00:00
Rob Findley
ed71c57206 internal/lsp: consolidate progress reporting
This change contains several improvements for progress reporting:

 + Consolidate the 'progressWriter' interface into the workDone
   interface.  Now all progress reporting should use workDone, and the
   workDoneWriter struct is just an io.Writer adapter on top of
   workDone.
 + Factor out the pattern of progress reporting, and use for all
   asynchronous commands.
 + Make several commands that were previously synchronous async.
 + Add a test for cancellation when the WorkDone API is not supported.
 + Always report workdone progress using a detached context.
 + Update 'run tests' to use the -v option, and merge stderr and stdout,
   to increase the amount of information reported.
 + Since $/progress reporting is now always run with a detached context,
   the 'NoOutstandingWork' expectation should now behave correctly. Use
   it in a few places.

A follow-up CL will improve the messages reported on command completion.

For golang/go#40634

Change-Id: I7401ae62f7ed22d76e558ccc046e981622a64b12
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248918
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-21 14:46:03 +00:00
Heschi Kreinick
c8f3937451 internal/lsp/source: fix unexported references of non-workspace packages
qualifiedObjsAtProtocolPos returned too early. Have it keep looking in
the rest of the candidate packages.

This changes the returned error slightly but AFAICT nobody cares.

Updates golang/go#40809.

Change-Id: Ic8199a484f0abcaa48cb6a3bcdd782195802d670
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249637
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-20 18:02:10 +00:00
Heschi Kreinick
d94536333c internal/lsp/cache: don't always type check in default mode
CL 248380 forced all type checking to be in the default workspace mode.
In that CL, I said I couldn't think of any features that would break. It
appears I didn't think very hard. Navigation features inside of
dependencies are something I use all the time and they broke.

Reintroduce the ability to get packages in a particular mode, and make
it convenient to get them in all relevant modes. Update some critical
features to do so, and add regression tests.

Fixes golang/go#40809.

Change-Id: I96279f4ff994203694629ea872795246c410b206
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249120
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-19 19:22:15 +00:00
Danish Dua
188abfa753 internal/lsp/mod: add go mod vendor and go mod tidy lenses
go mod tidy lens only shows up for a dirty mod file. Tidying a dirty
mod file makes the lens disappear.

Closes golang/go#40422

Change-Id: I3356ffe94deaf7a5cdfc159c16470894336edf9e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248717
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-18 00:58:47 +00:00
Heschi Kreinick
118ac038d7 internal/lsp: improve handling of non-Go folders
CL 244117 introduced a bug when modFile == os.DevNull: v.root is left
uninitialized, resulting in a view that appears to own all files. Fixing
that exposes a problem where opening a folder with no Go files and
GO111MODULE=on shows a popup. Skip the popup when no Go files are found.

Change-Id: I7f8b2d6fd2f954af64c3a65156ff44c649f3a5b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248620
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-17 19:03:02 +00:00
Heschi Kreinick
d926bd178c internal/lsp/cache: always type check in default mode
Every package has a default type checking mode dictated by whether it's
in the workspace or not. Some features force full rather than exported
type checking, but AFAICT that ends up being more harm than good. For
example, let's say we want to Find References on fmt.Printf in the stdlib.
Before this CL, we'd force a new type check of the fmt package, then
find no references because nothing else would have been checked against
that new version.

While there may be some features that work better in the current regime,
I can't think of any, and we have no test coverage for them. So I'd
rather start with what makes sense, and if we want to change it maybe
let's write some tests.

Change-Id: Iea589efb4b4374fd2a54451c868b6e2bd5484e20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248380
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-13 18:10:07 +00:00
Rebecca Stambler
48de4c84f0 internal/lsp: add a configuration to limit workspace scope to root URI
Some users may intentionally be opening subdirectories to avoid having
gopls load the whole module. Allow this via a configuration.

Fixes golang/go#40567

Change-Id: I6167f62a74a1c0b7cf07c1cb247adda839ee41f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247617
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-11 01:57:04 +00:00
Rob Findley
30a0893a98 internal/lsp: use the token supplied by the client for progress
Our WorkDone reporting was generating a random token for each unit of
work, even if a token was supplied by the client.  Change this to use
the client token if it is non-empty, and skip the
workDoneProgress/create request.

After this change we can no longer rely on tokens being a string.
Update our progress tracking accordingly.

For golang/go#40527

Change-Id: I702f739c466efb613b69303aaf07005addd3b5e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247321
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10 15:15:12 +00:00
Rebecca Stambler
383b97c0b5 internal/lsp: watch directories in replace targets and update on changes
This change adds the notion of a "workspace directory", which is
basically the set of directories that contains workspace packages. These
are mainly used for replace targets right now. It's a little trickier
than expected because the set of workspace directories can technically
change on any go.mod change.

At first, I wanted DidModifyFiles to report whether there was a change,
but I don't think it's actually that expensive to check on each call
and it complicates the code a bit. I can change it back if you think
it's worth doing.

The parse mod handle changes are because I needed an unlocked way of
parsing the mod file, but I imagine they'll conflict with CL 244769
anyway.

The next CL will be to "promote" replace targets to the level of
workspace packages, meaning we will be able to find references in them.

Change-Id: I5dd58fe29415473496ca6634a94a3134923228dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245327
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-07 23:35:17 +00:00
Rebecca Stambler
b5fad4ed8d internal/lsp: invalidate all packages on go.mod file changes
Previously, we were only invalidating workspace packages when the go.mod
changed, but we really need to be invalidating all known packages.

Fixes golang/go#40456

Change-Id: I9ad353a26ab40c74c7760ed7a1c5de517640cfab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245779
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-31 06:09:45 +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
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
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
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
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
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
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
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
62a0bb781d gopls, internal/lsp: support an extra formatting hook for gofumpt
This change reworks CL 240118 to apply gofumpt directly as a formatter,
not an analyzer. Depending on how gofumpt changes, we may be able to use
it as an analyzer in the future, but for now it's just easier to add it
as a formatting hook.

Fixes golang/go#39805

Change-Id: I227fde4b1916d8a82557f30dfca88e155136dff5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-13 21:42:47 +00:00
Rebecca Stambler
fd294ab11a internal/lsp: watch go.{mod,sum} files, as well as Go files
Not sure how this hasn't come up earlier. We aren't noticing file
changes on-disk for go.mod/go.sum files.

Fixes golang/vscode-go#297

Change-Id: I4532a252f330404515efec244a9c266a765e6bcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242160
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-13 01:13:07 +00:00
Rebecca Stambler
f1c4188a97 internal/lsp, go/packages: reproduce and fix golang/go#39646
This test exposes a lot of fundamental issues with `go list` overlays.
Now that we have the regression test framework, we can copy it over and
make any flakes completely reproducible by waiting for each change to
complete.

The issue here ended up being that initial workspace load returns
workspace packages with no associated files due to golang/go#39986.
Working around this allows invalidation to proceed as usual.

In the process of debugging this, I also noticed that packages can get
loaded as command-line-arguments even when we can get the correct
package path for them. It's pretty complicated to fix this, so
restructured the code a tiny bit and left a TODO. I'd like to come back
to this at some point, but it's not pressing since I was able to fix
this bug.

Fixes golang/go#39646.
Updates golang/go#39986.

Change-Id: Id6b08a5e92d28eddc731feb0ef3fd3b3fc69e64b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-07-10 04:28:08 +00:00
Rebecca Stambler
125cc70a40 internal/lsp: add new go.mod requires to files with unused require
This change addresses an underlying issue with the go.mod code, which is
that it was modifying go.mod files without cloning them. This could've
resulted in some ugly race conditions.

We also handle the fact that new dependencies weren't being added
cleanly to files that already had unused dependencies.

Fixes golang/go#39041

Change-Id: I96ee0052d8d29a25e24f0bda9688e780a0fa7442
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241443
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-10 04:15:23 +00:00
Rob Findley
e327e1019d internal/lsp/regtest: standardize on 20s timeout
The default regtest timeout was increased to 60s a while ago to try to
avoid flakes.

We think we've figured out the flakes, so drop it back down.  Start with
20s, since TestGOMODCACHE (which had overridden its timeout to 5s)
occasionally fails on freebsd:
https://build.golang.org/log/6c607c7fd3e886cf23127c958ae17637440fbc63

Change-Id: I10e6927985b8fb1d149070e204bd8ca120309c7b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240060
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-09 18:17:11 +00:00
Rob Findley
e404ca24fb internal/lsp/regtest: use a common directory for regtest sandboxes
For easier debugging (and less cruft if regtests are ctrl-c'ed), root
all regtest sandboxes in a common directory.

This also tries one last time to clean up the directory, and fails on an
error. This might be flaky on windows, but hasn't been so far...

Also give regtest sandboxes names derived from their test name.

Updates golang/go#39384
Updates golang/go#38490

Change-Id: Iae53c29e75f5eb2b8d938d205fbeb463ffc82eb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-09 18:15:24 +00:00
Heschi Kreinick
df98bc6d45 internal/lsp/regtest: remove stray short timeout
Apparently 5 seconds is too short for the freebsd-race builder and we
care about that.

Fixes golang/go#40090.

Change-Id: I6dad57402ed18e7c9e01f695ada7cc8b0cf35fb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241521
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-08 18:38:56 +00:00
Rebecca Stambler
3544e8c95d internal/lsp/regtest: add a regtest for golang/go#39296
Looks like the issue described in
https://github.com/golang/go/issues/39296#issuecomment-652058883 is
resolved with Go 1.15.

Fixes golang/go#39296

Change-Id: Ibe694281d4d4d40df3dc7bacc5837f593bb1dc5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241398
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-08 18:30:29 +00:00
Rebecca Stambler
134513de88 internal/lsp/regtest: await IWL before running tests
This change makes these tests a little more resilient by making sure
that the initial workspace load is completed before we run them.

Change-Id: I24735da31cc045cb78dfb280e88136b5835351a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241276
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-08 00:37:08 +00:00
Rebecca Stambler
682c4542fd all: update dependencies in go.mod file
CL 226639 changed the positions of the go.mod diagnostics (in a
negligible way), but the tests are quite brittle and can't handle the
different position. Rewrite this test as a regression test to handle it.

The special // indirect marker can be removed from the go/expect package
as a result, since it was only used in this one place.

Change-Id: I7d9a62e32e53d477838e65673635ed231c07b659
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240691
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-06 23:19:48 +00:00
Rebecca Stambler
f01a4bec33 internal/lsp: support opening single files
This change permits starting gopls without a root URI or any workspace
folders. If no view is found for an opened file, we try to create a new
view based on the module root of that file. In GOPATH mode, we just
use the directory containing the file.

I wrote a regtest for this by adding a new configuration that gets
propagated to the sandbox. I'm not sure if this is the best way to do
that, so I'll let Rob advise.

Fixes golang/go#34160

Change-Id: I3deca3ac1b86b69eba416891a1c28fd35658a2ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01 22:10:12 +00:00
Rebecca Stambler
ea7be8d74e internal/lsp/regtest: add regression tests for on-disk file changes
There are still many more cases to check, but this is a good starting
point. A few tests have skips in them because I encountered bugs, which
I plan to go back and fix.

Change-Id: I0b7bbeb632d38c09d6bdb1f4866d81a1690d6ca7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238917
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01 22:06:37 +00:00
Rob Findley
aa94e735be internal/lsp/source: add a new symbolStyle configuration option
Add a symbolStyle configuration option, and use it to parameterize the
following behavior when computing workspace symbols:

 + package (default): include package name in the workspace symbol.
 + full: fully qualify the symbol by import path
 + dynamic: use as the symbol the shortest suffix of the full path that
   contains the match.

To implement this, expose package name in the source.Package interface.
To be consistent with other handling in the cache package, define a new
cache.packageName named string type, to avoid confusion with packageID
or packagePath (if confusing those two identifiers was a problem, surely
it is a potential problem for package name as well).

Change-Id: Ic8ed6ba5473b0523b97e677878e5e6bddfff10a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236842
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
2020-06-26 17:13:37 +00:00
Paul Jolly
84cfedeb1e internal/lsp: fully qualify workspace Symbol matches
Fully qualify (using a package's import path) symbols when matching
them. This is a minimal change in preparation for later changes to add
more advanced query functionality to the workspace Symbol method.

Add more comprehensive regtest tests (covering case sensitive, case
insensitive and fuzzy matchers) in addition to updating lsp tests.

Change-Id: I675cde2f7b492158988cf9c3206a0a55fe29622a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228123
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-26 16:54:09 +00:00
Rebecca Stambler
bcbc01e07a internal/lsp: add a new regtest to reproduce golang/go#39646
Change-Id: I51d8c66a83ecae1c8fc1f39c0e90a03a732c263b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/240063
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-26 03:28:29 +00:00
Heschi Kreinick
e31c80b82c all: rework goimports environment, support GOMODCACHE
This CL got away from me a little.

For a number of reasons, the existing goimports API of passing in values
for various GO* values was not working. For one, the number of necessary
variables kept growing. For another, we tried to avoid calling `go env`
in GOPATH mode by using `build.Default`, but that turns out to be buggy;
see golang/go#39838. And finally, it created massive confusion about
whether the values were intended to be read from the OS environment, or
fully evaluated by the `go` command.

There are only two users of the internal imports API, so there really
shouldn't need to be more than two modes. For the command line tool, we
have to call `go env` to deal with the `go/build` bug. So we just do it.
Tests use that same path, but can augment the enviroment to set
themselves up. In contrast, `gopls` needs to fully specify the
environment. It can simply pass in the fully evaluated GO* values.

Finally, make the change I was actually here to make: propagate
GOMODCACHE and use it where appropriate.

Fixes golang/go#39761.

Change-Id: I720c69839d91d66d98e94dfc5f065ba0279c5542
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239754
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-25 17:33:20 +00:00