fix(lsp): directly rename the existing buffers when renaming (#27690)

Problem:
`vim.lsp.util.rename()` deletes the buffers that are affected by
renaming. This has undesireable side effects. For example, when renaming
a directory, all buffers under that directory are deleted and windows
displaying those buffers are closed. Also, buffer options may change
after renaming.

Solution:
Rename the buffers with :saveas.

An alternative approach is to record all the relevant states and restore
it after renaming, but that seems to be more complex. In fact, the older
version was attempting to restore the states but only partially and
incorrectly.

runtime/doc/lsp.txt

@@ -2068,6 +2068,14 @@ preview_location({location}, {opts})         *vim.lsp.util.preview_location()*
 rename({old_fname}, {new_fname}, {opts})               *vim.lsp.util.rename()*
     Rename old_fname to new_fname
 
+    Existing buffers are renamed as well, while maintaining their bufnr.
+
+    It deletes existing buffers that conflict with the renamed file name only
+    when
+    • `opts` requests overwriting; or
+    • the conflicting buffers are not loaded, so that deleting thme does not
+      result in data loss.
+
     Parameters: ~
       • {old_fname}  (`string`)
       • {new_fname}  (`string`)

runtime/lua/vim/lsp/util.lua

@@ -675,12 +675,23 @@ local function get_bufs_with_prefix(prefix)
   return buffers
 end
 
+local function escape_gsub_repl(s)
+  return (s:gsub('%%', '%%%%'))
+end
+
 --- @class vim.lsp.util.rename.Opts
 --- @inlinedoc
 --- @field overwrite? boolean
 --- @field ignoreIfExists? boolean
 
 --- Rename old_fname to new_fname
+---
+--- Existing buffers are renamed as well, while maintaining their bufnr.
+---
+--- It deletes existing buffers that conflict with the renamed file name only when
+--- * `opts` requests overwriting; or
+--- * the conflicting buffers are not loaded, so that deleting thme does not result in data loss.
+---
 --- @param old_fname string
 --- @param new_fname string
 --- @param opts? vim.lsp.util.rename.Opts Options:
@@ -700,24 +711,36 @@ function M.rename(old_fname, new_fname, opts)
     return
   end
 
-  local oldbufs = {}
+  local buf_rename = {} ---@type table<integer, {from: string, to: string}>
-  local win = nil
+  local old_fname_pat = '^' .. vim.pesc(old_fname_full)
-
+  for b in
-  if vim.fn.isdirectory(old_fname_full) == 1 then
+    vim.iter(get_bufs_with_prefix(old_fname_full)):filter(function(b)
-    oldbufs = get_bufs_with_prefix(old_fname_full)
+      -- No need to care about unloaded or nofile buffers. Also :saveas won't work for them.
-  else
+      return api.nvim_buf_is_loaded(b)
-    local oldbuf = vim.fn.bufadd(old_fname_full)
+        and not vim.list_contains({ 'nofile', 'nowrite' }, vim.bo[b].buftype)
-    table.insert(oldbufs, oldbuf)
+    end)
-    win = vim.fn.win_findbuf(oldbuf)[1]
+  do
-  end
+    -- Renaming a buffer may conflict with another buffer that happens to have the same name. In
-
+    -- most cases, this would have been already detected by the file conflict check above, but the
-  for _, b in ipairs(oldbufs) do
+    -- conflicting buffer may not be associated with a file. For example, 'buftype' can be "nofile"
-    -- There may be pending changes in the buffer
+    -- or "nowrite", or the buffer can be a normal buffer but has not been written to the file yet.
-    if api.nvim_buf_is_loaded(b) then
+    -- Renaming should fail in such cases to avoid losing the contents of the conflicting buffer.
-      api.nvim_buf_call(b, function()
+    local old_bname = vim.api.nvim_buf_get_name(b)
-        vim.cmd('update!')
+    local new_bname = old_bname:gsub(old_fname_pat, escape_gsub_repl(new_fname))
-      end)
+    if vim.fn.bufexists(new_bname) == 1 then
+      local existing_buf = vim.fn.bufnr(new_bname)
+      if api.nvim_buf_is_loaded(existing_buf) and skip then
+        vim.notify(
+          new_bname .. ' already exists in the buffer list. Skipping rename.',
+          vim.log.levels.ERROR
+        )
+        return
+      end
+      -- no need to preserve if such a buffer is empty
+      api.nvim_buf_delete(existing_buf, {})
     end
+
+    buf_rename[b] = { from = old_bname, to = new_bname }
   end
 
   local newdir = assert(vim.fs.dirname(new_fname))
@@ -733,17 +756,16 @@ function M.rename(old_fname, new_fname, opts)
     os.rename(old_undofile, new_undofile)
   end
 
-  if vim.fn.isdirectory(new_fname) == 0 then
+  for b, rename in pairs(buf_rename) do
-    local newbuf = vim.fn.bufadd(new_fname)
+    -- Rename with :saveas. This does two things:
-    if win then
+    -- * Unset BF_WRITE_MASK, so that users don't get E13 when they do :write.
-      vim.fn.bufload(newbuf)
+    -- * Send didClose and didOpen via textDocument/didSave handler.
-      vim.bo[newbuf].buflisted = true
+    api.nvim_buf_call(b, function()
-      api.nvim_win_set_buf(win, newbuf)
+      vim.cmd('keepalt saveas! ' .. vim.fn.fnameescape(rename.to))
-    end
+    end)
-  end
+    -- Delete the new buffer with the old name created by :saveas. nvim_buf_delete and
-
+    -- :bwipeout are futile because the buffer will be added again somewhere else.
-  for _, b in ipairs(oldbufs) do
+    vim.cmd('bdelete! ' .. vim.fn.bufnr(rename.from))
-    api.nvim_buf_delete(b, {})
   end
 end
 

test/functional/plugin/lsp_spec.lua

@@ -2383,12 +2383,13 @@ describe('LSP', function()
         [[
         local old = select(1, ...)
         local new = select(2, ...)
+        local old_bufnr = vim.fn.bufadd(old)
+        vim.fn.bufload(old_bufnr)
         vim.lsp.util.rename(old, new)
-
+        -- the existing buffer is renamed in-place and its contents is kept
-        -- after rename the target file must have the contents of the source file
+        local new_bufnr = vim.fn.bufadd(new)
-        local bufnr = vim.fn.bufadd(new)
+        vim.fn.bufload(new_bufnr)
-        vim.fn.bufload(new)
+        return (old_bufnr == new_bufnr) and vim.api.nvim_buf_get_lines(new_bufnr, 0, -1, true)
-        return vim.api.nvim_buf_get_lines(bufnr, 0, -1, true)
       ]],
         old,
         new
@@ -2400,87 +2401,6 @@ describe('LSP', function()
       eq(true, exists)
       os.remove(new)
     end)
-    it('Kills old buffer after renaming an existing file', function()
-      local old = tmpname()
-      write_file(old, 'Test content')
-      local new = tmpname()
-      os.remove(new) -- only reserve the name, file must not exist for the test scenario
-      local lines = exec_lua(
-        [[
-        local old = select(1, ...)
-	local oldbufnr = vim.fn.bufadd(old)
-        local new = select(2, ...)
-        vim.lsp.util.rename(old, new)
-	return vim.fn.bufloaded(oldbufnr)
-      ]],
-        old,
-        new
-      )
-      eq(0, lines)
-      os.remove(new)
-    end)
-    it('new buffer remains unlisted and unloaded if the old was not in window before', function()
-      local old = tmpname()
-      write_file(old, 'Test content')
-      local new = tmpname()
-      os.remove(new) -- only reserve the name, file must not exist for the test scenario
-      local actual = exec_lua(
-        [[
-        local old = select(1, ...)
-        local oldbufnr = vim.fn.bufadd(old)
-        local new = select(2, ...)
-        local newbufnr = vim.fn.bufadd(new)
-        vim.lsp.util.rename(old, new)
-        return {
-          buflisted = vim.bo[newbufnr].buflisted,
-          bufloaded = vim.api.nvim_buf_is_loaded(newbufnr)
-        }
-      ]],
-        old,
-        new
-      )
-
-      local expected = {
-        buflisted = false,
-        bufloaded = false,
-      }
-
-      eq(expected, actual)
-
-      os.remove(new)
-    end)
-    it('new buffer is listed and loaded if the old was in window before', function()
-      local old = tmpname()
-      write_file(old, 'Test content')
-      local new = tmpname()
-      os.remove(new) -- only reserve the name, file must not exist for the test scenario
-      local actual = exec_lua(
-        [[
-        local win =  vim.api.nvim_get_current_win()
-        local old = select(1, ...)
-        local oldbufnr = vim.fn.bufadd(old)
-        vim.api.nvim_win_set_buf(win, oldbufnr)
-        local new = select(2, ...)
-        vim.lsp.util.rename(old, new)
-        local newbufnr = vim.fn.bufadd(new)
-        return {
-          buflisted = vim.bo[newbufnr].buflisted,
-          bufloaded = vim.api.nvim_buf_is_loaded(newbufnr)
-        }
-      ]],
-        old,
-        new
-      )
-
-      local expected = {
-        buflisted = true,
-        bufloaded = true,
-      }
-
-      eq(expected, actual)
-
-      os.remove(new)
-    end)
     it('Can rename a directory', function()
       -- only reserve the name, file must not exist for the test scenario
       local old_dir = tmpname()
@@ -2497,21 +2417,25 @@ describe('LSP', function()
         [[
         local old_dir = select(1, ...)
         local new_dir = select(2, ...)
-	local pathsep = select(3, ...)
+        local pathsep = select(3, ...)
-	local oldbufnr = vim.fn.bufadd(old_dir .. pathsep .. 'file')
+        local file = select(4, ...)
-
+        local old_bufnr = vim.fn.bufadd(old_dir .. pathsep .. file)
+        vim.fn.bufload(old_bufnr)
         vim.lsp.util.rename(old_dir, new_dir)
-	return vim.fn.bufloaded(oldbufnr)
+        -- the existing buffer is renamed in-place and its contents is kept
+        local new_bufnr = vim.fn.bufadd(new_dir .. pathsep .. file)
+        vim.fn.bufload(new_bufnr)
+        return (old_bufnr == new_bufnr) and vim.api.nvim_buf_get_lines(new_bufnr, 0, -1, true)
       ]],
         old_dir,
         new_dir,
-        pathsep
+        pathsep,
+        file
       )
-      eq(0, lines)
+      eq({ 'Test content' }, lines)
       eq(false, exec_lua('return vim.uv.fs_stat(...) ~= nil', old_dir))
       eq(true, exec_lua('return vim.uv.fs_stat(...) ~= nil', new_dir))
       eq(true, exec_lua('return vim.uv.fs_stat(...) ~= nil', new_dir .. pathsep .. file))
-      eq('Test content', read_file(new_dir .. pathsep .. file))
 
       os.remove(new_dir)
     end)
@@ -2609,6 +2533,11 @@ describe('LSP', function()
         vim.cmd.write()
         local undotree = vim.fn.undotree()
         vim.lsp.util.rename(old, new)
+        -- Renaming uses :saveas, which updates the "last write" information.
+        -- Other than that, the undotree should remain the same.
+        undotree.save_cur = undotree.save_cur + 1
+        undotree.save_last = undotree.save_last + 1
+        undotree.entries[1].save = undotree.entries[1].save + 1
         return vim.deep_equal(undotree, vim.fn.undotree())
       ]],
         old,
@@ -2645,6 +2574,31 @@ describe('LSP', function()
       eq(true, exec_lua('return vim.uv.fs_stat(...) ~= nil', new))
       eq(true, undo_kept)
     end)
+    it('Does not rename file when it conflicts with a buffer without file', function()
+      local old = tmpname()
+      write_file(old, 'Old File')
+      local new = tmpname()
+      os.remove(new)
+
+      local lines = exec_lua(
+        [[
+          local old = select(1, ...)
+          local new = select(2, ...)
+          local old_buf = vim.fn.bufadd(old)
+          vim.fn.bufload(old_buf)
+          local conflict_buf = vim.api.nvim_create_buf(true, false)
+          vim.api.nvim_buf_set_name(conflict_buf, new)
+          vim.api.nvim_buf_set_lines(conflict_buf, 0, -1, true, {'conflict'})
+          vim.api.nvim_win_set_buf(0, conflict_buf)
+          vim.lsp.util.rename(old, new)
+          return vim.api.nvim_buf_get_lines(conflict_buf, 0, -1, true)
+        ]],
+        old,
+        new
+      )
+      eq({ 'conflict' }, lines)
+      eq('Old File', read_file(old))
+    end)
     it('Does override target if overwrite is true', function()
       local old = tmpname()
       write_file(old, 'Old file')