rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.06k stars 893 forks source link

alphabetical order for type in trait definition also #5078

Open tisonkun opened 3 years ago

tisonkun commented 3 years ago

I have code snippet below:

pub trait DataValue {
    type Value;
    type Mutation;
}

impl DataValue for Int64 {
    type Mutation = Int64Mutation;
    type Value = Int64Value;
}

where rustfmt will force types in the impl block in alphabetical order, but not for the trait block.

IntelliJ complains "Different impl member order from the trait" in this case.

calebcartwright commented 3 years ago

where rustfmt will force types in the impl block in alphabetical order, but not for the trait block.

Could you elaborate on what you mean by this? If I recall correctly the non-default reorder_impl_items will both regroup const and type items within an impl, which includes sorting within those groups, but that should never be "forced".

Are you requesting a new feature that's something of an analog of reorder_impl_items configuration that can be used for a similar non-default sorting of types in traits?

tisonkun commented 3 years ago

@calebcartwright rustfmt will change

impl DataValue for Int64 {
    type Value = Int64Value;
    type Mutation = Int64Mutation;
}

to

impl DataValue for Int64 {
    type Mutation = Int64Mutation;
    type Value = Int64Value;
}

but not applied on the trait. I'd like to apply this rule on trait which means

pub trait DataValue {
    type Value;
    type Mutation;
}

will be formatted to

pub trait DataValue {
    type Mutation;
    type Value;
}
calebcartwright commented 3 years ago

@calebcartwright rustfmt will change

No, rustfmt does not do this. The only way to force rustfmt to make this change in impl blocks is to change the default value of the reorder_impl_items configuration option. You can see this in action on this playground sample.

If you are convinced that the reordering is the default behavior of rustfmt it would be great if you could provide some steps to reproduce with a minimal example, as that would be a bug.

As such, I'd echo my earlier question to you on what specifically you are asking. Are you requesting:

tisonkun commented 3 years ago

@calebcartwright you're right that reorder_impl_items = true is configured.

Either of the below should resolve my request:

reorder_trait_items may be better.

tisonkun commented 3 years ago

@calebcartwright any thoughts?

calebcartwright commented 3 years ago

@calebcartwright any thoughts?

I'd rather go with an explicit, new option (reorder_trait_items) as opposed to expanding the behavior of the existing option, especially given its name and age