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

Add LSP support to air #38

Merged
merged 28 commits into from
Nov 26, 2024
Merged

Add LSP support to air #38

merged 28 commits into from
Nov 26, 2024

Conversation

lionel-
Copy link
Collaborator

@lionel- lionel- commented Nov 18, 2024

Branched from #37.

Install it in your ~/.cargo/bin folder with:

cargo install --path crates/air_cli

Summary:

  • New air lsp subcommand. It only has stdio support for now but it will be easy to add TCP later on. Stdio is the standard and avoids the need for any sort of handshakes and prevents race conditions with port availability if decided by the client.

  • New lsp_test crate that implements a basic LSP client for unit tests. The client launches our LSP server on creation. Depends on some private files imported from tower_lsp. These are stored in a tower_lsp subfolders with clear headers documenting the source.

    In the tests_macros crate, new lsp_test attribute macro. Its main purpose is to shutdown the LSP client and server when the test block goes out of scope. It's required that the test block returns the client so the macro can manage it.

  • The LSP server now properly handles shutdown and exit notifications, something we haven't supported in Ark. Requires a custom revision of tower-lsp that integrates this fix: Actually exit after receiving exit notification ebkalderon/tower-lsp#428.

  • The Document struct now contains:

    • An augmented LineIndex. This augmented line index is imported from rust-analyzer. It wraps the base line index (originally from https://github.com/rust-lang/rust-analyzer/tree/master/lib/line-index but we use biome's fork for compatibility with other biome infra).

      The base line index is an array of all the positions of \n characters in the document. It allows to efficiently convert between row/col coordinates (for communication with LSP clients) and byte offsets (for internal usage).

      The augmented line index keeps track of two additional properties. The encoding of position coordinates that we negotiated with the client (UTF-8 or the mandated UTF-16, we don't support UTF-32), and whether the document originally contained Unix or Windows line endings.

      This is useful for conversions of data structures at the protocol boundary (internal to LSP). We store the document contents in normalised form (UTF-8 Rust string with Unix line endings, and UTF-8 based offsets) and convert them back to the form expected by the client at the boundary.

      For this we import the conversion tools in rust-analyzer that consume this augmented line index: https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/lsp/from_proto.rs and https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/lsp/to_proto.rs. These files are stored in a rust_analyzer subfolder and have clear headers documenting the source. We also sometimes use the from_proto and to_proto Biome tools in https://github.com/biomejs/biome/tree/main/crates/biome_lsp_converters. And we have our own from_proto.rs and to_proto.rs files as well that contain custom tools.

      We lean more on Rust-Analyzer because of their choice to normalize line endings and use the augmented line index at the boundary. By comparison Biome doesn't normalize and just respects the formatter configuration for line endings. We think normalisation will give us more flexibility in more diverse situations (refactorings, assists) and allow us to make more assumptions.

    • The syntax tree that we compute eagerly without incrementality for now. We're going to need it right away for diagnostics so there is not much to be gained from laziness.

      On document updates we just reparse everything from scratch. That's what they do in Rust-Analyzer as well as this hasn't been a bottleneck for them.

    • The contents of the file as a simple String. In theory we shouldn't need it as the syntax tree is a full CST with all trivia tokens (newlines, comments, ...) and the SyntaxNode API offers ways to retrieve the text. But it's useful to have a String for document updates. If ever a bottleneck, we can switch to a rope data structure as in Ark.

  • New handlers_format.rs file that contains support for formatting whole documents.

#[cfg(test)]
pub(crate) async fn start_test_client() -> lsp_test::lsp_client::TestClient {
lsp_test::lsp_client::TestClient::new(|server_rx, client_tx| async {
start_lsp(server_rx, client_tx).await
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also consider an integration test harness that calls air lsp directly using the test binary to simulate "real" usage

@lionel- lionel- force-pushed the feature/import-ark-lsp branch from cfcaef0 to 20bc062 Compare November 26, 2024 09:41
@lionel- lionel- force-pushed the feature/lsp branch 3 times, most recently from 3cd1e70 to 278e5eb Compare November 26, 2024 09:56
Base automatically changed from feature/import-ark-lsp to main November 26, 2024 10:00
@lionel- lionel- merged commit 20b8c51 into main Nov 26, 2024
@lionel- lionel- deleted the feature/lsp branch November 26, 2024 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants