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

lsp: deleting entire file breaks incremental synchronization #27383

Open
XeroOl opened this issue Feb 8, 2024 · 11 comments
Open

lsp: deleting entire file breaks incremental synchronization #27383

XeroOl opened this issue Feb 8, 2024 · 11 comments
Labels
bug issues reporting wrong behavior lsp

Comments

@XeroOl
Copy link

XeroOl commented Feb 8, 2024

Problem

Neovim generates an invalid textDocument/didChange message when deleting and pasting the entire file.
The message references the range {line 0 col 0 - line 1 col 0}, but when you delete the entire file, there is no line 1.

I'm making a language server, and my server gets confused by this range, because it goes past the end of the file.

Every other language server I've tried seems to tolerate the invalid message, but I'm fairly sure Neovim's behavior isn't correct.

Steps to reproduce using "nvim -u minimal_init.lua"

You should be able to reproduce this really easily with almost any setup, as long as the language supports the {"change":2} mode. (pretty much all language servers do this).

At this point, skip down to the Steps section.

Probably unnecessary details

If you really want the exact same setup as me, you can follow these steps:

The exact steps to set up the environment

Download fennel-ls and put it on the path. To run it, you also need need lua on the path, because fennel-ls runs in lua.
Then, init.lua:

--- CHANGED THESE
local pattern = '*.fnl'
local cmd = {'fennel-ls'}
-- Add files/folders here that indicate the root of a project
local root_markers = {'.git', '.editorconfig'}
-- Change to table with settings if required
local settings = vim.empty_dict()

vim.api.nvim_create_autocmd('BufEnter', {
  pattern = pattern,
  callback = function(args)
    print("reached callback")
    local match = vim.fs.find(root_markers, { path = args.file, upward = true })[1]
    local root_dir = match and vim.fn.fnamemodify(match, ':p:h') or nil
    vim.lsp.start({
      name = 'fennel-ls',
      cmd = cmd,
      root_dir = root_dir,
      settings = settings
    })
  end
})

I know bufenter is the wrong autocmd, but there is no filetype for .fnl.

Then, touch .editorconfig so that the current dir is a root dir. fennel-ls needs a workspace to run in, so this is important.
Finally, run the following steps, using broken.fnl as the filename.

Steps

  1. Open a new file that starts a language server
  2. Type any text. (in my logs, I type the letter o)
  3. Move the cursor to the start of the file and use dG to delete everything
  4. Press p to paste everything back
  5. Observe that the message sent to the language server isn't valid.

Description of each sync step

For each step, I'll show the textDocument/didOpen and textDocument/didChange messages that are sent to the server, and what the file should contain.

Open a new file that starts a language server

{
  "params": {
    "textDocument": {
      "version": 0,
      "text": "\n",
      "uri": "file:///home/xerool/Documents/bug/broken.fnl",
      "languageId": "fennel"
    }
  },
  "jsonrpc": "2.0",
  "method": "textDocument/didOpen"
}

Okay, so Neovim starts an "empty" file with a newline. That's fine. The contents of the file are "\n".

Type any text.

In this instance, I simply typed the letter o.

{
  "params": {
    "textDocument": {
      "uri": "file:///home/xerool/Documents/bug/broken.fnl",
      "version": 4
    },
    "contentChanges": [
      {
        "range": {
          "end": {
            "line": 0,
            "character": 0
          },
          "start": {
            "line": 0,
            "character": 0
          }
        },
        "text": "o",
        "rangeLength": 0
      }
    ]
  },
  "jsonrpc": "2.0",
  "method": "textDocument/didChange"
}

Okay, so we take the range {line 0 char 0 - line 0 char 0} and replace it with "o". The contents of the file are "o\n".

Delete everything

{
  "params": {
    "textDocument": {
      "uri": "file:///home/xerool/Documents/bug/broken.fnl",
      "version": 5
    },
    "contentChanges": [
      {
        "range": {
          "end": {
            "line": 1,
            "character": 0
          },
          "start": {
            "line": 0,
            "character": 0
          }
        },
        "text": "",
        "rangeLength": 2
      }
    ]
  },
  "jsonrpc": "2.0",
  "method": "textDocument/didChange"
}

Okay, so we replace from {line 0 char 0 - line 1 char 0} with "". Now, the current content of the file is "", with no newline. The file is empty.

Press p

{
  "params": {
    "textDocument": {
      "uri": "file:///home/xerool/Documents/bug/broken.fnl",
      "version": 6
    },
    "contentChanges": [
      {
        "range": {
          "end": {
            "line": 1,
            "character": 0
          },
          "start": {
            "line": 0,
            "character": 0
          }
        },
        "text": "\no\n",
        "rangeLength": 1
      }
    ]
  },
  "jsonrpc": "2.0",
  "method": "textDocument/didChange"
}

Here's where I've got a problem. According to the previous messages, the file's contents are "". It's empty. However, the server is being asked to replace from {line 0 char 0 - line 1 char 0} with some text.

There is no position that corresponds to line 1 char 0, because there is no newline character in the file, and therefore no line 1.

Now, you may argue that the protocol says:

If you want to specify a range that contains a line including the line ending character(s) then use an end position denoting the start of the next line.

However, this doesn't apply here, because there is no "line ending character(s)" that could possibly be included here. I believe that Neovim's message isn't valid, and that it is correct for the language server to crash.

Expected behavior

