Skip to content

RIIR vscode extension #16515

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

Closed
wants to merge 1 commit into from

Conversation

HKalbasi
Copy link
Member

@HKalbasi HKalbasi commented Feb 8, 2024

Triggered by #14589 (comment) this PR adds basic infrastructure for using Rust in the vscode extension using webassembly.

There are some benefits and drawbacks in using Rust in the extension:

Benefits:

  • We are the rust-analyzer, so much more contributors and maintainers are familiar with Rust than with TS. So this will enable accepting more contribution and helps in maintaining more complex features like the vscode test api.
  • It may enable reusing some codes between the server and the extension (I'm not sure about this, but I guess we can reuse some things in configs and commands).
  • Rust the language has some benefits over TS (sound type system, traits, ...)
  • Using some wasm and async in the rust-analyzer code base may trigger improvements in supporting those in r-a due dogfooding.

Drawbacks:

  • TS is the only first class supported language by vscode, using any other language will create extra work.
  • Wasm will increase the size of the vsix package.
  • Currently the build time of the extension is almost instant, and by using Rust we will lose that.

This PR only exists for receiving feedback and I've put minimal effort into it. If more info is needed about how the Rust code will look like, I can rewrite something bigger (for example the entire ctx.ts file).

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2024
@HKalbasi HKalbasi force-pushed the riir-vscode-extension branch 8 times, most recently from 8ef54b7 to b97a486 Compare February 9, 2024 08:07
@HKalbasi HKalbasi force-pushed the riir-vscode-extension branch from b97a486 to 0873c4e Compare February 9, 2024 08:35
@Veykril
Copy link
Member

Veykril commented Feb 9, 2024

I'm not happy with that idea. Typescript is the first class language for extensions as you've mentioned, setting up any extension features is as such trivial to do. Moving over to Rust + wasm makes this entire ordeal a lot more complicated, requiring to set up bindings for the given APIs (correctly) and probably also losing the easy extension debugging VSCode offers.

We are the rust-analyzer, so much more contributors and maintainers are familiar with Rust than with TS.

The majority of code is supposed to be in the server for things we wanna offer support for, so this is a rather weak argument imo.

So this will enable accepting more contribution and helps in maintaining more complex features like the vscode test api.

This I also don't think is right. Typescript is honestly really simple to use. The bigger issue here is the VSCode API which takes a lot of resources to learn and maintain codebases for. Switching to Rust + wasm won't help in that aspect. So the point in #14589 (comment) would still stand (honestly we might need to draw a line in general regarding feature additions, as any extra we take up is more maintenance work for a team that does not seem to grow, but that's a different discussion).

It may enable reusing some codes between the server and the extension (I'm not sure about this, but I guess we can reuse some things in configs and commands).

Unlikely for that to be the case. The only things shared would maybe be the lsp extensions, which arguably are so few that the duplication across two languages isn't a problem.

I personally would rather work in typescript than Rust here.

@Veykril Veykril added S-waiting-on-decision and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 9, 2024
@davidbarsky
Copy link
Contributor

While I would personally prefer to work in Rust over Typescript, I'm sure whether it makes sense to consider this approach until WASI preview 3 comes around, at which point interoperability between JavaScript Promises and Rust futures becomes a bit easier. That being said, I'm not really plugged into the current state of WebAssembly or if there's anything like autocxx for Typescript.

This I also don't think is right. Typescript is honestly really simple to use.

I will somewhat disagree here, but I don't feel strongly enough to push one way or the other: I've found that getting something to work reliably in TypeScript is substantially harder than it is Rust. In the past month, I've wanted the equivalents of the following in TypeScript:

  • OnceCell
  • Path.ancestors(),
  • Pattern matching

To elaborate on these points:

  • I mimicked OnceCell via Promises, but that was a racy mess that I backed out of pretty quickly.
  • I was able to implement Path.ancestors() myself, but the absence of something like this in the Node standard library was an annoying papercut (I can't believe that this is my complaint about Node.js—the stdlib is too small!)
  • I've wanted pattern matching to in order to make evolving configuration APIs easier, such that a user could specify a string or string[] and the implementation inside of the extension could handle/translate things appropriately.

In any case, I don't think those features alone are enough to outweigh the cost of integrating with the VS Code API. If that portion can be trivially automated— and again, I'm not really sure how difficult that is these days—then I think something like this would be worth considering, but my vote doesn't really carry too much weight.

@Veykril
Copy link
Member

Veykril commented Feb 9, 2024

Simple != convenient. I do agree on your points, the standard library (as well as the language) is very much lacking in features and the typescript I write is a verbose mess usually. But I'd prefer that over having to glue rust to js via wasm exports in various ways, especially when having to do so manually.

@davidbarsky
Copy link
Contributor

Simple != convenient. I do agree on your points, the standard library (as well as the language) is very much lacking in features and the typescript I write is a verbose mess usually. But I'd prefer that over having to glue rust to js via wasm exports in various ways, especially when having to do so manually.

yeah, i think i'm in agreement with you here.

@HKalbasi
Copy link
Member Author

HKalbasi commented Feb 9, 2024

I don't personally have any problem with TS, I just tried to fix the road blocks in the way of vscode related features like the test api. Now that the problem isn't with TS, I think I'm not understanding the problem. What makes vscode api different from LSP or cargo api? For example in the case of test api, it's ~3 classes with ~10 straight forward functions. Would it being an LSP feature make things different?

we might need to draw a line in general regarding feature additions

We need to do that, but I think the test explorer is on the other side of any reasonable line (ignoring the TS/vscode problem, assuming it is a normal feature implementable in the server). It is extremely popular (The deprecated test explorer extension has 3M installs, and vscode-rust-test-explorer has 100K installs) and any foreign extension implementing it needs to either ask cargo about test information, which make it slow, lose on the fly updates, conflict with r-a, ... or ask r-a, which will create an equivalent maintenance burden to creating an interface for other extensions to ask tests information.

I'm sure whether it makes sense to consider this approach until WASI preview 3 comes around,

I don't see how WASI is relevant here. WASI is about calling system apis like file system and networking, but the vscode extension almost exclusively only uses the vscode apis. The state of async in wasm_bindgen is near perfect, much better than cxx and cxx-async.

Also, note that the current implementation is not the ideal one. It's just something I hacked together in an hour to start the discussion. An idiomatic Rust crate mirroring the api of the vscode api, living outside of this repository, with no JsValue or similar things in it's api is viable and you can assume it's existence in the decision. Even assuming that, there are valid reasons for not using Rust in the extension code, and I don't want to insist on it. I just would really like to find some path forward for the test api.

@flodiebold
Copy link
Member

FWIW, I tend to agree that rewriting the extension in Rust doesn't really resolve the concerns with having lots of code in it or make it more maintainable. I understand wanting to find a way to deliver something like the test explorer though. For me, the question is whether we should be in the business of maintaining a big VSCode-only feature like this. I'm biased since I don't use VSCode, but I see the role of the VSCode extension more as the "reference implementation" of a rust-analyzer client. So from that perspective 1. it should be written in the canonical language for VSCode extensions, and 2. I agree with David that a large feature like the test explorer would be better in a separately maintained extension. OTOH, this would mean maintaining a large API surface of LSP extensions without an actual client in the repo, which also isn't ideal...

@davidbarsky
Copy link
Contributor

davidbarsky commented Feb 11, 2024

I think—and I might be wrong!—that maintaining non-standard LSP extensions is much more tractable and sustainable than maintaining a whole UI for VS Code (heck, at work, some colleagues wrote a React shim over VS Code's tree views because manually manipulating that view was so challenging and fiddly, writing a compiler was preferable!).

I think the other point in favor of exposing non-standard LSP extensions for some advanced functionality is that the cost (in terms of lines of code and ongoing maintenance) of each non-standard extension is relatively tiny compared to fully-fleshed UI features. There is, of course, some design and documentation costs to these features, but that exists with any feature.

OTOH, this would mean maintaining a large API surface of LSP extensions without an actual client in the repo, which also isn't ideal...

Few notes on this:

  • At least for the test explorer, I believe that only one or two additional non-standard requests are necessary. Each new feature can be evaluated on a case-by-case basis.
  • For a companion extension to work (communication mechanisms with rust-analyzer-the-server changing notwithstanding), VS Code would need to become an extremely thin client that proxies the responses to a companion extension. You are, however, correct that the extension would stray a bit from its original purpose of a reference implementation.

@HKalbasi
Copy link
Member Author

Keeping the client side code small makes sense, and for a small code it is less important what language it is written in, so I'm going to close this. It seems the path forward to something like the test explorer is a lsp extension. I still believe with a focused lsp extension this can become reasonably small on the client side so that it can be included in the main extension.

By the way, I have an (incomplete) autogenerated bindings for the vscode extension api. If anyone seeing this thread is interested in writing some other vscode extensions in Rust, tell me to publish it somewhere.

@CGMossa
Copy link

CGMossa commented Mar 11, 2024

This reminds me of the beautiful WIP https://github.com/rust-analyzer/rust-analyzer-wasm, that can be seen here https://ra-wasm.netlify.app/. It might not be exactly on the path to realise this, but it is an advantage of Rust Analyzer as LSP was WASM/rust based...

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

Successfully merging this pull request may close these issues.

6 participants