fix: autostart=false: attach when editing new, nonexistent file

[Diomendius]

Problem

Currently, nvim-lspconfig tries to attach servers automatically via two autocommands:

1. Created if config.autostart == true and triggered by FileType, if config.filetypes is set, else BufReadPost. Calls try_add().
2. Created for each workspace root, triggered by BufReadPost matching paths starting with the root. Calls try_add_wrapper().

BufReadPost does not fire when creating a buffer for a file that doesn't exist. This means that if config.autostart == true and config.filetypes is set and includes the detected filetype for the buffer, the server is attached automatically regardless of whether the file exists, but in all other cases the server is only attached for existing files.

Solution

1. Where these autocommands trigger on BufReadPost, also trigger on BufNewFile.
2. Wrap the autocommand callbacks in vim.schedule() to ensure filetype is set first, as the BufReadPost/BufNewFile autocommands will trigger before FileType if nvim-lspconfig is set up early enough during Nvim init (see startup: precedence of :syntax/:filetype autocmds neovim#7367 and fix: autostart=false: attach when editing new, nonexistent file #2712 (comment)).

I did consider including a test with this PR, but there doesn't seem to be any existing test infrastructure for tests involving actually running a language server (or a mock of one).

Fixes #2711

[glepnir]

BufNewFile run before FileType

[Diomendius]

au FileType * echo 'FileType ' . &ft
au BufNewFile * echo 'BufNewFile ' . &ft
au BufReadPost * echo 'BufReadPost ' . &ft

Using the above for testing, both BufNewFile and BufReadPost run before FileType. Weirdly, when I run these commands with my usual config loaded, FileType actually executes first. I have no idea what part of my config changes the order.

If there's any issue with this callback triggering before FileType, it affects BufReadPost as well. I'm not sure if it matters here, since BufReadPost and BufNewFile are only used if config.filetypes is unset. Does it make sense for a server to be configured without config.filetypes if it matters what filetype is when try_add() runs?

At line 139 it's definitely important that try_add_wrapper() runs after filetype is set, however.

vim.schedule() seems to be enough to defer a function until after FileType, but this seems like a hacky solution to me.

[Diomendius]

With the following in test.lua:

vim.api.nvim_create_autocmd({ "BufReadPost", "FileType", "BufNewFile" }, {
    callback = function(e)
        print(vim.o.ft, vim.inspect(e))
    end,
})

If I run XDG_DATA_HOME=xdg_data XDG_STATE_HOME=xdg_state nvim -u test.lua, BufReadPost/BufNewFile triggers first.
If I run XDG_DATA_HOME=xdg_data XDG_STATE_HOME=xdg_state nvim -u NORC +source\ test.lua, FileType triggers first.
If I wrap the entire contents of test.lua in vim.schedule(), FileType always triggers first regardless of whether I :source test.lua or load it as the rc file with -u.

BufReadPost and BufNewFile autocommands only seem to trigger earlier than FileType if they were set up sufficiently early during startup. This seems weird to me, but I can reproduce it consistently. I don't know what the intended execution order of these events is, or if there is even meant to be a guaranteed order at all.

[Diomendius]

I can't think of a better solution than vim.schedule(), but it seems to work fine in testing and in normal use.

It'd be nice to have an explicit guarantee that the callback will run only after filetype detection. The other idea I had was to create a one-shot FileType autocommand when triggering on BufNewFile, but this just seems generally worse and would break if FileType fires before BufNewFile, which does happen sometimes, seemingly depending on user configuration.

I looked at the Nvim source to try to confirm that vim.schedule() callbacks couldn't fire between BufNewFile and FileType, but I couldn't make much sense of it. It should be enough to confirm that every code path that can trigger BufNewFile also triggers (or has already triggered) filetype detection before the event loop runs. Intuitively, I would expect this to be the case, and it appears to be in practice.

[justinmk]

this seems fine, can you rebase it. also would help to have a clear Problem/Solution summary.

[Diomendius]

I've rebased this and expanded the OP to better describe this PR.

It looks like neovim/neovim#7367 already describes the issue with FileType not having a consistent ordering relative to BufReadPost/BufNewFile.

[justinmk]

Had to revert: #3347

But, if you can find a solution then we can try again.

[Diomendius]

That's a shame. @glepnir said something about this causing an invalid buffer ID, but it's not obvious to me what would trigger that or in what context.

If someone could give instructions to reproduce, I'd appreciate it.

[justinmk]

@BrunoKrugel fb453a1#commitcomment-147560628

[Diomendius]

My working hypothesis is that the buffer could be wiped out after schedule() but before the next event loop. It's a bit pointless, but there's nothing stopping someone from creating an autocommand to call :bwipeout on an event like BufEnter for certain buffers, which would probably cause this.

The real-world cause might not be so contrived, but assuming the buffer ID is still valid when the scheduled callback runs is clearly wrong. Thankfully, the solution is probably just to do nothing in the callback if the buffer isn't valid; a buffer that doesn't exist obviously can't have a language server attach to it, so nothing needs to be done.

[AlexMasterov]

If someone could give instructions to reproduce, I'd appreciate it.

Repro for my case:

-- fb453a1.lua
vim.env.LAZY_STDPATH = '.repro'
load(vim.fn.system('curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua'))()

require('lazy.minit').repro({
  spec = {
    {
      'neovim/nvim-lspconfig',
      commit = 'fb453a1', -- https://github.com/neovim/nvim-lspconfig/pull/2712
      config = function()
        local lspconfig = require 'lspconfig'

        lspconfig.lua_ls.setup({
          autostart = true,
        })
      end
    }
  },
})

Steps To Reproduce (NVIM v0.10.2)

1. nvim -u fb453a1.lua
2. edit fb453a1.lua
3. Use default maps for built-in commenting like gcc

Error executing vim.schedule lua callback: .../nvim-data/lazy/nvim-lspconfig/lua/lspconfig/manager.lua:247: Invalid buffer id: 3
stack traceback:
        [C]: in function '__index'
        .../nvim-data/lazy/nvim-lspconfig/lua/lspconfig/manager.lua:247: in function 'try_add'
        .../nvim-data/lazy/nvim-lspconfig/lua/lspconfig/configs.lua:111: in function <.../nvim-data/lazy/nvim-lspconfig/lua/lspconfig/configs.lua:110>

[Diomendius]

Thanks for that, @AlexMasterov.

Details of cause

So, this bug only occurs when config.filetypes is set and includes the relevant filetype. For lua_ls, this is set by default but can be disabled with filetypes = false, which prevents the bug from occurring.

It happens when vim.api.nvim_get_option_value(opt, { filetype = ft }) is called where ft is one of the filetypes of a server configured in nvim-lspconfig.

When using gcc, this gets run here

local cur_cs = vim.filetype.get_option(ft, 'commentstring')

vim.filetype.get_option() calls vim.api.nvim_get_option_value() here:

ft_option_cache[filetype][option] = api.nvim_get_option_value(option, { filetype = filetype })

That then calls do_ft_buf() here, which creates an unlisted buffer, sets its filetype and runs ftplugins and FileType autocommands for that filetype. The buffer is then wiped out here:

buf_T *ftbuf = do_ft_buf(filetype, &aco, err);
-- … --
wipe_buffer(ftbuf, false);

So it wasn't exactly a plugin triggering this, but rather a side effect of a function in Neovim proper (see :h nvim_get_option_value()). That :bwipeout autocommand I suggested would probably also trigger this, but someone else can test that if they're bored enough.

The obvious solution still seems to be to bail early if the buffer isn't valid. I've pushed a fix and opened #3355.