Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

Support rust-analyzer as alternate LSP engine #793

Merged
merged 20 commits into from
May 12, 2020
Merged

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented May 11, 2020

This mostly reshuffles stuff around and makes some bits more generic and less tied to the current RLS engine.

The behaviour can be changed by setting rust-client.engine accordingly, and now the client implementations live at rls.ts and rustAnalyzer.ts for the respective engine.

Particularly, the workspace configuration is no longer synchronized for every rust-prefixed setting and the list is hardcoded to the existing rust.<CFG> settings for backwards compatibility - this is to prevent RLS from warning about unrecognized future settings and that's because we want to possibly unify all of our configuration under a single rust prefix (rather than have a separate rust-client for client-relevant bits).

Previously we spawned each instance of RLS lazily depending on the Rust file that was opened (we crawled the FS and determined a project root where to spawn RLS) but Rust Analyzer currently eagerly crawls every opened workspace (not in sense of an opened text editor tab but rather active workspace folders). To keep the existing logic somewhat compatible, we keep the multi-project layout but only spawn and refer to a singleton client in the rustAnalyzer.ts to keep the implementation simple.

What's left is tweaking the README, documenting more rust-analyzer-specific options (ideally coupled with general settings refresh), reword some tasks and UI to not refer to RLS unconditionally and wait for rust-analyzer to use more LSP-compliant messages in order to fully support its capabilities 😅

cc @matklad

Fixes #780
Addresses #781 partially

}

/** Returns a path where persistent data for rust-analyzer should be installed. */
function metadataDir(): string | undefined {
Copy link
Member Author

Choose a reason for hiding this comment

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

@matklad fyi it'd be great to support something like rust-analyzer --version --tag or simply include just it in the output to drop the persistent data logic (that bit is copied in spirit from the existing rust-analyzer extension) and query the binary itself

Copy link
Member

Choose a reason for hiding this comment

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

The problem here is that nightly tag moves. I guess, we can change version string to rust-analyzer yyyy-mm-dd [nightly]. Yeah, I if I am not missing something, this would probably be a better way to handle this.

Copy link
Member

Choose a reason for hiding this comment

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

Though, @Xanewok, could you explain why we need to effectively implement our own persistance here? Shouldn't this be handled by VS Code persistance mechanism? Ie, this:

https://github.com/rust-analyzer/rust-analyzer/blob/62730ae30db453f53eb0c37647b5c76058eafca0/editors/code/src/persistent_state.ts

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't this be handled by VS Code persistance mechanism?

You're absolutely right! What I did at the beginning was to implement everything except the persistence just to get it working; while polishing it I just bolted it on thinking it's implemented similarly (didn't know about the Memento API). It'd be a good idea to drop this altogether :)

To de-emphasize the current RLS and emphasize that we use either RLS
or Rust Analyzer as our underlying language server.
@Xanewok
Copy link
Member Author

Xanewok commented May 12, 2020

I've been trying to test this extensively in both modes on both Linux and Windows and it seems more or less things are in place and not regressed. What's left is reworking the extension settings a bit, e.g. to separate rustup or rls-specific ones rather than lumping them all together but let's do that as a part of a next PR.

@matklad
Copy link
Member

matklad commented May 12, 2020

(I want to review this closesly, but I am pretty swamped at the moment, but, anyway, I am wondering, what would be the easiest way to bridge the rest of rust-analyzer's typescript extension here...)

@kjeremy
Copy link
Contributor

kjeremy commented May 12, 2020

One thing I may have missed in looking at this are support for call hierarchy and semantic tokens: https://github.com/rust-analyzer/rust-analyzer/blob/master/editors/code/src/client.ts#L50-L51

@Xanewok
Copy link
Member Author

Xanewok commented May 12, 2020

@kjeremy I've had some problems with module imports while hacking up the prototype and just used more broad registerProposedFeatures for the lsp client:
https://github.com/rust-lang/rls-vscode/pull/793/files#diff-f18ac56573e8e79333ee19a04a9a70f1R275

This covers more features than call hierarchy and semantic token but does include it as well and looks to be working (although only sending that after modifying the file? might have to do with not porting the busy middleware workaround for semantic tokens, idk 🤷 )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement support for the Rust Analyzer engine
3 participants