Skip to content

feat(tools): rustfmt sort tool #4614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 30, 2025
Merged

Conversation

marvelshan
Copy link
Contributor

@marvelshan marvelshan commented May 26, 2025

Which issue does this PR close?

closes #4360

Why

This change introduces a deterministic Rust file sorting tool (rustfmt-sort) into the proto code generation pipeline. The motivation is to ensure that all generated Rust files have a stable, alphabetically sorted item order, which:

  • Reduces unnecessary diffs in generated code,
  • Improves reproducibility and code review experience,
  • Makes it easier to track meaningful changes in generated files. This is especially important for large proto-generated codebases where minor changes in proto definitions or generation order can otherwise cause noisy diffs.
    Additionally, due to limitations in the proto generator, a manual patch for the timestamp field in CanonicalVote is still required after generation.

Implementation details

  • Uses syn to parse and mutate the Rust syntax tree, and quote to regenerate the code.
  • Recursively sorts items in modules as well.
  • The tool is invoked automatically as part of the proto generation process.

Modules Affected

  • tools/rustfmt-sort
  • tools/rust-proto.nix
  • tools/tools.nix
  • Cargo.lock
  • Cargo.toml

@marvelshan marvelshan changed the title Rustfmt sort tool fear(tools): Rustfmt sort tool May 26, 2025
@marvelshan marvelshan changed the title fear(tools): Rustfmt sort tool fear(tools): rustfmt sort tool May 26, 2025
Copy link

vercel bot commented May 26, 2025

@marvelshan is attempting to deploy a commit to the unionbuild Team on Vercel.

A member of the Team first needs to authorize it.

@marvelshan marvelshan changed the title fear(tools): rustfmt sort tool feat(tools): rustfmt sort tool May 26, 2025
@marvelshan
Copy link
Contributor Author

@benluelo I tried running the CI for it but ran into a couple of issues that I’m currently stuck on, and I was wondering if you might have any suggestions.

Here are the main errors I encountered:

  1. dprint formatting failed:

    Error resolving plugin /nix/store/.../dprint-plugin-typescript-0.95.4: Is a directory (os error 21)
    

    It seems like dprint is failing to resolve the TypeScript plugin from the Nix store — possibly due to a misreference to a directory instead of a plugin file?

  2. rustfmt errors related to gen keyword:

    error: expected identifier, found reserved keyword `gen`
    

    This occurs in multiple files using schemars::gen::SchemaGenerator, where gen appears to be conflicting with the new gen keyword in Rust 2024 edition. I assume the fix is to rename it to r#gen, but I’m not entirely sure if that’s the expected approach here.

Would you happen to know what might be causing the dprint plugin resolution issue, and do you have any suggestions on how best to address the gen keyword conflicts for rustfmt?
Any pointers would be really appreciated. Thank you!

@benluelo
Copy link
Contributor

@marvelshan are you using the rust version from the repo? this should not be an issue as we have not yet bumped to the 2024 edition. you should use nix develop to enter the devshell for the repo

@marvelshan marvelshan marked this pull request as draft May 26, 2025 15:34
@marvelshan marvelshan force-pushed the rustfmt-sort-tool branch from fc1477c to 0911553 Compare May 26, 2025 17:36
@marvelshan marvelshan force-pushed the rustfmt-sort-tool branch from 0911553 to 67c5724 Compare May 27, 2025 09:03
@marvelshan
Copy link
Contributor Author

@benluelo I'm using the Rust version provided by the repo via nix develop, and I'm inside the dev shell:

(nix:union-devShell-env) zaki@ubuntu:~/union$ echo $IN_NIX_SHELL
impure

(nix:union-devShell-env) zaki@ubuntu:~/union$ rustc --version
rustc 1.87.0-nightly (3f5502370 2025-03-27)

(nix:union-devShell-env) zaki@ubuntu:~/union$ cargo --version
cargo 1.87.0-nightly (a6c604d1b 2025-03-26)

The edition field in Cargo.toml is still set to "2021":

[workspace.package]
edition = "2021"

However, when I run nix fmt, I still get formatting errors that seem to assume edition 2024 is in use:

ERRO formatter | rustfmt: failed to apply with options '[--config skip_children=true --edition 2024]': exit status 1

error: expected identifier, found reserved keyword `gen`

This happens in multiple files, mostly in code using schemars::gen::SchemaGenerator, where gen is being flagged as a reserved keyword (which is new in edition 2024).

Also, I noticed nix fmt prints the following:

evaluation warning: crane requires at least nixpkgs-25.05, supplied nixpkgs-24.11

Could this be causing rustfmt to use the wrong edition somehow? Let me know if there’s something I should adjust in my setup. Thanks!

@benluelo
Copy link
Contributor

you've updated the flake.lock again. revert changes to that file, and don't update it.

@marvelshan
Copy link
Contributor Author

marvelshan commented May 27, 2025

Thank you for your reminder, and I apologize for any unnecessary changes caused by the changes to flake.lock. I have reviewed the Nix documentation regarding flake.lock and have reverted the file to its original state. Please let me know if there is anything else I should adjust.

@marvelshan marvelshan marked this pull request as ready for review May 27, 2025 12:02
@marvelshan marvelshan force-pushed the rustfmt-sort-tool branch from 88686c6 to 46f8fd2 Compare May 27, 2025 14:18
@benluelo benluelo force-pushed the rustfmt-sort-tool branch from fb26b79 to 15416ac Compare May 29, 2025 20:22
Copy link
Contributor

@benluelo benluelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for doing the monke work here, pushed a few more small changes and regenerated the protos

@benluelo benluelo merged commit 947e2a2 into unionlabs:main May 30, 2025
110 of 111 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

write a tool to sort rust files alphabetically
2 participants