Commit Graph

618 Commits

Author SHA1 Message Date
Rob Findley 4fc0492b8e internal/lsp/cache: keep a cached workspace module dir
Keep a workspace module dir around for running the go command against a
snapshot, bound to the contents of the workspace modfile.

This uses the cache's resource model to share the workspace module dir
across snapshots if it is not invalidated, and to delete it when it is
no longer in-use by a snapshot. Of course, the go command will still
only see files on the filesystem, but using this immutable model was
most consistent with the immutable workspace.

For golang/go#41836

Change-Id: Iaec544283b2f545071e5cab1d0ff2a66e6d24dff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263938
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
2020-10-30 20:42:49 +00:00
Rob Findley d36b6f6806 internal/memoize: add a final argument to Bind for cleaning up
With its memoization and refcounting, the cache is well suited to the
sharing of other expensive resources, specifically those that interact
with the file system. However, it provides no means to clean up those
resources when they are no longer needed.

Add an additional argument to Bind to clean up any values produced by
the bound function when they are no longer referenced.

For golang/go#41836

Change-Id: Icb2b12949de06f2ec7daf868f12a9c699540fa5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263937
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
2020-10-30 19:59:21 +00:00
Rob Findley f239dba448 internal/lsp/cache: consider gopls.mod when finding workspace root
gopls.mod files should take precedence over go.mod files when finding a
workspace root. To allow this, implement our own algorithm for expanding
the workspace, rather than using the go command.

For golang/go#41837

Change-Id: I943c08bdbdbdd164f108e44bae95d2c659a6e21e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263897
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
2020-10-30 19:58:41 +00:00
Rob Findley d463eb0e41 internal/lsp/cache: introduce a workspace abstraction
When incorporating the gopls.mod file, the invalidation logic for
workspace module information becomes quite complicated. For example:
 + if a modfile changes we should only invalidate if it is in the set of
   active modules
 + the set of active modules can be provided by either the filesystem or
   gopls.mod
 + if gopls.mod changes, we may gain or lose active modules in the
   workspace
 + if gopls.mod is *deleted*, we may need to walk the filesystem to
   actually find all active modules

Doing this with only concrete changes to the snapshot proved
prohibitively complex (at least for me), so a new workspace type is
introduced to manage the module state. This new abstraction is
responsible for tracking the set of active modules, the workspace
modfile, and the set of workspace directories. Its invalidation logic is
factored out of snapshot.clone, so that it can be tested and to
alleviate some of the growing complexity of snapshot.clone.

The workspace type is idempotent, allowing it to be shared across
snapshots without needing to use the cache. There is little benefit to
the cache in this case, since workspace module computation should be
infrequent, and the type itself consumes little memory.

This is made possible because the workspace type itself depends only on
file state, and therefore may be invalidated independently of the
snapshot. The new source.FileState interface is used in testing, and so
that the workspace module may be computed based on both the session file
state as well as the snapshot file state.

