-
Notifications
You must be signed in to change notification settings - Fork 55
Sort derives #154
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
Comments
The style team discussed this when initially developing the default Rust style and style guide. Our concern at that time was partly that derives are not always reorderable (because a proc-macro derive might care what other derives go inside/outside), and partly that people do sometimes list derives in a specific order for a logical reason. I'm not opposed to changing this, but we'd need to address those points. |
Do you know who can be contacted for more insight on this? I have no knowledge of how ordering derives may make a semantic difference for proc macros, and cannot seem to find any resources online.
This is a good point. I think a reasonable solution would be to treat groups of derives like imports are treated. That is, traits in a single |
Try the rust-lang Zulip under t-lang or t-compiler, or internals.rust-lang.org, and in either case mention specifically that you're seeking examples where a proc-macro derive could be order-dependent relative to other derives. |
The problem is that proc-macros can do whatever they want at compile-time. For example one derive macro could write something to a file and the other would read it (if they were reordered the file might not exist), but to me this seems like an anti-pattern, because proc-macros are designed to be order independent (you can not infer which other traits have been derived from a proc-macro) |
Going to close this. I appreciate the recurring interest in this and that many folks could leverage the feature without running into issues. However, we can't categorically prescribe something within the style guide that could knowingly be problematic and a subtle source of bugs, and similarly I don't want to have a feature in rustfmt that could cause such bugs. |
Could we consider designing the feature so that it won't trigger these bugs? An idea for this: designate a set of well-known traits which we know we can reorder at will. That is, teach Alternatively or as an extension to the above, you could let A system like that would be good enough for many usecases, while still being safe. I think it would make a lot of users happy here. |
I agree with mgeisler, but I would extend it to this: from the derive token tree, perform a take_while that only accepts well-known derives. Then, these can be sorted. let's say, derives starting with Then, the following example: #[derive(A1, A2, A7, A4, B1, B4, B2, A3)] Will only consider A1, A2, A7, and A4, and sort those. This would guarantee that there can not be any weird bugs, as these derives are well understood, and absolutely none of the third-party derives are affected in any way. |
Hi all! In 2017, a request was made about handling the order of Rust derive traits in It’s now 2024, and I'm unsure if this reasoning still applies. I have created a simple example with two traits,
For now, I’ve created a tool to help maintain derive order until this feature is possibly implemented in Thank you! UPD: for one-time modifications one can use sort-derive-traits.github.io |
This PR sorts Rust `#[derive(...)]` traits to maintain a consistent style. The idea comes from a (now closed) feature request to the `rust-fmt` style team for adding default sorting, [link](rust-lang/style-team#154). The sorting follows the 'canonical' order, where `std` traits come first, and the rest follow in alphabetical order. It does not modify autogenerated rust code from protobuf files. The changes were made automatically using the `keepsorted` CLI app, [repo](https://github.com/maksym-arutyunyan/keepsorted), with the `rust_derive_canonical` experimental feature.
You're correct. This has now largely been fixed. There is one remaining sensitivity to execution ordering: one derive can emit That said, this case is:
All of these combinations together make it an excellent candidate, IMO, for a lint with an aim of turning it into a hard error. |
See rust-lang/rustfmt#4112 and rust-lang/rustfmt#1867 for more context and discussion.
This rfc proposes the sorting of items listed in a derive attribute. Sorting derives keeps uniformity in a project, since derive attributes can often list many items, and expecting a consistent order helps readers more quickly scan an item's derives.
Alternatives
These three alternatives are taken from rust-lang/rustfmt#4112; here I enumerate them and provide what I think are advantages/disadvantages of each.
(1) Alphabetical sorting
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
Advantages:
Disadvantages:
Eq
andPartialEq
are often derived together and placed adjacently, implicitly implying a relationship; such a relationship cannot be preserved with alphabetical ordering.(2) "Canonical" sorting
Specifically, by "canonical" sorting I mean the sorting of derivable
std
traits in the order they are listed in the rust API guidelines. Traits not included in that list are ordered alphabetically at the end of the derive list.#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Display, Default, CustomTrait, CrateSpecificTrait)]
Advantages:
Disadvantages:
#[derive(CustomDebug, Injective, Surjective, Bijective)]
a tool like rustfmt cannot (consistently) deduce that the latter three traits are related, and would format the derive as
#[derive(Bijective, CustomDebug, Injective, Surjective)]
I'm not convinced that only partially supporting implicit relationships is a bad thing, but this is important to mention.
(3) Sort by resolved trait path
Given the following derive
try to format the derives alphabetically by their full path
So the original example would be sorted as
Advantages:
Disadvantages:
Risks
Suggestion
My suggestion is to provide options (1): Alphabetical Sorting and (2): "Canonical" Sorting in rustfmt. I hypothesize that the most common use case is to only use std-provided derivable traits; in this situation, "Canonical" sorting is likely the nicest alternative for a user due to its preservation of implicit relationships. For more complex use cases, alphabetical sorting may be more readable and consistent across a code base.
As for the rust style guide, I suggest specifying that derives should be sorted alphabetically.
This is just my opinion; more insights are requested.
The text was updated successfully, but these errors were encountered: