1
0
mirror of https://github.com/golang/go synced 2024-10-01 05:28:33 -06:00
Commit Graph

193 Commits

Author SHA1 Message Date
Heschi Kreinick
c9619e8fac internal/lsp: separate LSP files from FS files
FileHandle currently includes LSP-level information about Version and
Session. That's dangerous, because the cache operates in terms of
URIs and content only -- we explicitly want to share results across
sessions and versions if they happen to be the same.

Split the LSP information into separate types, VersionedFileHandle and
VersionedFileIdentity.

Change-Id: I158646b783375b58245468599301e2a29c657e71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245058
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-03 22:16:06 +00:00
Heschi Kreinick
39fdd541e6 internal/lsp: move builtin package to Snapshot
The builtin package was the one special case where we parsed Go outside
the context of a Snapshot. Move it up.

Change-Id: I1f4bb536adb40019e0ea9c5c89f38b15737abb8c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245057
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-03 22:09:14 +00:00
Heschi Kreinick
412b8bda49 internal/lsp/cache: ref-count snapshots
To manually collect cache entries, we need to know when a snapshot is
idle. Add a reference count in the form of a WaitGroup and keep track of
its uses. The pattern is that any time a snapshot is returned, it comes
with a release function that decrements the ref count.

Almost all uses of a snapshot originate in a user-facing request,
handled in beginFileRequest. There it's mostly an exercise in passing
Snapshots around instead of Views.

In the other places I took the path of least resistance. For file
modifications I tried to minimize the amount of code that needed to deal
with snapshots. For diagnostics I just acquired the snapshot at the
diagnostics call.

Change-Id: Id48a2df3acdd97f27d905e2c2be23072f28f196b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241837
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-03 22:08:54 +00:00
Heschi Kreinick
6467de6f59 internal/lsp: remove Mod handles
Continuing the massacre, remove ParseModHandle, and Mod*Handle, from the
source API.

Notably, having the snapshot available means we can simplify the go
command invocation paths a lot.

Change-Id: Ief4ef41e42f93d653f719a230004861e5e1ef70b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244769
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-29 19:44:36 +00:00
Heschi Kreinick
b6476686b7 internal/lsp: remove PackageHandle
Just like ParseGoHandle, PackageHandle isn't very useful as part of the
public API. Remove it.

Having PackagesForFile take a URI rather than a FileHandle seems
reasonable, and made me wonder if that logic applies to other calls like
ParseGo. For now I'm going to stop here. I could also revert that part
of the change.

Change-Id: Idba8e9fdba0b0c48e841a698eb97e47fd5f23cf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244637
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:35:25 +00:00
Heschi Kreinick
a9439ae9c1 internal/lsp: replace ParseGoHandle with concrete data
ParseGoHandles serve two purposes: they pin cache entries so that
redundant calculations are cached, and they allow users to obtain the
actual parsed AST. The former is an implementation detail, and the
latter turns out to just be an annoyance.

Parsed Go files are obtained from two places. By far the most common is
from a type checked package. But a type checked package must by
definition have already parsed all the files it contains, so the PGH
is already computed and cannot have failed. Type checked packages can
simply return the parsed file without requiring a separate Check
operation. We do want to pin the cache entries in this case, which I've
done by holding on to the PGH in cache.pkg.

There are some cases where we directly parse a file, such as for the
FoldingRange LSP call, which doesn't need type information. Those parses
can actually fail, so we do need an error check. But we don't need the
PGH; in all cases we are immediately using and discarding it.

So it turns out we don't actually need the PGH type at all, at least not
in the public API. Instead, we can pass around a concrete struct that
has the various pieces of data directly available.

This uncovered a bug in typeCheck: it should fail if it encounters any
real errors.

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

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

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

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

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

Change-Id: I6c5a3f462b1f19cbca6a267fedc36ce54613b6fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244018
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28 17:34:24 +00:00
Rebecca Stambler
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
37a045f3b9 internal/lsp: move undeclaredname suggested fix out of analysis
This CL is a follow-up from CL 241983. I didn't realize that the
undeclaredname analysis was also using the go/printer.Fprint trick,
which we decided was both incorrect and inefficient. This CL does
approximately the same things as CL 241983, with a few changes to make
the approach more general.

