This change moves from caching package information the file object to
caching in a map that gets invalidated when content changes.
This simplifies cache invalidation and reduces the number of fields
guarded by the (*goFile).mu lock.
Change-Id: I33fef6e0b18badb97e49052d9d6e3c15047c4b63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196984
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change shifts our approach to make sure that a top-level package
only ever imports "trimmed" packages.
Change-Id: I63c35791ef6efad7dac248a9ff877835f46df9ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196523
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This will allow view configuration to modify the set of analyzers being applied, and also allow the main gopls to inject new analyzers
Change-Id: Ic2a76118c3e29b059e19b31bd1fb54b1d9e15e54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196320
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This changes adds basic support for running `go mod tidy` as a code
action when a user opens a go.mod file. When we have a command
available like `go mod tidy -check`, we will be able to return edits as
part of the codeAction. For now, we execute the command directly.
This change also required a few modifications to our handling of file
kinds so that we could distinguish between a Go file and a go.mod file.
Change-Id: I343079b8886724b67f90a314e45639545a34f21e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196322
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We had too many options for functions to use to get type information for
a package. Now we stick with having one option to get the check package
handles, and then the caller can refine the results as needed.
Change-Id: I81f69a670e1539854ee23b6f364159a7de9b782f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is to remove the confusion around having only handles that have had Get
called pin the value into memory.
Instead now there is a single handle per key, and it is the handle that is
weakly held not the value.
Change-Id: I9e813a0dfe2adf4cb651af9b5cfc8878fa71c041
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186839
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This function incorrectly used cached packages to get ASTs and type
information that should have been directly found from the origin
package. Shift to using pkg.FindFile instead.
Change-Id: I9f73209bb1a1343f53b430150e78ffd180e14a44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change allows to remove some of the special handling for the
builtin package.
Change-Id: I105fcefd8812af2d42ff42edca954824c98db429
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195758
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
A mapper is always uniquely tied to a file at a specific version, so
just build it when we get a new *ast.File. We build the mapper using the
*token.File associated with the particular *ast.File, which is why there
is one per ParseGoHandle instead of FileHandle.
Change-Id: Ida40981ef91f6133cdd07e9793337fcd67510fba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Previous changes to the config mechanism made the config options
per-view, not per-session. We should now make sure to obey config
changes per-view. This does not fix the configuration handling for
"watchChangedFile" however. This should be done in a future CL.
Change-Id: I73f6236386c36d2587fdb9c0601670833a4366c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194818
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This fixes the issue of config options not being applied.
Also, handle config errors and deprecation by showing a message to the
user.
Change-Id: I850d5303a7a1e301c97324060a87929710ee6700
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194682
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our original caching plan was to use only the file ParseGoHandles as
cache keys to define a given package. However, because of package test
variants, we cannot rely on files alone. A package may have the exact
same set of files, but be a test variant. Add the ID to the key to avoid
clobbering entries in the cache.
Also, remove the unused metadata ID cache.
Change-Id: I4b33de1f83f6c769d23441e98a2a7324ff0fa1b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194571
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now when a file is deleted we force the file's packages to refresh
go/packages metadata, and kick off diagnostics.
I made a couple other changes to watched file handling:
- Kick off diagnostics in a goroutine to match how DidChange works.
This will allow us to work through big sets of file changes faster,
and will save duplicated work once type checking can be canceled.
- Don't assume a watched file is only part of one view.
Two interesting cases we don't handle yet:
- If the deleted file was the only file in the package, we don't
currently update diagnostics for dependent packages. This requires
rejiggering how diagnostics are invoked a bit.
- If the deleted file is still open in the editor and then later
closed, we don't trigger metadata/diagnostics refresh on DidClose.
Updates golang/go#31553
Change-Id: I65768614c24d9800ffea149ccdbdbd3cb7b2f3d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193121
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We now wait to build views until we have the options for that view,
and pass the options in to the view constructor.
The environment and build flags are now part of the view options.
Change-Id: I303c8ba1eefd01b66962ba9cadb4847d3d2e1d3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This cl is the first in a set that change the configuration behaviour.
This one should have no behaviour differences, but makes a lot of preparatory changes.
The same options are set to the same values in the same places.
The options are now stored on the Session instead of the Server
The View supports options, but does not have any yet.
Change-Id: Ie966cceca6878861686a1766d63bb8a78021259b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193726
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The "Organize imports" code action uses internal/imports that needs a
valid GOPATH set. Since Go 1.8 setting GOPATH manually is not required,
and if it isn't set gopls will sometimes fail to properly import
packages.
This CL sets GOPATH to the default if the env var GOPATH isn't set.
Fixesgolang/go#33918.
Change-Id: Ib63a26a801e15af730197999de4d1d4901694a30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191600
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is a straight move of some code with no changes.
It splits the part of the telemetry code that will become a standalone library from the bit that belongs in the lsp.
Change-Id: Icedb6bf1f3711da9251450531729984df6df7787
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190403
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change fixes a race condition in the metadata caching logic.
Also, some minor fixes to comments and invalidation logic (it's not
necessary to invalidate ASTs when a package is invalidated).
Change-Id: I927bf6fbc661a86ef0ba99b29a9ed979cd1eb95d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change eliminates the need for the package cache map, and instead
stores package type information in the store. We still have to maintain
invalidation logic because the key is not computed correctly.
Change-Id: I1c2a7502b99491ef0ff68d68c9f439503d531ff1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185438
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
On MacOS the default open file limit is 256 files per process. For big
projects this could cause issues when reading all files (more or less)
at the same time.
By moving up the `parseLimit` so the reading of the file is only done
when it is allowed to start parsing the file, we stay well below the 256
files per process (as the `parseLimit` is currently set to 20).
Since `parseLimit` is actually only used to limit IO access, let's also
rename the const to `parseLimit`.
Change-Id: Ie8744030875d84d0d6095ee4ec2d9d553911bed1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189437
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
The results of running 'go list -m' are only valid as long as the
current module and the modules in its replace directives do not
change their go.mod files. Store the 'go.mod' versions that are
used in the imports call, and reinitialize the module resolver if
they change.
Change-Id: Idb73c92b9e4dc243a276885e5333fafd2315134d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186597
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a Logf field to the packages.Config.
Change-Id: I144a9a1e1181585bbe621898c4a3e6a007a38322
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185993
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
The imports ProcessEnv contains cached module and filesystem state. This change
allows gopls to use the same ProcessEnv and resolver across multiple calls to the
internal/imports library.
A ProcessEnv belongs to a view, because the cached module state depends
on the module that is open in the workspace.
Since we do not yet track whether the 'go.mod' file has changed, we
conservatively reset the cached state in the module resolver before
every call to imports.Process.
Change-Id: I27c8e212cb0b477ff425d5ed98a544b27b7d92ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184921
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change removes the need for the ast and token fields on the *goFile
object. We switch to using source.ParseGoHandles on the package, which
means that we can easily access both the AST and token via the package,
which is already cached.
Change-Id: I5f78bbe09362f4d95eb15556617bdbd809a7a55d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185878
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The first deadlock involved differing mutex acquisition order in two
code paths:
1. loadParseTypecheck() holds the "mcache" mutex then eventually
acquires the "handleMu" file mutex.
2. (*goFile).invalidateContent() acquires the "handleMu" mutex first and
then the "mcache" mutex.
Fix by changing the acquisition order in invalidateContent().
The second deadlock involved the file watcher. The two code paths
involved were:
1. (*goFile).GetPackages() holds the view mutex and eventually calls
(*WatchMap).Watch, which acquires the watcher mutex.
2. (*session).openOverlay acquires the watcher mutex as it triggers a
file's callbacks, and then the callback
"(*goFile).invalidateContent" acquires the view mutex.
Fix by not holding the watcher mutex as we invoke the callbacks.
Fixesgolang/go#32910
Change-Id: I9d060e0d80fd86a317a1d6c7aaa736a8ce10bd07
GitHub-Last-Rev: 04944fa0249c0e6f1022a415787e23abce21bc2e
GitHub-Pull-Request: golang/tools#129
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184880
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change modifies gopls to use the internal goimports library, which
allows us to manually configure the ProcessEnv. We also add a logger to
the ProcessEnv to allow this change not to conflict with gopls's logging
mechanism.
Fixesgolang/go#32585
Change-Id: Ic9aae69c7cfbc9b1f2e66aa8d812175dbc0065ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184198
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
There was a situation where we were trying to re-acquire a lock that was
already held. This change solves this issue.
Change-Id: I97cf6bad7e7c219a267e3ca5d174a2573f70ebe2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184217
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There has been a race condition that occasionally appears in test runs
on TryBots. Multiple threads perform type-checking, so they may race on
setting the fields of the *goFiles. Add a mutex to synchronize this.
Change-Id: If52c9d792c6504fc89044964998b06de7dfbd19c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/183978
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change adds supports for a package belonging to multiple files.
It requires additional packages.Loads for all of the packages to which a
file belongs (for example, if a non-test file also belongs to a package's
test variant).
For now, we re-run go/packages.Load for each file we open, regardless of
whether or not we already know about it.
This solves the issue of packages randomly belonging to a test or not.
Follow-up work needs to be done to support multiple packages in
references, rename, and diagnostics.
Fixesgolang/go#32791Fixesgolang/go#30100
Change-Id: I0a5870a05825fc16cc46d405ef50c775094b0fbb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/183628
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change just separates minor changes made along the course of the
memoization CL out into their own change. This will clean up the diffs
in the memoization CL.
Change-Id: I7d59e05ba6472af5f1bf516b1e5b879a5815b9a5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/183250
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This adds an IDs map to the metadata cache, which maps package paths to
IDs. This is only ever used by the Import function in the type checker.
Change-Id: I8677d9439895bc6cbca5072e3fa9fddad4e165d5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181683
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Previously when you added a new file to an existing package, the new
file would get stuck with the "no package for file" error until you
saved the file and then made changed a different file in the
package. There were two changes required to fix the errors:
First, we need to invalidate the package cache when a new file is
added to a package so that the package will actually re-parse and
re-type check. We now notice if file names changed when updating a
package's metadata and invalidate the package cache accordingly.
Second, when dealing with overlay (unsaved) files, we need to map
the *goFile to the package even if we fail to parse the
file (e.g. the new file fails to parse when it is empty). If we don't
map it to the package, the package won't get refreshed as the file is
changed.
Fixesgolang/go#32341
Change-Id: I1a728fbedc79da7d5fe69554a5893efcd1e1d902
GitHub-Last-Rev: e7c3d4c1f8f73b12c87ee76d868cc04893e55808
GitHub-Pull-Request: golang/tools#111
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change replaces the strings that were previously used for both the
ID and package path fields. This is a precursor to the change that will
replace the uses of package path with package ID.
Change-Id: I353e98aedede9b85c7a183fdd49048ff43b1e26d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We panic if the uri was not a valid file uri instead
They always are a valid file URI, and we would fail miserably to cope if they were
not anyway, and there are lots of places where we need to be able to get the filename
and don't want to cope with an error that cannot occur.
If we ever have not file uri's, you will have to check if it is a file before calling
.Filename, which seems reasonable anyway.
Change-Id: Ifb26a165bd43c2d310378314550b5749b09e2ebd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181017
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change marks any unresolved packages discovered as imports by
go/packages.Load. It re-runs go/packages.Load for any package with
missing imports, which will result in the LSP registering changes in
dependencies.
However, this means that we re-run go/packages.Load on
package's with unresolved imports much more than we normally would, so
it may result in a slowdown or unexpected behavior. I'm not sure if this
is necessarily the correct approach here.
Updates golang/go#32232
Change-Id: Id611fa1876e42c88ca2c3e4db30da66dc66945fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/180537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change correctly invalidates the cache when we
have to go from a trimmed to untrimmed AST.
The "ignoreFuncBodies" behavior is still disabled due to a racy test.
Updates golang/go#30309
Change-Id: I6b89d1d2140d77517616cb3956721a157c25ab71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/180857
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
On FileHandle Read now just returns the data hash and error
This makes it more obvious that you should handle the error, rather than hiding
it all in a struct.
We also change the way we get and return content, the main source.File
constructs now hold a FileHandle that then updates on invalidation
Change-Id: I20be1b995355e948244342130eafec056df10081
Reviewed-on: https://go-review.googlesource.com/c/tools/+/180417
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This updates overlays immeditely, and uses the handle identity change to
correctly update the content on demand.
Fixesgolang/go#32348
Change-Id: I3125a6350cac358b7c0f7dc11f2bd11ae1f41031
Reviewed-on: https://go-review.googlesource.com/c/tools/+/179922
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add 'buildFlags' config to processConfig and pass that value to packages.Config.
We can avoid incorrect diagnostics such as if current source codes require any build tags.
Change-Id: Id191469ec75eedaa82b75ec4fdec084fa78c2c5d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/178782
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
This change fixes a regression introduced by the building the builtin
package on demand. Although this change increases the startup tasks of
gopls, it is necessary to ensure that we ignore diagnostics from
builtin.go.
Change-Id: I897e747a273056d70cecba486a74c75a736d8f80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/179921
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
