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

185 Commits

Author SHA1 Message Date
Danish Dua
bc8aaaa29e internal/lsp: ignore period ('.') triggered completions in comments
Period triggered completions don't provide any use in comments and in
worst case can be nuisance. LSP provides a completion context which
provides more info about what triggered a completion and hence we can
use this to ignore period triggererd completions. This will also provide
us options to deal with retriggered completions etc. better in the
future.

Change-Id: I8449aee0fe3cf5f9acf315865ac854d5c894d044
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250337
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-26 04:07:57 +00:00
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
Danish Dua
1365742343 internal/lsp: add incoming calls hierarchy to gopls
* Adds incoming calls hierarchy to gopls. Returns function declarations/function literals/files enclosing the call/s to the function being inpected.
* Updates cmd to show ranges where calls to function in consideration are made by the caller.
* Added tests for incoming calls.

Example:
This example shows call hierarchy for PathEnclosingInterval in tools/go/ast/astutil.go
Show Call Hierarchy View: https://imgur.com/a/9VhspgA
Peek Call Hierarchy View: https://imgur.com/a/XlKubFk

Note:
* Function literals show up as <scope>.func() in call hierarchy since they don't have a name. Here scope is either the function enclosing the literal or a file for top level declarations
* Top level calls (calls not inside a function, ex: to initialize exported variables) show up as the file name
* Clicking on an item shows the the range where a call is made in the scope

Change-Id: I56426139e4e550dfabe43c9e9f1838efd1e43e38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247699
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-13 20:36:30 +00:00
Rebecca Stambler
fd80f4dbb3 internal/lsp: fix a few small staticcheck warnings
Address unused context / unused error warnings.

Change-Id: Ibd302d58a3d6db5aa274ed95fb6d5a337978779a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247597
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Danish Dua <danishdua@google.com>
2020-08-11 03:20:01 +00:00
Heschi Kreinick
c1903db4db internal/memoize: switch from GC-driven to explicit deletion
The GC-based cache has given us a number of problems. First, memory
leaks driven by reference cycles: the Go runtime cannot collect cycles
involving finalizers, which prevents us from writing natural code in
Bind callbacks. If we screw it up, we get a mysterious leak that takes a
long time to track down. Second, the behavior is generally mysterious;
it's hard to predict how long a value lasts, and harder to tell if a
value being live is a bug. Third, we think that it may be interacting
poorly with the GC, resulting in unnecessary memory usage.

The structure of the values we put in the cache is not actually that
complicated -- there are only 5 significant types: parse, typecheck,
analyze, parse mod, and analyze mod. Managing them manually should not
be conceptually difficult, and in fact we already do most of the work
in (*snapshot).clone.

In this CL the cache adds the concept of "generations", which function
as reference counts on cache entries. Entries are still global and
shared across generations, but will be explicitly deleted once no
generations refer to them. The idea is that each snapshot is a new
generation, and can inherit entries from the previous snapshot or leave
them behind to be deleted.

One obvious risk of this scheme is that we'll leave dangling references
to values without actually inheriting them across generations. To
prevent that, getting a value requires passing in the generation at
which it's being read, and an error will be returned if that generation
is dead.

Change-Id: I4b30891efd7be4e10f2b84f4c067b0dee43dcf9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242838
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-10 19:02:17 +00:00
Danish Dua
9221131678 internal/lsp: release resources for call hierarchy file requests
* adds call to release resources for beginFileRequest in
  lsp/call_hierarchy.go
* fixes source_test

Change-Id: Id45f61ccd474a2df9f46be89fdb059cfe8584f0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247497
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-07 21:04:51 +00:00
Danish Dua
a5d4502270 internal: add call hierarchy cmd and lsp scaffolding
* adds gopls command line tool for call hierarchy
* adds lsp setup for call hierarchy
* adds handler for textDocument/prepareCallHierarchy to display selected
  identifier and get incoming/outgoing calls for it
* setup testing

