prabirshrestha / vim-lsp

async language server protocol plugin for vim and neovim
MIT License
3.07k stars 303 forks source link

Bug: Updating buffer (open, reload) creates duplicated server-side content when multiple LSP servers are enabled #1547

Closed rgoya closed 1 week ago

rgoya commented 3 months ago

Summary

When loading or updating the contents of a file (e.g. :e), vim-lsp correctly sends the full content to the file to the first LSP registered server, but erroneously tells subsequent servers that the file has had changes. This causes the subsequent LSP servers to have a warped view of what the file contents are.

In some cases the only effect will be an overloaded list of diagnostics, (e.g. when no code actions are requested). In other cases, like requesting :LspDocumentFormat, the server will return its own warped version of the file, leaving the user with a buffer containing multiple copies of the actual content.

A suggested fix is provided in an accompanying PR (#1548)

Setup

Tell vim-lsp to start both ruff and pyright servers:

let g:lsp_settings_filetype_python = ['pyright-langserver', 'ruff-lsp']

The formatting step in the reproduction below will work only when ruff is assigned the second slot int he server list; however, the duplication of diagnostics should also show up for pyright if that is the one assigned the second slot.

Create a test file that will trigger some diagnostics for both servers. The following has unused imports (ruff fix), extra blank lines (ruff format) and unknown module (pyright):

import pandas

import numpi

Reproduce

The initial opening of the file correctly identifies the issues of the simple file: image

Executing :e on the file once causes ruff to start hallucinating, while pyright is doing well: image

And this continues as we reload the file: image

Asking for :LspDocumentFormat, which is executed by ruff, results in a very different buffer, caused by ruff imposing its perceived reality onto the developer: image

Debug

The offending line can be found in the function s:text_changes(buf, server_name) in the following line: https://github.com/prabirshrestha/vim-lsp/blob/f7ccf006df1aefd327c0e2c55cc8632a2db577c1/autoload/lsp.vim#L752-L754

When the first LSP server is being sent the data s:file_content does not have an entry for the buffer yet, and is correctly sent the full content: https://github.com/prabirshrestha/vim-lsp/blob/f7ccf006df1aefd327c0e2c55cc8632a2db577c1/autoload/lsp.vim#L765-L768

When the second LSP server is being sent the data, the condition is met and the diff is sent. However, the check is not making the distinction of which LSP server is actually being updated, a distinction that should be meade given the expected content of s:file_content: https://github.com/prabirshrestha/vim-lsp/blob/f7ccf006df1aefd327c0e2c55cc8632a2db577c1/autoload/lsp.vim#L9-L20

Fix

The fix seems to be simple, just add a statement to the condition statement to ensure the right server is being evaluated and updated, change: https://github.com/prabirshrestha/vim-lsp/blob/f7ccf006df1aefd327c0e2c55cc8632a2db577c1/autoload/lsp.vim#L752-L754 to:

   if l:sync_kind == 2 && has_key(s:file_content, a:buf) && has_key(s:file_content[a:buf], a:server_name)
prabirshrestha commented 1 week ago

This is now fixed. Thanks for the detailed issue as well as the PR fix.