🐛 Incorrect handling of incremental LSP changes

[rchl]

Environment information

CLI:
  Version:              11.0.0
  Color support:        true

Platform:
  CPU Architecture:     aarch64
  OS:                   macos

What happened?

It appears there is a bug with handling of the textDocument/didChange LSP request.

(Note that this reproduces in Sublime Text when using sublimelsp/LSP-rome#1 but I don't expect you to set it up to test.)

With a document like:

/*
 * comment
 */

import * as path from 'node:path';

1. Place cursor on the first line and press cmd + /
2. ST removes both the initial /* and trailing */ characters which results in:

 * comment
 

import * as path from 'node:path';

4. LSP server receives a textDocument/didChange notification:

{
  "contentChanges": [
    {
      "range": {
        "end": {
          "character": 2,
          "line": 0
        },
        "start": {
          "character": 0,
          "line": 0
        }
      },
      "rangeLength": 2,
      "text": ""
    },
    {
      "range": {
        "end": {
          "character": 3,
          "line": 2
        },
        "start": {
          "character": 1,
          "line": 2
        }
      },
      "rangeLength": 2,
      "text": ""
    }
  ],
  "textDocument": {
    "uri": "file:///usr/local/workspace/github/typescript-language-server/src/test.ts",
    "version": 6
  }
}

at which point its internal state gets confused as evident by the diagnostic which says

expected a statement but instead found '* comment */'

Note how the server still sees the trailing */ which was removed.

As far as I can see this bug triggers because of the order of changes in the contentChanges list. When the order is from top to bottom of the document then the bug triggers. Otherwise not.

I've tried reproducing it in VSCode by selecting both ranges and removing them with backspace but VSCode sends changes in opposite order so it doesn't reproduce there.

Expected result

Server should correctly process didChange notification regardless of the order of changes.

ST behavior is valid per spec. The server just has to ensure that the change it is currently applying is applied for the document state that includes previously applied change.

Code of Conduct

• I agree to follow Rome's Code of Conduct

[github-actions]

👋 @rome/staff please triage this issue by adding one of the following labels: S-Bug: confirmed, S-Planned , S-Wishlist or umbrella

[leops]

This should have been fixed in #4135, I'll try to push a new nightly release that includes the fix