Change-Id: I0a0904abdbe11273a56162b6e5be93b97ceb9c26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246521
Run-TryBot: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-08-07 20:22:52 +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
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
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
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
4025ed8474 internal/lsp: move fillstruct suggested fixes out of analysis
This change moves the suggested fixes logic for fillstruct out of the
analysis and into internal/lsp/source. This logic is then used as part
of a new fillstruct command. This command is returned along with the
code action results, to be executed only when the user accepts the code
action.

This led to a number of changes to testing. The suggested fix tests in
internal/lsp doesn't support executing commands, so we skip them. The
suggested fix tests in internal/lsp/source are changed to call
fillstruct directly. A new regtest is added to check the command
execution, which led to a few regtest changes.

Also, remove the `go mod tidy` code action, as it's made redundant by
the existence of the suggested fixes coming from internal/lsp/mod.

Change-Id: I35ca0aff1ace8f0097fe7cb57232997facb516a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241983
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-20 20:42:44 +00:00
Josh Baum
6d307edf52 internal/lsp: support extract function
Extract function is a code action, similar to extract variable. After
highlighting a selection, if valid, the lightbulb appears to trigger
extraction. The current implementation does not allow users to
extract selections with a return statement.

Updates golang/go#37170

Change-Id: I5fc3b19cf7dbca4407ecf0cc37017661223614d1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241957
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-20 18:55:41 +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
Rob Findley
20370b0cb4 internal/lsp: honor GOPRIVATE in documentLinks and go.mod hovers
Several fixes related to GOPRIVATE handling and links:
 + In Go source, fix links matching GOPRIVATE for external modules.
   Previously, in these cases we'd try to match <mod>@v1.2.3/<suffix>,
   which wasn't the correct input into the GOPRIVATE matching algorithm.
 + Similarly check GOPRIVATE for go.mod require statement hovers.
 + Likewise, for documentLink requests (both mod and source).
 + Move the existing hover regtest to link_test.go, and expand to cover
   all these cases.

Along the way, I encountered a couple apparent bugs, which I fixed:
 + Correctly handle the case where there is only one require in a go.mod
   file. This was exercised by the regtest, so took some debugging.
 + Only format links [like](this) if the requested format is actually
   markdown.

Fixes golang/go#36998

Change-Id: I92011821f646f2a7449dcca619483f83bdeb54b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238029
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-18 13:42:42 +00:00
Rebecca Stambler
e31b568ad1 internal/lsp: plumb fillstruct through analysis
Now that fillstruct is an analyzer, we can simplify the code that calls
it in code_action.go. We introduce a new class of analyzer --
convenience analyzers, which are closer to commands. These represent
suggestions that won't necessarily improve the quality or correctness of
your code, but they offer small helper functions for the user.

This CL also combines the refactor rewrite tests with the suggested fix
tests, since they are effectively the same.

For now, we only support convenience analyzers when a code action was
requested on the same line as the fix. I'm not sure how to otherwise
handle this without bothering the user with unnecessary diagnostics.

Change-Id: I7f0aa198b5ee9964a907d709bae6380093d4ef21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237687
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-15 21:22:08 +00:00
Rebecca Stambler
9001f16f58 internal/lsp: cancel the initial workspace load when view shuts down
The error messages from view cancellation clutter up the logs when
testing, especially if you're running a single subtest.

A few quick staticcheck fixes in the CL also.

Change-Id: Ia1ed5360ac754023c589ed526ec0ed3e94a79b2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237637
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:06:40 +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
Pei Xian Chee
9b20fe4cab internal/lsp: added a fill struct code action
This code action generates key-value pairs of fields and default values between a struct's enclosing braces.

Fixes #37576

Change-Id: Ia0555164d2164c2bc90bb9ecabbb55042cdd3846
GitHub-Last-Rev: 0e4db3effad7212b6c2b226079fa8fc5464eb0b9
GitHub-Pull-Request: golang/tools#220
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227437
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-04 04:23:27 +00:00
Pontus Leitzler
1c0c6cf43e x/tools/internal/lsp/source: avoid panic in failing Highlight test
When testing Highlight the highlight count is checked against expected
number of highlights. If it doesn't match t.Errorf(...) is called and
the test continues.

A few lines below the test ranges over results using the index for both
result and expected result leading to a panic if there are less then
expected highlights.

