Add support for `textDocument/implementation`

[Seelengrab]

My LanguageServer.jl version is (I think) LanguageServer v4.3.1.

When running LanguageServer.jl in helix, I encounter the following error quite frequently:

2022-10-14T10:09:46.351 helix_lsp::transport [ERROR] err <- "ERROR: Unknown method textDocument/implementation.\n"
2022-10-14T10:09:46.351 helix_lsp::transport [ERROR] err <- "Stacktrace:\n"
2022-10-14T10:09:46.371 helix_lsp::transport [ERROR] err <- " [1] error(s::String)\n"
2022-10-14T10:09:46.371 helix_lsp::transport [ERROR] err <- "   @ Base ./error.jl:35\n"
2022-10-14T10:09:46.372 helix_lsp::transport [ERROR] err <- " [2] dispatch_msg(x::JSONRPC.JSONRPCEndpoint{Base.PipeEndpoint, Base.PipeEndpoint}, dispatcher::JSONRPC.MsgDispatcher, msg::Dict{String, Any})\n"
2022-10-14T10:09:46.373 helix_lsp::transport [ERROR] err <- "   @ JSONRPC ~/.julia/packages/JSONRPC/P0G1p/src/typed.jl:81\n"
2022-10-14T10:09:46.373 helix_lsp::transport [ERROR] err <- " [3] run(server::LanguageServerInstance)\n"
2022-10-14T10:09:46.373 helix_lsp::transport [ERROR] err <- "   @ LanguageServer ~/.julia/packages/LanguageServer/0vsx2/src/languageserverinstance.jl:382\n"
2022-10-14T10:09:46.373 helix_lsp::transport [ERROR] err <- " [4] runserver(pipe_in::Base.PipeEndpoint, pipe_out::Base.PipeEndpoint, env_path::String, depot_path::String, err_handler::Nothing, symserver_store_path::Nothing)\n"
2022-10-14T10:09:46.373 helix_lsp::transport [ERROR] err <- "   @ LanguageServer ~/.julia/packages/LanguageServer/0vsx2/src/runserver.jl:41\n"
2022-10-14T10:09:46.374 helix_lsp::transport [ERROR] err <- " [5] runserver()\n"
2022-10-14T10:09:46.374 helix_lsp::transport [ERROR] err <- "   @ LanguageServer ~/.julia/packages/LanguageServer/0vsx2/src/runserver.jl:39\n"
2022-10-14T10:09:46.374 helix_lsp::transport [ERROR] err <- " [6] top-level scope\n"
2022-10-14T10:09:46.374 helix_lsp::transport [ERROR] err <- "   @ none:1\n"

after which the language server is unresponsive/crashed. It would be nice either not to crash, allowing me to continue using the language server but just not that specific feature, or to support that feature.

As an aside, which version of LSP is supported? This feature in particular has existed since 3.6.0, which was released in February of 2018..

[davidanthoff]

I believe this is an error in the helix lsp implementation. Our LS reports that it does not have the implementationProvider server capability, and in that case a client should not send this request.

[Seelengrab]

Ok, regardless of that, can this be supported? This is not helix specific after all, any editor could send that. Or, barring that, not crash so I can continue using the remaining functionality without having to go through a full LS.jl-restart cycle? It's quite expensive UX wise to do that and I'd really rather write code than restart the LS.

[davidanthoff]

I would suggest you open a bug report with helix ls on this, I would consider it a bug if it sends a request to a LS that previously specifically indicated that it doesn't support that kind of request. The proper way for an editor to implement this here is to a) check which kind of requests the language server supports, and then b) only send those kinds of requests.

In terms of functionality, I think that just depends on whether someone wants to add this feature :) I think in terms of priorities this is probably pretty low, there are just a lot of other things that we probably should fix first.

[Seelengrab]

Well I did end up asking on matrix and got this as a response why "just check" is difficult:

To be fair, yes helix should also not just send things it doesn't know the server supports, but still, not crashing on faulty client behavior would be nice :) I'm honestly surprised there's no default error communication channel in LSP itself..

I'm also fairly certain that the incantation on the helix side (gi in this case) did work at some point in the past few weeks, so I'm not sure what changed :shrug: My helix version should be the same as back then, not sure if my LS.jl version changed..

Regardless, issue is here.

[davidanthoff]

To be fair, yes helix should also not just send things it doesn't know the server supports, but still, not crashing on faulty client behavior would be nice

That is the same topic we had over at https://github.com/julia-vscode/LanguageServer.jl/issues/1155, we should probably keep that discussion over there.

[fredrikekre]

What should this do in Julia? Return the same as textDocument/definition?

[Seelengrab]

No, these two are subtly different, e.g. in the case of forward declarations like

"""
    foobar

MyDoc
"""
function foobar end

foobar(a::Int) = ...
foobar(a::Float64) = ...

I'd expect textDocument/definition to jump to the forward declaration, while textDocument/implementation jumps to either the Int method or the Float64 method, depending on which method would be called (if that can be determined statically - else the forward declaration again. Or a method selector, if that's possible to communicate to the client within LSP).

[fredrikekre]

Yes I understand why the protocol have two different methods here, but don't really see a point of the distinction in Julia.

[Seelengrab]

Well, an interface package like Tables.jl can have the (sometimes) only docstring, which can be jumped to with textDocument/definition, while a package implementing that interface then has textDocument/implementation. Seems more convenient than having to manually jump/look it up when e.g. a docstring has to be changed. I don't see why julia should behave differently here, especially when it can be very interesting to know where an interface is defined.

Maybe this distinction makes more sense if/when julia provides discoverable API surface of functions programmatically & in a semantically well-defined way, but I certainly wouldn't say that there's no point to the distinction in julia at all.

I'm aware though that this is a bit difficult, since the source code is not necessarily a static thing - especially the "jumping to different methods" part requires (at least in some cases) type inference and can't be done exclusively from the source code.