As a result of this change, the following improvements in functionality
are possible:
 + in the presence of a gopls.mod file, we avoid walking the filesystem
   to detect modules. This could be helpful for working in large
   monorepos or in GOPATH, when discovering all modules would be
   expensive.
 + The set of active modules should always be consistent with the
   gopls.mod file, likely fixing some bugs (for example, computing
   diagnostics for modfiles that shouldn't be loaded)

For golang/go#41837

Change-Id: I2da888c097748b659ee892ca2d6b3fbe29c1942e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261237
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
2020-10-30 19:58:30 +00:00
Rebecca Stambler 443cd81abb Revert "internal/lsp: move initialization entirely into the snapshot"
This reverts commit deb1282f04.

Reason for revert: Merge conflicts for Rob's CL stack

Change-Id: I166b3bd60ec2b727a79a090049f0764864656d47
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266699
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-10-30 19:36:23 +00:00
Rebecca Stambler deb1282f04 internal/lsp: move initialization entirely into the snapshot
The majority of the initialization logic is already based on the
snapshot, not the view, so it makes sense to move it there. The
initialization status of the snapshot is copied and invalidated in
clone.

Fixes golang/go#41764

Change-Id: Icebcf8f4dc750ae4d31e19aa93f9a4c0a57beb4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266343
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-10-30 19:34:16 +00:00
Heschi Kreinick 061905c3e8 internal/lsp/cache: stop unnecessarily waiting for IWL
These were added in https://golang.org/cl/237419 and I don't think they
actually fixed anything. I'd like to remove them and see if anything
goes south.

Change-Id: I2f3a48d0e8e40b7a79b963a9319d426cbc6f5d9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266341
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-29 19:59:06 +00:00
Heschi Kreinick e7a17c4c13 internal/lsp/cache: preserve OS environment
In https://golang.org/cl/265237 I accidentally lost the os.Environ()
call, so we weren't using the OS environment as the defaults any more.

Change-Id: Id36e3c04f3c13d512b0b71b8d73ed086d9ee9f1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266337
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2020-10-29 18:29:19 +00:00
Heschi Kreinick 2c115999a7 internal/lsp: use the go command to fix go.mod files
Modifying go.mod files directly leaves go.sum unchanged, and therefore
in need of updates later. Leaving work for the users to clean up isn't
ideal, so it'd be better to use the go command to make modifications.

Unfortunately, the go command has something of a mind of its own. The
most obvious problem is that using go get to add a new require adds a
// indirect comment to that new require, and there's no way to prevent
it. The only thing we can do is add the require first, then use go get
to do nothing but update the go.sum file.

The other inherent problem is that the go command operates on files as
they exist on disk, not the in-memory versions. As discussed, we issue
an error for this case. The alternative would be to work on temporary
files based on the in-memory contents, but that would be much larger
change, so I'd rather not at least right now.

To support Commands for quick fixes, add a new Command field to
source.SuggestedFix, and use it when forming the CodeAction response.

Fixes golang/go#38209.

Change-Id: I0c13ea39199368623e7494e222ba38587323d417
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265981
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-28 22:47:54 +00:00
Heschi Kreinick 49729134c3 internal/lsp: unify go command invocation logic
We have two parallel code paths for Load and other go command
invocations. Unify them by introducing a mode argument to the various
functions that run the go command, indicating the purpose of the
invocation. That purpose can be used to infer what features should be
enabled.

In the future, I hope we can use the mode to decide whether mod
file updates and network access should be allowed.

Change-Id: I49c67fcefc9141287b78c56e9812ee6a8ac8378a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265238
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-28 22:19:10 +00:00
Heschi Kreinick 2b84a066b2 internal/lsp: use gocommand.Invocation more
We pass around verb/args/wd in many places. Bundle them up as an
Invocation instead. goCommandInvocation now updates and returns an input
Invocation.

packages.Config is conceptually an extra layer of parsing and
type-checking on top of a go list invocation. It doesn't make sense for
us to construct the latter using the former. A later CL will construct
the Config in terms of the Invocation; for now duplicate a bit of logic.

Use the environment in the cache key for various module operations
rather than the Config hash. I'm not sure either is fully correct but I
think the environment captures everything that's likely to matter. This
way we don't need Configs in those functions at all.

Change-Id: Iebee2705e63638ab365b3ee18b23f8c3e8ca30ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265237
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-28 21:55:01 +00:00
Rebecca Stambler 6652d1fb14 internal/gocommand: add a GoVersion function to avoid duplicating logic
It seems easier to have this as a shared internal function.

Updates golang/go#41598

Change-Id: If35bdbdf5499624dd55ae9daede1ff1ae9495026
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263985
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>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-10-21 20:36:10 +00:00
Rebecca Stambler d105bfabbd internal/lsp: dynamically register semantic tokens
This CL switches from automatically registering the semantic tokens
capability to dynamic registration. This allows us to turn it on and
off as the option is switched--otherwise it defaults on always.

To achieve this, we also have to set session options on
didChangeConfiguration. It turns out that the passed-in "changed"
parameter can be null, which is why we always refetch the workspace
configuration.

Fixes golang/go#41963

Change-Id: I58d742577ce7f3da67db32011ba21bd9813eb203
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263525
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-10-21 17:10:30 +00:00
Rebecca Stambler d49c4edd7d internal/lsp: do not allow non-Go files to be loaded in packages.load
Fixes golang/go#41962

Change-Id: I47ec376213541095386add09e63a5127473a6d27
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263983
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2020-10-21 00:02:07 +00:00
Heschi Kreinick b894a3290f all: use explicit -mod, -modfile fields for gocommand.Invocation
Build flags, -mod, and -modfile are all accepted by disjoint go command
verbs. Mixing them together had the effect of forcing gocommand users to
figure out which went where themselves, which was annoying and
error-prone. Add ModFlag and ModFile fields to Invocation and update all
uses to use them.

go/packages has a BuildFlags field that's bad for the same reason. Add
private modFlag and modFile fields that forward to the corresponding
Invocation fields.

imports.ProcessEnv gets the same treatment.

Fixes golang/go#41826.

Change-Id: If74d19146e9e62930d7c34f40859c27c25566c4e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263213
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>
2020-10-19 17:57:15 +00:00
Rob Findley 0a3dcccdcf internal/lsp/source: add a FileSource interface
Rename Snapshot.GetFile to GetVersionedFile, and make the signature of
GetFile consistent with the corresponding method on session and cache.
This allows algorithms that depend only on file state to be expressed
using this API. In a subsequent CL, this is used for building and
testing the workspace module.

Preeemptively add the FileSource interface for use in these algorithms.

Change-Id: I550906e554fd290dcdf4cac442d5f223e0f644c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263522
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>
2020-10-19 16:07:06 +00:00
Rebecca Stambler b2746f1d7a internal/lsp: fix nil pointer exception in initialization
Fixes golang/vscode-go#802

Change-Id: Ide3e834d413efcb394370a0c2e5ffa412a45e1a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263520
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>
2020-10-19 15:59:33 +00:00
Rob Findley 6003fad69a internal/lsp/source: delete unused Snapshot.IsSaved
Change-Id: I0f71798d81618a0a9bb992056a3c2d5f71aed150
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263207
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>
2020-10-17 00:14:24 +00:00
Rob Findley 593bd9b1d2 internal/lsp/cache: delay transitive invalidation in snapshot.clone
Currently, walking the transitive closure of invalidated packages
happens eagerly while we invalidate each file. Delay this until after
looping through invalidated files, so that we may more easily consider
invalidations not directly associated with an individual file change.

Change-Id: Id6a95098ce6d5d3ec70143c029ac7d2c9b8c7d28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263205
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>
2020-10-16 23:30:29 +00:00
Rob Findley eae18abd75 internal/lsp/cache: factor out guessPackagesForURI from snapshot.clone
To simplify snapshot.clone a bit, factor out the logic to figure out
which packages to invalidate when a new file arrives. This also made it
easier to improve the algorithm by caching os.Stat results when walking
known packages.

Change-Id: I012b60e8ad73eb4df80950aac48791204877e9e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263204
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: Heschi Kreinick <heschi@google.com>
2020-10-16 21:48:57 +00:00
Heschi Kreinick 828e89dd29 internal/lsp: fix env setting type
We stored the user's environment settings as a string slice, but users
set it as a map. It turns out that we have code spread around to convert
it both back and forth, so rather than hack up the doc generator,
so centralize that code and change the native representation to a map.
We end up with slightly less code.

Fixes golang/go#41964.

Change-Id: I975c69524c7d79c7d09079f44cc44a27e72ba285
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262351
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>
2020-10-15 17:44:03 +00:00
Rebecca Stambler 2db1cd7910 internal/lsp: extract mod errors in load and treat them like diagnostics
This change pulls out the logic to process module-related errors from
go/packages.Load out of internal/lsp and into internal/lsp/cache. This
simplifies diagnostic handling in internal/lsp, as we can just return
a *source.ErrorList. Most of this is just a code move, not really a
logical change.

This also fixes the staticcheck error :)

