x/tools/gopls: support selection range request

[stamblerre]

Forked from microsoft/vscode-go#2984.

See https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_selectionRange.

[rneatherway]

I'm interested in having a look at this one. Would you suggest that I follow the same advice as over here (#47558 (comment)) to get started?

[findleyr]

Hi @rneatherway, sorry for the slow response. Sure this is a good starter project, and a good way to get started would be to solve the problem outside of gopls using go/ast: given a position and an ast.File, can you find interesting selection ranges?

Then we can help you wire this into gopls' request handling.

[rneatherway]

No problem at all, thanks for the guidance 👍

[rneatherway]

👋 I've had a go at this and put together a function of this signature:

type Selection struct {
	StartLine int
	StartCol  int
	EndLine   int
	EndCol    int
}

func expandSelection(
	fileSet *token.FileSet,
	file *ast.File,
	s Selection,
) Selection

It's up at https://github.com/rneatherway/expand along with some tests/examples.

Note that it also requires a token.FileSet to get the location of AST nodes. It works by doing simple descent of the tree ignoring any nodes that do not include the current selection, returning the last node that is a strict superset of the current selection. I noticed that the column numbers displayed in VSCode count a tab as multiple characters according to the configured tab width, but I'd guess that's already accounted for by other gopls features or the language server API.

[findleyr]

Thanks! We'll take a look.

[rneatherway]

Sorry to prod, but if you have any advice on wiring this up I'd be interested to try that too.

[findleyr]

@rneatherway so sorry that this fell through the cracks. Entirely my fault, and thank you for following up.

What you've done looks good! I think there is a utility that may be helpful, which operates similarly to your algorithm:
https://pkg.go.dev/golang.org/x/[email protected]/go/ast/astutil#PathEnclosingInterval

Using this traversal, you can start from the innermost enclosing interval, and work outwards until you find an interesting node, then compute its range.

One note: for simplicity I suggest working with a token.Pos rather than token.Position. Language servers use UTF-16 column positions anyway, so we will always have to do a conversion before returning to the client.

[rneatherway]

What you've done looks good! I think there is a utility that may be helpful, which operates similarly to your algorithm:
https://pkg.go.dev/golang.org/x/[email protected]/go/ast/astutil#PathEnclosingInterval

Oh nice, thanks! I've simplified the code in my repository using that helper method. The input and output are now in terms of offsets so hopefully this is ready to try integrating. It's hard to go much further without knowing what the language server has available when this will be invoked.

[findleyr]

Yep, let's start integrating it.

The entry point for this handler is x/tools/gopls/internal/lsp/server_gen.go. In this case, you'll be writing a handler that needs what we call the "ParsedGoFile" corresponding to the request URI. A ParsedGoFile is just a wrapper around *ast.File that bundles additional related informatoin.

A similar type of request is DocumentSymbols, which returns information about symbols in a given file, using the AST. You can look at how that handler is structured: get a snapshot using Server.beginFileRequest, then use that snapshot to parse. Want to give it a shot and see how far you get? Now that you're working in x/tools, the simplest way to collaborate will be to send me a CL (even if it is a work in progress) to comment on.

[findleyr]

(aside: converting to int does not convert token.Pos to offsets, as token.Pos is relative to some file base position. You must use token.File.Offset to convert to an offset)

[rneatherway]

@findleyr I've got a branch visible at https://github.com/golang/tools/compare/master...rneatherway:tools:selectionrange?expand=1 including extending the tests to support this new case. A few things came up while working on it that I'd appreciate your opinion on:

• I found a few minor typos and other issues in READMEs and the code generation helper doesn't work on Windows. Do you mind me including those in this change or should I submit them separately?

• The tests for my standalone version that I wrote over at https://github.com/rneatherway/expand/blob/master/expand_test.go include multiline cases, which seem to be necessary for proper testing of this feature. Does the annotation testing support marking spans over multiple lines right now? Update: the inputs are positions so multiline annotations are not necessary.

• I wasn't sure if it is necessary or desirable to implement command-line support for this feature. I also claim that it doesn't need "source" level testing, but I'm not sure what that means.

• My implementation interprets position[0] and position[1] as specifying a range, as I had in mind the idea of extending a single selection by one step. The LSP documentation states:

  A selection range in the return array is for the position in the provided parameters at the same index. Therefore positions[i] must be contained in result[i].range. To allow for results where some positions have selection ranges and others do not, result[i].range is allowed to be the empty range at positions[i].

  Some background reading at Feature request: "document/extendSelection" for semantic selection microsoft/language-server-protocol#613 helped to clarify that this is intended for multiple cursors and to return the entire path to the root of the AST in order to avoid round-trips. So I can probably pretty much return astutil.PathEnclosingInterval(...) for each position. I'll update accordingly and try to think about how to re-phrase the tests. Update: I have made these updates and changed the tests so they output numeric and textual representations of the ranges to a golden file.

[gopherbot]

Change https://go.dev/cl/452315 mentions this issue: gopls/internal/lsp: add selection range request

[rneatherway]

I got far enough that I decided to open the pull request: golang/tools#416

I guess my last question would be whether you had any advice on how to test this change with VSCode "live". So far I have tried go installing from my branch and invoking the "expand selection" command in a subsequently launched VSCode, but it doesn't behave any differently.

[findleyr]

@hyangah do we need to register a new selection range provider? It may be that this needs a vscode-go change as well.

[rneatherway]

@hyangah do we need to register a new selection range provider? It may be that this needs a vscode-go change as well.

I've got a feeling that we need to do something like https://github.com/elm-tooling/elm-language-server/blob/c7e21144c0833690a58bbf5f5e6b44b123c1fc9f/src/providers/selectionRangeProvider.ts, but looking in the vscode-go repository a lot of the providers are in a legacy directory, so I am not sure if I should be adding new similar code or if there is a new way of doing things.

[gopherbot]

Change https://go.dev/cl/458199 mentions this issue: gopls/internal/lsp: announce selectionRangeProvider capability

[hyangah]

Thanks for implementing the feature. That's neat.
VSCode Go uses LSP so the server needs to announce this new capability.

[rneatherway]

Thanks for implementing the feature. That's neat.
VSCode Go uses LSP so the server needs to announce this new capability.

Ah thanks! I wasn't sure how the final wiring up should look. Is there anything more we need to do in vscode-go?