bug: fallback lsp range format modifies the wrong buffer

[rtgiskard]

Neovim version (nvim -v)

neovim: 0.9.5
conform: v5.1, commit: ad2b5ec

Operating system/version

archlinux latest

What is the severity of this bug?

breaking (some functionality is broken)

Steps To Reproduce

Provide two files:

a.cc: maybe valid cpp source is enough, formatted or not

#define A_CC

#include <iostream>
using namespace std;

int main() { cout << "Hello World!"; }

b.cc: lsp format will change the content

#define B_CC

#include <iostream>
using namespace std;



int main() {
  cout << "Hello World!";
  return 0;
}   

Configuration:

1. clangd lsp setup for cpp
2. conform format with fallback to lsp for the filetype: cpp

Operations

1. nvim a.cc
2. apply conform format (run at least once)
3. load b.cc with :e b.cc in nvim
4. apply conform format (this will trigger the bug)

bug behavior base on the above operation:

For the conform format operation on b.cc:
the first one may finish success, the latter format will raise Error without running the format callback:

Error executing vim.schedule lua callback: /usr/share/nvim/runtime/lua/vim/lsp/util.lua:501: Invalid 'start_col': out of range
stack traceback:
	[C]: in function 'nvim_buf_set_text'
	/usr/share/nvim/runtime/lua/vim/lsp/util.lua:501: in function 'apply_text_edits'
	.../share/nvim/lazy/conform.nvim/lua/conform/lsp_format.lua:24: in function 'apply_text_edits'
	.../share/nvim/lazy/conform.nvim/lua/conform/lsp_format.lua:103: in function 'handler'
	/usr/share/nvim/runtime/lua/vim/lsp.lua:1393: in function ''
	vim/_editor.lua: in function <vim/_editor.lua:0>

Once b.cc get formatted, a.cc get will be modified with extra empty(or maybe) lines. and b.cc will not be changed.

Additional context

You may use the lazyNvim: dev branch to apply the test, use key shot for conform format operation. the format() wrapper is located in init/utils.lua:format().

[stevearc]

I found one way this could happen, but it's a race condition and the repro steps you're using don't look like they would trigger it. Worth giving it a shot anyway to see if this fixes it.

Could you set log_level = vim.log.levels.DEBUG and include the output of the log output from the repro steps? That should help provide more context for what's happening.

Also, the error is just about the format changes being out of range. What makes you so certain that it's because the formatting is applied to the wrong buffer, as opposed to some other bug related to generating the text changes?

[rtgiskard]

I found one way this could happen, but it's a race condition and the repro steps you're using don't look like they would trigger it. Worth giving it a shot anyway to see if this fixes it.

Could you set log_level = vim.log.levels.DEBUG and include the output of the log output from the repro steps? That should help provide more context for what's happening.

Also, the error is just about the format changes being out of range. What makes you so certain that it's because the formatting is applied to the wrong buffer, as opposed to some other bug related to generating the text changes?

Not seems to be a race condition for the issue, the log with DEBUG show three entry of LSP format both on a.cc:

# vi a.cc; <C-f> to format on current buffer (a.cc)
13:07:02[DEBUG] Running LSP formatter on /mnt/tmpfs/a.cc
# :e b.cc; <C-f> to format on current buffer (b.cc)
13:07:11[DEBUG] Running LSP formatter on /mnt/tmpfs/a.cc
13:07:11[DEBUG] Running LSP formatter on /mnt/tmpfs/a.cc
# :q: prompt for modification of a.cc, there be extra empty line on the tail of  a.cc

The comments in the log is extra info add to show the operation what I have done at the time.

And, updated the bug behavior base on the above operation:, sorry for missing such important part.

[rtgiskard]

Well, seems I'm making a mistake in the format() wrapper by passing a table from module directly to different format() operations:

function M.format()
	local format_args = require('init.options').plugins.format_args

	local have_fmt, fmt_util = pcall(require, 'conform')
	if have_fmt then
		-- get current formatter names
		local formatters = fmt_util.list_formatters()
		local fmt_names = {}

		if not vim.tbl_isempty(formatters) then
			fmt_names = vim.tbl_map(function(f)
				return f.name
			end, formatters)
		elseif fmt_util.will_fallback_lsp(format_args) then
			fmt_names = { 'lsp' }
		else
			return
		end

		-- notify with noice progress api
		local noice_progress = require('noice.lsp.progress')

		local fmt_info = 'fmt: ' .. table.concat(fmt_names, '/')
		local msg_id = noice_progress.progress_msg(fmt_info)

		-- format with callback, and notify on err
		fmt_util.format(format_args, function(err)
			noice_progress.progress_msg_end(msg_id)
			if err then
				vim.notify(err, vim.log.levels.WARN, { title = fmt_info })
			end
		end)
	else
		vim.lsp.buf.format(format_args)
	end
end

The fmt_util.will_fallback_lsp(format_args) check will insert bufnr into the table,
and the modified table is always passed to next format operation which may be applied on different buffer.

The original format_args looks like this:

{
  async = true,
  lsp_fallback = true,
  quiet = true,
  timeout_ms = 2000
}

To fix that, make format_args a fresh deep copy on every new format():

format_args = vim.deepcopy(format_args)