-
Notifications
You must be signed in to change notification settings - Fork 21
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
Generate Rust types from the official specification #151
Comments
It is wonderful to see interest in this.
@dbaeumer Does the LSP model json contain proposed features.
The python package does not explicitly enable or disable proposed feature. We could add support for this in the Rust code.
I like the first idea:
The LSP model in this repro was generated with 3.17.0. We currently generate from the latest only. Before this we did not have a JSON model for the LSP spec. |
I don't think the current 3.17.0 specification includes any proposed features, but it would be awesome if the JSON model and the generator crate had support for this going forward. Previous versions did have some proposed features (inline value and inlay hints for a while, I think), so having a |
Yes, the model has support for proposed features. See the proposed properties in the meta meta model itself: https://github.com/microsoft/vscode-languageserver-node/blob/56c23c557e3568a9f56f42435fd5a80f9458957f/tools/src/metaModel.ts#L1 |
@ebkalderon I have started work on this. I created a meta item to track various bits of work. #164 |
That's awesome! Really glad to see a pull request land on this. 😄 |
Love what I'm seeing in #166, though I think we should probably remove the I'm not familiar with this particular nuance of the specification, but does LSP include the full pub enum ErrorCode {
ParseError,
InvalidRequest,
MethodNotFound,
InvalidParams,
InternalError,
ServerNotInitialized,
UnknownErrorCode,
Other(i64),
} I used Any thoughts on this? |
@ebkalderon Enum types that can be extended have |
Published alpha version of |
Introduction
It would be awesome if this project also supported generating Rust types from the official spec as well.
This ticket was created in response to a brief discussion with @brettcannon over at ebkalderon/tower-lsp#361 (comment) and is intended track future developments. Happy to get some productive dialog going! 😄
Background
Considering that installing and running a Python package as a prerequisite from a
cargo build
would be pretty messy, I would assume this effort would likely entail the creation of a pure Rust equivalent that can consume the same spec. Consider the following setup:lsprotocol-codegen
crate would process the spec and generate a Rust file containing type and optionally trait definitions.build.rs
script.generate()
function that takes an input spec and some configuration switches.lsprotocol.rs
file in$OUT_DIR
that can be included in a Rust project with:src/bin.rs
that wraps this library and provides a CLI interface, should others prefer to use that.prost-build
(Protobuf),tonic-build
(gRPC), anddbus-codegen
(dbus) for prior art.schemafy
for processing the JSON Schema (draft 4) file in this repo.lsprotocol
crate would export native Rust types to the user.lsprotocol-codegen
and vendored directly into the repository.std
traits (Clone
,Debug
,Default
,Eq
,PartialEq
, etc).serde::Serialize
andserde::Deserialize
for easy (de)serialization to and from JSON usingserde_json
.lsp-types
for prior art.pub
fields, enums used to represent multiple states, and everything implementsSerialize
andDeserialize
.Notification
andRequest
traits are very handy for generic code. Would love to have that here too.Ideally, the above Rust crates would share the same spec file as the Python project so the codegen for both could be tested for correctness in CI.
Open Questions
One notable open question is how we should best handle "proposed" features. It would be nice if downstream users of
lsprotocol
could opt into certain not-quite-standardized features in advance, provided they explicitly agree to the API instability when switching this on. The de-facto community standard cratelsp-types
does precisely this (as does my own downstream LSP library for Rusttower-lsp
) using an off-by-defaultproposed = []
Cargo feature which, if enabled at compile-time, would activate these types in the public API.If we choose to offer the same thing here, it would be fairly trivial on the surface: the
lsprotocol
crate would expose Rust types for the entire superset of the LSP specification, marking certain types#[cfg(feature = "proposed")]
and offering users an off-by-defaultproposed = []
feature they can choose to enable if they wish. This would fall in line with other popular LSP crates used in the community today.However, what remains to be seen is how fancy we'd like to be with
lsprotocol-codegen
. Consider the following questions:lsprotocol
package supports today? Is this something we even want to support?lsprotocol-codegen
implement said support?lsprotocol-codegen
would output a singlelsprotocol.rs
file containing all types with some marked#[cfg(feature = "proposed")]
, ready to be vendored into this repo aslsprotocol/src/lib.rs
and published directly to Crates.io as thelsprotocol
crate.lsprotocol_codegen::generate()
and the CLI interface (if we choose to provide one) for the user to change the"proposed"
feature name string in the output#[cfg]
s to something else?lsprotocol.rs
output entirely?Another open question is regarding specification version support in general. Do we support the latest version of the LSP spec only? Or do we want to be able to perform codegen for all available versions of the spec? I presume the former, but I think it's good to get solid confirmation on this.
Versioning
Crates published to Crates.io are expected to adhere to semantic versioning as strictly as possible. This is quite significant for us because adding new
pub
fields to an existingstruct
or new variants to an existingenum
is considered a breaking API change, unless those types are explicitly marked#[non_exhaustive]
(reference), at which point users are forced to include a_ =>
fallthrough case or wildcard..
when matching on or destructuring those types.We should not annotate every single type in the generated code with
#[non_exhaustive]
, of course, since this would severely hamper the crate's usability. Downstream code would not be able to directly construct instances of any of the generated structs, even with..Default::default()
syntax, even though all fields arepub
(see bug report rust-lang/rust#70564). We should carefully consider whether or not to mark certain types#[non_exhaustive]
and when it would be most applicable to do so.Thankfully,
serde
seems to support serializing from and deserializing to#[non_exhaustive]
types out of the box, so this should not be a factor in us deciding when or not to apply this attribute in thelsprotocol-codegen
output.The text was updated successfully, but these errors were encountered: