helix: Gleam LSP fails to format because the end range is out of bounds

Summary

The textDocument/formatting response from the gleam LSP sends a line number that is way past the end of the document:

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": [
    {
      "newText": "fn main() {\n  let x = 5\n}\n",
      "range": {
        "end": {
          "character": 0,
          "line": 4294967295
        },
        "start": {
          "character": 0,
          "line": 0
        }
      }
    }
  ]
}

That causes lsp_pos_to_pos to return None which causes an early return here: https://github.com/helix-editor/helix/blob/ce0837dbb75badf39c9b1ac251fba9c3efbc57c4/helix-lsp/src/lib.rs#L323-L327

making the Transaction a no-op.

Reproduction Steps

I tried this:

hx file.gleam
add some contents like
```
fn main() {
    let x = 5
}
```
:format

I expected this to happen: the formatter should turn the 4 column indent into a 2 column indent.

Instead, this happened: no change to the document.

Helix log

~/.cache/helix/helix.log

2023-02-16T13:51:01.704 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"general":{"positionEncodings":["utf-32","utf-8","utf-16"]},"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"deprecatedSupport":true,"insertReplaceSupport":true,"resolveSupport":{"properties":["documentation","detail","additionalTextEdits"]},"snippetSupport":false,"tagSupport":{"valueSet":[1]}},"completionItemKind":{}},"hover":{"contentFormat":["markdown"]},"publishDiagnostics":{},"rename":{"dynamicRegistration":false,"honorsChangeAnnotations":false,"prepareSupport":false},"signatureHelp":{"signatureInformation":{"activeParameterSupport":true,"documentationFormat":["markdown"],"parameterInformation":{"labelOffsetSupport":true}}}},"window":{"workDoneProgress":true},"workspace":{"applyEdit":true,"configuration":true,"didChangeConfiguration":{"dynamicRegistration":false},"executeCommand":{"dynamicRegistration":false},"symbol":{"dynamicRegistration":false},"workspaceFolders":true}},"clientInfo":{"name":"helix","version":"22.12 (78a1e2db)"},"processId":192552,"rootPath":"/home/michael/tmp","rootUri":"file:///home/michael/tmp","workspaceFolders":[{"name":"tmp","uri":"file:///home/michael/tmp"}]},"id":0}
2023-02-16T13:51:01.705 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":0,"result":{"capabilities":{"completionProvider":{"triggerCharacters":["."," "]},"definitionProvider":true,"documentFormattingProvider":true,"hoverProvider":true,"textDocumentSync":{"change":1,"save":{"includeText":false}}}}}
2023-02-16T13:51:01.706 helix_lsp::transport [INFO] <- {"capabilities":{"completionProvider":{"triggerCharacters":["."," "]},"definitionProvider":true,"documentFormattingProvider":true,"hoverProvider":true,"textDocumentSync":{"change":1,"save":{"includeText":false}}}}
2023-02-16T13:51:01.706 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialized","params":{}}
2023-02-16T13:51:01.706 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":"create-compiling-progress-token","method":"window/workDoneProgress/create","params":{"token":"compiling-gleam"}}
2023-02-16T13:51:01.706 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"$/progress","params":{"token":"compiling-gleam","value":{"cancellable":false,"kind":"begin","title":"Compiling Gleam"}}}
2023-02-16T13:51:01.706 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"$/progress","params":{"token":"compiling-gleam","value":{"kind":"end"}}}
2023-02-16T13:51:01.712 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"gleam","text":"fn main() {\n    let x = 5\n}\n","uri":"file:///home/michael/tmp/file.gleam","version":0}}}
2023-02-16T13:51:01.712 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","result":null,"id":"create-compiling-progress-token"}
2023-02-16T13:51:02.665 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"textDocument/formatting","params":{"options":{"insertSpaces":true,"tabSize":2},"textDocument":{"uri":"file:///home/michael/tmp/file.gleam"}},"id":1}
2023-02-16T13:51:02.665 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":1,"result":[{"newText":"fn main() {\n  let x = 5\n}\n","range":{"end":{"character":0,"line":4294967295},"start":{"character":0,"line":0}}}]}
2023-02-16T13:51:02.666 helix_lsp::transport [INFO] <- [{"newText":"fn main() {\n  let x = 5\n}\n","range":{"end":{"character":0,"line":4294967295},"start":{"character":0,"line":0}}}]

Platform

Linux

Terminal Emulator

Kitty 0.26.2

Helix Version

78a1e2db6035b326d7536fbd0fb60f9fc586d978

About this issue

Original URL
State: closed
Created a year ago
Comments: 16 (8 by maintainers)

Most upvoted comments

Thanks for digging into this folks! Happy to make the change in Gleam.

It would be fab if the spec could be made more clear here, my reading of it was that this was supported (I can see nothing that suggests otherwise) and VSCode’s accepting of it solidified that in my mind.

lpil on Mar 2, 2023

Gleam does this on purpose: https://github.com/gleam-lang/gleam/blob/94caeb35b284dff0b4b8c3b0b800e3b3021b45a4/compiler-cli/src/lsp.rs#L860

In my opinion this is not really LSP compliant but I guess it doesn’t hurt to still accept such positions. It should be quite easy. We should log a warning in that case tough in-case it’s a genuine bug

pascalkuthe on Feb 16, 2023

I just tried it out building from source and it works great! Thanks @VlkrS and @lpil!

the-mikedavis on Mar 3, 2023

Hmm not quite I think the

To fix it in Helix, lsp_pos_to_pos could return the document end when the given position is past the end of the document.

Actually, instead we should probably just handle the None return of lsp_pos_to_pos in this case to set end to the document end char.

Hmm not sure I think we should return Some(document.len_chars()) here https://github.com/helix-editor/helix/blob/ce0837dbb75badf39c9b1ac251fba9c3efbc57c4/helix-lsp/src/lib.rs#L129

I guess since other LSP client’s don’t have problems with such positions other servers might do this for other things than formatting too. And lsp_pos_to_pos can also return None if an invalid (between chars) utf-16 /utf-8 character offset is returned. I still we should abort in that case as we do currently

pascalkuthe on Feb 16, 2023