Pass the current selection for a Hover Request

[alanz]

In some languages, such as Haskell, it is meaningful to show the type of an expression, rather than just an identifier.

To allow this, extend the Hover Request to also include a Range if the selection is active.

At the moment in the haskell vscode plugin this is done in a roundabout way using workspace/executeCommand, which means it has to be separately implemented in each client.

[rictic]

+1, I've also wanted this functionality in TypeScript. The current workaround I've used is to declare a local const with the expression to get the type.

[rcjsuen]

Can someone explain this feature to me with a TypeScript/JavaScript/Java example?

Also, what happens if the hover is not over the selection in the editor?

[rictic]

Suppose a user wonders what the type of 'foo' + 'bar' is in typescript. Is it 'foobar' or is it string?

One option is that the user can do: const tempVar = 'foo' + 'bar'; and then hover over tempVar. Under this proposal, the user could select the text 'foo' + 'bar' in their editor, then mouse over any part of the selection. The selection would then be sent up to the typescript LSP server as part of the hover request, allowing it to return the type of 'foo' + 'bar' as a whole.

Other motivating expressions:

await Promise.all([foo, bar, baz])
foo.then((f) => f.bar())
foo === undefined ? null : bar

[vavans]

I had to implement this in the haskero plugin too. (As it communicates throughout the language server protocol).
In many languages this feature would be great.

@alanz i should try to tweak my haskero plugin to use your language server, it could be nice

[dbaeumer]

It sounds reasonable to me to include the range in the Hover request. Anyone willing to work on a PR to add this. If so please also check if this can be added to the VS Code client LSP library (https://github.com/Microsoft/vscode-languageserver-node)

[rcjsuen]

@dbaeumer What would be added to Microsoft/vscode-languageserver-node? You mean update the protocol to reflect the new additional parameter in the request? It's not possible to make any client changes unless VS Code supports this given that HoverProvider needs to be augmented to include selection information.

/**
 * The hover provider interface defines the contract between extensions and
 * the [hover](https://code.visualstudio.com/docs/editor/intellisense)-feature.
 */
export interface HoverProvider {

	/**
	 * Provide a hover for the given position and document. Multiple hovers at the same
	 * position will be merged by the editor. A hover can have a range which defaults
	 * to the word range at the position when omitted.
	 *
	 * @param document The document in which the command was invoked.
	 * @param position The position at which the command was invoked.
	 * @param token A cancellation token.
	 * @return A hover or a thenable that resolves to such. The lack of a result can be
	 * signaled by returning `undefined` or `null`.
	 */
	provideHover(document: TextDocument, position: Position, token: CancellationToken): ProviderResult<Hover>;
}

I think to move forward with this we need to decide on three things:

1. What do we call the new request's parameter's interface? Currently, it's a TextDocumentPositionParams but there will be new selection information. When textDocument/completion was changed CompletionParams extends TextDocumentPositionParam was added. Do we just call it HoverParams? It seems more generic than that though. I don't think we want to just add a new parameter to TextDocumentPositionParams anyway...or do we?

2. What do we call the new parameter? range or selection or activeSelection or something else?

3. In the capabilities, a new clientCapabilities.textDocument.completion.context was added for context support in textDocument/completion. Is something like this needed for this new hover "capabilitiy"? If yes, what do we call it?

[jdneo]

I hope the CompletionParams can also have the selection information.

[TonalidadeHidrica]

Any updates on this? I want the "get type of the expression" for rust-analyzer, but this issue seems to be blocking it.

[rgrinberg]

This would be quite useful for ocaml-lsp-server as well.

[matklad]

rust-analyzer adds this as an extension:

Hover Range

Experimental Server Capability: { "hoverRange": boolean }

This extension allows passing a Range as a position field of HoverParams.
The primary use-case is to use the hover request to show the type of the expression currently selected.

interface HoverParams extends WorkDoneProgressParams {
    textDocument: TextDocumentIdentifier;
    position: Range | Position;
}

Whenever the client sends a Range, it is understood as the current selection and any hover included in the range will show the type of the expression if possible.

https://github.com/rust-analyzer/rust-analyzer/blob/406eeb39bec3d81f924e31d33d6d4a40ae670f30/docs/dev/lsp-extensions.md#hover-range

Turns out, it's very straightforward to just support this in VS Code as well:

https://github.com/rust-analyzer/rust-analyzer/pull/9693/files#diff-bca0ed357f47caaadd008edc9b2df19f36869583b78c2c0265b9f0db3650df2eR59

[rchl]

What does the client send as position when a range is selected but cursor is not on the range? Just a Position, I presume?

[flodiebold]

The current implementation just sends the position, yes.

[dbaeumer]

An implementation IMO must look like this:

• guarded by a server capability since clients can't suddenly send a range
• need to think if LSP should support a kind of selection with an anchor to know how the range got created.
• VS Lib implementation: take the selection from the editor of the document. If pos inside selection use it otherwise use the pos.

[predragnikolic]

FWIW, I wouldn't mix the Range type with the position:

interface HoverParams extends WorkDoneProgressParams {
    textDocument: TextDocumentIdentifier;
    position: Range | Position;
}

instead I would add a optional range property:

interface HoverParams extends WorkDoneProgressParams {
    textDocument: TextDocumentIdentifier;
    position: Position;
    range?: Range;
}

The server can then return whatever it wants based if the range is present or not.

[santiweight]

I don't have experience with LSP nor TS enough to contribute in the near term, but this submission would be of great benefit to Haskell's LSP server!

[adonovan]

It seems there is a great deal of enthusiasm for this simple extension to the protocol from maintainers of clients for Haskell, Rust, Go, TS, Scala, Ocaml and perhaps other languages.

Do we all agree with @predragnikolic that the appropriate change is a new optional range field alongside position?

interface HoverParams ... {
    textDocument: TextDocumentIdentifier;
    position: Position;
+    range?: Range;
}

Should this change be made just to HoverParams, or to the TextDocumentPositionParams base class? In all other existing uses of TextDocumentPositionParams the range field would not enable any new behavior as the commands are mostly concerned with identifiers, not compound expressions, but it nonetheless seems like a better abstraction of "the current selection or cursor position" that may be useful to future commands.

@dbaeumer asks whether "LSP should support a kind of selection with an anchor to know how the range got created": Is this necessary? I suspect we can all benefit from the current modest proposal without it.

@dbaeumer also says the feature must be "guarded by a server capability since clients can't suddenly send a range". However @predragnikolic's suggestion of using an additional field makes this unnecessary.

Any objections to adding a range?: Range field to the TextDocumentPositionParams class?

[HighCommander4]

Should this change be made just to HoverParams, or to the TextDocumentPositionParams base class? In all other existing uses of TextDocumentPositionParams the range field would not enable any new behavior as the commands are mostly concerned with identifiers, not compound expressions, but it nonetheless seems like a better abstraction of "the current selection or cursor position" that may be useful to future commands.

Having a range in TextDocumentPositionParams would be useful for features other than hovers as well, for reasons discussed in #1029.

[adonovan]

83 thumbs up on the issue body, 8 thumbs up on the specific proposal two comments above, no objections in 3 weeks.

Please review this PR: #2027