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

87 Commits

Author SHA1 Message Date
Danish Dua
b793a1359e internal/lsp: add outgoing calls call hierarchy
* Adds outgoing calls call hierarchy for function declarations to gopls. Returns all call ranges and call items for functions/literals being called.
* Adds tests for outgoing call.
* Updates cmd to account for call ranges and call items being in different files for outgoing calls.
* Updates prepare call hierarchy to return declaration as root instead of cursor position.

Example:
Example shows https://github.com/golang/tools/blob/master/internal/lsp/source/call_hierarchy.go

Show Call Hierarchy View: https://imgur.com/a/DA5vc6l
Peek Call Hierarchy View: https://imgur.com/a/fuiG0Be

Note:
* While incoming calls for a function defined in an interface return references to that function, outgoing calls don't return anything since we don't know what implementation to return outgoing calls for.* Outgoing calls to function literals show as variable name used to define the literal, compared to <scope>.func() for incoming calls.

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

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

Fixes golang/go#40809.

Change-Id: I96279f4ff994203694629ea872795246c410b206
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249120
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-19 19:22:15 +00:00
Heschi Kreinick
f29cbc7105 internal/lsp: remove source.Cache
snapshot.View().Session().Cache().FileSet() has been driving me crazy
for a while. Add it to snapshot. Along the way, discover that the Cache
interface is now totally unused and delete it.

I also changed a bunch of View arguments to Snapshot while I was in the
area.

Change-Id: I1064d0020b1567c2ed28d2d55e0f4649eb94c060
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245324
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-05 22:08:24 +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
Rebecca Stambler
449c585133 internal/lsp: properly check for nil on i.enclosing
Fixes golang/go#40479

Change-Id: If827a86fd603574d3adde295c1165bb6267732eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245486
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-29 16:35:35 +00:00
Rebecca Stambler
df70183b18 internal/lsp/source: add extra nil check in searchForEnclosing
I still keep seeing this crash too, even after CL 244841.

Fixes golang/go#40464

Change-Id: Ic587045e65f34c24bd6df452e24517fd90e36bbe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245440
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-29 04:18:21 +00:00
Rob Findley
e7a7e3a8a0 internal/lsp/source: add a unit test for searchForEnclosing
The logic to resolve the enclosing type for an identifier is somewhat
tricky. Add a unit test to exercise a few edge cases.

This would probably be easier to read and write using a hybrid approach
that extracts markers from the source.

This test uncovered a bug, that on the SelectorExpr branch we were
accidentally returning a nil *Named types.Type, rather than a nil
types.Type.

Change-Id: I43e096f51999b2a6e109c09d3805ad70a4780398
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244841
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-28 18:51:24 +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
Rebecca Stambler
eaaaedc6af internal/lsp: fix hover link for embedded fields and methods
Our logic to generate documentation links did not account for embedded
fields and methods. The types.Info.ObjectOf an embedded field returns
the *types.Var created for the field, not its types.TypeName, so we have
to navigate back to the actual definition of the field. This requires
traversing through all of the named types in the top-level type.

Fixes golang/go#40294

Change-Id: Ia6573aebe66b7f60e2d6861a381cd7b07e7d7eaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244178
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-27 15:46:26 +00:00
Rebecca Stambler
7017fd6b13 internal/lsp: fix hover for implicit type switch variable declarations
There was a bug in the hover for type switch variables. For example:

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

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

Change-Id: Ia9bd0afc4ddbb9d33bbd0c78fa32ffa75836a326
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-07-24 02:27:22 +00:00
Rebecca Stambler
e66011cbf8 internal/lsp: fix error in CL 242457
I didn't properly understand Heschi's comment, and missed a case where
we failed to return early.

