From 669a139956681edf13c9fa21f08fa27979d5c345 Mon Sep 17 00:00:00 2001
From: topecongiro <seuchida@gmail.com>
Date: Thu, 24 Aug 2017 23:46:22 +0900
Subject: [PATCH] Only merge consecutive derives

---
 Configurations.md                          |  2 -
 src/visitor.rs                             | 46 +++++++++++++---------
 tests/source/configs-merge_derives-true.rs | 36 +++++++++++++++++
 tests/target/configs-merge_derives-true.rs | 34 +++++++++++++++-
 4 files changed, 96 insertions(+), 22 deletions(-)

diff --git a/Configurations.md b/Configurations.md
index 373447bf24f..972c2e58241 100644
--- a/Configurations.md
+++ b/Configurations.md
@@ -1272,8 +1272,6 @@ Merge multiple derives into a single one.
 - **Default value**: `true`
 - **Possible values**: `true`, `false`
 
-*Note*: The merged derives will be put after all other attributes or doc comments.
-
 #### `true`:
 
 ```rust
diff --git a/src/visitor.rs b/src/visitor.rs
index b0df0bcaf9b..2f6a503dc13 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -927,7 +927,8 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
 
         let mut derive_args = Vec::new();
 
-        for (i, a) in self.iter().enumerate() {
+        let mut iter = self.iter().enumerate().peekable();
+        while let Some((i, a)) = iter.next() {
             let a_str = try_opt!(a.rewrite(context, shape));
 
             // Write comments and blank lines between attributes.
@@ -954,44 +955,51 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
                 } else if multi_line {
                     result.push('\n');
                 }
-                result.push_str(&indent);
+                if derive_args.is_empty() {
+                    result.push_str(&indent);
+                }
             }
 
             // Write the attribute itself.
+            let mut insert_new_line = true;
             if context.config.merge_derives() {
+                // If the attribute is `#[derive(...)]`, take the arguments.
                 if let Some(mut args) = get_derive_args(context, a) {
-                    // If the attribute is `#[derive(...)]`, take the arguments and skip rewriting.
-                    // We will merge the all arguments into a single `#[derive(...)]` at last.
                     derive_args.append(&mut args);
+                    match iter.peek() {
+                        // If the next attribute is `#[derive(...)]` as well, skip rewriting.
+                        Some(&(_, next_attr)) if is_derive(next_attr) => insert_new_line = false,
+                        // If not, rewrite the merged derives.
+                        _ => {
+                            result.push_str(&format!("#[derive({})]", derive_args.join(", ")));
+                            derive_args.clear();
+                        }
+                    }
                 } else {
                     result.push_str(&a_str);
-
-                    if i < self.len() - 1 {
-                        result.push('\n');
-                    }
                 }
             } else {
                 result.push_str(&a_str);
-
-                if i < self.len() - 1 {
-                    result.push('\n');
-                }
             }
-        }
 
-        // Add merged `#[derive(...)]` at last.
-        if context.config.merge_derives() && !derive_args.is_empty() {
-            if !result.is_empty() && !result.ends_with('\n') {
-                result.push_str(&indent);
+            if insert_new_line && i < self.len() - 1 {
                 result.push('\n');
             }
-            result.push_str(&format!("#[derive({})]", derive_args.join(", ")));
         }
-
         Some(result)
     }
 }
 
+fn is_derive(attr: &ast::Attribute) -> bool {
+    match attr.meta() {
+        Some(meta_item) => match meta_item.node {
+            ast::MetaItemKind::List(..) => meta_item.name.as_str() == "derive",
+            _ => false,
+        },
+        _ => false,
+    }
+}
+
 /// Returns the arguments of `#[derive(...)]`.
 fn get_derive_args(context: &RewriteContext, attr: &ast::Attribute) -> Option<Vec<String>> {
     attr.meta().and_then(|meta_item| match meta_item.node {
diff --git a/tests/source/configs-merge_derives-true.rs b/tests/source/configs-merge_derives-true.rs
index 804a48c43c0..18b8443f0d7 100644
--- a/tests/source/configs-merge_derives-true.rs
+++ b/tests/source/configs-merge_derives-true.rs
@@ -8,3 +8,39 @@
 #[foobar]
 #[derive(Copy, Clone)]
 pub enum Foo {}
+
+#[derive(Eq, PartialEq)]
+#[derive(Debug)]
+#[foobar]
+#[derive(Copy, Clone)]
+pub enum Bar {}
+
+#[derive(Eq, PartialEq)]
+#[derive(Debug)]
+#[derive(Copy, Clone)]
+pub enum FooBar {}
+
+mod foo {
+#[bar]
+#[derive(Eq, PartialEq)]
+#[foo]
+#[derive(Debug)]
+#[foobar]
+#[derive(Copy, Clone)]
+pub enum Foo {}
+}
+
+mod bar {
+#[derive(Eq, PartialEq)]
+#[derive(Debug)]
+#[foobar]
+#[derive(Copy, Clone)]
+pub enum Bar {}
+}
+
+mod foobar {
+#[derive(Eq, PartialEq)]
+#[derive(Debug)]
+#[derive(Copy, Clone)]
+pub enum FooBar {}
+}
diff --git a/tests/target/configs-merge_derives-true.rs b/tests/target/configs-merge_derives-true.rs
index 8cf7b9a346d..4d0148b1c6e 100644
--- a/tests/target/configs-merge_derives-true.rs
+++ b/tests/target/configs-merge_derives-true.rs
@@ -2,7 +2,39 @@
 // Merge multiple derives to a single one.
 
 #[bar]
+#[derive(Eq, PartialEq)]
 #[foo]
+#[derive(Debug)]
 #[foobar]
-#[derive(Eq, PartialEq, Debug, Copy, Clone)]
+#[derive(Copy, Clone)]
 pub enum Foo {}
+
+#[derive(Eq, PartialEq, Debug)]
+#[foobar]
+#[derive(Copy, Clone)]
+pub enum Bar {}
+
+#[derive(Eq, PartialEq, Debug, Copy, Clone)]
+pub enum FooBar {}
+
+mod foo {
+    #[bar]
+    #[derive(Eq, PartialEq)]
+    #[foo]
+    #[derive(Debug)]
+    #[foobar]
+    #[derive(Copy, Clone)]
+    pub enum Foo {}
+}
+
+mod bar {
+    #[derive(Eq, PartialEq, Debug)]
+    #[foobar]
+    #[derive(Copy, Clone)]
+    pub enum Bar {}
+}
+
+mod foobar {
+    #[derive(Eq, PartialEq, Debug, Copy, Clone)]
+    pub enum FooBar {}
+}
-- 
2.44.0