x/tools/gopls: support advanced query syntax for Symbol

[myitcv]

What version of Go are you using (go version)?

$ go version
go version devel +b7689f5aa3 Fri Jan 31 06:02:00 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200214144324-88be01311a71
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20200214144324-88be01311a71

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build646330205=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This is a request/proposal more than an issue.

We were discussing with @findleyr a means by which symbol search can be made more useful (in Vim). With #37236 fixed, the existing Symbol method will return matches from within the workspace (main module). But a fairly common query is to find symbols matching the query within the "current" package, where "current" is defined by the file within which the cursor is located. i.e. I want to jump to the definition of the method (*T).M.

Because the editor/client knows nothing about packages, I think this will require a query similar to the following. Imagine we are searching using the term hello, with the cursor in the file main.go

package:/path/to/main.go hello

gopls would then translate the file URI to a package and suitably constrain the results.

---

cc @stamblerre @findleyr

FYI @leitzler

[myitcv]

Per #37236 (comment), we should also add a module scope, again parameterised by file, to limit the search scope to the module containing the file.

[myitcv]

To make this request more formal I'd like to sketch out a potential query syntax. It is based on the popular fzf search syntax (FYI @junegunn).

In the text that follows, the term "item" refers to what we are matching against (noting that more information/context is returned in protocol.SymbolInformation in the form of location etc). At present an "item" is just the symbol name (variable, function, method etc), but in #38273 I have proposed making the fully qualified symbol the "item".

Search syntax

Unless otherwise specified via an option (see below), Symbol will search in extended-search mode, a mode that takes multiple space-separated search terms. The overall search query is the conjunction of these terms.

Token	Match type	Description
jsEnc	fuzzy-match	Items that match jsEnc
'json	exact-match (quoted)	Items that include json
^encoding	prefix-exact-match	Items that start with encoding
main$	suffix-exact-match	Items that end with main
!json	inverse-exact-match	Items that do not include json
!^encoding	inverse-prefix-exact-match	Items that do not start with encoding
!main$	inverse-suffix-exact-match	Items that do not end with main

To be explicit, / and . (which appear in some fully qualified symbol paths) also participate in the match if specified in a search term.

Options

Options are provided as special search terms in the form key::value.

• exact::true - enable exact matches (current behaviour)
• package::/path/to/file.go - constrain the search to the package that contains /path/to/file.go (filepath must be absolute)
• module::/path/to/file.go - constrain the search to the module that contains /path/to/file.go (filepath must be absolute)
• package::example.com/blah - constrain the search to the package example.com/blah
• module::example.com/blah - constrain the search to the module (that contains package) example.com/blah
• kind::X,Y,Z - constrain the search to the protocol.SymbolKind list provided (comma separated list of numbers corresponding to the SymbolKind enum

Examples

We assume for the following examples that:

• x/tools/gopls: fully qualify symbol names for Symbol #38273 is adopted
• the type of the symbol is not considered in the match
• the type of the symbol is returned to the client for display purposes

Example 1

A query of "jsEnc" would match encoding/json.Encoder and encoding/json.NewEncoder func(w io.Writer) *Encoder, and encoding/json.Encoder struct and encoding/json.NewEncoder func(w io.Writer) *Encoder respectively would be returned to the client

Example 2

A query of "package::/path/to/file.go 'main" would search within the package that contains /path/to/file.go for items that include the exact term main somewhere in their fully qualified path.

Outstanding questions

• Consider support for filepaths with spaces
• Support disjunctions?
• Further options

[gopherbot]

Change https://golang.org/cl/227677 mentions this issue: internal/lsp: first cut of advanced query syntax for WorkspaceSymbol

[findleyr]

I experimented with fzf-like syntax in http://golang.org/cl/248418, and it was very straightforward to add. Furthermore, it is backwards compatible with our current fuzzy matching: any query containing , ', ^, or $ will not currently match any symbols. So to me, this seems like an easy win that may allow us to deprecate the symbolMatcher configuration option and consolidate on a single matcher.

Regarding the rest of the proposal, supporting things like package::foo... to me, this blurs the line between user-interface and API. I doubt any user would ever type this interactively -- if they wanted to go to a symbol they would just type the symbol out. The purpose of the workspace symbols request is to find a single symbol to jump to, not a collection of symbols.

So because syntax like package::foo is only likely to be used by clients to implement features over the top of the workspace symbols RPC, it seems like it may be better to instead implement this functionality as a separate, proper API, supported as a non-standard request in gopls.

[myitcv]

The only one comment to add is that the approach I've experimented with in https://go-review.googlesource.com/c/tools/+/227677 makes no additional requirements of the LSP client, only the editor/user. You're quite right: there is no expectation that a user would ever type one of these options interactively, rather that via a user-defined custom key mapping, the editor user would augment a typed search query with certain options.

Either way we end up exposing the end user to some sort of API, namely the structure of the options to specify a particular type of search/scope.

Whilst my approach in this CL works in a setting like Vim where there are powerful key mapping capabilities, I quite acknowledge it's not guaranteed to work in all editors. As such a proper API might well be more appropriate. But that might place a requirement on each LSP client to support it (I have no experience with non-standard requests so can't comment on the merits one way or the other).

Hence it felt like, for the very short term, this approach provided a nice, purely opt-in way to experiment with the types of options that would be appropriate, with zero impact on existing clients, editors, users etc.

[findleyr]

Having discussed this at length with @myitcv today, I'm starting to come around to the idea of supporting at a subset of the advanced filters proposed above within the workspace symbols request.