This change fails fast with t.Fatalf(...) instead to avoid the panic.

Change-Id: I33d0973f3145c307d9084d037ffbb73b244a3acb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236099
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-06-03 17:03:22 +00:00
Muir Manders
cf2d1e09c8 internal/lsp/source: offer smart "append()" completions
Assigning a slice to the appendage of itself is common and tedious
enough to warrant a special case completion candidate. We now offer
smarter "append()" candidates:

    var foo []int
    foo = app<> // offer "append(foo, <>)"
    fo<> // offer "foo = append(foo, <>)"

The latter is only offered if the best completion candidate is a
slice. It is inserted as the second-best candidate because it seems
impossible to avoid annoying false positives if it is ranked first.

I added a new debug option to disable literal completions. This was to
clean up some test logic that was disabling snippets for all tests
just to defeat literal completions. My tests were failing mysteriously
due to having snippets disabled, and it was hard to figure out why.

Change-Id: I3e8313e00a1409840cb99d5d71c593435a7aeb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221777
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-20 22:05:37 +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
Paul Jolly
ca5866bcf9 internal/lsp: add config option for SymbolMatch
In preparation for later changes to the workspace Symbol method, we add
a separate configuration option keyed by "symbolMatcher" that specifies
the type of matcher to use for workspace symbol requests. We also define
a new type SymbolMatcher, the type of this new option. We require
SymbolMatcher to be a separate type from Matcher because a later CL adds
a type of symbol matcher that does not make sense in the context of
other uses of Matcher, e.g. completion.

Change-Id: Icde7d270b9efb64444f35675a8d0b48ad3b8b3dd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228122
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 19:58:58 +00:00
Paul Jolly
e48fac377d internal/lsp: change workspace symbols to use session config for matcher
WorkspaceSymbols matches symbols across views using the given query,
according to the matcher Matcher.

The workspace symbol method is defined in the spec as follows:

 > The workspace symbol request is sent from the client to the server to
 > list project-wide symbols matching the query string.

It is unclear what "project-wide" means here, but given the parameters
of workspace/symbol do not include any workspace identifier, then it has
to be assumed that "project-wide" means "across all workspaces".  Hence
why WorkspaceSymbols receives the views []View.

However, it then becomes unclear what it would mean to call
WorkspaceSymbols with a different configured Matcher per View.

Therefore we assume that Session level configuration will define the
Matcher to be used for the WorkspaceSymbols method.

As part of this change we also tidy up lsp_test.go and source_test.go to
remove some repetition.

Change-Id: I444f9a78303ac9d2c8d8ac6496603b5758e4aafd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228763
Run-TryBot: Paul Jolly <paul@myitcv.org.uk>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-11 19:21:19 +00:00
Rebecca Stambler
5744cfde56 internal/lsp: fix imports issue with duplicate package decl
We shouldn't apply formatting fixes when we are
organizing imports.

Fixes golang/go#38412

Change-Id: I082f4d9a7d1d1dd571304733c287b0d5e90ea448
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228264
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-16 06:17:24 +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
a3568bac92 internal/lsp: disable unimported completions for snippet tests
Turns out that these were already disabled in the
golang.org/x/tools/internal/lsp tests, which is why we only ever save
the failures in golang.org/x/tools/internal/lsp/source tests. Discussing
the reasons that this is necessary on the issue, but I think completion
scoring in general needs to be centralized and documented, so hopefully
we can handle this case as part of that larger fix.

Fixes golang/go#38269

Change-Id: Ie1b615315e417ab8211a3580cf8f27cbdc3b74e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-07 14:37:52 +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
31583a0dbb internal/lsp/tests: remove ellipses from test names
This change makes working with tests easier by trimming the folder name
up to "testdata". This will remove any ellipses from the test folder name.

Fixes golang/go#38103

Change-Id: I33b931e527de63713b8fc370c50b1c382796b2b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226377
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-30 17:55:17 +00:00
Marwan Sulaiman
63da46f303 x/tools/gopls: run go generate through CodeLens
This change adds support for recognizing a //go:generate directive
and offering a CodeLens that will then send a "generate" command to
the server to run "go generate" or "go generate ./...". Because
"go generate" can only be executed per package, there is no need to show
the CodeLens on top of every //go:generate comment. Therefore, only the
top directive will be considered.