source.Analyzer now has a field to indicate if its suggested fix needs
to be computed separately, and that is used to determine which
code actions get commands. We also make helper functions to map
analyses to their commands.

I figured out a neater way to test suggested fixes in this CL, so I
reversed the move to source_test back to lsp_test (which was the right
place all along).

Change-Id: I505bf4790481d887edda8b82897e541ec73fb427
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242366
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-23 23:54:27 +00:00
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
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
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
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
Muir Manders
1837592efa internal/lsp/source: speed up completion candidate formatting
Completion could be slow due to calls to astutil.PathEnclosingInterval
for every candidate during formatting. There were two reasons we
called PEI:

1. To properly render type alias names, we must refer to the AST
   because the alias name is not available in the typed world.
   Previously we would call PEI to find the *type.Var's
   corresponding *ast.Field, but now we have a PosToField cache that
   lets us jump straight from the types.Object's token.Pos to the
   corresponding *ast.Field.

2. To display an object's documentation we must refer to the AST. We
   need the object's declaring node and any containing ast.Decl. We
   now maintain a special PosToDecl cache so we can avoid the PEI call
   in this case as well.

We can't use a single cache for both because the *ast.Field's position
is present in both caches (but points to different nodes). The caches
are memoized to defer generation until they are needed and to save
work creating them if the *ast.Files haven't changed.

These changes speed up completing the fields of
github.com/aws/aws-sdk-go/service/ec2 from 18.5s to 45ms on my laptop.

Fixes golang/go#37450.

Change-Id: I25cc5ea39551db728a2348f346342ebebeddd049
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221021
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-01 04:11:22 +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
Rebecca Stambler
fadf93ffb2 internal/lsp: watch all files in the module and replace target
Previously, our file watching only considered the root directory of the
view, which may not include the entire module or its replaced
dependencies. Now we expand our watching to include the whole module.

As part of testing this, I noticed that VS Code's file watcher actually
only sends updates for files in the workspace, even if we request
notifications for all files in the module. I filed an issue to ask about
this: https://github.com/microsoft/vscode-languageserver-node/issues/641.

Change-Id: I9499d31aff273f69e9c117511e7985ff58b7fdc4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239198
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-25 21:08:52 +00:00
Rebecca Stambler
1745ac5bc6 internal/lsp: refactor various module-specific handles in cache
This change separates out different functions of mod handles.
Previously, we had ModHandle and ModTidyHandle. ModHandle was used to
parse go.mod files and get the results of `go mod why` and possible
dependency upgrades.

Now, we factor this out into 4 handles: ParseModHandle, ModWhyHandle,
ModUpgradeHandle, and ModTidyHandle. This allows each handle to be
specific to its own functionality. It also simplifies the code a bit,
as the handles can be written in terms of ParseModHandles instead of
FileHandles.

I may have some follow-up CLs to refactor the `go mod tidy` logic out of
the cache package, though I'm no longer certain that that's a good
choice.

Change-Id: I8e12299dfdda7bb61b05903d9aa474461d7f4836
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-23 18:31:46 +00:00
Rebecca Stambler
20e05c1c8f internal/lsp: don't use -modfile for go mod commands
These are commands whose changes should be reflected in the existing
go.mod file, as they do not provide edits. Add a third way of running
the go command, explicitly without -modfile. Update the regression test
accordingly.

Change-Id: I866b5db83b504fae190e58c306c01a7a4296672d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-22 20:30:43 +00:00
Heschi Kreinick
4fd1c64487 internal/lsp/cache: fix ignored file check
The return of IgnoreFile. We continue using go list's ignore rules,
but only apply them to the part of the path underneath the source root.

This should be fixed to include replace targets once we have them.

