Typescript Language Sever crashes on 'Move to a new file`

[David-Else]

Summary

When a code action 'Move to a new file` is available, if selected it crashes with:

thread 'main' panicked at 'Failed to parse ApplyWorkspaceEdit params: Error { code: InvalidParams, message: "Invalid params: invalid value: integer `-1`, expected u32.", data: None }', helix-lsp/src/lib.rs:211:22
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Reproduction Steps

example.ts

function example() {}

place cursor on example, press space a and 'Move to a new file'

Maybe this feature is not built in yet, or maybe it is a bug?

Helix log

~/.cache/helix/helix.log

2022-04-08T16:30:45.238 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"snippetSupport":false},"completionItemKind":{}},"hover":{"contentFormat":["markdown"]},"rename":{"dynamicRegistration":false,"honorsChangeAnnotations":false,"prepareSupport":false}},"window":{"workDoneProgress":true},"workspace":{"configuration":true,"didChangeConfiguration":{"dynamicRegistration":false},"workspaceFolders":true}},"processId":708186,"rootPath":"/home/david/Documents/apps-sites/typescript/zombie-attack-4","rootUri":"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4","workspaceFolders":[{"name":"zombie-attack-4","uri":"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4"}]},"id":0}
2022-04-08T16:30:45.454 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"[lspserver] Using Typescript version (workspace) 4.5.5 from path \"/home/david/Documents/apps-sites/typescript/zombie-attack-4/node_modules/typescript/lib/tsserver.js\""}}
2022-04-08T16:30:45.454 helix_term::application [INFO] window/logMessage: LogMessageParams { typ: Info, message: "[lspserver] Using Typescript version (workspace) 4.5.5 from path \"/home/david/Documents/apps-sites/typescript/zombie-attack-4/node_modules/typescript/lib/tsserver.js\"" }
2022-04-08T16:30:45.724 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":0,"result":{"capabilities":{"textDocumentSync":2,"completionProvider":{"triggerCharacters":[".","\"","'","/","@","<"],"resolveProvider":true},"codeActionProvider":{"codeActionKinds":["source.fixAll.ts","source.removeUnused.ts","source.addMissingImports.ts","source.organizeImports.ts"]},"definitionProvider":true,"documentFormattingProvider":true,"documentRangeFormattingProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"executeCommandProvider":{"commands":["_typescript.applyWorkspaceEdit","_typescript.applyCodeAction","_typescript.applyRefactoring","_typescript.organizeImports","_typescript.applyRenameFile"]},"hoverProvider":true,"renameProvider":true,"referencesProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",",","<"]},"workspaceSymbolProvider":true,"implementationProvider":true,"typeDefinitionProvider":true,"foldingRangeProvider":true,"semanticTokensProvider":{"documentSelector":null,"legend":{"tokenTypes":["class","enum","interface","namespace","typeParameter","type","parameter","variable","enumMember","property","function","member"],"tokenModifiers":["declaration","static","async","readonly","defaultLibrary","local"]},"full":true,"range":true},"callsProvider":true}}}
2022-04-08T16:30:45.724 helix_lsp::transport [INFO] <- {"capabilities":{"callsProvider":true,"codeActionProvider":{"codeActionKinds":["source.fixAll.ts","source.removeUnused.ts","source.addMissingImports.ts","source.organizeImports.ts"]},"completionProvider":{"resolveProvider":true,"triggerCharacters":[".","\"","'","/","@","<"]},"definitionProvider":true,"documentFormattingProvider":true,"documentHighlightProvider":true,"documentRangeFormattingProvider":true,"documentSymbolProvider":true,"executeCommandProvider":{"commands":["_typescript.applyWorkspaceEdit","_typescript.applyCodeAction","_typescript.applyRefactoring","_typescript.organizeImports","_typescript.applyRenameFile"]},"foldingRangeProvider":true,"hoverProvider":true,"implementationProvider":true,"referencesProvider":true,"renameProvider":true,"semanticTokensProvider":{"documentSelector":null,"full":true,"legend":{"tokenModifiers":["declaration","static","async","readonly","defaultLibrary","local"],"tokenTypes":["class","enum","interface","namespace","typeParameter","type","parameter","variable","enumMember","property","function","member"]},"range":true},"signatureHelpProvider":{"triggerCharacters":["(",",","<"]},"textDocumentSync":2,"typeDefinitionProvider":true,"workspaceSymbolProvider":true}}
2022-04-08T16:30:45.724 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialized","params":{}}
2022-04-08T16:30:45.724 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"typescript","text":"import {\n  calculateCenter,\n  createMultiple,\n  Vector2,\n} from \"../src/helperFunctions\";\nimport {\n  heroFactory,\n  zombieFactory,\n  bulletFactory,\n  levelTextFactory,\n  GameEntityFactory,\n} from \"../src/factories\";\nimport type { GameCanvas } from \"../src/GameCanvas\";\nimport { PubSub } from \"../src/EventObserver\";\nimport type { Zombie } from \"../src/entities/Zombie\";\n\n// type EntityKeys = \"hero\" | \"zombies\" | \"bullets\" | \"text\";\n\nexport interface Entity {\n  position: Vector2;\n  velocity: Vector2;\n  widthHeight: Vector2;\n  rotation: number;\n  update: () => void;\n  draw: (gC: Readonly<GameCanvas>) => void;\n}\n\nfunction example() {}\n\n/**\n * =============================================================================\n * Axis-aligned bounding boxes, test if two game entities are overlapping or not\n * =============================================================================\n */\nfunction checkCollision(entity1: Entity, entity2: Entity): boolean {\n  const left = entity1.position[0];\n  const right = entity1.position[0] + entity1.widthHeight[0];\n  const top = entity1.position[1];\n  const bottom = entity1.position[1] + entity1.widthHeight[1];\n\n  const otherLeft = entity2.position[0];\n  const otherRight = entity2.position[0] + entity2.widthHeight[0];\n  const otherTop = entity2.position[1];\n  const otherBottom = entity2.position[1] + entity2.widthHeight[1];\n\n  return !(\n    left > otherRight ||\n    right <= otherLeft ||\n    top >= otherBottom ||\n    bottom <= otherTop\n  );\n}\n\nconst EntityKeys = [\"hero\", \"zombies\", \"bullets\", \"text\"] as const;\ntype EntityKey = typeof EntityKeys[number];\n\n/**\n * The World stores all the game entities and the pubsub message bus\n * It contains methods to add/remove entities, check for collisions\n * and take actions when collisons happen\n *\n * Each type of event has its own pubsub object, and these are composed into the\n * entities that need access to data outside of themselves\n */\nclass World {\n  gameCanvas;\n  level = 1;\n  entities;\n  bulletFiredPubSub;\n  zombieDiesPubSub;\n\n  constructor(gameCanvas: Readonly<GameCanvas>) {\n    this.gameCanvas = gameCanvas;\n    // Create PubSub event buses, they will be injected into entities that need them\n    this.bulletFiredPubSub = new PubSub<string>();\n    this.zombieDiesPubSub = new PubSub<string>();\n    // Subscribe to any that need access to World scope\n    this.bulletFiredPubSub.subscribe(() => this.addBullet());\n\n    // https://www.reddit.com/r/typescript/comments/jfzkor/how_can_i_get_the_keys_from_a_mapkeys_iterator_as/\n    const entities = new Map<EntityKey, Entity[]>(\n      EntityKeys.map((k) => [k, []])\n    );\n\n    this.entities = entities;\n  }\n\n  addHero(): void {\n    if (this.entities.get(\"hero\")) {\n      this.entities.get(\"hero\")?.push(\n        heroFactory(\n          this.gameCanvas.getMiddle(), // maybe inject this!\n          this.bulletFiredPubSub\n        )\n      );\n    }\n  }\n\n  async addZombies(): Promise<void> {\n    if (this.entities.get(\"zombies\")) {\n      this.entities.get(\"zombies\")?.push(\n        await zombieFactory(\n          this.gameCanvas.getWidthHeight(),\n          this.gameCanvas.getMiddle(), // TODO SHOULD be hero.position somehow\n          [0, 0]\n        )\n      );\n    }\n  }\n\n  addText(): void {\n    if (this.entities.get(\"text\")) {\n      this.entities.get(\"text\")?.push(\n        levelTextFactory({\n          position: [190, 50],\n          velocity: [0, 0],\n          rotation: 0,\n          text: `Level: ${this.level}\nBullets left:`,\n          textAlignment: \"right\",\n          fillStyle: \"serif\",\n          font: \"serif\",\n          fontSize: 32,\n          widthHeight: [0, 100],\n        })\n      );\n    }\n  }\n\n  addBullet(): void {\n    const hero = this.entities.get(\"hero\");\n    if (hero) {\n      // get possible undefined?\n      this.entities.get(\"bullets\")?.push(\n        bulletFactory({\n          position: calculateCenter(hero[0]),\n          rotation: hero[0].rotation,\n        })\n      );\n    }\n  }\n\n  getEntity(entityKey: EntityKey, index: number): Entity | undefined {\n    const result = this.entities.get(entityKey)?.[index];\n    return result;\n  }\n\n  deleteEntity(entity: EntityKey, index: number): void {\n    this.entities.get(entity)?.splice(index, 1);\n  }\n\n  checkCollision(): void {\n    this.checkIfGroupsColliding(\n      this.entities.get(\"zombies\"), // WHY?\n      this.entities.get(\"bullets\"),\n      this.zombieBulletCollisionHandler\n    );\n    this.checkIfGroupsColliding(\n      this.entities.get(\"hero\"),\n      this.entities.get(\"zombies\"),\n      this.heroZombieCollisionHandler\n    );\n  }\n\n  private readonly checkIfGroupsColliding = (\n    entitiesGroupOne: Entity[] | undefined,\n    entitiesGroupTwo: Entity[] | undefined,\n    collisionHandler: (indexOne: number, indexTwo: number) => void\n  ): void => {\n    entitiesGroupOne?.some((entity, indexOne) =>\n      entitiesGroupTwo?.some((entityTwo, indexTwo) => {\n        if (checkCollision(entity, entityTwo)) {\n          collisionHandler(indexOne, indexTwo);\n          return true; // return from some() when first match is found\n        }\n        return false;\n      })\n    );\n  };\n\n  private readonly zombieBulletCollisionHandler = (\n    zombieIndex: number,\n    heroIndex: number\n  ): void => {\n    this.deleteEntity(\"zombies\", zombieIndex);\n    this.deleteEntity(\"bullets\", heroIndex);\n  };\n\n  private readonly heroZombieCollisionHandler = (heroIndex: number): void => {\n    this.deleteEntity(\"hero\", heroIndex);\n  };\n}\n\n// [\n//   [\n//     \"hero\",\n//     createMultiple(1, () =>\n//       GameEntityFactory.getHero(\n//         this.level,\n//         gameCanvas.getMiddle(),\n//         this.bulletFiredPubSub\n//       )\n//     ),\n//   ],\n//   [\n//     \"zombies\",\n//     createMultiple(options.numberOfZombies, () =>\n//       zombieFactory(\n//         gameCanvas.getWidthHeight(),\n//         gameCanvas.getMiddle(), // TODO SHOULD be hero.position somehow\n//         [0, 0]\n//       )\n//     ),\n//   ],\n//   [\"bullets\", []],\n//   [\n//     \"text\",\n//     createMultiple(1, () =>\n//       textFactory({\n//         position: [190, 50],\n//         velocity: [0, 0],\n//         rotation: 0,\n//         text: `Score:\n// Bullets left:`,\n//         textAlignment: \"right\",\n//         fillStyle: \"serif\",\n//         font: \"serif\",\n//         fontSize: 32,\n//         widthHeight: [0, 100],\n//       })\n//     ),\n//   ],\n// ]\n","uri":"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","version":0}}}
2022-04-08T16:30:45.733 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":0,"method":"window/workDoneProgress/create","params":{"token":"d35a9c98-2ac7-47f4-b3a3-2e464c4ddba2"}}
2022-04-08T16:30:45.733 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","result":null,"id":0}
2022-04-08T16:30:45.734 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"$/progress","params":{"token":"d35a9c98-2ac7-47f4-b3a3-2e464c4ddba2","value":{"kind":"begin","title":"Initializing JS/TS language features…"}}}
2022-04-08T16:30:47.170 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"$/progress","params":{"token":"d35a9c98-2ac7-47f4-b3a3-2e464c4ddba2","value":{"kind":"end"}}}
2022-04-08T16:30:47.231 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","diagnostics":[]}}
2022-04-08T16:30:47.354 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","diagnostics":[{"range":{"start":{"line":113,"character":25},"end":{"line":124,"character":9}},"message":"Argument of type '{ position: number[]; velocity: number[]; rotation: number; text: string; textAlignment: string; fillStyle: string; font: string; fontSize: number; widthHeight: number[]; }' is not assignable to parameter of type 'string'.","severity":1,"code":2345,"source":"typescript"},{"range":{"start":{"line":2,"character":2},"end":{"line":2,"character":16}},"message":"'createMultiple' is declared but its value is never read.","severity":4,"code":6133,"source":"typescript"},{"range":{"start":{"line":10,"character":2},"end":{"line":10,"character":19}},"message":"'GameEntityFactory' is declared but its value is never read.","severity":4,"code":6133,"source":"typescript"},{"range":{"start":{"line":14,"character":0},"end":{"line":14,"character":53}},"message":"'Zombie' is declared but its value is never read.","severity":4,"code":6133,"source":"typescript"},{"range":{"start":{"line":27,"character":9},"end":{"line":27,"character":16}},"message":"'example' is declared but its value is never read.","severity":4,"code":6133,"source":"typescript"},{"range":{"start":{"line":64,"character":6},"end":{"line":64,"character":11}},"message":"'World' is declared but never used.","severity":4,"code":6196,"source":"typescript"}]}}
2022-04-08T16:30:48.707 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"textDocument/codeAction","params":{"context":{"diagnostics":[]},"range":{"end":{"character":10,"line":34},"start":{"character":9,"line":34}},"textDocument":{"uri":"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts"}},"id":1}
2022-04-08T16:30:48.741 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":1,"result":[{"title":"Move to a new file","command":{"title":"Move to a new file","command":"_typescript.applyRefactoring","arguments":[{"file":"/home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","startLine":35,"startOffset":10,"endLine":35,"endOffset":11,"refactor":"Move to a new file","action":"Move to a new file"}]},"kind":"refactor.move"},{"title":"Convert parameters to destructured object","command":{"title":"Convert parameters to destructured object","command":"_typescript.applyRefactoring","arguments":[{"file":"/home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","startLine":35,"startOffset":10,"endLine":35,"endOffset":11,"refactor":"Convert parameters to destructured object","action":"Convert parameters to destructured object"}]},"kind":"refactor"}]}
2022-04-08T16:30:48.741 helix_lsp::transport [INFO] <- [{"command":{"arguments":[{"action":"Move to a new file","endLine":35,"endOffset":11,"file":"/home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","refactor":"Move to a new file","startLine":35,"startOffset":10}],"command":"_typescript.applyRefactoring","title":"Move to a new file"},"kind":"refactor.move","title":"Move to a new file"},{"command":{"arguments":[{"action":"Convert parameters to destructured object","endLine":35,"endOffset":11,"file":"/home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","refactor":"Convert parameters to destructured object","startLine":35,"startOffset":10}],"command":"_typescript.applyRefactoring","title":"Convert parameters to destructured object"},"kind":"refactor","title":"Convert parameters to destructured object"}]
2022-04-08T16:30:50.610 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"workspace/executeCommand","params":{"arguments":[{"action":"Move to a new file","endLine":35,"endOffset":11,"file":"/home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts","refactor":"Move to a new file","startLine":35,"startOffset":10}],"command":"_typescript.applyRefactoring"},"id":2}
2022-04-08T16:30:50.672 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":1,"method":"workspace/applyEdit","params":{"edit":{"changes":{"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/World.ts":[{"range":{"start":{"line":15,"character":0},"end":{"line":15,"character":0}},"newText":"import { checkCollision } from \"./checkCollision.1\";\n"},{"range":{"start":{"line":29,"character":0},"end":{"line":53,"character":0}},"newText":""}],"file:///home/david/Documents/apps-sites/typescript/zombie-attack-4/docs/checkCollision.1.ts":[{"range":{"start":{"line":-1,"character":-1},"end":{"line":-1,"character":-1}},"newText":"import { Entity } from \"./World\";\n\n/**\n * =============================================================================\n * Axis-aligned bounding boxes, test if two game entities are overlapping or not\n * =============================================================================\n */\nexport function checkCollision(entity1: Entity, entity2: Entity): boolean {\n    const left = entity1.position[0];\n    const right = entity1.position[0] + entity1.widthHeight[0];\n    const top = entity1.position[1];\n    const bottom = entity1.position[1] + entity1.widthHeight[1];\n\n    const otherLeft = entity2.position[0];\n    const otherRight = entity2.position[0] + entity2.widthHeight[0];\n    const otherTop = entity2.position[1];\n    const otherBottom = entity2.position[1] + entity2.widthHeight[1];\n\n    return !(\n        left > otherRight ||\n        right <= otherLeft ||\n        top >= otherBottom ||\n        bottom <= otherTop\n    );\n}\n"}]}}}}