The stdout/stderr of the go generate command will be piped to the logger
while stderr will also be sent to the editor as a window/showMessage

The user will only know when the process starts and when it ends so that they wouldn't
get bogged with a large number of message windows popping up. However, they can
check the logs for all the details.

If a user wants to cancel the "go generate" command, they will be able
to do so with a "Cancel" ActionItem that the server will offer to the client

Fixes golang/go#37680

Change-Id: I89a9617521eab20859cb2215db133f34fda856c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222247
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-17 04:34:34 +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
Rohan Challa
a0897bacdd internal/lsp/cmd: improve flexibility of suggested fixes
This change adds support for passing a span to cmd/suggested_fix.go, originally it would just take the filename and apply all the fixes for that file. Now, it can also take just a span within that file and only apply the fixes relevant to that span.

The //@suggestedfix marker now contains an extra parameter to specify which type of codeaction is expected.

Change-Id: I4e94b8dad719f990dc2d0ef3c50816f70f59f737
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222137
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-06 14:31:35 +00:00
Ian Cottrell
4183ba16a9 internal/lsp: move the debug.Instance onto the Context
This allows us to register a telemetry exporter that works with mulitple active
debug instances.
It also means we don't have to store the debug information in our other objects.

Change-Id: I9a9d5b0407c3352b6eaff80fb2c434ca33f4e397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-03 22:54:53 +00:00
Rohan Challa
c4f5635f10 internal/lsp: add an upgrade all dependencies codelens
This change adds an upgrade all dependencies codelens on the go.mod file if there are available upgrades.

Updates golang/go#36501

Change-Id: I86c1ae7e7a6dc01b7f5cd7eb18e5a11d96a3acc1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221108
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-02 21:30:18 +00:00
Rob Findley
e02f5847d1 internal/lsp/debug: move all debug state onto the Instance
For testability, and to support the exchange of debug information across
Forwarder and server, it is helpful to encapsulate all debug information
on the instance object.

This CL moves all state in the debug package into a new 'State' type,
that is added as a field on the debug.Instance. While doing so, common
functionality for object collections is factored out into the objset
helper type.

Also add two new debug object types: Client and Server. These aren't yet
used, but will be in a later CL (and frankly it was easier to leave them
in this CL than to more carefully rewrite history...).

Updates golang/go#34111

Change-Id: Ib809cd14cb957b41a9bcbd94a991f804531a76ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220078
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 22:50:47 +00:00
Heschi Kreinick
f7b8cc7bd0 internal/span,lsp: disambiguate URIs, DocumentURIs, and paths
Create a real type for protocol.DocumentURIs. Remove span.NewURI in
favor of path/URI-specific constructors. Remove span.Parse's ability to
parse URI-based spans, which appears to be totally unused.

As a consequence, we no longer mangle non-file URIs to start with
file://, and crash all over the place when one is opened.

Updates golang/go#33699.

Change-Id: Ic7347c9768e38002b4ad9c84471329d0af7d2e05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219482
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-14 22:51:03 +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
Daisuke Suzuki
885dec1b2d internal/lsp/source: make matchers selectable in WorkspaceSymbols
This change allows to use fuzzy or case-sensitive matchers in addition
to case-insensitive when searching for symbols.
Matcher is specified by UserOptions.Matcher just like Completion.

Updates golang/go#33844

Change-Id: I4000fb7984c75f0f41c38d740dbe164398032312
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218737
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-13 22:46:20 +00:00
emahiro
f8c9a4c3ab internal/lsp: export DiffSymbols to avoid duplication
Change-Id: I9002d19067011e0fcff22782eebc593a124594c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
2020-02-06 20:09:13 +00:00
Rebecca Stambler
6224300ba8 internal/lsp: remove unnecessary source.SignatureInformation type
We should just use the protocol.SignatureInformation type, as it's
essentially the same thing. Refactor tests a bit to make use of the
shared type.