Change-Id: I054fee8c12fc860a279b0d0c1fd670f44d78a63f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239288
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-06-22 19:29:24 +00:00
Rebecca Stambler
47c907e258 internal/lsp: use a new temporary go.mod for every go list call
Refactor internal/lsp/cache to use a new temporary go.mod file for each
go command invocation. This cleans up the abstraction in the source
package, as we no longer are aware of temporary go.mod files.

This will also fix the raciness of reusing the same temporary go.mod
file for each invocation.

Updates golang/go#37318.
Fixes golang/go#39504.

Change-Id: I90bc17a678b5df222ab598c8f7dbf6c6fdd393f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-18 02:37:23 +00:00
Rebecca Stambler
6aa8f57aac internal/lsp: un-export (*snapshot).Config to limit it to cache
Limiting the Config to the view seems reasonable, considering that it is
only used to run the `go` command. I prefer just having the cache run
go commands, so that source doesn't have to deal with the environment.

This also enables CL 237517.

Change-Id: I639c082592de30e9682dc25cdd12c7751ddb4f97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-15 22:28:25 +00:00
Rebecca Stambler
3c1b287bbd internal/lsp: await the initial workspace load in ModHandle
ModHandle races with the initial workspace load if the go.mod file does
not yet exist. We should await for the initial workspace load to
complete before proceeding with update codelenses, etc.

Part of trying to figure out the flakes in golang/go#39504.
Also a few staticcheck fixes, and fix the Windows line endings in
fill_struct.go, because `git gofmt` complains.

Change-Id: Ide21a47137390792d1afb924740cff0bb6f0b764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-12 18:21:55 +00:00
Heschi Kreinick
f520afa52e internal/lsp: remove Ignore feature
Ignore ignored the builtin package and files that start with _. The
latter should already be ignored by "go list". The former seems
like too much effort to me. People shouldn't edit random parts of the
stdlib, and ignoring changes to (e.g.) the Error interface seems like
the least of the trouble they can get themselves into.

Remove it for now. If we get complains I'll re-add it, probably by
rejecting the write entirely somewhere.

We incidentally relied on this in the identifier functions; change those
to treat the builtin package slightly more specially.

Change-Id: I005b02a66b1a987c50a3074d53a2d28ff07d3324
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237597
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-11 22:55:14 +00:00
Heschi Kreinick
ecd3fc4348 internal/lsp: read files eagerly
We use file identities pervasively throughout gopls. Prior to this
change, the identity is the modification date of an unopened file, or
the hash of an opened file. That means that opening a file changes its
identity, which causes unnecessary churn in the cache.

Unfortunately, there isn't an easy way to fix this. Changing the
cache key to something else, such as the modification time, means that
we won't unify cache entries if a change is made and then undone. The
approach here is to read files eagerly in GetFile, so that we know their
hashes immediately. That resolves the churn, but means that we do a ton
of file IO at startup.

Incidental changes:

Remove the FileSystem interface; there was only one implementation and
it added a fair amount of cruft. We have many other places that assume
os.Stat and such work.

Add direct accessors to FileHandle for URI, Kind, and Version. Most uses
of (FileHandle).Identity were for stuff that we derive solely from the
URI, and this helped me disentangle them. It is a *ton* of churn,
though. I can revert it if you want.

Change-Id: Ia2133bc527f71daf81c9d674951726a232ca5bc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237037
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-11 22:11:59 +00:00
Ian Cottrell
ce53dc4445 internal/lsp: clean out the debug handling
This removes all the cache/session/view hooks from the lsp and instead
uses normal introspection from the client to find all the entities
rather than keeping shadow copies of the object graph in the debug page
handler.
This required the addition only of the ability to view the sessions open
on a cache and exposing the unique identifier for all the objects, both
of which are useful and reasonable things to have in the API anyway.

Change-Id: Ic19903e7b19832ca79290954ec373d4177789742
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236197
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:14 +00:00
Rebecca Stambler
4d5ea46c79 internal/lsp: support go mod vendor as a command
In addition to adding a `go mod vendor` command option, which can be
exposed via an editor client frontend, we show a suggestion to users who
experience the "inconsistent vendoring" error message.