Change-Id: I692c26678b2e659c6b748085bb4b16090e04472d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/243117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-16 17:41:35 +00:00
Rebecca Stambler
ddb87c8c46 internal/lsp: add an importShortcut configuration
This configuration deals with the fact that VS Code has the same
shortcut for go-to-definition and clicking a link. Many users don't like
the behavior of this shortcut triggering both requests, so we allow
users to choose between each behavior.

Fixes golang/go#39065

Change-Id: I760c99c1fade4d157515d3b0fd647daf0c8314eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-15 23:00:36 +00:00
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
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
Rebecca Stambler
8b0f8a7919 internal/lsp/source: handle nil pointer in package name hover
Updates golang/go#38977.

Change-Id: I8cbf0b058d77e749285cfe41b4b49de3764be861
Reviewed-on: https://go-review.googlesource.com/c/tools/+/233177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-05-11 19:44:01 +00:00
Ian Cottrell
535e1470ec internal/lsp: use %w in error wrappers
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.

Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 14:52:14 +00:00
Rebecca Stambler
b8428586f4 internal/lsp: handle hover documentation for package declarations
We had previously not been generating documentation on hover for
package declarations or import specs. We do this by adding a few special
cases, since package declarations don't appear in type information.

Throughout, we make the assumption that only one file in a package will
have the documentation for the package. go/doc just appends
documentation as it sees it. We may be able to do better by checking for
a "Package ..." but that still is not guaranteed. Not sure what the
right approach is, so this assumption may be the best option.

Fixes golang/go#38526

Change-Id: Ibc515f8729e1aba0d11090be62371be6b18d0cfa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/230417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-04-29 20:57:41 +00:00
Ian Cottrell
7b212d60a1 internal/event: renaming the main event API functions
event.Log removed
event.Print -> event.Log
event.Record -> event.Metric
event.StartSpan -> event.Start

In order to support this core now exposes the MakeEvent and Export functions.

Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:21:36 +00:00
Ian Cottrell
cf0cb92717 internal/telemetry: renaming to internal/event
internal/telemetry/event was renamed to internal/event/core
Some things were partly moved from internal/telemetry/event straight to
internal/event to minimize churn in the following restructuring.

Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:20:48 +00:00
Ian Cottrell
206ec5b82a internal/lsp: migrate telemetry to using the event package
Change-Id: Idc662385ed08bd62593ccd1d54afd3fa8c1a7d29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 03:59:16 +00:00
Danish Prakash
c94e1fe145 internal/lsp/source: return location(s) for imported packages
Fixes golang/go#32993

Change-Id: Iff2f7ab8e34eea1ce7c1ab99fb2e51589dce95c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-09 16:25:02 +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
ab391d50b5 internal/lsp: don't show links in hover for test functions
source.Identifier previously was used for references and rename, so it
needed to take a package policy. Now, it's only used for definition and
hover, so it should always be the narrowest package handle. We can use
this fact to determine if the identifier is located in its declaring
package, and if that package is a test variant, we don't link to the
documentation on pkg.go.dev, since it doesn't exist.

Change-Id: I5686828858a3feafb8ff2e4c5964b562f66db9fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-03 21:56:10 +00:00
Muir Manders
f80fb1dfa1 internal/lsp: refactor find-references and rename
The main goal is to push the package variant logic from internal/lsp
into internal/lsp/source so all users of internal/lsp/source benefit.

"references" and "rename" now have top-level source.References() and
source.Rename() entry points (as opposed to hanging off
source.Identifier()). I expanded objectsAtProtocolPos() to know about
implicit objects (type switch and import spec), and to
handle *ast.ImportSpec generically. This gets rid of special case
handling of *types.PkgName in various places.

The biggest practical benefit, though, is that "references" no longer
needs to compute the objectpath for every types.Object comparison it
does, instead using direct types.Object equality. This speeds up
"references" and "rename" a lot.

Two other notable improvements that fell out of not using
source.Identifier()'s logic:

- Finding references on an embedded field now shows references to the
  field, not the type being embedded.