Change-Id: I169949f6e23757ce0a6f54de36560c4c8e0479ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217731
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-06 19:05:38 +00:00
Rohan Challa
2529d2857a internal/lsp/tests: standardize testdata folder format
This change standardizes the folder structure for testdata that are used for testing the lsp. In particular, it uses the following format:
- dir
  - primarymod
    - .go files
    - packages
    - go.mod (optional)
  - modules
    - repoa
      - mod1
        - .go files
        -  packages
        - go.mod (optional)

As we can see, any folder inside of testdata should be of this format, where the primary test files with the markers are all located inside the primarymod folder. The modules folder is used to hold any potential dependencies that are used for testing.

A consequence of this change is that we can have one directory separated by folders, where each folder is it's own module, this allows us to use internal/lsp/tests with go.mod files. Now, tests.Load() will return an array of Data objects, where each object corresponds to one of the directories structured above.

Updates golang/go#36091

Change-Id: I437cc2a2a9fc1bac93779845737aa74383fbf9c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217541
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 14:14:23 +00:00
Muir Manders
dd0d5d4851 internal/lsp: filter keyword completions in tests
Filter keywords out of completion results in tests similar to
builtins. You don't care about keyword completions unless you are
explicitly testing keyword completion.

Change-Id: I0caaaef8b0f5b08c4b15ba3ada1a963f35a14028
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217499
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:08:30 +00:00
Daisuke Suzuki
6e8b36d2c7 internal/lsp: add support for workspace symbol
This change adds support for the LSP workspace/symbol. Unlike
documentSymbol, the target is symbols that exist not only in a specific
file, but also in the current or imported packages. It returns symbols
whose name contains the query string of the request(case-insensitive),
or all symbols if the query string is empty.

However, the following is not implemented:
- Setting of deprecated and containerName fields in SymbolInformation
- Consideration of WorkspaceClientCapabilities
- Progress support
- CLI support

Updates golang/go#33844

Change-Id: Id2a8d3c468084b9d44228cc6ed2ad37c4b52c405
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213317
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-05 19:03:17 +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
Rebecca Stambler
ab094738a9 internal/lsp: fix active parameter for incomplete parentheses
I had originally thought I might be able to use exprAtPos for this,
which is why I ended up eliminating that function when I saw it only had
one use.

One test also had to change in order to fit better with the spec.
Specifically: "If [the active parameter is] omitted or the value
lies outside the range of `signatures[activeSignature].parameters`
it defaults to 0 if the active signature has parameters."

Fixes golang/go#36766.

Change-Id: I400d5b2db2985bfaa5efbcd91225151ca8b5f46a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216309
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-27 19:24:44 +00:00
Rebecca Stambler
1b668f2091 internal/lsp: disable literal completion candidates for some clients
If a client doesn't support the snippet format in completion insert
text, they can't take full advantage of the literal completion
candidates. Disable it in those cases, and remove the setting in
internal/lsp/source/options.go.

Fixes golang/go#36655.

Change-Id: Ibc045a0f2945aab753b0187194a03d0c0398dba5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-24 20:07:20 +00:00
Rebecca Stambler
219d3418f5 internal/lsp: batch file changes in didChangeWatchedFiles
Remove the special handling for go.mod file saves. This was only really
added to be extra careful, but our cancellation logic should cope with
this.

Fixes golang/go#31553

Change-Id: I0a69bcdeaf6369697e79aba4689a7b714484ccc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215908
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 20:51:52 +00:00
Rebecca Stambler
e54d0edf47 internal/lsp: support batched on-disk changes in source.DidModifyFiles
We don't yet propagate these batched changes in text_synchronization.go,
but this is the next step in moving towards a batched approach.

Updates golang/go#31553

Change-Id: Id6496af9d5422cc50ccb995f81c71ec1886f965a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215907
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 20:28:28 +00:00
Rebecca Stambler
ba161d9e22 internal/lsp: add tests for references includeDeclaration setting
Make sure to test both modes, as this is the second time we've
accidentally broken this.

Fixes golang/go#36598.

Change-Id: I3993af3d106b18c76c44ada558b2c6cd9cbfcf17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-23 01:39:50 +00:00