In particular, having seen that the workspace symbols RPC can get noticeably slow in very large projects, it could generally be helpful to expose a mechanism that limits the scope of packages that are walked when looking for symbol matches. Limiting to the current workspace, module, or package seems natural.

We could of course do this via configuration rather than a dynamic query field. But in that case we'd be supporting the complexity of implementing the filter anyway. How much more complexity is added to support parsing package:path/to/foo.go out of the query? Probably not much. Also, for filtering to e.g. the current module, we'd need to know which file the cursor is in, and we have no way to know that server-side.

I'm less convinced by the argument that users will regularly want to bounce back and forth between searching within all packages, within the current package, and within the current module. In my experience, as long as the symbols request is fast, I have no difficulty jumping to my desired symbol across all packages. If I want to go to the Error symbol in package foo, I just type foo.error or 'foo 'error. It would be more work to think about whether or not package foo is in my module (or even my workspace -- sometimes I forget where I am...). I have similar objections to filtering by kind: I'd have to remember which 'kinds' are supported, whether my symbol is a const or a var, etc.

So in summary: I think we should do a cost-benefit analysis of a subset of the proposed syntax. Specifically, how much complexity would it add, and how much our users would benefit from it.

Regarding complexity, I think the following filters would not add much, the reason being that they only affect the set of packages walked, not the symbol search itself. It would be pretty easy just to put together a CL to do this (sans tests) that we can consider.

• package:path/to/foo.go
• module:path/to/foo.go
• workspace:path/to/foo.go

Regarding benefit, I think we should try to put some numbers behind the performance argument. I wrote a benchmark in CL 250798 that can be used for this: let's pick some known large repo (Kubernetes?) and benchmark different queries. It's worth caring about anything above ~100ms.

I'd be happy to do this work myself, though I won't get to it for at least a couple weeks.

But fair warning, we may look at all this and decide it's not worth it, given all of our other priorities.

[jackieli-tes]

FWIW, I'm implementing an coc.nvim plugin that lists tests of current file using DocumentSymbols request. So that I can append actions like run test, yank test command to each test item.

However, in most cases, I found myself wanting to locate the tests in the current package, not just the current file. With this WorkspaceSymbols requests that's able to return all symbols in the current package would make this possible.

[findleyr]

Things have evolved a bit since we last talked about this. Gopls now has much better support for custom commands using workspace/executeCommand.

I'd rather build a custom command for package symbols than try to build an API on top of workspaceSymbols: workspaceSymbol results are interpreted by many different LSP clients, who do things like apply additional filters.

@hyangah could VS Code also use a PackageSymbol or more generalized SymbolQuery API? Is there something to design here?

[jackieli-tes]

Generalised SymbolQuery as a command would be great. My feeling is go's package scope creates quite a few things that's specific to Go. E.g. finding all the methods of a struct: it would be in the current module, not a DocumentSymbol query.

On a similar issue (maybe worth another issue): Query the package's full name and partial name of current buffer. I'm using go list at the moment as a work around. I see vim-go does the same. But this one is used everywhere, and calling out go list affects performance quite a bit. Caching it in the plugins would be a solution for this, but I reckon having it in LSP seems proper.

[hyangah]

@findleyr time to resurrect https://go-review.googlesource.com/c/tools/+/315669 (This was part of an unfinished attempt to demo Package Outline View in vscode-go)

[findleyr]

@jackieli-tes I think we're generally open to offloading work to gopls using custom commands. However, we want to avoid the proliferation of commands with overlapping functionality. Could you please file a separate issue for the requirements of what you're using go list for? It would be helpful to have a separate thread for discussion, and to lay out exactly the API you need. I'll note that there's some overlap with #49018 though: generally it would be great if gopls provided a go/packages-like API, but that is a major project.

In this issue, it would be great if you could specify your requirements for a SymbolQuery API, in terms of arguments and results.

(another side note: since this issue was originally opened, our workspace symbol queries have gotten up to 40x faster depending on the query: we keep a symbol table active in memory, so some of the constraints have changed).

[jackieli-tes]

Thanks @findleyr for the info. I quite like the suggestions in the link especially around using a light weight memory representation on the symbols.

I don't agree with the external plugin approach though. I might understand it wrong, but I reckon either way, it should come out as an LSP command: LSP clients e.g. coc.nvim already have easy ways to invoke the command. Integrating with the editors would be easier.

Filled the retrieve package info command issue: #51490

Also filed the package symbols proposal: #51492

[findleyr]

Coming back to this: I think various users and clients struggle with the fact that the LSP concepts of "workspace" and "document" do not align nicely with go package/module/workspace boundaries. I think we should revisit adding a custom command that provides the query search syntax that we want, and let workspace/symbols and textDocument/documentSymbols be a projection of this command onto the LSP API. (a while back I rewrote our DocumentSymbols implementation to be more similar to WorkspaceSymbols). This would also generalize the recently added "symbolScope" setting.

@myitcv @hyangah what do you think about this?

[myitcv]

I can't say I'm fully appraised on the pros/cons of putting this in a separate command vs overloading workspace/symbols. Overloading workspace/symbols means no changes from an LSP perspective in any LSP client/configuration. The alternative is the polar opposite as far as I can tell. I defer to your experience however on the cost/load this creates for users of gopls.

Also, give my exploration/experimentation detailed above it appears clear to me (at least) that such a command will only ever work with editor support: a user will never type in the long-form qualifiers that alter the scope of a given search to be either file, package, module, workspace etc. As a Vim user, such editor support would be achieved via a Vim command that adds the qualifier based on the current buffer's filename etc. As such from a Vim perspective, it doesn't per se matter whether this is an existing command or a new one, because there will be some automation to add these scope qualifiers on top of anything the user types.