fix(lsp): inlay hints: "Failed to delete autocmd" when closing buffer #24469

Problem:
"Failed to delete autocmd" error when deleting LspNotify autocmd. #24456

Solution:
Change a few things in the inlay_hint and diagnostic LSP code:
1. Re-introduce the `enabled` flag for the buffer state tables. Previously I was
   relying on the presence of an autocmd id in the state table to track whether
   inlay_hint / diagnostic was enabled for a buffer. There are two reasons why
   this doesn't work well:
  - Each time inlay_hint / diagnostic is enabled, we call `nvim_buf_attach` on
    the buffer, resulting in multiple `on_reload` or `on_detach` callbacks being
    registered.
  - Commands like `bwipeout` delete buffer local autocmds, sometimes before our
    `on_detach` callbacks have a chance to delete them first. This causes the
  - Use module local enabled state for diagnostic as well. bwipeout can race
    with on_detach callbacks for deleting autocmds. Error referenced in #24456.
2. Change the `LspDetach` autocmd to run each time (i.e., remove the `once`
   flag). Since we're only registering autocmds once per buffer now, we
   need to make sure that we set the enabled flag properly each time the LSP
   client detaches from the buffer.
  - Remove `once` from the LspDetach autocmds for inlay_hint and diagnostic.
    We only set up the autocmd once now. Gets removed when buffer is deleted.
3. Have the `LspNotify` handler also refresh the inlay_hint / diagnostics when
   receiving the `textDocument/didOpen` event. Before this point, the LSP
   backend doesn't have the contents of the buffer, so can't provide inlay hints
   or diagnostics.

Downsides of this approach:
* When inlay_hint / diagnostics are disabled on a buffer, it will continue to
  receive `LspNotify` events for that buffer. The callback exits early since the
  `enabled` flag is false.

Alternatives:
* Can we wrap the call to `nvim_del_autocmd` in `pcall` to swallow any errors
  resulting from trying to delete the autocmd?

Fixes #24456

Helped-by: Maria José Solano <majosolano99@gmail.com>

2 files changed, 67 insertions, 53 deletions

diff --git a/runtime/lua/vim/lsp/diagnostic.lua b/runtime/lua/vim/lsp/diagnostic.lua
index 34be13096d..44bb90d985 100644
--- a/runtime/lua/vim/lsp/diagnostic.lua
+++ b/runtime/lua/vim/lsp/diagnostic.lua
@@ -392,19 +392,18 @@ local function clear(bufnr)
   end
 end
 
---- autocmd ids for LspNotify handlers per buffer
---- @private
---- @type table<integer,integer>
-local _autocmd_ids = {}
+---@class lsp.diagnostic.bufstate
+---@field enabled boolean Whether inlay hints are enabled for this buffer
+---@type table<integer, lsp.diagnostic.bufstate>
+local bufstates = {}
 
 --- Disable pull diagnostics for a buffer
 --- @private
 local function disable(bufnr)
-  if not _autocmd_ids[bufnr] then
-    return
+  local bufstate = bufstates[bufnr]
+  if bufstate then
+    bufstate.enabled = false
   end
-  api.nvim_del_autocmd(_autocmd_ids[bufnr])
-  _autocmd_ids[bufnr] = nil
   clear(bufnr)
 end
 
@@ -416,38 +415,46 @@ function M._enable(bufnr)
     bufnr = api.nvim_get_current_buf()
   end
 
-  if _autocmd_ids[bufnr] then
-    return
+  if not bufstates[bufnr] then
+    bufstates[bufnr] = { enabled = true }
+
+    api.nvim_create_autocmd('LspNotify', {
+      buffer = bufnr,
+      callback = function(opts)
+        if
+          opts.data.method ~= 'textDocument/didChange'
+          and opts.data.method ~= 'textDocument/didOpen'
+        then
+          return
+        end
+        if bufstates[bufnr] and bufstates[bufnr].enabled then
+          util._refresh('textDocument/diagnostic', { bufnr = bufnr, only_visible = true })
+        end
+      end,
+      group = augroup,
+    })
+
+    api.nvim_buf_attach(bufnr, false, {
+      on_reload = function()
+        if bufstates[bufnr] and bufstates[bufnr].enabled then
+          util._refresh('textDocument/diagnostic', { bufnr = bufnr })
+        end
+      end,
+      on_detach = function()
+        disable(bufnr)
+      end,
+    })
+
+    api.nvim_create_autocmd('LspDetach', {
+      buffer = bufnr,
+      callback = function()
+        disable(bufnr)
+      end,
+      group = augroup,
+    })
+  else
+    bufstates[bufnr].enabled = true
   end
