Skip to content

write a tool to sort rust files alphabetically #4360

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.

Sign up for GitHub

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

Jump to bottom
Closed
benluelo opened this issue Apr 16, 2025 · 5 comments · Fixed by #4614
Closed

write a tool to sort rust files alphabetically #4360

benluelo opened this issue Apr 16, 2025 · 5 comments · Fixed by #4614
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@benluelo
Copy link
Contributor

benluelo commented Apr 16, 2025 •
edited
Loading

see rust-lang/rustfmt#2359 (comment) for why. should be trivial to do with syn, just ensure doc comments are retained (shouldn't require any special handling since they parse as #[doc] attributes anyways).

after some experimentation, this is slightly more annoying than i thought it would be. here is a working example, using syn and quote:

syn = { version = "*", features = ["full", "visit-mut", "extra-traits", "printing"] }
quote = "1.0.40"
use std::{env, fs};

use quote::ToTokens;
use syn::{visit_mut::VisitMut, Item};

fn main() {
    let mut file =
        syn::parse_file(&fs::read_to_string(env::args().nth(1).unwrap()).unwrap()).unwrap();

    SortItemsVisitor.visit_file_mut(&mut file);

    let out = file.into_token_stream().to_string();

    println!("{out}");
}

struct SortItemsVisitor;

impl VisitMut for SortItemsVisitor {
    fn visit_file_mut(&mut self, i: &mut syn::File) {
        syn::visit_mut::visit_file_mut(self, i);

        sort_items(&mut i.items);
    }

    fn visit_item_mod_mut(&mut self, i: &mut syn::ItemMod) {
        syn::visit_mut::visit_item_mod_mut(self, i);

        if let Some((_, ref mut content)) = i.content {
            sort_items(content);
        }
    }
}

fn sort_items(content: &mut Vec<Item>) {
    let (uses, mut items) = content
        .drain(..)
        .partition::<Vec<_>, _>(|i| matches!(i, Item::Use(_)));

    // the token display repr should be stable since we pin the rustc version in the repo
    items.sort_by_cached_key(|i| i.to_token_stream().to_string().into_bytes());

    *content = uses.into_iter().chain(items).collect();
}

make a new bin under tools/ called rustfmt-sort, expose it as a package in tools.nix, and use it here in rust-proto.nix, after the rustfmt call. once this is all done, run nix run .#generate-rust-proto and commit the changes in a separate commit. you will need to manually patch generated/rust/protos/src/tendermint.types.rs to add back the timestamp field to CanonicalVote.
@benluelo benluelo added C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Apr 16, 2025
@marvelshan marvelshan mentioned this issue May 25, 2025
Merged
@marvelshan
Copy link
Contributor

take

@benluelo
Copy link
Contributor Author

updated the description @marvelshan, let me know if you have any questions

@marvelshan
Copy link
Contributor

Thanks so much for the clear and detailed explanation, it was super helpful and really clarified a lot for me. I was able to implement everything without issues! That said, I did run into a small hiccup when running the Nix commands. Specifically, I encountered the following error:

error: experimental Nix feature 'nix-command' is disabled; add '--extra-experimental-features nix-command' to enable it

To work around it, I've been running commands like this:

nix --extra-experimental-features 'nix-command flakes' <command>

It seems to be a common issue for users who haven't enabled these experimental features globally. I found this helpful thread that talks about the problem in more detail: https://discourse.nixos.org/t/error-experimental-nix-feature-nix-command-is-disabled/18089
Let me know if there's a preferred way you'd like me to handle this in the repo or if I should update my Nix config permanently. Thanks again!

@benluelo
Copy link
Contributor Author

benluelo commented May 25, 2025 •
edited
Loading

you can add experimental-features = nix-command flakes to your nix.conf (mine is located at /etc/nix/nix.conf). how you set this (either manually or via a nixos-rebuild) depends on how you've installed nix. the link you sent also mentions ~/.config/nix/nix.conf, which i believe should also work.

@marvelshan marvelshan mentioned this issue May 26, 2025
Merged
benluelo added a commit that referenced this issue May 30, 2025
@benluelo
feat(tools): rustfmt sort tool (#4614)
947e2a2 
## 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
Copy link
Contributor

@benluelo I’ve found two issues that I’m interested in working on: #3393 and #3396. Could you please advise which issue would be better for me to start with? Or, do you have any other recommendations for issues that would be suitable for me to work on?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(tools): rustfmt sort tool marvelshan/union
2 participants
@benluelo @marvelshan