Change-Id: Ic6cad3f59a7e4492dea85b8dcfa9e3f24a371803
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260805
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>
2020-10-13 05:33:47 +00:00
Rebecca Stambler 5bd0538631 internal/lsp: move the workspaceMode into the snapshot
Workspace mode makes more sense as a property of the snapshot, since
it is determined based on the modules in the workspace. Move it to the
snapshot and enable the GOPATH to modules test. The mode switch means
that we may run `go mod` commands before a `go.mod` is on-disk, so add
handling for that case.

Also, remove the code added in CL 258121 to treat packages starting with
a "_/" the same way as command-line arguments--that's not actually
correct because perfectly valid packages can also have a "_/" package
path prefix.

Fixes golang/go#40340

Change-Id: I35044f5d108983ba00df1359698bf14217caa982
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260078
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>
2020-10-12 19:26:20 +00:00
Heschi Kreinick d01b322e6f internal/lsp/cache: extract goimports code
Many of the View's fields are for goimports, which is a good sign it
should be extracted into a separate struct. Do so in preparation for
redesigning the lifecycle.

I'm not certain this is the right direction but I don't want to deal
with a zillion merge conflicts as I figure it out. I'll move it back
later if it does turn out to have been a mistake.

No functional changes intended, just moved stuff around.

