2019 Strategy for Rustc and the RLS

Let’s discuss problems with RLS :smiley: (this is the last part I think?)

I think there’s an agreement that currently RLS is not quite the IDE experience we strive for. Some people think that this would be fixed by incrementally improving RLS. I don’t agree: steady stream of improvements is impossible if the foundation is wrong, and I think that’s the case with RLS. I’d like to list specific problems.

First and foremost, you can’t fix code that you can’t build, and its next to impossible to build RLS: cargo test works only if stars are aligned, and only the person who build RLS last knows this alignment. CI is perma-red, and applying the not rocket science rule is impossible.

Code Analyzers are interesting in a sense that they are as deep as compilers, but they are also extremely broad. There’s a huge number of relatively shallow features, and this breadth is a significant part of what constitutes a good IDE experience. To give an example, once a colleague at JetBrains had a code like

if (foo() && bar()) {

and wanted to refactor it to

if (foo()) { 
  /* stuff */ 
  if (bar()) { 
    /* more stuff */`

They thought “hm, I guess my IDE should be able to do this refactoring”, placed the cursor at if, pressed Alt+Enter and didn’t see the required intention. Then they moved cursor to && and surely they saw “Split into two if’s” light bulb. That is, ideally one does not ask “can Code Analyzer do X”, one just searches this feature. Given the community-driven nature of Rust, implementing a myriad of such small helpers shouldn’t be hard, if the contribution experience is actually good, and being able to cargo test is a huge part of such an experience.

The second big RLS problem is its “shared mutable state is fine” handling of concurrency. One of the hardest problems of Code Analyzers, which is not typically faced by traditional compilers, is state management. Code Analyzer is a long-lived process which manages state affected by various asynchronous sources: user’s actions, file system events and background analysis jobs. It’s important to provide consistent view of the state to the actual analysis queries: it’s hard to do computations if you can’t rely on repeatable read property. See https://github.com/rust-lang-nursery/rls/issues/962 for details.

Fun aside: LSP, unlike the Dart protocol, includes version numbers for files, which seems misguided? If an edit applies to file A of version X, and A has version X currently, the edit might actually be invalid if the version of other file B hash changed since edit has been calculated. Client and Server must agree on the whole state to be able to make correctness guarantees. Why LSP designers haven’t just ripped of the Dart protocol? It’s so much better for important stuff…

The third big RLS problem is that it is deeply integrated with the build system and runs rustc to get save-analysis data. My and @petrochenkov’s comments in a sibling thread illustrate why this is not a good idea. The main points are that build systems for non-toy projects are non-trivial, and can’t handle on-demand requirements of IDEs. It’s a fine optimization to use save-analysis for readonly dependencies, but using it for the current crate create a lot of complexity (a significant chunk of RLS is build orchestration, which could have been just cargo check otherwise) and poor user experience (for non-trivial projects, RLS takes significant time to answer simple queries after edits). Code Analyzers should not care about build system used, and, as usual, Dart Analyzer is a great example here. It’s build system integration consists of a single abstract class, and a couple of implementations for Bazel and pub.

17 Likes