x.py feature request: support "x.py fmt" as rust-analyzer formatter

[RalfJung]

When working with vscode in the rustc repo, one needs to set a rustfmt invocation to use for formatting files. The dev guide recommends

    "rust-analyzer.rustfmt.overrideCommand": [
        "./build/$TARGET_TRIPLE/stage0/bin/rustfmt",
        "--edition=2021"
    ],

However, this command fails when rustfmt has not been extracted yet, so I often get errors from vscode here and then I have to do x.py fmt on the terminal by hand to make things work again. (Also the user has to figure out the right TARGET_TRIPLE themselves.)

It would be nice if one could set the overrideCommand to x.py such that extracting the bootstrap rustfmt can happen automatically. However, x.py fmt doesn't work since that always formats everything, not just a single file RA does formatting via stdin/stdout, which is not how x.py fmt works.

Cc @jyn514

[jyn514]

However, this command fails when rustfmt has not been extracted yet

That was fixed in #101969.

However, x.py fmt doesn't work since that always formats everything, not just a single file.

Have you tried x.py fmt src/bootstrap/lib.rs (or whatever file you care about)? That should already work today.

[RalfJung]

That was fixed in #101969.

Oh, nice!

Have you tried x.py fmt src/bootstrap/lib.rs (or whatever file you care about)? That should already work today.

I tried that a while ago, and back then it just re-formatted everything, even when given a particular filename. But looks like that was fixed. :)
So can one set ./x.py fmt as formatter for RA then? Should the dev guide be adjusted?

[RalfJung]

Answering my own question: yes it works but it is annoying because then formatting has to wait for the workspace lock.

[RalfJung]

Also the per-file thing does not entirely work... when I ask x.py to format library/core/src/intrinsics.rs, it also formats library/core/src/panicking.rs.

[jyn514]

Answering my own question: yes it works but it is annoying because then formatting has to wait for the workspace lock.

Hmm, good point. We added that because our sysroot handling wasn't parallel safe; I wonder if it's possible to remove that restriction only for stuff that doesn't touch the sysroot? I won't have time to look into it but happy to mentor.

[Mark-Simulacrum]

I don't think we should be removing it selectively - in particular, this operation could need the lock if you had changed stage0.json (and need to redownload). I think if we want to make the lock more granular we will need to actually protect specific resources (e.g., the stage0 sysroot distinct from everything else would suffice here, probably).

[RalfJung]

Turns out I was entirely off, and individual files are not even what RA needs. What RA needs is to pass the code via stdin and retrieve the formatted result on stdout.

[jyn514]

Ok. That seems like a major expansion of scope and I think we should continue to recommend using stage0 rustfmt instead.

[Mark-Simulacrum]

FWIW my own hack is that I just manually copy over the rustfmt binary (it doesn't dynamically link to librustc) into a directory somewhere and only update the path configured in rust analyzer when I notice divergence between in tree and my copy. This happens pretty rarely; rustfmt is pretty stable.

You could imagine a helper script that's purely conditional on stage0.json updates that does something similar, but given our difficulties in scripts for cross platform compat I'm inclined to agree with closing.