Feature Request: Add reorder_impl_blocks option

[JonathanWoollett-Light]

Discussion for reorder_impl_blocks configuration option. Implemented by: TBD.

This option enforces ordering of impl blocks such that struct implementations precede trait implementations.

struct MyStruct { ... }
impl fmt::Debug for MyStrict { ... }
impl MyStruct { ... }
impl Drop for MyStruct { ... }

to

struct MyStruct { ... }
impl MyStruct { ... }
impl fmt::Debug for MyStrict { ... }
impl Drop for MyStruct { ... }

[ytmimi]

Interesting! Any implementation would need to play nice with reorder_impl_items.

---

Questions:

Would this be a binary option like reorder_impl_items? I imagine users might want to specify their own ordering, so that's something we'd need to consider.

Would the following be considered ordered?

struct MyStruct { ... }

fn one() {. .. }

impl MyStruct { ... }

fn two() {. .. }

impl fmt::Debug for MyStrict { ... }

fn three() {. .. }

impl Drop for MyStruct { ... }

or would we need to hoist all the impls to come after the struct defintion?

struct MyStruct { ... }

impl MyStruct { ... }

impl fmt::Debug for MyStrict { ... }

impl Drop for MyStruct { ... }

fn one() {. .. }

fn two() {. .. }

fn three() {. .. }

what happens when the item is defined in a different file than the impl?

// a.rs
struct MyStruct { ... }

// b.rs
impl MyStruct { ... }

What happens when the item is defined in another crate, and we want to implement traits defined in our crate. How do those get ordered? (Essentially the same question as the one above).

use a::MyStruct;

trait LocalTraitOne { ... }
trait LocalTraitTwo { ... }

impl LocalTraitOne for MyStruct { ... }

impl LocalTraitTwo for MyStruct { ... }

What happens when there's more than one item (e.g struct One; struct Two;)? Assuming we hoist the impls and anchor them after the item definition is this what you'd expect:

Input:

struct Two;

struct One;

impl fmt::Debug for One { ... }

impl fmt::Debug for Two { ... }

impl One { ... }

impl Two { ... }

impl Drop for One { ... }

impl Drop for Two { ... }

output (Note: Two comes first because it was defined first in the input):

struct Two;

impl Two { ... }

impl fmt::Debug for Two { ... }

impl Drop for Two { ... }

struct One;

impl One { ... }

impl fmt::Debug for One { ... }

impl Drop for One { ... }

Again, the two previous questions about the ordering when we don't have an item to anchor the other definitions to might complicate things here.

What about the order of generic items with both generic and concrete implementations?

struct Foo<T> { ... };

impl<T> Foo<T> { ... };
impl<T> Foo<Vec<T>> { ... };
impl Foo<bool> { ... };
impl Foo<i32> { ... };

---

Limitations:

Not sure if this is something you'd like out of this feature request, but rustfmt doesn't do any macro expansion, so it would be impossible for us to know if a macro added an impl for a trait, and reorder the macro call along with the impl blocks. For example:

macro_rules! add_custom_trait {
    ($name:ident) => {
        impl CustomTrait for $name {...}
    };
}

add_custom_trait(One)
add_custom_trait(Two)

[JonathanWoollett-Light]

Would the following be considered ordered?

Yes, as the narrow scope is only struct vs trait implementations. The rule could be extended to more generic block ordering (but I think this overlaps with other rules and would require other changes).

what happens when the item is defined in a different file than the impl?

Even without the struct definition in the file it would ideally still guarantee that trait implementations for any given type are after their respective struct implementation.

impl LocalTraitOne for MyOtherStruct { ... }
impl LocalTraitOne for MyStruct { ... }
impl MyStruct { ... }
impl LocalTraitTwo for MyStruct { ... }

to

impl LocalTraitOne for MyOtherStruct { ... }
impl MyStruct { ... }
impl LocalTraitOne for MyStruct { ... }
impl LocalTraitTwo for MyStruct { ... }

In essence the rule only specifies that all impl T will precede all impl Y for T, so in any case we only take the minimum action necessary.

In specific functionality I would favour simply swapping impl T and impl Y for T blocks to achieve the order so it doesn't affect the ordering or position of other blocks in a file.

For your example

struct Two;

struct One;

impl fmt::Debug for One { ... }

impl fmt::Debug for Two { ... }

impl One { ... }

impl Two { ... }

impl Drop for One { ... }

impl Drop for Two { ... }

becomes:

struct Two;

struct One;

impl One { ... }

impl Two { ... }

impl fmt::Debug for One { ... }

impl fmt::Debug for Two { ... }

impl Drop for One { ... }

impl Drop for Two { ... }

What about the order of generic items with both generic and concrete implementations?

There would be no required changes here to meet this rule.

Not sure if this is something you'd like out of this feature request, but rustfmt doesn't do any macro expansion, so it would be impossible for us to know if a macro added an impl for a trait, and reorder the macro call along with the impl blocks. For example:

Macros are difficult even if we consider widening this rule to ordering of various blocks. For now I would just ignore them.