nvim-telescope / telescope.nvim

Find, Filter, Preview, Pick. All lua, all the time.
MIT License
15.06k stars 816 forks source link

`lsp_definitions` doesn't work for builtins and deps in Deno LSP #2768

Open solson opened 9 months ago

solson commented 9 months ago

Description

Using require("telescope.builtin").lsp_definitions() fails even when vim.lsp.buf.definition() works in the Deno LSP for a dependency or builtin (like console.log). See recordings in expected/actual sections.

Neovim version

NVIM v0.9.4
Build type: Release
LuaJIT 2.1.1693350652

Operating system and version

nixos-unstable (Linux 6.1.60)

Telescope version / branch / rev

telescope 4522d7e3ea75ffddabdc39957168a8a7060b5df0

checkhealth telescope

telescope: require("telescope.health").check()

Checking for required plugins ~
- OK plenary installed.
- OK nvim-treesitter installed.

Checking external dependencies ~
- OK rg: found ripgrep 13.0.0
- OK fd: found fd 8.7.1

===== Installed extensions ===== ~

Telescope Extension: `frecency` ~
- OK sqlite.lua installed.
- OK nvim-web-devicons installed.

Telescope Extension: `smart_history` ~
- No healthcheck provided

Steps to reproduce

  1. Install Deno (which includes the LSP).
  2. Make a scratch directory with minimal.lua and the following.
  3. touch deno.json (so the LSP sees this as a project root).
  4. echo 'console.log("hello world")' > foo.ts
  5. nvim -nu minimal.lua
  6. :e foo.ts
  7. :Telescope lsp_definitions (on console or log).

Alternately, to see the successful outcome, replace step 7 with :lua vim.lsp.buf.definition(), but not during the same execution because vim.lsp.buf.definition and lsp_definitions change each other's outcomes if run in succession in the same Neovim session.

Expected behavior

When I use vim.lsp.buf.definition() for a dependency or builtin (like console.log), it works correctly: asciinema

Actual behavior

But when I use require("telescope.builtin").lsp_definitions(), it fails: asciinema

Error executing vim.schedule lua callback: ...-unwrapped-0.9.4/share/nvim/runtime/lua/vim/lsp/util.lua:1157: Cursor position outside buffer                                                                                                                       stack traceback:                                                                                                                         [C]: in function 'nvim_win_set_cursor'                                                                                           ...-unwrapped-0.9.4/share/nvim/runtime/lua/vim/lsp/util.lua:1157: in function 'jump_to_location'                                 ...nvim/lazy/telescope.nvim/lua/telescope/builtin/__lsp.lua:208: in function 'handler'                                           ...eovim-unwrapped-0.9.4/share/nvim/runtime/lua/vim/lsp.lua:1393: in function ''                                                 vim/_editor.lua: in function <vim/_editor.lua:0>                                                                         

Minimal config

vim.cmd [[set runtimepath=$VIMRUNTIME]]
vim.cmd [[set packpath=/tmp/nvim/site]]
local package_root = '/tmp/nvim/site/pack'
local install_path = package_root .. '/packer/start/packer.nvim'
local function load_plugins()
  require('packer').startup {
    {
      'wbthomason/packer.nvim',
      {
        'nvim-telescope/telescope.nvim',
        requires = {
          'nvim-lua/plenary.nvim',
        },
      },
      -- ADD PLUGINS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
      'neovim/nvim-lspconfig',
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. '/plugin/packer_compiled.lua',
      display = { non_interactive = true },
    },
  }
end
_G.load_config = function()
  require('telescope').setup()
  -- ADD INIT.LUA SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
  require('lspconfig').denols.setup({})
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing Telescope and dependencies.")
  vim.fn.system { 'git', 'clone', '--depth=1', 'https://github.com/wbthomason/packer.nvim', install_path }
end
load_plugins()
require('packer').sync()
vim.cmd [[autocmd User PackerComplete ++once echo "Ready!" | lua load_config()]]
solson commented 9 months ago

I've done some investigation and I believe I know the cause of the issue as well. Note that in every case, the file being loaded has a "virtual" name like deno:/asset/lib.deno.shared_globals.d.ts or slightly different for dependencies.

Following the definition of vim.lsp.buf.definition():

function M.definition(options)
  local params = util.make_position_params()
  request_with_options('textDocument/definition', params, options)
end

local function request_with_options(name, params, options)
  local req_handler
  if options then
    req_handler = function(err, result, ctx, config)
      local client = vim.lsp.get_client_by_id(ctx.client_id)
      local handler = client.handlers[name] or vim.lsp.handlers[name]
      handler(err, result, ctx, vim.tbl_extend('force', config or {}, options))
    end
  end
  request(name, params, req_handler)
end

I noticed that it calls client.handlers[name] and sure enough, lspconfig's denols settings defines relevant handlers:

https://github.com/neovim/nvim-lspconfig/blob/b44737605807023d32e6310b87ba69f4dbf10e0e/lua/lspconfig/server_configurations/denols.lua#L91-L92

Then this calls into here:

https://github.com/neovim/nvim-lspconfig/blob/b44737605807023d32e6310b87ba69f4dbf10e0e/lua/lspconfig/server_configurations/denols.lua#L36-L62

Notably, this checks for the deno: prefix like in our original example of deno:/asset/lib.deno.shared_globals.d.ts, and it handles it using an LSP request to deno/virtualTextDocument.

I don't think telescope's lsp_definitions can work like vim.lsp.buf.definition in general unless it routes through these server-specific handlers.

solson commented 9 months ago

(Worth noting that this can also affect lsp_type_definitions and the others, as well as not being Deno-specific, if any other LSP uses handlers like this, but this is the case I ran into and knew how to make a minimal repro for.)

solson commented 9 months ago

It is probably interesting to note that trouble.nvim appears to have the same kind of bug (not invoking client-specific LSP handlers, and thus failing to handle Deno in the same way), so I opened a discussion.

jamestrew commented 9 months ago

Thanks for the report and investigative findings. Really helps a lot.

I think I can put together a fix for this. Tricky this is that client handlers can do arbitrary things and it looks like the denols handler in lspconfig calls the base neovim lsp handlers, which already handles the jumping to definition or opening a quickfix list. Obviously we want telescope to open instead of a quickfix list.

Luckily there's a on_list config option we can pass to the these handlers whereby we can pass function to collect the lsp response. I think telescope can leverage this so I'll try this.

jamestrew commented 9 months ago

I created a PR https://github.com/nvim-telescope/telescope.nvim/pull/2770 In my limited testing, this seems to fix these kinds of issues for the definition, references, typeDefinition and implementation pickers. I think the rest of the lsp based pickers will also need a similar treatment but if you can give this PR a try it'll be great. I'd like to get @Conni2461 to take a look as well.

marcelarie commented 2 months ago

This is still broken.

I don't know if it is of any help, but after running vim.lsp.buf.references once, the telescope built-in alternative works without preview. This might happen with definitions too.

Screen recording:

https://github.com/nvim-telescope/telescope.nvim/assets/62728887/56469a87-01fb-404c-98b9-e6fec06c56bc