Rls: RLS won't format code until it is done building

[programmerjake]

I have format-on-save turned on. When I save while RLS is building crates, it doesn't format the saved file until after it finished building, then it formats just fine.

I'm using:

• rls-preview 1.31.6 (daa138c 2018-11-20)
• Ubuntu 18.04.1 on amd64
• Atom 1.28.2 with
  
  
  
  • ide-rust 0.18.5
  
  
  • language-rust 0.4.12
  
  

[programmerjake]

From what I can tell, this is caused by my not adding edition = "2015" to my Cargo.toml files. I think this should be fixed to assume the 2015 edition if rls can find a Cargo.toml file that doesn't have an edition tag.

[alexheretic]

https://github.com/rust-lang/rls/blob/master/src/actions/requests.rs#L787

The code doesn't know which edition it is so doesn't format the code, which is correct. However, we could improve fn file_edition to handle pre-build cases maybe.

[nrc]

We should assume 2015 if there is no explicit edition, but we should not format until we know the edition (IOW, we should not default to 2015 if the edition is an unknown unknown, only if it is a known unknown). We should also be able to deduce the edition before the build is finished.

[programmerjake]

I can format all the code in a fraction of a second using cargo fmt. Since RLS has all the same information, it should be able to at least format a single file in the time that cargo uses to format the entire project.

[Sh4rK]

I use VS Code with RLS, invoking rustfmt with a keybind, and this bugs me very much as it happens quite often. I change something, hit format, and nothing happens, except the RLS error console pops up with an error.

[alexheretic]

I think we all agree, rls just needs the code to fix it now. This could be a good first issue maybe @Xanewok?

[Xanewok]

Unfortunately this is somewhat of a (unnecessarily) complex issue.

We can query Cargo for

• package edition
• crate target ([lib], [[bin]] etc.) and its root source file

However:

1. crate targets can have different editions specified but share files otherwise (mod inner;) (ambiguous editions; given a shared source file, what is the associated edition?)
   @alexcrichton is there a real benefit for having different editions per crate targets?
2. ~We can't know specific set of input source files until we at least execute the build script and in the future (hopefully not) we compile our immediate dependencies (Cargo build plan inputs comment).~
   EDIT: Technically we might need conditional compilation but we can sidestep that by processing every mod (https://github.com/rust-lang/rls/issues/1148#issuecomment-443230903)

Working around 2. would need work on the Rust compiler, as currently we only know our inputs after we do parse & expansion but we can't even create a correct AST without having access to dependency .rlib or .rmetas.
I assume we could try and insert a dummy nodes if we only run the compiler with --emit=dep-info equivalent?
However, this would still require executing build script to expand conditional includes, e.g.

include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

In this case, I think we should emit a warning like: "Couldn't detect edition used for this file. Either specify edition key manually in rustfmt.toml or wait for the indexing to finish".
@alexcrichton @alexheretic @nrc thoughts?

[alexheretic]

It does seems annoyingly valid to counter with _"but cargo fmt can do it"_.

Are we better off starting rustfmt as a process and letting it figure it out? I assume it has an option to output a single file into stdout.

[Xanewok]

Ah, I'm mistaken! This differs from detecting input set of files since we care about the Rust source file tree but technically we still need to resolve conditional compilation.

Turns out Rustfmt takes a more total approach of traversing every ast::ItemKind::Mod (list_files list_submodules), even if it's not used during compilation.

Before the actual building (or every time, until indexing is finished?) we could do exactly what Rustfmt does and run it for every crate target source file to detect to which root crate target a given file belongs to.

This may seem excessive but it's still necessary and will take no longer than cargo fmt itself - does this approach sound better?

[alexcrichton]

@alexcrichton is there a real benefit for having different editions per crate targets?

This was necessary for cross-edition testing as well as incremental upgrades of crates, a pretty useful feature in a pinch!

[Xanewok]

Fair enough, makes sense :sweat_smile: Just imagined it'd make more sense on a package level, is all!

[nrc]

Are we better off starting rustfmt as a process and letting it figure it out? I assume it has an option to output a single file into stdout.

We can't do this since we have to be able to pass unsaved files in memory.

[norru]

We can't do this since we have to be able to pass unsaved files in memory.

Dump the unsaved onto a temp file?

[programmerjake]

What about something like LLVM's virtual file system, where all file access goes through a trait that allows adaptor implementations to provide the in-memory files? This will also allow useful things like compilation directly from a zip file or git objects, decompressing on the fly.

[programmerjake]

We could pass in-memory files through a pipe (works on both Unix and Windows) that we pass the handle/fd of through a command line argument.