-
Notifications
You must be signed in to change notification settings - Fork 925
enhancement request - derive sorting #1867
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
We can't do this because the order of derives can be semantically important (a derive only sees following derives, so in your example, |
Ok didnt know |
Although order of derives can be semantically important, I believe majority of custom derives don't care about other derives, and it could almost certainly be surprising if a custom derive is sensitive to the order. So I think it is probably a good idea for rustfmt to support sorting derives (maybe as an opt-in for now), and discourage custom derive to rely on its position in derive list. There are projects, e.g. Stylo, try to sort the derives. An example can be seen here. Without rustfmt's help, it needs to be maintained manually, which is error-prone. |
I didn't know before now the order is important. If a derive is sensitive to order, then I imagine the confusion from the user in case of an error. |
As I mentioned, I think it should generally be considered a bad practice to have derive order sensitive. Doing so would surprise users anyway, given how few people are aware of it, regardless of whether rustfmt would sort it. |
Even if there is a derive that is dependent on the order, it would be good practice by the maintainer to chose the name in a way so that the order is enforced by rustfmt :) |
This issue should be reopened. Many options could already theoretically break custom proc macros (e.g. all options that modify AST: 1, 2, 3, 4, 5, 6, 7...). That is fine because proc macros shouldn't depend on such details and deserve to break if they do. Same applies here. Plus, this option would be opt-in anyways, so users would know what they're getting into when enabling this option. It would be nice to have this option for serenity |
Any updates on this? Think this is really nice to bring consistency to codebases (less cognitive load when reading code too). |
Seems to be a simple sorting tool only, which doens't address the problem of some derives that depend on others, where order matters, right? |
maybe like you are sorting the 'use' lines and values, you could also sort the derive values, for example
#[derive(Serialize, Deserialize, Debug, Clone)]
can be sorted by abc order and keep the code consistent all over.
The text was updated successfully, but these errors were encountered: