Rustfmt: Sort derives

[Luro02]

I always try to keep derives in a certain order, so they are uniform with the rest of the crate. It would be nice if rustfmt could automatically sort them :)

I could imagine sorting them:

• alphabetically:
  rust #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]

• after some sort of priority
  I like the order in which the traits are listed in the rust-api-guidelines

  #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Display, Default)]
  

  I think they simply grouped similar traits together:

  • Copy + Clone
  • Eq + PartialEq
  • Ord + PartialOrd
  • Debug + Display

[calebcartwright]

This was previously requested back in #1867 but rejected given the potential for unintentional and undesirable impacts.

[Luro02]

As I mentioned, I think it should generally be considered a bad practice to have derive order sensitive.

I kinda agree with this statement. I never heard of order specific derives.

[Luro02]

We can't do this because the order of derives can be semantically important (a derive only sees following derives, so in your example, Debug only sees Clone) and re-ordering could thus break some custom derives.

Is the aforementioned behavior documented somewhere?

#[derive(Eq, PartialEq)]
struct Example;

For example shouldn't this fail to compile, because Eq requires PartialEq, which can not be seen by the Eq derive?

btw the above compiles as expected.

Are there any real world proc-macros, where this characteristic is abused?

[calebcartwright]

For example shouldn't this fail to compile

AIUI, it's more that a change of the ordering when custom derives are used could fail to preserve semantics/impact the resulting generated items, not that the ordering of built in derives could cause compilation failures.

If you still feel strongly that there should be a standard ordering of derives that could be enforced by rustfmt then I'd suggest making the request in https://github.com/rust-dev-tools/fmt-rfcs to get that ordering codified in the Style guide. If the Style guide were to be updated to prescribe an ordering for derives then we would update rustfmt accordingly.

[topecongiro]

IIRC reordering derives led to a compilation error against some, including ones from the standard library. That does not seem to be the case anymore, though, so I think having an option to sort derives sounds ok.

[ckaran]

I just wanted to give a +1 to this feature request, and to make a couple of suggestions.

First, Instead of sorting by the derive used directly, sort by the complete path. That is, sort the following:

#[derive(
    Debug,
    Clone,
    PartialEq,
    Eq,
    PartialOrd,
    Ord,
    Hash,
    Serialize,
    Deserialize,
    TypeGenerator,
    CopyGetters,
    Getters,
    Builder,
)]
pub struct Foo{
    ..
}

as if it were actually defined as follows:

#[derive(
    std::fmt::Debug,
    std::clone::Clone,
    std::cmp::PartialEq,
    std::cmp::Eq,
    std::cmp::PartialOrd,
    std::cmp::Ord,
    std::hash::Hash,
    serde::ser::Serialize,
    serde::de::Deserialize,
    bolero::generator::TypeGenerator,
    getset::CopyGetters,
    getset::Getters,
    derive_builder::Builder,
)]
pub struct Foo{
    ..
}

The main advantage of this is that traits that are defined in the same crate will be sorted close to one another. E.g.:

#[derive(
    bolero::generator::TypeGenerator,
    derive_builder::Builder,
    getset::CopyGetters,
    getset::Getters,
    serde::de::Deserialize,
    serde::ser::Serialize,
    std::clone::Clone,
    std::cmp::Eq,
    std::cmp::Ord,
    std::cmp::PartialEq,
    std::cmp::PartialOrd,
    std::fmt::Debug,
    std::hash::Hash,
)]
pub struct Foo{
    ..
}

Second, considering what @calebcartwright said:

This was previously requested back in #1867 but rejected given the potential for unintentional and undesirable impacts.

This sort could be made an option (--sort_derives) that defaults to being false. That way the end user opts into this behavior, and (if they were smart and did a commit before doing a potentially dangerous command) they can always roll back to the prior commit if it doesn't work.

[calebcartwright]

This sort could be made an option (--sort_derives) that defaults to being false. That way the end user opts into this behavior, and (if they were smart and did a commit before doing a potentially dangerous command) they can always roll back to the prior commit if it doesn't work

It would definitely require a config option (not a command line flag) like the other reorder_* options.

However, a config option itself wouldn't have been sufficient to address the original reordering issues because we couldn't have a config option that could knowingly break code.

Fortunately it seems like the problem that blocked the original request may have subsequently been resolved and is no longer an issue.

In order to proceed though, someone would need to submit a request in the style guide/rfc repo (https://github.com/rust-dev-tools/fmt-rfcs) with the proposal.

Going through the style guide rfc process to get the available ordering options (and default ordering) added to the style guide (that rustfmt would then be able to benefit implement) should also help provide definitive clarity on whether the original problems with reordering still exist.

[ayazhafiz]

Opened RFC: https://github.com/rust-dev-tools/fmt-rfcs/issues/154