Allow to query workspace/symbols by a SymbolDescriptor

[felixfbecker]

Use cases are constructing symbol URLs and global j2d.

Discussed with @rothfels and @beyang

[rothfels]

This LGTM, @beyang any comments?

[rothfels]

@felixfbecker think it might be good to give examples of the different use cases of query and symbol?

[felixfbecker]

Will add an examples section.

[emidoots]

Since we must modify the type of query to be optional with this extensive anyway, what is the reason to have the fields be separate?

i.e. why not just have query be either a string or a Partial<SymbolDescriptor> ?

[felixfbecker]

I thought about that too, but there are languages that are strictly typed and cannot have it either be a string or an object. LSP will avoid these union types in the future, see microsoft#39. There isn't really a disadvantage to it, but it increases the likelihood of the proposal getting accepted later.

[felixfbecker]

query	symbol
user input in UI	used programmatically
matches as fuzzily as possible	matches as exact as possible
returns as many results as possible	returns as few results as possible

[emidoots]

Makes sense, and sounds good. In that case:

1. How about naming it querySymbol instead?
2. Can we clarify that only one of query or querySymbol should be present? If not, what is the behavior of a LS when both are specified?

[felixfbecker]

1. I like to keep it short and precise - if any, I would prefer symbolQuery, and that is kind of confusing since the whole thing you are doing is a symbol query. I thought about naming it symbolDescriptor, but it is not a full SymbolDescriptor, and I would like to change it to SymbolInformation soon.
2. Yes, this should be specified. The semantics should be an AND, because it could be useful to query with a fuzzy query but restrict it to a specific SymbolKind or package.

[emidoots]

The motivation behind this PR is to enable querying based on Sourcegraph URLs like sourcegraph.com/<repo>/<language/mode>/<language_specific_string> (e.g. sourcegraph.com/github.com/gorilla/mux/go/github.com/gorilla/mux/-/mux/-/Router/Serve for a Go example), right?

I assume it is. So the question arises: how does Sourcegraph know to get a SymbolDescriptor from a language-specific URL string? Will we need a language-specific Sourcegraph piece to do that?

If we would need a language-specific Sourcegraph piece to do this, why not just pass the string directly into the LS and let it parse it? e.g. we could change symbol type from Partial<SymbolDescriptor> to type string and say it's the LS responsibility to define/parse that string?

[emidoots]

(requesting changes so this doesn't get merged prematurely)

[felixfbecker]

@slimsag The idea was to add a new field to SymbolDescriptor named id, and we would query by that to resolve the URL. The language server defines how it looks like, it just should be as unique as possible. This was the result of discussion with @beyang and @rothfels, since we need an endpoint to query by a SymbolDescriptor anyway later for global refs, it makes the most sense to add it now and use it to query by the symbol ID instead of adding new query parameters for every use case that will come up in the future. This is forward-compatible with #13.

[felixfbecker]

TODO: need to add ServerCapability. Suggestions on the name welcome

[emidoots]

since we need an endpoint to query by a SymbolDescriptor anyway later for global refs

Can you elaborate on this? Not sure if I understand which endpoint you are talking about

[emidoots]

ServerCapability -> that is tricky, how about workspaceSymbolExact? (I don't feel strongly about this, just the best suggestion I can come up with)

[emidoots]

Ok, I see why you'd want to pack id into SymbolDescriptor. That sounds logical / good to me. (I'd still like to understand your comment about the endpoint though, just to make sure I understand fully).

[emidoots]

LGTM

[felixfbecker]

since we need an endpoint to query by a SymbolDescriptor anyway later for global refs

Can you elaborate on this? Not sure if I understand which endpoint you are talking about

This was @beyang's point, I think it is needed because in TypeScript global j2d would normally not resolve to the actual declaration in the TS source, but to the TS definition file (which is just a stub). So I think the steps needed there are to do xdefinition, get the repository info, then query the repository with the SymbolDescriptor to get the actual definition.

[emidoots]

Got it, that makes sense. So it was talking about the TS/JS cross-repo j2d. Thanks for the clarification @felixfbecker !

[felixfbecker]

Who's gonna implement this for the Go LS?

[rothfels]

@felixfbecker I think @slimsag should implement this on the Go side, see my note in slack.

[emidoots]

Yes, I will implement for the Go LS

[rothfels]

@beyang please provide final LGTM