Merge pull request #2593 from topecongiro/issue-2455

Add reorder_impl_items config option

Author: nrc

Configurations.md

@@ -1413,6 +1413,42 @@ mod sit;
 **Note** `mod` with `#[macro_export]` will not be reordered since that could change the semantic
 of the original source code.
 
+## `reorder_impl_items`
+
+Reorder impl items. `type` and `const` are put first, then macros and methods.
+
+- **Default value**: `false`
+- **Possible values**: `true`, `false`
+- **Stable**: No
+
+#### `false` (default)
+
+```rust
+struct Dummy;
+
+impl Iterator for Dummy {
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+
+    type Item = i32;
+}
+```
+
+#### `true`
+
+```rust
+struct Dummy;
+
+impl Iterator for Dummy {
+    type Item = i32;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+}
+```
+
 ## `report_todo`
 
 Report `TODO` items in comments.

src/config/mod.rs

@@ -75,6 +75,7 @@ create_config! {
     reorder_imported_names: bool, true, false,
         "Reorder lists of names in import statements alphabetically";
     reorder_modules: bool, true, false, "Reorder module statemtents alphabetically in group";
+    reorder_impl_items: bool, false, false, "Reorder impl items";
 
     // Spaces around punctuation
     binop_separator: SeparatorPlace, SeparatorPlace::Front, false,

src/items.rs

@@ -11,7 +11,7 @@
 // Formatting top-level items - functions, structs, enums, traits, impls.
 
 use std::borrow::Cow;
-use std::cmp::min;
+use std::cmp::{min, Ordering};
 
 use config::lists::*;
 use regex::Regex;
@@ -660,12 +660,55 @@ pub fn format_impl(
 
         if !items.is_empty() || contains_comment(&snippet[open_pos..]) {
             let mut visitor = FmtVisitor::from_context(context);
-            visitor.block_indent = offset.block_only().block_indent(context.config);
+            let item_indent = offset.block_only().block_indent(context.config);
+            visitor.block_indent = item_indent;
             visitor.last_pos = item.span.lo() + BytePos(open_pos as u32);
 
             visitor.visit_attrs(&item.attrs, ast::AttrStyle::Inner);
-            for item in items {
-                visitor.visit_impl_item(item);
+            if context.config.reorder_impl_items() {
+                // Create visitor for each items, then reorder them.
+                let mut buffer = vec![];
+                for item in items {
+                    visitor.visit_impl_item(item);
+                    buffer.push((visitor.buffer.clone(), item.clone()));
+                    visitor.buffer.clear();
+                }
+                // type -> const -> macro -> method
+                use ast::ImplItemKind::*;
+                fn need_empty_line(a: &ast::ImplItemKind, b: &ast::ImplItemKind) -> bool {
+                    match (a, b) {
+                        (Type(..), Type(..)) | (Const(..), Const(..)) => false,
+                        _ => true,
+                    }
+                }
+
+                buffer.sort_by(|(_, a), (_, b)| match (&a.node, &b.node) {
+                    (Type(..), _) => Ordering::Less,
+                    (_, Type(..)) => Ordering::Greater,
+                    (Const(..), _) => Ordering::Less,
+                    (_, Const(..)) => Ordering::Greater,
+                    (Macro(..), _) => Ordering::Less,
+                    (_, Macro(..)) => Ordering::Greater,
+                    _ => Ordering::Less,
+                });
+                let mut prev_kind = None;
+                for (buf, item) in buffer {
+                    // Make sure that there are at least a single empty line between
+                    // different impl items.
+                    if prev_kind
+                        .as_ref()
+                        .map_or(false, |prev_kind| need_empty_line(prev_kind, &item.node))
+                    {
+                        visitor.push_str("\n");
+                    }
+                    visitor.push_str(&item_indent.to_string_with_newline(context.config));
+                    visitor.push_str(buf.trim());
+                    prev_kind = Some(item.node.clone());
+                }
+            } else {
+                for item in items {
+                    visitor.visit_impl_item(item);
+                }
             }
 
             visitor.format_missing(item.span.hi() - BytePos(1));

tests/source/configs/reorder_impl_items/false.rs

@@ -0,0 +1,11 @@
+// rustfmt-reorder_impl_items: false
+
+struct Dummy;
+
+impl Iterator for Dummy {
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+
+    type Item = i32;
+}

tests/source/configs/reorder_impl_items/true.rs

@@ -0,0 +1,11 @@
+// rustfmt-reorder_impl_items: true
+
+struct Dummy;
+
+impl Iterator for Dummy {
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+
+    type Item = i32;
+}

tests/target/configs/reorder_impl_items/false.rs

@@ -0,0 +1,11 @@
+// rustfmt-reorder_impl_items: false
+
+struct Dummy;
+
+impl Iterator for Dummy {
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+
+    type Item = i32;
+}

tests/target/configs/reorder_impl_items/true.rs

@@ -0,0 +1,11 @@
+// rustfmt-reorder_impl_items: true
+
+struct Dummy;
+
+impl Iterator for Dummy {
+    type Item = i32;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+}