-
-  _autocmd_ids[bufnr] = api.nvim_create_autocmd('LspNotify', {
-    buffer = bufnr,
-    callback = function(opts)
-      if opts.data.method ~= 'textDocument/didChange' then
-        return
-      end
-      util._refresh('textDocument/diagnostic', { bufnr = bufnr, only_visible = true })
-    end,
-    group = augroup,
-  })
-
-  api.nvim_buf_attach(bufnr, false, {
-    on_reload = function()
-      util._refresh('textDocument/diagnostic', { bufnr = bufnr })
-    end,
-    on_detach = function()
-      disable(bufnr)
-    end,
-  })
-
-  api.nvim_create_autocmd('LspDetach', {
-    buffer = bufnr,
-    callback = function()
-      disable(bufnr)
-    end,
-    once = true,
-    group = augroup,
-  })
 end
 
 return M
diff --git a/runtime/lua/vim/lsp/inlay_hint.lua b/runtime/lua/vim/lsp/inlay_hint.lua
index 912ce6898e..2bf0fb9cb2 100644
--- a/runtime/lua/vim/lsp/inlay_hint.lua
+++ b/runtime/lua/vim/lsp/inlay_hint.lua
@@ -3,12 +3,12 @@ local log = require('vim.lsp.log')
 local api = vim.api
 local M = {}
 
----@class lsp._inlay_hint.bufstate
+---@class lsp.inlay_hint.bufstate
 ---@field version integer
 ---@field client_hint table<integer, table<integer, lsp.InlayHint[]>> client_id -> (lnum -> hints)
 ---@field applied table<integer, integer> Last version of hints applied to this line
----@field autocmd_id integer The autocmd id for the buffer
----@type table<integer, lsp._inlay_hint.bufstate>
+---@field enabled boolean Whether inlay hints are enabled for this buffer
+---@type table<integer, lsp.inlay_hint.bufstate>
 local bufstates = {}
 
 local namespace = api.nvim_create_namespace('vim_lsp_inlayhint')
@@ -31,6 +31,9 @@ function M.on_inlayhint(err, result, ctx, _)
     return
   end
   local bufstate = bufstates[bufnr]
+  if not bufstate or not bufstate.enabled then
+    return
+  end
   if not (bufstate.client_hint and bufstate.version) then
     bufstate.client_hint = vim.defaulttable()
     bufstate.version = ctx.version
@@ -122,10 +125,9 @@ local function disable(bufnr)
     bufnr = api.nvim_get_current_buf()
   end
   clear(bufnr)
-  if bufstates[bufnr] and bufstates[bufnr].autocmd_id then
-    api.nvim_del_autocmd(bufstates[bufnr].autocmd_id)
+  if bufstates[bufnr] then
+    bufstates[bufnr] = { enabled = false, applied = {} }
   end
-  bufstates[bufnr] = nil
 end
 
 --- Enable inlay hints for a buffer
@@ -136,24 +138,27 @@ local function enable(bufnr)
   end
   local bufstate = bufstates[bufnr]
   if not bufstate then
-    bufstates[bufnr] = { applied = {} }
-    bufstates[bufnr].autocmd_id = api.nvim_create_autocmd('LspNotify', {
+    bufstates[bufnr] = { applied = {}, enabled = true }
+    api.nvim_create_autocmd('LspNotify', {
       buffer = bufnr,
       callback = function(opts)
-        if opts.data.method ~= 'textDocument/didChange' then
+        if
+          opts.data.method ~= 'textDocument/didChange'
+          and opts.data.method ~= 'textDocument/didOpen'
+        then
           return
         end
-        if bufstates[bufnr] then
+        if bufstates[bufnr] and bufstates[bufnr].enabled then
           util._refresh('textDocument/inlayHint', { bufnr = bufnr })
         end
       end,
       group = augroup,
     })
     util._refresh('textDocument/inlayHint', { bufnr = bufnr })
-    api.nvim_buf_attach(bufnr, true, {
+    api.nvim_buf_attach(bufnr, false, {
       on_reload = function(_, cb_bufnr)
         clear(cb_bufnr)
-        if bufstates[cb_bufnr] then
+        if bufstates[cb_bufnr] and bufstates[cb_bufnr].enabled then
           bufstates[cb_bufnr].applied = {}
           util._refresh('textDocument/inlayHint', { bufnr = cb_bufnr })
         end
@@ -167,9 +172,11 @@ local function enable(bufnr)
       callback = function()
         disable(bufnr)
       end,
-      once = true,
       group = augroup,
     })
+  else
+    bufstate.enabled = true
+    util._refresh('textDocument/inlayHint', { bufnr = bufnr })
   end
 end
 
@@ -180,7 +187,7 @@ local function toggle(bufnr)
     bufnr = api.nvim_get_current_buf()
   end
   local bufstate = bufstates[bufnr]
-  if bufstate then
+  if bufstate and bufstate.enabled then
     disable(bufnr)
   else
     enable(bufnr)