- Finding references on an imported object now works
  correctly (previously it searched the importing package's dependents
  rather than the imported package's dependents).

Finally, I refactored findIdentifier() to use pathEnclosingObjNode()
instead of astutil.PathEnclosingInterval. Now we only need a single
call to get the path because pathEnclosingObjNode() has the
"try pos || try pos-1" logic built in.

Change-Id: I667be9bed6ad83912404b90257c5c1485b3a7025
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211999
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 06:24:15 +00:00
Rebecca Stambler
96555e0fa5 internal/lsp/cache: initialize view before LookupBuiltin
This will crash otherwise.

Change-Id: I4fbce813283291792ed21fa5d83186ec59543ff1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214948
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-16 00:42:58 +00:00
Rebecca Stambler
831fdb1e18 internal/lsp: push initialization tasks into one function
This change moves as much view initialization code into the
initialization function, instead of having it happen on view create.
Also, the `go env` variables that are collected at inconsistent times
are all collected on view creation. That is sufficient, since the view
is recreated if the environment changes.

I had originally hoped that the initial call to `go env` and the
-modfile detection could become part of this parallel initialization as
well, but you can't create a *packages.Config until the temporary
modfile has been set up, so it still makes sense to do that on view
create. This is, however, the reasoning behind the refactorings in
the -modfile detection in this CL. The main changes are a few renamings
and a split between snapshot.ModFiles and view.modFiles to maximize the
amount of work done in the view. I changed view.modfiles to moduleInformation
because I thought we might want to store additional information there at some
point. Rohan, please let me know if you disagree with any of the changes I made,
and I can revert them.

Fixes golang/go#36487

Change-Id: I504db5a4f41b79bee99ebd391e32e7b520a19569
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
2020-01-15 04:46:56 +00:00
Rebecca Stambler
4a54ec1d38 internal/lsp: remove view.FindPosInPackage and view.FindMapperInPackage
There is no reason for these functions to live on the view. They make
more sense as unexported functions in internal/lsp/source.

Initially, I had to propagate contexts through a lot of functions in
internal/lsp/source, but instead I removed the unused contexts forom
snapshot.GetFile.

Change-Id: I8323419d0356feb2010091fe8d3ed35e511f801a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214384
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 18:51:11 +00:00
Dan Kortschak
6c68fec0bc all: remove many cases of space-space
Change-Id: I49eb8410d4143c67dfccf027f8b2794e66963415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212580
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-30 19:07:42 +00:00
Rebecca Stambler
2208e1677e internal/lsp: eliminate source.File type and move GetFile to snapshot
This change eliminates the extra step of calling GetFile on the view and
getting the FileHandle from the snapshot. It also eliminiates the
redundant source.File type. Follow up changes will clean up the file
kind handling, since it still exists on the fileBase type.

Change-Id: I635ab8632821b36e062be5151eaab425a5698f60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-19 20:51:25 +00:00
Muir Manders
640059499b internal/lsp: kill some dead code
Clear out a few things I noticed aren't really used anymore.

Change-Id: I03f955015632197b56230ae0443cfb3871b54db2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211717
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-17 19:30:50 +00:00
Rebecca Stambler
42268a1f04 internal/lsp: fix link anchors for struct fields
This change fixes the link anchors for fields within a struct or
composite literal by getting the enclosing types.Type.

Fixes golang/go#36138

Change-Id: I534a900fad6fa6fa1b1acaa5a63ca264c5d34c39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211582
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-17 02:48:38 +00:00
Rebecca Stambler
cfcbc7796e internal/lsp: improve link in documentation on hover
This change refactors some of the logic that builds a link anchor for
a given symbol, pushing the actual Link into the HoverInformation struct.
This is necessary because type information is needed to build up that
link in certain cases, like methods.

The last step will be to correctly display struct fields.

Updates golang/go#34240
Fixes golang/go#36031

Change-Id: I7f989faddbaa07f91838a870b4477bf78ce8ddf7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210201
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-12 22:39:21 +00:00
Muir Manders
a27fdba277 internal/lsp: check all package variants in find-implementations
We previously only searched for implementations of the object we found
in the "widest" package variant. We instead need to search all
variants because each variant is type checked separately, and
implementations can be located in packages associated with different
variants.

For example, say you have:

-- foo/foo.go --
package foo
type Foo int
type Fooer interface { Foo() Foo }

-- foo/foo_test.go --
package foo
func TestFoo(t *testing.T) {}

-- bar/bar.go --
package bar
import "foo"
type impl struct {}
func (impl) Foo() foo.Foo { return 0 }

When you run find-implementations on the Fooer interface, we
previously would start from the (widest) foo.test's Fooer named
type. Unfortunately bar imports foo, not foo.test, so bar.impl
does not implement foo.test.Fooer. The specific reason is that
bar.impl.Foo returns foo.Foo, whereas foo.test.Fooer.Foo returns
foo.test.Foo, which are distinct *types.Named objects.

Starting our search instead from foo.Fooer resolves this issue.
However, we also need to search from foo.test.Fooer so we match any
implementations in foo_test.go.

Change-Id: I0b0039c98925410751c8f643c8ebd185340e409f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210459
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-11 21:44:05 +00:00
Rebecca Stambler
786dcba013 internal/lsp: respect References.IncludeDeclaration setting
Previously, (*IdentifierInfo).References was returning the declaration
of the identifier among the reference results. This change alters the
behavior of this function to only ever return non-declaration
references. Declarations can be accessed through the
IdentifierInfo.Declaration field.

Fixes golang/go#36007

Change-Id: I91d82b7e6d0d51a2468d3df67f666834d2905250
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210238
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-06 20:05:57 +00:00
Heschi Kreinick
660eba4da3 internal/lsp/source: extract helper, improve error messages
Lack of context in error messages is making my life difficult. Add
context to a few, refactoring out some duplicate code along the way.

Change-Id: I3a940b12ec7c82b1ae1fc477694a2b8b45f6ff71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209860
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-04 19:34:30 +00:00
Rebecca Stambler
a51b8faf84 internal/lsp: rename CheckPackageHandle to PackageHandle
Change-Id: I4ea5fed9fcb71b77da4a15c9d85792bda815ddf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-12-02 18:29:46 +00:00
Rebecca Stambler
73cd2cc3b5 internal/lsp: don't run analyses on the entire view
Running staticcheck on the entire workspace causes a slowdown, and most
likely users don't want to see staticcheck reports for every
subdirectory of their workspace. Only run staticcheck on open files.

Also, fixed a staticcheck warning that showed up along the way. Filed
golang/go#35718 to remind ourselves to fix all of the staticcheck warnings
that showed up when we ran gopls with staticcheck on x/tools.

Finally, made sure that we don't send empty diagnostics when diagnosing
the snapshot on start-up, as that is not necessary.

Change-Id: Ic51d1abfc80b1b53397057f06a4cfd7e2dc930f9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-11-25 22:48:44 +00:00
Heschi Kreinick
ef6787d357 internal/lsp: track and parse non-compiled go files
When packages.Load'ing cgo packages, the authored files show up in
GoFiles, and the generated files show up in CompiledGoFiles. We need the
AST and type information for the latter, since they're the only thing we
can type check. But we also need the contents (and column mapper) for
the authored file so that we can navigate into it.

Store GoFiles in package metadata and checked Packages. Parse the extra
files, just for their mappers. Refactor the View functions a little bit,
since there's only one place that actually needs to find the mapper for
a file.

Updates golang/go#35720.

Change-Id: I9f96872a9a592bf0e11da27ebd8976c6db8752c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208502
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-25 19:20:50 +00:00
Rebecca Stambler
cc15fab314 internal/lsp: use AST to construct placeholders
Type aliases don't work well with types.TypeString. Work around that by
using the AST to build this information. Follow up from CL 201677.

Fixes golang/go#33500

Change-Id: I8b2d4ea238eb5d284a419f2b0bbf9655e69d434d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208497
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-11-25 01:11:57 +00:00
Rebecca Stambler
35ba81b9fb internal/lsp: reorganize and refactor code
This change cleans up internal/lsp/source/view.go to have a more logical
ordering and deletes the view.CheckPackageHandle function. Now, the only
way to get a CheckPackageHandle is through a snapshot (so all of the
corresponding edits).

Also, renamed fuzzy tests to fuzzymatch. Noticed this weird error when
debugging - I had golang.org/x/tools/internal/lsp/fuzzy in my module
cache and it conflicted with the test version.

Change-Id: Ib87836796a8e76e6b6ed1306c2a93e9a5db91cce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-11-21 02:33:28 +00:00
Heschi Kreinick
8fd459516a internal/lsp: rename Files to CompiledGoFiles
As we improve support for cgo we'll need to reference GoFiles, not just
CompiledGoFiles. "Files" is right out.

I think I got everything that needs renaming but please let me know if
not.

Updates golang/go#35720.

Change-Id: I97a6ebf5b395535de0d5f4f8b3f84b46ca34643f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208101
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-20 22:19:51 +00:00
Heschi Kreinick
4bf2f4069d internal/lsp: fix and re-enable godef tests
None of the godef tests were running due to a mistake in the test
harness code. Fix them and re-enable.

We decided that the range for an import statement should be the whole
import path, not just the first character, so make that change and
adjust the PrepareRename tests accordingly.

Change-Id: I45756a78f2a1beb3c5180b5f288ce078075624bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207900
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-11-20 21:51:18 +00:00
Rebecca Stambler
bc1376d635 internal/lsp: look up files in packages by position instead of URI
This change makes sure that we only return files that contain the given
position. There are a few instances of needing to look up files by URI
in the internal/lsp/cache package, so use an unexported package for
that. This allows us to remove some code in the implementations code.

Change-Id: Ifa7a62c67271826e6c632e4c88667d60f8b760c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206880
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2019-11-13 16:34:02 +00:00
Rebecca Stambler
8456940f41 internal/lsp: remove the pkg.view field, in preparation for CL 204079
This change encompasses the refactorings needed to correctly implement
CL 204079. The goal of this CL is to make the actual relevant diffs more
clear.

Change-Id: I38acfd436e2380be790910e01b6e37d8280e9100
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204139
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-10-29 23:14:01 +00:00
Rebecca Stambler
3d91e92cde internal/lsp: stop caching diagnostics on the package
Now that we are using the memoize package to cache analysis results, we
can use that cache for suggested fixes.

Change-Id: I42905a6fe575f49d38979d53d58ea8ec59210ae0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203278
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-10-24 22:03:59 +00:00
Rebecca Stambler
2b779830f9 internal/lsp: don't associate package with snapshot
This change effectively reverts CL 202039. This CL was a mistake, as it
creates a cycle. Snapshots hold CheckPackageHandles, which in turn hold
pkgs.

Change-Id: I944304cb365f0ef98b5e54ea38edea6cece40453
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202740
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-10-23 20:24:04 +00:00
Rebecca Stambler
0bbdbb2ef6 internal/lsp: associate package with its snapshot
A package really should always be associated with its snapshot rather
than its view. This eliminates some extra parameters in a few utility
functions.

Change-Id: I60f9b7286e0072d3268602f6bd32052a3d2e5559
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202039
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-18 19:08:19 +00:00
Rebecca Stambler
638914d249 internal/lsp: return an error from {Narrowest,Widest}CheckPackageHandle
This will prevent us from panicking in cases with errors.

Fixes golang/go#34824

Change-Id: I02c20655f6926ec00c1591a905ff5a107cc44192
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200300
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-10-11 16:51:06 +00:00