Feedback on Rust bindings

[osiewicz]

Hi,
as promised in #425 (comment), I wanted to share a bit of feedback on existing state of lsprotocol.

Who am I and what's my use case

I am a member of the core team of zed; we currently use an in-house fork of lsp-types. For the most part we pull in external changes from the community (we're not doing much of custom development in our fork).
This brings me to a main reason why we (IMHO) should switch over; lsp-types is pretty much unmaintained. PRs pile on top of it. They had some controversial changes around handling of URIs which we could not incorporate.
I have high trust in lsprotocol, as it:

• Comes from Microsoft
• Is intended for use with multiple languages

Now, looking at the codegen of Rust generator, there are a couple issues I see with status quo. Note that I am speaking from a perspective of somebody trying to migrate from lsp-types, so my judgement is biased.

There is no relationship between types

One of the best features of lsp-types is that it bridges Request and Response types via Rust's traits. See the following traits:

• Request
• Notification

All Request and Notification implementations are also neatly stored aside in request/notification modules, but that's a cherry on top and not a crucial part of the deal.

Now, having the relationships defined at the type level allows one to implement functions that automatically infer the correct return type for a given set of parameters, just like we do in zed (another example being rust-analyzer). This is very, very nice for us and we definitely don't want to give up on it. Plus, it seems like a metamodel you're generating code from already includes the info about these relationships.

Some types seem unnecessary

Building upon my previous point, some types in the public API seem straight up unnecessary to me. Take ImplementationRequest as an example; it includes things like request id, method or JSONRPC version.
The thing is, with JSONRPC such enumeration of types does not make much sense; namely, we always want to deserialize the message into a generic "json rpc with payload" struct that then dispatches on method AND id. (for example: Zed's handler). It makes sense to deal with ImplementationParams (that's what anything in Zed cares about) and ImplementationResponse (again though, without the nitty-gritty-details of JSONRPC).

Documentation contains dead links

When running rustdoc against this crate there's a bunch of warnings about dead links in doc comments. I get that it's not entirely your fault, but doc quality is of high importance to me (even if it's just forwarded from the meta file). As an implementer of LSP client I stare at those a fair bunch. This is still a nitpick compared to the other two points

Closing thoughts

I am aware that my perspective is stained by having worked with a single library for the past two years, but I really do believe they got a bunch of stuff right in a way that makes implementing a LSP client/server pretty straightforward. I guess we don't need to reinvent the wheel here; it's sufficient to have a library that's cared for, which is what I am missing with lsp-types.

[osiewicz]

I've started implementing some of these changes in https://github.com/zed-industries/lsprotocol/tree/rust-clean-up-api

[debonte]

@osiewicz, I appreciate your thoughts on this. Tomorrow is my last day at Microsoft and I'm not sure how much time my team will be investing on this in the near future. That said, we do want to improve our Rust support and would welcome PRs.

Just making you aware. I didn't want you to think I was ghosting you. 😄

[osiewicz]

Thank you and good luck in your future endeavors!