yioneko / vtsls

LSP wrapper for typescript extension of vscode
Other
486 stars 7 forks source link

Error with a Neovim nightly built-in completion #164

Closed kristijanhusak closed 3 months ago

kristijanhusak commented 3 months ago

Hi, I'm testing new Neovim builtin lsp completion (https://neovim.io/doc/user/lsp.html#vim.lsp.completion.enable()) and I'm getting some errors when I confirm the selection. It seems that JSON that is being sent is invalid on labelDetails.

Here are steps to reproduce:

  1. Create minimal_init.lua file with this content:
    
    local tmp_dir = vim.env.TMPDIR or vim.env.TMP or vim.env.TEMP or '/tmp'
    local nvim_root = tmp_dir .. '/nvim_vtsls'
    local lazy_root = nvim_root .. '/lazy'
    local lazypath = lazy_root .. '/lazy.nvim'

-- Install lazy.nvim if not already installed if not vim.loop.fs_stat(lazypath) then vim.fn.system({ 'git', 'clone', '--filter=blob:none', 'https://github.com/folke/lazy.nvim.git', '--branch=stable', -- latest stable release lazypath, }) end vim.opt.rtp:prepend(lazypath)

require('lazy').setup({ { 'neovim/nvim-lspconfig', dependencies = { { 'yioneko/nvim-vtsls' }, { 'williamboman/mason.nvim' }, }, config = function() require('mason').setup({ install_root_dir = nvim_root..'/mason', log_level = vim.log.levels.TRACE }) require('lspconfig').vtsls.setup({ on_attach = function(client, bufnr) vim.lsp.completion.enable(true, client.id, bufnr, { autotrigger = true }) vim.lsp.set_log_level(vim.lsp.log_levels.TRACE) end }) end, }, }, { root = lazy_root, lockfile = nvim_root .. '/lazy.json', install = { missing = true, }, })

require('lazy').sync({ wait = true, show = false, })

2.  Make sure you have latest nightly version of Neovim. I use [bob](https://github.com/MordechaiHadad/bob) to achieve that: `bob use nightly`
3. Create a test typescript file: `echo "const test = { hello: 'world' }" > init.ts`
4. Open up neovim with the minimal init: `nvim -u minimal_init.lua`
5. Once the lazy finishes installing, install vtsls with mason: `:MasonInstall vtsls`
6. Once vtsls is installed, open up the ts file: `:edit init.ts`
7. On new line, try writing `test.`, autocompletion should automatically show
8. Select the only entry in the autocompletion and press `<C-y>` to accept it
9. See the error:
![screenshot_2024_06_05_19_44_00](https://github.com/yioneko/vtsls/assets/1782860/c86dab9b-83bc-4fdb-9b78-4ca80327757e)

I checked why it can't encode the JSON, and it turns out there is a `labelDetails` object where key is `true`: This is what is in lsp log:

```lua
{
    command = {
      arguments = { { cacheId = 3, index = 0, providerId = 2 } },
      command = '_vtsls.completionCacheCommand',
      title = '',
    },
    commitCharacters = { '.', ',', ';', '(' },
    data = { cacheId = 3, index = 0, providerId = 2 },
    filterText = '.hello',
    insertText = '.hello',
    insertTextFormat = 1,
    kind = 5,
    label = 'hello',
    labelDetails = { [true] = 6 },
    sortText = '11',
    textEdit = {
      newText = '.hello',
      range = {
        ['end'] = { character = 5, line = 2 },
        start = { character = 4, line = 2 },
      },
    },
  }

When you try to do vim.json.encode on it, it errors out. Once the labelDetails is removed, it works.

Just to note that I don't have any issues with vtsls with cmp.

Also, I'm not 100% sure if this is issue with vtsls or Microsoft's tsserver, but I confirmed that I don't have this issue with typescript-language-server.

yioneko commented 3 months ago

I could reproduce the error but I suspect this is not a problem on server side. The invalid field labelDetails = { [true] = 6 } is sent during a completionItem/resolve request, while I notice the corresponding item in textDocument/completion response has field labelDetails = vim.empty_dict(), which means that the completion response from server side is valid and deserialized by client correctly. For reference, labelDetails fields could be optional in spec.

It seems that new builtin completion has problem with an empty object as a field of completion item. And I guess typescript-language-server does not have the same problem because it does not set labelDetails field if the client does not support it or the server does not support labelDetails at all.

EDIT: If you'd like to submit an issue to nvim, you can post the following test snippet:


local function trigger()
  local item = {
    word = "a",
    abbr = "a",
    kind = "v",
    user_data = {
      some_field = vim.empty_dict(),
    },
  }
  vim.fn.complete(1, { item })
end

vim.keymap.set("i", "<C-a>", trigger, { nowait = true, noremap = true, silent = false })

vim.api.nvim_create_autocmd("CompleteDone", {
  callback = function()
    local reason = vim.api.nvim_get_vvar("event").reason --- @type string
    if reason == "accept" then
      local completed_item = vim.api.nvim_get_vvar("completed_item")
      print(vim.inspect(completed_item))
    end
  end,
})

---trigger complete by <C-A>, then accpet with <C-Y>, the printed item is:
local printed = {
  abbr = "a",
  info = "",
  kind = "v",
  menu = "",
  user_data = {
    some_field = {
      [true] = 6
    }
  },
  word = "a"
}
kristijanhusak commented 3 months ago

I opened up the issue on Neovim. Is there a way to temporarily disable these labelDetails through the attach function?

yioneko commented 3 months ago

Here is one possible workaround:

vim.api.nvim_create_autocmd("CompleteDonePre", {
  callback = function()
    local item = vim.v.completed_item
    if item and vim.tbl_get(item, "user_data", "nvim", "lsp", "completion_item", "labelDetails") then
      item.user_data.nvim.lsp.completion_item.labelDetails = nil
      vim.v.completed_item = item
    end
  end,
})

You can also try to override vim.lsp.hanlders[vim.lsp.protocol.Methods.textDocument_completion].

yioneko commented 3 months ago

I found that there are still some problems while testing nvim builtin completion, which might be useful to you.

Some completion items are missing: this is the problem on server side, and should be fixed in https://github.com/yioneko/vtsls/commit/f0beac03e90531ea17d37a134d4af83c94132b2c (not released yet).

Manually triggered completion yields many items unrelated to typed prefix:

This is due to that the client only uses filterText to do filtering: https://github.com/neovim/neovim/blob/d8e384b7bfd5829e5ff5006202faa584b3211e84/runtime/lua/vim/lsp/completion.lua#L241-L244

While this is how the spec describe filterText:

A string that should be used when filtering a set of completion items. When omitted the label is used as the filter text for this item.

Currently only a few items from response have filterText set and we expect the client to use label for filtering in such case. You can work around this by manually setting label to filterText in CompleteDonePre event similar to the code I posted in the previous comment.

kristijanhusak commented 3 months ago

Thanks, the workaround works great!

Manually triggered completion yields many items unrelated to typed prefix:

This is due to that the client only uses filterText to do filtering: neovim/neovim@d8e384b/runtime/lua/vim/lsp/completion.lua#L241-L244

While this is how the spec describe filterText:

A string that should be used when filtering a set of completion items. When omitted the label is used as the filter text for this item.

Yeah, I noticed this, but I thought I'm not understanding the LSP spec. I'll open up a separate issue for that, thanks!

If you want we can close this, since the issue is not in here.

yioneko commented 3 months ago

Then let's close this for now. If you found any further problems, feel free to raise another issue.