Skip to content

Instantly share code, notes, and snippets.

@nikomatsakis
Last active April 28, 2020 13:12
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save nikomatsakis/c31e74c279025aefcdc485bab05a7fdf to your computer and use it in GitHub Desktop.
Save nikomatsakis/c31e74c279025aefcdc485bab05a7fdf to your computer and use it in GitHub Desktop.

OK, so I've been going over the thread, and I see that there are maybe three major points of discussion thus far that I want to highlight. I've created a summary comment highlighting key points and how they've been resolved thus far. Please avoid relitigating things covered in the summary, unless you feel the summary is missing something.

Distribution of rust-analyzer

Currently things are distributed in a rather ad-hoc way, and there were several comments in support of distributing rust-analyzer through rustup. @pietroalbini writes:

I'd prefer for rust-analyzer to be shipped through rustup soon after the RFC is merged: being able to pull it like any other component would be great, especially for people who are not using VSCode.

I agree with this sentiment, and I suspect @matklad does too, as he wrote:

Additionally, distribution of binaries comes with security implications. The current distribution story (using github actions to publish github releases and marketplace extension) seems fine for a 3rd party tool, but would be underwhelming for an official project. [..] So, if/when we complete the transition period, we definitely want to ship rust-analyzer via rustup, just like any other official tool. Given that we want this eventually, we might as well start early, somewhere before the "deprecation" phase

I think the only "detail" is the exact timing to make this transition. I think that specifying that it occurs during this first phase is sufficient, and I will make that change. There are some details that I think can and should be worked out separately from this RFC, such as whether rust-analyzer will continue to try and upgrade to newer releases when they're available and so forth (I do appreciate, for example, getting regular upgrades with new functionality without having to do any work, especially during this period of rapid change).

I consider this resolved.

Transition and clients

One key comment that I think has gone overlooked is this one, by @Xanewok:

I'd like to chime in and propose that for VSCode specifically we continue to use the existing rust-lang.rust plugin and implement support for the Rust Analyzer engine.

This is the most popular downloaded extension for Rust with almost 0.5M downloads now and should considerably reduce the churn and smooth the transition period, especially if it's going to be as easy as prompting the user with a "Using Rust Analyzer engine is preferred now. Switch?" modal.

I do think it's very important to make the transition over to using rust-analyzer as smooth as possible. I'm not 100% sure what is being suggested here, though, so I'd like for folks to flesh it out (perhaps offline from the RFC thread -- this seems like a technical discussion that would be well-suited to Zulip). It does seem to me that it would be best if we can reduce the number of options in the VSCode marketplace and consolidate on a single plugin, but I also know that the rust-analyzer plugin has a number of custom elements so that may not be too easy.

I consider this resolved -- but only because the RFC leaves these details unspecified, I think that we should discuss offline and work out the best plan.

Conforming the LSP protocol

rust-analyzer presently supports only the "latest and greatest" version of the LSP protocol, and it also includes various pending extensions to the protocol, as @matklad describes in this comment:

At the moment, rust-analyzer isn't strictly conforming here: we usually only implement the latest versions of things. It seems like this works for the majority of users: so far we've receive relatively few reports (but some were quite vocal) about us not supporting older versions of requests.

This support causes problems for the ycmd editor, as @bstaletic discusses here, because they do not support snippet-based completions, but rust-analyzer is sending them anyway. (As of yesterday, this has since been fixed!)

In his comment, @matklad went on to say that rust-analyzer should support the full conformance protocol:

But, if we transition rust-analyzer as the official IDE, I think it makes sense to make full protocol conformance a pre-requisite. There are a lot of external benefits in following the standards to the letter, and it's not like it's hard to do, just annoying.

I agree that, before rust-analyzer is adopted as our official client, it should fully conform to the LSP protocol, and I will add a note to that effect. Note though that r-a would not need to support optional features, or older features that aren't required (currently, they are). I've pushed text to this effect.

I consider this resolved -- note that further discussion of specific aspects of the protocol is best taken to rust-analyzer repository.

Support for clients beyond VSCode

We discussed the level of support for clients beyond VSCode. Currently, the only "official" plugin is the one for VSCode -- this is also true for the RLS. Other plugins are independently maintained. I added some language to clarify that, while rust-analyzer is under rapid development, this situation is not expected to change, but that as rust-analyzer matures, it may make sense to consider how we can best support other editors, and in particular if we should make other clients official.

I consider this resolved.

Level of protocol support required

In his commment, @matklad raised the point that rust-analyzer currently includes a number of extensions beyond the LSP protocol:

rust-analyzer makes a rather heavy use of such extensions. The reason is that I want rust-analyzer to be a good base for "perfect" IDE support. LSP as it exists today is a rather limited protocol. I wouldn't call it the lowest common denominator, but it definitely is way more restricted than, for example, capabilities of IntelliJ Platform. So we ship some features before they are implemented in the LSP.

I think this is well-motivated, but it's worth clarifying the stability story here, and I added some text to that effect. In short:

  • We should document the extensions and their stability level, and make disruptive/unstable extensions "opt-in"
  • Removing extensions is not considered a semver violation, since the LSP permits feature negotiation

There may also be some implications here for rustup distribution, in that the rust-analyzer VSCode client and the rust-analyzer binary that is executing must be in sync? I don't know the details, though, and I don't think that we need to discuss them there.

I consider this resolved.

Feature parity

One of the key points in this RFC is that feature parity with RLS is not strictly required. While rust-analyzer offers a number of things that the RLS does not support, there are three specific ways that it lags behind:

  • It does not support reporting errors or lints without saving
  • It does not support precise find-all-usages, goto-definition, or renames, in some cases falling back to approximations.
  • It does not persist data to disk, which can lead to large startup times.

The reasons behind these limitations are that it will take some time to implement those features "the right way" (i.e., using the demand-driven approach that rust-analyzer is pioneering). Initially, we expected to require full feature parity, but we realized that this would lead to us creating "throwaway" code to temporarily patch over the limitation, and that this would in turn slow the progress towards our ultimate goals. Therefore, we decided not to require this, but instead to opt for a "feedback" period to assess the biggest pain points and see what we can do to relieve them.

Thus far, for example, feedback on the "files must be saved" has been relatively positive, though -- as @sbromberger noted here -- that requirement can be particularly inconvenient over a high-latency network mount.

Nonetheless, I think that overall the decision to not require feature parity is the right one. We may able to alleviate some of those specific failures (for example, rustc does support a "virtual file system" feature, which is used by the RLS, though it's not clear whether rust-analyzer could use it very easily), but we will decide based on the total volume of feedback and other priorities whether it makes sense to pursue that.

I consider this resolved.

Other concerns

Naming of the periods

I named the three periods

  • Feedback
  • Deprecation
  • Final transition

but it was suggested that this terminology is sub-optimal. I agree, but I opted to keep it, though, as I noticed that it was already in use elsewhere in the RFC and thread.

Naming of "library-ification"

The compiler team has been using the obviously ungrammatical, but playful, term "library-ification". This RFC is mostly just referencing this term, which is in use elsewhere, so I don't intend to change that here.

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