Platform

Linux

Terminal Emulator

Kitty

Helix Version

helix 22.05-dev (2d4f94e)

[the-mikedavis]

I've seen this pop up in odd scenarios in elixir-ls but haven't had the time to make a nice reproduction case. My understanding is that the language server is breaking the spec by sending -1 for a field that the spec says must be a u32 (i.e. non-negative) (but I don't have a link to where the spec says this). Snippet from the logs:

[{"range":{"start":{"line":-1,"character":-1},"end":{"line":-1,"character":-1}}

Though helix should probably not be panicking: it would make sense to discard a message from the language server that cannot be correctly decoded and log it as an error.

[David-Else]

@the-mikedavis I think this is the issue you are talking about is:

Fix invalid ranges

tsserver uses non-compliant ranges in some code actions (most notably "Move to a new file"), which makes them not work properly in Neovim. The plugin fixes these ranges so that the affected actions work as expected.

You can enable this feature by calling setup_client in your configuration (see below).

https://github.com/jose-elias-alvarez/nvim-lsp-ts-utils which links to neovim/neovim#14469

Also, there is a new Neovim plugin that attempts to fix and enhance the TypeScript Language Server (for Neovim, and probably any client that is not VS Code): https://github.com/jose-elias-alvarez/typescript.nvim

Is it possible to port the fix?

[the-mikedavis]

Here's the spec on ranges that says they must be 0-based: https://microsoft.github.io/language-server-protocol/specification#range

If I'm reading this right, here's the fix in that plugin: jose-elias-alvarez/nvim-lsp-ts-utils@d21761d#diff-1aff290f09c6b3579514b767764dbe457f42b96b9425b888c9f7e3ba8b2121b7R214-R219. It just sets it to 0 if it's -1.

On the one hand it seems like an easy fix to make but on the other hand this is a pretty blatant violation of the LSP spec. I can't find any issues in the typescript LS repo about negative ranges. Maybe it hasn't been reported yet?

[zen3ger]

I've seen this pop up in odd scenarios in elixir-ls but haven't had the time to make a nice reproduction case. My understanding is that the language server is breaking the spec by sending -1 for a field that the spec says must be a u32 (i.e. non-negative) (but I don't have a link to where the spec says this). Snippet from the logs:

[{"range":{"start":{"line":-1,"character":-1},"end":{"line":-1,"character":-1}}

It's at the Position description:

Position in a text document expressed as zero-based line and zero-based character offset. A position is between two characters like an ‘insert’ cursor in an editor. Special values like for example -1 to denote the end of a line are not supported.

[andreicek]

For elixir-ls looks like this is a quick fix:

diff --git a/apps/language_server/lib/language_server/build.ex b/apps/language_server/lib/language_server/build.ex
index e8d050c..1d6a441 100644
--- a/apps/language_server/lib/language_server/build.ex
+++ b/apps/language_server/lib/language_server/build.ex
@@ -279,7 +279,7 @@ defmodule ElixirLS.LanguageServer.Build do
   end
 
   defp range(position, nil) when is_integer(position) do
-    line = position - 1
+    line = if position - 1 < 0, do: 0, else: position - 1
 
     # we don't care about utf16 positions here as we send 0
     %{
@@ -289,7 +289,7 @@ defmodule ElixirLS.LanguageServer.Build do
   end
 
   defp range(position, source_file) when is_integer(position) do
-    line = position - 1
+    line = if position - 1 < 0, do: 0, else: position - 1
     text = Enum.at(SourceFile.lines(source_file), line) || ""
 
     start_idx = String.length(text) - String.length(String.trim_leading(text)) + 1

I've had elixir-ls crash helix just while sitting at an open file. I'll test more and then open a PR to elixir-lsp/elixir-ls.

[the-mikedavis]

Closed by 7ae6cad

Malformed LSP messages are now logged as an error and discarded instead of panicing.