The main change made here is that we save the view initialization error,
and we return it if the view has absolutely no metadata. This seems
reasonable enough, but my fear is that it may lead to us showing
outdated error messages. I will spend some time improving the handling
of initialization errors in follow-up CLs.

Updates golang/go#39100

Change-Id: Iba21fb3fbfa4bca956fdf63736b397c47fc7ae44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235619
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-04 18:33:45 +00:00
Michael Matloob
10921354bc go/packages: add a Module field to the Package struct
This change introduces Module and ModuleError struct types to the
packages package with the same types as defined in the cmd/go
documentation for module information output by go list (with the
exception of the Module type's Versions and Update fields).
go/packages will fill the module struct with the module information
output by go list. Drivers that support modules can also provide
module information by filling the Module fields in the packages in
their driverResponses.

Fixes golang/go#35921

Change-Id: Icbdf79869f09d26f6a01c3670146ace4f6ffa25e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234219
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-19 14:27:18 +00:00
Heschi Kreinick
d3bf790afa internal/lsp: add Regenerate Cgo code lens
Now that we support authoring cgo packages better, we need a way to
regenerate the C definitions. Doing it automatically is very difficult;
in particular, referencing a new symbol from the C package may require
regeneration, but we don't want to do that for every typo.

For now, give the user a button and make them push it. We attach a
code lens to the import "C" line. This is vulnerable to the usual
user-didn't-save glitches.

Updates golang/go#35721.

Change-Id: Iaa3540a9a12bbd8705e7f0e43ad0be1b22e87067
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234103
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-15 22:01:28 +00:00
Rob Findley
8ddc06776e internal/lsp/source: don't link to packages matching GOPRIVATE in hover
Currently, our hover text by default links point to public documentation
sites (e.g. pkg.go.dev). This doesn't make sense for private repos, so
hide the hovertext link when the import path matches GOPRIVATE.

Implementing this was a little messy. To be optimal I had to thread
the value of goprivate through cache.view, and to be correct I had to
duplicate some code from cmd/go internal.

Regtest will follow after https://golang.org/cl/232983 is submitted.

Updates golang/go#36998

Change-Id: I1e556471bf919fea30132d9642426a08fdb7f434
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233524
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-15 19:36:02 +00:00
Rob Findley
cb8d9cd245 internal/lsp: support configurable codeLens
Some code lenses may be undesirable for certain users or editors -- for
example a code lens that runs tests, when VSCode already supports this
functionality outside of the LSP. To handle such situations, support
configuring code lenses via a new 'codelens' gopls option.

Add support for code lens in regtests, and use this to test the new
configuration. To achieve this, thread through a new 'EditorConfig' type
that configures the fake editor's LSP session. It made sense to move the
test Env overlay onto this config object as well.

While looking at them, document some types in source.Options.

Change-Id: I961077422a273829c5cbd83c3b87fae29f77eeda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232680
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-08 18:47:35 +00:00
Rob Findley
08cbf656ce internal/lsp/cache: add an UnsavedFiles method to Session
It is useful to know whether the session has any unsaved files, for
example to warn/error when executing a command that interacts only with
files on disk.

Add a new UnsavedFiles method to the Session.

Change-Id: Iea4bf472e3ed6897979306b670eb974a2ee0d3bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232747
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-07 17:51:45 +00:00
Rebecca Stambler
88e38c1d8d internal/lsp: make sure diagnostics only refer to existing files
We were previously sending diagnostics for nonexistent files, and then
adding them to the snapshot in the process. Remove this behavior, and
add a regression test. Case insensitive filesystems were too confusing
to write a test for, but fortunately, Filippo reported another instance
of this bug, so I used that for the regression test.

Fixes golang/go#38602

Change-Id: I4ef6b51944f3338e838875a5aafffd066e8392f4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230315
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-07 02:01:22 +00:00
Rebecca Stambler
4d14fc9c00 internal/lsp: add type error fixes to existing diagnostics
This change is the first step in handling golang/go#38136. Instead of
creating multiple diagnostic reports for type error analyzers, we add
suggested fixes to the existing reports. To match the analyzers for
FindAnalysisError, we add an ErrorMatch function to source.Analyzer.

This is not an ideal solution, but it was the best one I could come up
with without modifying the go/analysis API. analysisinternal could be
used for this purpose, but it seemed to complicated to be worth it, and
this is fairly simple. I think that go/analysis itself might need to be
extended for type error analyzers, but these temporary measures will
help us understand the kinds of features we need for type error
analyzers.

A follow-up CL might be to not add reports for type error analyzers
until the end of source.Diagnostic, which would remove the need for the
look-up.

Fixes golang/go#38136

Change-Id: I25bc6396b09d49facecd918bf5591d2d5bdf1b3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-08 01:45:16 +00:00
Rebecca Stambler
3304cfb00f internal/lsp: temporarily disable type error analyzers by default
If we release gopls/v0.4.0 soon, we should keep these new analyzers off
by default. They were just merged, so they haven't been used enough to
be enabled, I think. We'll turn them on by default for gopls/v0.5.0.

Also, ended up creating a helper function to check if analysis has been
abled (which fixed a small bug in FindAnalysisError), and another helper
function to enable all analyses for testing purposes.

Updates golang/go#38212

Change-Id: I5ee94b3582dfc0863978650fc6ce51bfa0606c13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226962
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-04-02 16:54:32 +00:00
Rohan Challa
e46a7b92c0 internal/lsp: add support for sourceFixAll analyzers
This change adds support for analyzers that have suggested fixes of the kind Source.FixAll. This will allow these fixes to be applied on save if the user desires.

To auto apply these fixes on save, make sure your settings.json looks like:

"[go]": {
	"editor.codeActionsOnSave": {
		...
		"source.fixAll": true,
		...
	},
	...
}

Update golang/go#37221

Change-Id: I534e4f6c8c51ec2848cf2899aab68f587ba68423
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223658
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 20:01:16 +00:00
Rohan Challa
02a6ca6dc3 internal/lsp: change disabledAnalyses setting to be general
This change removes the disabledAnalyses setting and replaces it with a more general feature named "analyses". This will allow users to opt in as well as opt out of analyzers that they do not find useful. This also updates some documentation to show users what analyzers gopls is using and which are enabled by default.

Change-Id: Id3239b4c4c9667e834f262c889270d14fdba0f93
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223662
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-26 19:30:34 +00:00
Rebecca Stambler
6fc5d0bc36 internal/lsp: print view-specific environment
For debugging purposes, we print the output of `go env` on start. Now,
we print a view-specific `go env`, which will helps us when debugging
multi-project workspaces. Additional information included: the folder of
the view, if the view has a valid build configuration, and the build
flags for the view.

Updates golang/go#37978

Change-Id: Iadffd46f8ac5410971558a304b8bbcb2f9bdbcc3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-24 20:15:47 +00:00
Rob Findley
bc073721ad internal/lsp/cache: include session IDs in some cache keys
When caching file data specific to a session (anything with a Version or
tied to a view), we now need to be more careful about the existence of
multiple sessions.

This change fixes a few places where we appear to cache session data
without explicitly referring to the session. In principal this could
cause data corruption in multi-session gopls instances, but I have not
been able to force this to occur in either manual or automated testing.

Also fix a data race to the unsaved overlays:
https://storage.googleapis.com/go-build-log/588ee798/linux-amd64-race_d0762522.log

Updates golang/go#34111

Change-Id: I47117da1aba1afc2e211785544ad3f8b3416d15d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-03-05 20:50:14 +00:00
Rohan Challa
abb57c682a internal/lsp: support textDocument/hover for .mod extension
This change implements support for textDocument/hover when it comes to go.mod files.

Updates golang/go#36501

Change-Id: Ie7da0194bb972955b7ab9cf7b9c9972bd9f4b8a9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220359
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-26 15:59:49 +00:00
Rohan Challa
48cfad2f5e internal/lsp: support textDocument/documentLink for .mod extension
This change implements support for textDocument/documentLink when it comes to go.mod files.

Updates golang/go#36501

Change-Id: Ic0974e3e858dd1c8df54b7d7abee085bbcb6d4ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219938
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-26 15:45:04 +00:00
Rohan Challa
7c4b6277d7 internal/lsp: add module versions from "go list" to pkg.go.dev links
This change appends to the pkg.go.dev link the version of the module that is being used. To get this functionality, go/packages.Package now contains a module field which gets populated from the "go list" call. This module field is then used to get the version of the module that we are linking to.

Updates golang/go#36501

Change-Id: I9668a6da0fd3ec8f4cde017986419c8d28196765
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219079
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-19 19:55:21 +00:00
Rob Findley
c4d4ea9c79 internal/lsp/cache: return concrete types where possible
For testability, and to allow the exchange of debug information when
forwarding the LSP, it will be necessary to access debug information
stored in cache objects. This is cumbersome to do if our constructors
return source interfaces rather than concrete types.

This CL changes cache.New and (*Cache).NewSession to return concrete
types. This required removing NewSession from source.Cache. I would
argue that this makes sense from a philosophical perspective: everything
in the source package operates in a context where the Session and Cache
already exist.

Updates golang/go#34111

Change-Id: I01721db827d51117f9479f1544b15cedae0c5921
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220077
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-19 18:42:16 +00:00
Rohan Challa
bc664416c6 internal/lsp: fix error handling when getting go.mod codelens
This change has a fix for mod/codelens: check if we get an error from ParseModHandles().Upgrades(). This change also only runs codelens on save.

Change-Id: I6dab7ddf3a08c650e4c670b039b1e99153ec8187
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219478
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-14 19:54:16 +00:00
Rohan Challa
98b3097d01 internal/lsp: add codelens for go.mod dependency upgrades
This change adds a code lens for go.mod files that will let a user know if a module can be upgraded, once it is clicked gopls will run a command to update that module.

Updates golang/go#36501

Change-Id: Id22b8097ede4972cf73bc029ec927544a71b7150
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218557
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-14 14:15:08 +00:00
Muir Manders
49b8ac185c internal/lsp/cache: add file contents to ParseGoHandle
Currently there is no need for this because the file contents are part
of the file handle. This change is in preparation for an impending
improvement that tweaks the source code during the parse stage to fix
certain kind of terminal parse errors. Any code that wants to use
an *ast.File or *token.File in conjunction with the file contents
needs access to the doctored source code so things line up.

Change-Id: I59d83d3d6150aa1264761aa2c1f6c1269075a2ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218979
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 05:05:14 +00:00
Rebecca Stambler
7cfd24942e internal/lsp/cache: hardcode parse modes instead of guessing them
This change largely reverts CL 217139, which attempted to guess a
package's parse mode based on whether or not it was in the user's
workspace. This ignored the fact that a user may jump to the definition
of a file outside of their workspace.

Fixes golang/go#37045

Change-Id: Icb6b9d055bd1f260013227db1a6a34873c45b680
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218499
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-07 20:00:15 +00:00
Rebecca Stambler
3d51b05cfb internal/lsp: don't use overlays from the session in the snapshot
Hold the session's overlay mutex the whole time we compute new overlays,
and then pass these overlays directly into clone. This avoids us calling
s.session.GetFile, which can return overlays that the snapshot doesn't
yet "know" about.

Change-Id: I1a10c78e26f8fec64550bfe0a97b5975ea8f976b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 19:21:16 +00:00
Rebecca Stambler
b753a1ba74 internal/lsp: build overlays through the snapshot
This is the first in a series of changes to move overlay handling to the
snapshot instead of the session. We may not be able to fully get away
from managing overlays on the session, but we should be able to only use
overlays when they are known to the snapshot.

Change-Id: I88b125117cd2cfbd0ff9ef16a944a34297c81b10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218324
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-07 18:37:49 +00:00