Update to latest rustfmt

[thomaseizinger]

To make future upgrades easier, we also make the following changes:

1. Use a rust-toolchain file to clearly specify the requirements of
   our toolchain.
2. Refer to this toolchain in the CI configuration to avoid duplicating
   the version requirement.
3. Remove wasm feature in favour of cfg-based dependency declarations.
4. Remove patches directory as they are now outdated with the recent
   ap-crates releases.
5. Depend on rustfmt via git because the crates.io release is no longer
   being updated.

This is a draft PR because:

a) I am not sure if you want all the changes to the infrastructure.
b) We need this to be merged and released before this is going to build. I applied the patch locally and it works. We could also patch this the way rustc-ap-rustc_data_structures was patched before however, I thought it is nice to actually be able to get rid of this stuff hence I didn't want to re-introduce it again. We might need to wait for another rustc + rustfmt release though before we can use that.

[CLAassistant]

All committers have signed the CLA.

[dsherret]

@thomaseizinger looks great! Yeah, I'd love this change.

I wonder why it's not compiling though. I found this issue rust-lang/rust#56935 Edit: Oh, I see you referenced that already.

[thomaseizinger]

rust-lang/rust#81498 has been merged, waiting for it to be included in a nightly build and then rustfmt needs to update to that nightly as well.

[thomaseizinger]

The PR is included in the recent nightly but it still fails. I assume we would need to patch more crates with the same workaround 🙄

I'll look into this.

[dsherret]

That sucks. Thanks so much for taking this on and looking into it!

[thomaseizinger]

Further investigation revealed that rustfmt is just not yet depending on a recent enough version of the rustc-ap-rustc_index crate. The most recent version simply does not yet include the fix so we will need to be patient for some more days :)

That sucks. Thanks so much for taking this on and looking into it!

No worries. I am keen to make use of some recent features in rustfmt, that is why I am willing to push this forward :D

[dsherret]

I investigated this just now... looks like we're still waiting on a rustfmt release (last one was 1.4.36 on Feb 8th).

[thomaseizinger]

I investigated this just now... looks like we're still waiting on a rustfmt release (last one was 1.4.36 on Feb 8th).

Correct. I've subscribed to the repo and will pick this up once they cut a new release!

[thomaseizinger]

Alternatively, we could depend on a specific commit hash?

[thomaseizinger]

@dsherret What is your thought on depending on a specific commit hash vs a tagged one? I am tired of waiting for a release 😅

Additionally, it doesn't really make a big difference for dprint itself so I don't necessarily want to take up resources of the rustfmt team just so we can depend on a more "official" version.

I don't know if tagged releases of rustmft undergo more specific testing than the usual CI? If not, I see little risk in depending on a specific commit on their main branch vs depending on a tagged release.

[dsherret]

@thomaseizinger yeah depending on a specific commit sounds good to me!

[thomaseizinger]

@dsherret I've updated the patch to point to latest master of rustfmt. The specific revision is tracked within Cargo.lock. For future updates, a cargo update -p rustfmt-nightly should do! Potentially, also the rust-toolchain file needs to be updated to a newer Rust version.

From my perspective, this is ready be reviewed!

[dsherret]

@thomaseizinger great! Thanks! Seems like the release build failed on the CI for some reason. I didn't look into it yet, but it says it can't find "rustc_ast".

[thomaseizinger]

Uhm, that should have been installed through rustup, at least locally that worked but then I didn't actually build for wasm32 locally 🤦‍♂️

Let's hope that actually works for wasm ...

[thomaseizinger]

So here is what happened:

• Rustfmt changed from depending on rustc-ap crates to instead depend on the rustc-dev component: rust-lang/rustfmt@61ee108
• rustc-dev is not available for wasm32: https://rust-lang.github.io/rustup-components-history/wasm32-unknown-unknown.html
• As a result, we can't actually build rustfmt on wasm anymore.

I've opened an issue here to discuss this but I am not very confident that this will be resolved quickly. As far as I understand rustc-dev is literally the entire compiler as a library and I doubt that you can actually compile the entire compiler to wasm?

[dsherret]

That sucks… I’m thinking it might be time to change this to a dprint “process plugin”, which is unfortunate, but not too big of a deal. I can start on that tomorrow or Monday and then hopefully in the future rustfmt supports compiling to wasm.

[thomaseizinger]

Yeah, I found it quite elegant that rustfmt was a wasm plugin :)

If you change it to a process plugin, are you still going to compile it yourself? One of the things I like about dprint is that I can use nightly formatting features but still use stable Rust without having to muck around and install two toolchains in CI etc.

Will that be possible with a process plugin or are you going to rely on rustfmt being installed on the machine?

[dsherret]

If you change it to a process plugin, are you still going to compile it yourself? One of the things I like about dprint is that I can use nightly formatting features but still use stable Rust without having to muck around and install two toolchains in CI etc.

Will that be possible with a process plugin or are you going to rely on rustfmt being installed on the machine?

@thomaseizinger yup, will be possible!

I'm going to merge this PR even though it doesn't work due to changes in rustfmt. Thanks for all your help on this and hopefully we'll be able to make this a Wasm plugin again in the future so that it's more portable and secure.

[dsherret]

@thomaseizinger ok, the work here is done. You can update to the process plugin by following the instructions here:

https://github.com/dprint/dprint-plugin-rustfmt/releases/

(Slightly more verbose because a checksum needs to be provided because the plugin doesn't run sandboxed—unlike Wasm plugins)