Change-Id: Ide4c2002133d00f6aaa92d114dae2b2ea3ad18fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260557
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>
2020-10-08 18:49:44 +00:00
Rebecca Stambler ffec97847f internal/lsp: handle major versions above v0/v1 in workspace module mode
Workspace mode did not yet support major versions other than v0/v1. To
do so, we have to check the major version before creating the fake gopls
workspace pseudoversion that goes in the workspace module.

Fixes golang/go#41807

Change-Id: I108fe504fdf9e9a0ce23f7102991c9ae78f12a9f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260004
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>
2020-10-08 17:44:24 +00:00
Rob Findley 03e951c4ab internal/lsp/source: add an experimental new cache key for packages
We've recently noted that hashing the packages.Config into the cache key
for the type checked package is probably unnecessary, given that all the
other critical inputs into the typechecker are already included
(packageID, parsed files, and deps). Furthermore, when using a gopls
daemon this causes unnecessary cache misses, because the packages.Config
includes things like working directory that no longer matter once other
inputs to type checking have been computed.

Add an experiment flag that removes the packages.Config from the cache
key. An experiment is used to stage this change as comprehensively
testing the cache is ~impossible.

Change-Id: I7ba73daaa71a80ec996decaa9817ec515b5eeb6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260737
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>
2020-10-08 17:20:56 +00:00
Rebecca Stambler 9df69603ba internal/lsp: don't pass build flags to `go mod` commands
This is a temporary fix--golang/go#41826 is a better approach.

Fixes golang/go#41803

Change-Id: Ia055c5e171fe5d4f6d67e9e2e9e7c85fe254605e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260000
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>
2020-10-08 02:52:39 +00:00
Rebecca Stambler a00137c514 internal/lsp: default to workspace module mode even with vendor dirs
We were previously opting out of workspace module mode when any module
had vendor directories. While we still haven't decided how to handle
vendoring, it simplifies things to opt experimentalWorkspaceModule users
in to the workspace module mode when they have modules with vendoring.

