Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
solson opened this issue Nov 5, 2023 · 6 comments
Open

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

solson opened this issue Nov 5, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@solson
Copy link

solson commented Nov 5, 2023

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 4522d7e

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 solson added the bug Something isn't working label Nov 5, 2023
@solson
Copy link
Author

solson commented Nov 5, 2023

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
Copy link
Author

solson commented Nov 5, 2023

(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
Copy link
Author

solson commented Nov 6, 2023

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
Copy link
Contributor

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
Copy link
Contributor

I created a PR #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
Copy link
Contributor

marcelarie commented Jun 3, 2024

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:

Screen.Recording.2024-06-03.at.14.25.38.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants