Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: PathEnclosingInterval is inappropriate for single-point LSP queries #71068

Open
adonovan opened this issue Dec 30, 2024 · 1 comment
Labels
gopls Issues related to the Go language server, gopls.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Dec 30, 2024

The handlers for most gopls RPCs start by computing the node in the syntax tree that is identified by the selection.

An unfortunate limitation of LSP is that in most cases the RPC request indicates the selection by only a single point, the cursor position, losing valuable information. Indicating the selection by '[' and ']' and the cursor by '⁁', in this example:

[break]⁁ label

the break keyword is selected, but the server can tell only that the cursor is after break. In:

[break ]⁁label

the selection is the break keyword plus the following space, but the server knows only that the cursor is before label. And in:

[break]⁁

the selection is the break keyword alone, but the server gets only the position after the 'k'.

Gopls makes extensive use of astutil.PathEnclosingInterval for inferring the syntax node from a given selection (start, end). When invoked on an empty selection (end=start), which is the common case in gopls because of the limitation described above, it behaves as if invoked on the interval (start, start+1). That means that it operates

  • in the first example on the space between break and label, and returns the BranchStmt (a reasonable answer);
  • in the second case on the first "l" of label, returning the Ident (again reasonable); but
  • in the third on the newline following break, returning the enclosing BlockStmt, which is not helpful.

It is the wrong tool for the job.

Until such time as LSP furnishes the server with both the start and end points of the selection (which may be a long wait), I propose that we introduce a new function for all single-position queries in gopls that gives the right answers in all three cases (respectively: BranchStmt, Ident, BranchStmt).

@madelinekalil

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 30, 2024
@adonovan adonovan removed the Tools This label describes issues relating to any tools in the x/tools repository. label Dec 30, 2024
@gopherbot gopherbot added this to the Unreleased milestone Dec 30, 2024
@adonovan adonovan changed the title x/tools/gopls: PathEnclosingInterval is in appropriate for single-point LSP queries x/tools/gopls: PathEnclosingInterval is inappropriate for single-point LSP queries Dec 30, 2024
@findleyr
Copy link
Member

findleyr commented Jan 3, 2025

Thanks, this makes sense. We could call it PathEnclosingPos, perhaps, and include many tests for operations that accept a TextDocumentPositionParams.

@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls.
Projects
None yet
Development

No branches or pull requests

3 participants