null-ls.nvim
null-ls.nvim copied to clipboard
hoverProvider Should be false when no hover source registered
FAQ
- [X] I have checked the FAQ and it didn't resolve my problem.
Issues
- [X] I have checked existing issues and there are no issues with the same problem.
Neovim Version
NVIM v0.8.0-dev Build type: Release LuaJIT 2.1.0-beta3 Compiled by ray@ray-xps-arch Features: +acl +iconv +tui See ":help feature-compile" system vimrc file: "$VIM/sysinit.vim" fall-back for $VIM: "/usr/local/share/nvim" Run :checkhealth for more info
Operating System
archlinux
Minimal config
https://raw.githubusercontent.com/jose-elias-alvarez/null-ls.nvim/main/scripts/minimal_init.lua
Steps to reproduce
query lsp capacity with buf_get_clients
Expected behavior
hoverProvider should be false if no hover source is added
Actual behavior
it returns true, but textDocument/hover request failed
Debug log
[WARN Wed 08 Jun 2022 08:30:15] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 01:07:33] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 16:26:53] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 16:26:54] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 16:32:39] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 16:32:40] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 16:32:41] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1 [WARN Thu 09 Jun 2022 16:32:42] .../pack/packer/opt/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:64: failed to decode json: Expected value but found invalid token at character 1
Help
No
Implementation help
No response
Requirements
- [X] I have read and followed the instructions above and understand that my issue will be closed if I did not provide the required information.
First: I believe your logs are not coming from the minimal config, since with that config null-ls doesn't run at all. Could you try deleting the log file and running again to see if anything shows up?
Second: how are you querying the client's capabilities? If you are looking at the raw capabilities table, hoverProvider will be true for the reasons I mentioned in #902, but if you use client.supports_method, you'll get the correct results. This code correctly prints false when no hover sources are registered or active for the current buffer and prints true if a source is active:
vim.api.nvim_create_user_command("NullLsSupportsHover", function()
local null_ls_client
for _, client in ipairs(vim.lsp.get_active_clients()) do
if client.name == "null-ls" then
null_ls_client = client
end
end
if not null_ls_client then
return
end
print(null_ls_client.supports_method("textDocument/hover"))
end, {})
I know supports_method. But it is not a standard LSP field listed in lsp spec. Also, I checked the behaviour of other lsp clients, the result does not consistent. From what I understand, if the hoverProvider is true, null-ls should always handle the hover request regardless the source is enabled or not. It is strange that hoverProvider is true, but the hover request asserts.
If you can point to a specific problem this is causing, we can see if there's a way to work around it. For better or worse, null-ls relies on Neovim implementation details to function properly, and so LSP compliance is not always possible.
In my setup, I will check if hover is provided from LSP. If it is enabled, CursorHold autocommand hover will send. But if only null-ls is enabled in current buffer, e.g. markdown, I will overwhelm by error messages.
Currently, I have to by-pass null-ls.
The lsp should provide one source of truth for its capacities.
I understand the issue from the point of view from correctness, but in this case can't you just use supports_method? Standard LSP clients will respond as usual and null-ls will respond correctly.
because the supports_method is not standard lsp field, not all lsp provide this. I check a few of them and they just always return false.
I don't think that's right - you can see from the default implementation of supports_method that it defaults to checking the server's capabilities, so it should behave the same way (and if it doesn't, that's potentially an issue with Neovim itself).
e.g. If I send supports_method('textDocument/symbol') to copilot, null-ls and sumneko_lua, The result is true, false, true. But in fact, only sunmeko support this.
I don't know what copilot is, but if you find an actual language server that doesn't respond correctly to supports_method, it's a pretty serious issue, either with the language server's capability declaration or with Neovim itself.
Since supports_method is what Neovim itself uses to query language server capabilities, and since (to my knowledge) null-ls responds correctly when queried via the method, I don't know if there's much else we can do here. We'll be able to set capabilities accurately once Neovim supports dynamic capability registration, but until then, I think supports_method is an acceptable workaround.
But copilot return documentSymbolProvider false which is expected. Most of us are following spec to develop init.lua setup. But it is really annoying some plugin following one standard but some are following the other. Also, by default, null-ls does not provide hover, it is confusing to return hoverProvider true by default. Also even with directory enabled, it may still fail if the curl is not installed.
I think we're going in circles here. I see that copilot.vim seems to have added some type of LSP support, but if the client is not returning a correct response to supports_method, that's an issue, because Neovim relies on this information to determine which clients can and can't respond to a given request.
From what you are telling me, your use case is that you want to check if a server supports a given method before running code. That case is handled by supports_method, and if a client returns an inaccurate result, that's a bug. Once Neovim supports dynamic capability registration, I will make sure null-ls correctly updates its capabilities, but to be clear, supports_method already provides a solution.
Edit: You can see on this line that the default implementation of supports_method returns true if Neovim doesn't know about the method. If you run :lua print(vim.inspect(vim.lsp._request_name_to_capability)) you will see that Neovim is not aware of textDocument/symbol and so returns true (the null-ls implementation is more conservative). This issue describes what's happening and the rationale for returning true in this situation.
This means the textDocument/symbol situation is an edge case, though it has a few potential workarounds (for example, you could add an entry to the vim.lsp._request_name_to_capability). Once Neovim supports dynamic registration, we can make sure that the null-ls client's capabilities are updated, making it possible to directly check the capabilities table.
I think that is why I am not happy with supports_method. This logic for supports_methods is more
- if client explicitly support return true
- if client explicitly not support return false
- if client does not know this method (no matter whether it supports or not) return true
So this means there is a chance client does not support the method but still return true to indicate it supports this method.
Also neovim return true because some of the bad requests need pass through, otherwise there is no way to test some of the clients. It may be some LSP existed long before LSP spec come out and their capacity is super set of LSP spec and you can not relay on LSP spec, or the LSP is in beta phase and does not fully implement the spec ATM. In another word, this is a back door so requests can bypass.
Sure, I think your analysis is fair, and I see the value in directly querying the capabilities table. Still, until Neovim supports dynamic registration, I'm not sure there's a great solution right now.
For example: let's say we disable our custom supports_method logic and let request handling be fully determined by the capability table. If I'm in a buffer where both null-ls and another LSP client are attached, and I haven't registered a null-ls hover source, every time I call vim.lsp.buf.hover(), I'll see a message that says No information available (together with the hover results from the LSP client, if it supports it). That's annoying and misleading enough that we'd definitely have users complain about it, and it would happen constantly for every single null-ls user, regardless of their config.
One hacky solution: set client.server_capabilities to a metatable and move the current supports_method logic into the __index metamethod. I'm not in love with this because it couples us even more closely to Neovim implementation details, and it also wouldn't work for 0.7. But this would avoid the issue and is worth considering once 0.8 releases.
Another solution: since it's just a plain Lua table, we could modify client.server_capabilities directly. I'd be hesitant to do this because there is an on-spec solution, and apart from edge cases like textDocument/symbol, I think the current situation is 95% of the way there. We'd also have trouble differentiating between "null-ls has a source registered for this method" and "null-ls can actually run that source for the current buffer".
I'm definitely open to other ideas, but I think it's a complicated situation without a clear solution for now.
I am not keen on hacky solutions. my solution (client side only) is if client.name='null-ls' I will not send hoverProvider (I did not add dictionary source).
On server-side, TBH, I think if dict source is not configured by the user null-ls can still respond to the hover request. The response can either be an empty one or, indicates that no sources are set up and null-ls failed a hover request.
Also might be worthwhile to document that if the dictionary is not configured, the hover request will fail.
The only issue with sending an empty response is that Neovim will sometimes send a message when receiving it, which is the case for hover, specifically. Definitely makes sense to add something to the documentation for now, at least, and I'll see what else I can come up with.
someone open an new issue: https://github.com/neovim/neovim/issues/18939
Closing this as I think the right solution(s) will ultimately require upstream changes.