I expected the the content changes to be applicable to the file. For example: I would be okay if the final message did not try to reference line 1:

{
  "params": {
    "textDocument": {
      "uri": "file:///home/xerool/Documents/bug/broken.fnl",
      "version": 6
    },
    "contentChanges": [
      {
        "range": {
          "end": {
            "line": 0,
            "character": 0
          },
          "start": {
            "line": 0,
            "character": 0
          }
        },
        "text": "\no\n",
        "rangeLength": 1
      }
    ]
  },
  "jsonrpc": "2.0",
  "method": "textDocument/didChange"
}

It would also be okay if instead, there was an extra "contentChanges" message after the dP that re-introduces the newline after telling the server that the newline was deleted. I'm not sure exactly where the server and client are desyncing, but I would like them to stay in sync.

Neovim version (nvim -v)

0.9.5 Release

Language server name/version

fennel-ls 0.1.0

Operating system/version

Arch Linux

Log file

https://gist.github.com/XeroOl/873a8ab9968861cf33c73f8acea3ab28

@XeroOl XeroOl added bug issues reporting wrong behavior lsp labels Feb 8, 2024
@dylan-chong
Copy link

dylan-chong commented Mar 8, 2024

i get a similar error when yying some line then ping it somewhere on some other line. there's a chance that ping will make the lsp get very out of sync. (have noticed this on tsx files). often get diagnostics about syntax error or something ridiculous that will go away when :eing. treesitter syntax highlighting turns chaotic too

@XeroOl
Copy link
Author

XeroOl commented Mar 8, 2024

Yeah, I've seen desyncs happen like that, but I couldn't figure out how to reliably reproduce it. dGp was the first sequence of edits I tried that reliably reproduced the issue.
I suspect there are other combinations of edits that causes more serious desyncs

@dylan-chong
Copy link

This could be related #16669

cc @rish987 @mjlbach

@dylan-chong
Copy link

dear god this issue is constant. i'm running neovim NVIM v0.10.0-dev-1495+g3d8f0cb69. i have to run :e after writing a few lines of code, or pasting anything, otherwise the lsp is way out of sync. ocassionally :e doesnt even get the lsp to sync properly

@rudiejd
Copy link
Contributor

rudiejd commented Mar 27, 2024

For what it's worth, I've noticed similar issues with typescript-language-server as @dylan-chong. Could be related to #26483 - is dynamic registration enabled in this case?

@XeroOl
Copy link
Author

XeroOl commented Mar 27, 2024

This is a different issue. I can reproduce this one regardless of if didChangeWatchedFiles is on or off.

@iurimateus
Copy link

iurimateus commented Mar 31, 2024

Angular language service crashes with

[ERROR][2024-03-29 09:21:11] .../vim/lsp/rpc.lua:789    "rpc"   "ngserver"
"stderr"
"/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:35508\n

Debug.assert(oldText.length - textChangeRange.span.length + textChangeRange.newLength === newText.length);
\n                  ^\n\nError:
Debug Failure. False expression.\n    at checkChangeRange

(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:35508:19)\n
at Object.updateSourceFile2 [as updateSourceFile]
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:35213:11)\n
at updateSourceFile
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:27912:45)\n
at updateLanguageServiceSourceFile
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:139642:31)\n
at acquireOrUpdateDocument
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:131539:30)\n
at Object.updateDocumentWithKey
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:131464:14)\n
at Object.getOrCreateSourceFileByPath [as getSourceFileByPath]
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:139868:39)\n
at tryReuseStructureFromOldProgram
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:117260:61)\n
at createProgram
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:116680:25)\n
at synchronizeHostData
(/home/iuri/.local/share/nvim/mason/packages/angular-language-server/node_modules/typescript/lib/tsserverlibrary.js:139809:17)\n\nNode.js
v18.18.2\n"

Not sure if related but buf_state.refs https://github.com/neovim/neovim/blob/master/runtime/lua/vim/lsp/_changetracking.lua#L204 may be 0 while there's still clients attached to the loaded buffer, which causes buf_state[bufnr] to be nil on https://github.com/neovim/neovim/blob/master/runtime/lua/vim/lsp/_changetracking.lua#L174.


attempt to get length of a nil value when compute_diff received

  curr_lines = { "",
    [351] = "      @if (uploadingVideo) {",
    [352] = "",
    [353] = "      )}"
  },
  firstline = 353,
  prev_lines = { "",
    [351] = "      @if (uploadingVideo) {",
    [352] = ""
  }
}

note that prev_lines[firstline] is nil. (forgot to log the remaining values...)

@XeroOl
Copy link
Author

XeroOl commented Mar 31, 2024

Looks like this may be a duplicate of #16259

@XeroOl
Copy link
Author

XeroOl commented Mar 31, 2024

This line seems to be describing the issue, but I can still reproduce the bug.

https://github.com/neovim/neovim/blob/master/runtime/lua/vim/lsp/_changetracking.lua#L101

Perhaps this fixes it internally, but doesn't share with the language servers?
Perhaps this line was added recently and my version of Neovim isn't up to date?
At least I have some possibilities to investigate.

@sudo-tee

This comment was marked as off-topic.

@XeroOl
Copy link
Author

XeroOl commented Apr 11, 2024

That sounds like a very different issue.
This issue is specifically about the incremental textDocument/didChange sync, and your tsserver log isn't complaining about a malformed textDocument/didChange notification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior lsp
Projects
None yet
Development

No branches or pull requests

5 participants