Temporarily require default mode for the inconsistent vendoring test
(golang/go#41819).

Fixes golang/go#41725

Change-Id: Ifa494daea51a2a3bb2e6bc3026bfb9e8118d31a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259623
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>
2020-10-07 00:01:20 +00:00
Heschi Kreinick 23a3aa10a2 internal/lsp: improve handling of files not in views
didModifyFiles and DidModifyFiles were tightly coupled but also repeated
each other's work a bit, and inconsistencies in the implementation led
to golang/go#41777.

Push all the work of assigning a "best view" down to the Session, and
always assign it somewhere, matching the logic in ViewOf. This would
in principle allow us to diagnose random files, but we only diagnose
workspace packages.

Fixes golang/go#41777.

Change-Id: I6dab32b98bdff6edd07032d84a8fec1b82ecd283
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259877
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>
2020-10-06 18:39:45 +00:00
Rebecca Stambler 90a82dd33a internal/lsp/cache: actually remove the view's modURI and sumURI
Should've done this in an earlier CL.

Change-Id: Id0b39c80802c4902f6986d1909112ebba3a9751c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258939
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>
2020-10-06 17:02:58 +00:00
Dan Kortschak 1ccce621de internal/lsp/cache: prevent version from including line break
Change-Id: I3caf8900039eaff9676c5a4040e693e52590d417
Reviewed-on: https://go-review.googlesource.com/c/tools/+/259418
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2020-10-05 18:47:51 +00:00
Rebecca Stambler 22683886a9 internal/lsp: fix go.mod creation without experimental workspace module
We were previously adding modules to the snapshot, even if they weren't
relevant without the workspace module mode. Now, check that the modules
are relevant before adding them.

Change-Id: Ib7600482992d538db2f7451863fee5709a35ffb3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258719
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>
2020-10-02 14:15:43 +00:00
Rob Findley 8445f4f065 internal/lsp: add experimental support for multi-phase diagnostics
An experimental new feature is added to run parsing and checking on
modified files immediately, and run analysis and diagnostics for
transitive dependencies only after debouncing. This feature is disabled
by default.

Also, some refactoring is done along the way:
 + Clean up diagnostic functions a bit using a report collection type.
 + Factor out parsing diagnostics in options.go.

Change-Id: I2f14f9e30d79153cb4219207de3d9e77e1f8415b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/255778
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
2020-10-02 14:12:12 +00:00
Rebecca Stambler 4e032a7e1e internal/lsp: fix and add a test for non-workspace module mode
This issue was reported on Slack. We need to run more tests in the
default mode, but this is a quick fix.

Change-Id: Ic29d0332aa0410b6e80f77d894c8c007c7f251fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258657
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>
2020-10-01 21:53:43 +00:00
Rebecca Stambler af0a1b5f3c internal/lsp: fix staticcheck warning
Turns out I had staticcheck off. Maybe we should run it in presubmits...

Change-Id: Ia7089d67ab271af5161b73408df92de818a1ea67
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258720
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>
2020-10-01 19:14:22 +00:00
Heschi Kreinick 77e61d32f6 internal/lsp/source: remove unused Session method
Change-Id: I4be81fd9a450ed3991127ddc4d4186b8bcd57fba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258857
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>
2020-10-01 18:30:55 +00:00
Rebecca Stambler 1e3611d215 internal/lsp: remove logic for re-creating a view when a go.mod changes
This is no longer necessary now that we modify the modules field based
on newly created/deleted modules.

There was a race condition setting the metadata--we were reusing old
metadata that may have already been cached in some instances. Now, we
always override metadata.

Also, disabled the TestUseGoplsMod test -- there seems to be an issue
with it. Will discuss this offline in the AM.

Change-Id: Ie8c97557d4f0a319051256e5f130b9cdae479928
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258121
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>
2020-10-01 18:12:13 +00:00
Rebecca Stambler e57f6d466a internal/lsp: move hasValidBuildConfiguration into the snapshot
Update hasValidBuildConfiguration as modules are created and deleted.

Change-Id: I9196611225d42a87ea5790c564bc9ac1ea1871f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257968
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>
2020-09-30 21:31:15 +00:00
Rebecca Stambler 2e5f0cfadf internal/lsp: remove all but one use of the view's modURI field
The view reinitialization logic appears to be broken, and so needs one
remaining use of the modURI, which be fixed in a follow-up. Every other
use of view.modURI is removed.

Updates golang/go#32394

Change-Id: Ic051ed848c30e6981d42a576fb35f40efbeb17a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257417
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-09-30 20:43:29 +00:00
Rebecca Stambler 66e72d03b2 internal/lsp: print the go version only once the view is created
Printing the Go version without the session's go command runner means
that we may not find the right Go version. Also, panicking when we
cannot find a go command is not useful to the user--show the error as a
view initialization error instead.

Fixes golang/go#41701

Change-Id: I0e0753da9795b1c78331db1faecd27c2bfcee9b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258312
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>
2020-09-30 16:38:20 +00:00
Rebecca Stambler f1e51e6b94 internal/lsp: stop using modURI as much as possible
This change switches over load and RunProcessEnvFunc to use the
snapshot's modules instead of the view's modURI. These do not seem to
have been the racy parts of CL 257417.

Change-Id: I317a350fc4b0c62d77858455a0e2e61148804ecd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257969
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>
2020-09-29 19:10:02 +00:00
Rob Findley 5272f303b6 gopls: fix various staticcheck errors
Do a pass of unused code cleanup and other staticcheck errors.

Change-Id: Iaf5d27c4f5405d4cce3e48fa06a5d46ec757de40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257965
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
2020-09-29 17:30:36 +00:00
Rebecca Stambler a0ef9b62de internal/lsp: prepare for deletion of view.modURI
Splitting this CL out of CL 257417 to minimize the number of changes.
A few of the view's methods are moved to the snapshot, as they will
soon rely on the snapshot's modules field. Some dead code is also
deleted.

We now populate the snapshot's modules field even when
ExperimentalWorkspaceModule is not true, but we stop looking for modules
after searching the view's root.

Change-Id: Id0068ec10fafcfa6f7694dfcb8aaee8cb025078f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257961
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>
2020-09-28 20:19:43 +00:00
Rob Findley 19e0367891 internal/lsp/cache: use gopls.mod for the workspace module if it exists
When building the workspace module, prefer a gopls.mod file located at
the root of the view if it exists.

Also do some minor documenting/cleanup along the way.

For golang/go#32394

Change-Id: If87729a766d37e6c834fefe40cfb47b67a36a60c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256582
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
2020-09-28 18:20:47 +00:00
Rebecca Stambler 5d1fdd8fa3 internal/lsp: allow multiple go.mod files in a view
This change allows a view to have multiple go.mod files associated
with it. This doesn't actually make any changes in internal/lsp/cache
with regards to the view's modURI, but it does do the necessary plumbing
in the client packages.

The next CL will delete modURI.

Updates golang/go#32394

Change-Id: I2312efd69c364aed4244ee3769679885a1ebc7e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256941
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>
2020-09-25 19:12:24 +00:00
Rebecca Stambler 4c77dbd9e5 internal/lsp: fix race condition caused by config in `go mod tidy`
Reusing the config causes a race condition because it may be modified.
Get a new config in the bound function instead.
See the logs here:
https://storage.googleapis.com/go-build-log/23cc16cd/linux-amd64-race_343e911a.log.

Change-Id: Iaab43631740e43ba97cd70f7cd8a3bbaa91c26c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257206
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>
2020-09-25 16:30:29 +00:00
Danish Dua 8d73f17870 internal/lsp: move package selection to before type checking
This change moves package selection to before type checking so we don't
unnecessarily type-check both variants of a package. As a result, exec
time and memory usage for features making calls to GetParsedFile are cut
by half since we only type check either the narrowest or the widest
package.

Change-Id: Ifd076f8c38e33de2bd3509fe17feafccd59d7419
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257240
Run-TryBot: Danish Dua <danishdua@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-09-24 22:42:22 +00:00
Rebecca Stambler 8a9a89368b internal/lsp/cache: don't set tempModFile is view's modURI is empty
This fixes panics caused by us thinking that we should use tempModFile
without a go.mod in the view. This code will likely go away after
view.modURI is deleted.

Change-Id: I77a9c9f62bcac27cafa69eb970e004dd58d29d07
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257201
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
2020-09-24 20:59:11 +00:00
Heschi Kreinick 7b6ac5b934 internal/lsp/cache: slightly more useful comment
Explain why we're ignoring errors, not just that we are.

Change-Id: I6cbb4945ef6066180953dcaba6f561843d7f20a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256820
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>
2020-09-24 18:10:31 +00:00
Rebecca Stambler 24570c0594 internal/lsp: handle initial workspace load failure per module
This CL adds diagnostics to the go.mod file if one of the modules in
the workspace is invalid and causes the initial workspace load to fail.
When the module is fixed, the initial workspace load will be retried.

This CL also introduces the *source.ErrorList error type, which will be
useful in the future as a way of producing diagnostics out of errors.

Updates golang/go#32394

Change-Id: Ib8860a4f16c1983b8512f75e26354512d5a9a86d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/254753
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>
2020-09-24 18:05:18 +00:00