From 7023184fca04e852c270341548d6a16207d13862 Mon Sep 17 00:00:00 2001
From: Bence Kalmar <bkalmar1996@gmail.com>
Date: Sat, 7 Apr 2018 13:30:58 +0200
Subject: [PATCH] fix(arg_enum!): Invalid expansions of some trailing-comma
 patterns

In particular, fix macros that take an enum of one of the the following forms:

 - `#[...] enum { ... , }`
 - `pub enum { ... , }`
 - `enum { ... , }`

Previously, these expansions would result in an error message like "error: no
rules expected the token `:`".

Add extensive tests for each pattern.  Only two of the patterns had tests
before, so these errors did not surface automatically.
---
 src/macros.rs   |  25 ++++++----
 tests/macros.rs | 125 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 123 insertions(+), 27 deletions(-)
 mode change 100644 => 100755 tests/macros.rs

diff --git a/src/macros.rs b/src/macros.rs
index 66778ffc0e4..384a3bf7ca7 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -362,10 +362,11 @@ macro_rules! arg_enum {
         );
     };
     ($(#[$($m:meta),+])+ enum $e:ident { $($v:ident $(=$val:expr)*,)+ } ) => {
-        arg_enum!($(#[$($m:meta),+])+
-            enum $e:ident {
-                $($v:ident $(=$val:expr)*),+
-            }
+        arg_enum!(@impls
+            ($(#[$($m),+])+
+             enum $e {
+                 $($v$(=$val)*),+
+             }) -> ($e, $($v),+)
         );
     };
     ($(#[$($m:meta),+])+ enum $e:ident { $($v:ident $(=$val:expr)*),+ } ) => {
@@ -377,9 +378,11 @@ macro_rules! arg_enum {
         );
     };
     (pub enum $e:ident { $($v:ident $(=$val:expr)*,)+ } ) => {
-        arg_enum!(pub enum $e:ident {
-            $($v:ident $(=$val:expr)*),+
-        });
+        arg_enum!(@impls
+            (pub enum $e {
+                $($v$(=$val)*),+
+            }) -> ($e, $($v),+)
+        );
     };
     (pub enum $e:ident { $($v:ident $(=$val:expr)*),+ } ) => {
         arg_enum!(@impls
@@ -389,9 +392,11 @@ macro_rules! arg_enum {
         );
     };
     (enum $e:ident { $($v:ident $(=$val:expr)*,)+ } ) => {
-        arg_enum!(enum $e:ident {
-            $($v:ident $(=$val:expr)*),+
-        });
+        arg_enum!(@impls
+            (enum $e {
+                $($v$(=$val)*),+
+            }) -> ($e, $($v),+)
+        );
     };
     (enum $e:ident { $($v:ident $(=$val:expr)*),+ } ) => {
         arg_enum!(@impls
diff --git a/tests/macros.rs b/tests/macros.rs
old mode 100644
new mode 100755
index fcd62c6f339..9823fe9ccaa
--- a/tests/macros.rs
+++ b/tests/macros.rs
@@ -251,24 +251,115 @@ fn group_macro_set_not_required() {
 
 #[test]
 fn arg_enum() {
-    arg_enum!{
-        #[derive(Debug, PartialEq, Copy, Clone)]
-        pub enum Greek {
-            Alpha,
-            Bravo
-        }
+    // Helper macros to avoid repetition
+    macro_rules! test_greek {
+        ($arg_enum:item, $tests:block) => {{
+            $arg_enum
+            // FromStr implementation
+            assert!("Charlie".parse::<Greek>().is_err());
+            // Display implementation
+            assert_eq!(format!("{}", Greek::Alpha), "Alpha");
+            assert_eq!(format!("{}", Greek::Bravo), "Bravo");
+            // fn variants()
+            assert_eq!(Greek::variants(), ["Alpha", "Bravo"]);
+            // rest of tests
+            $tests
+        }};
+    }
+    macro_rules! test_greek_no_meta {
+        {$arg_enum:item} => {
+            test_greek!($arg_enum, {
+                // FromStr implementation
+                assert!("Alpha".parse::<Greek>().is_ok());
+                assert!("Bravo".parse::<Greek>().is_ok());
+            })
+        };
+    }
+    macro_rules! test_greek_meta {
+        {$arg_enum:item} => {
+            test_greek!($arg_enum, {
+                // FromStr implementation
+                assert_eq!("Alpha".parse::<Greek>(), Ok(Greek::Alpha));
+                assert_eq!("Bravo".parse::<Greek>(), Ok(Greek::Bravo));
+            })
+        };
     }
-    assert_eq!("Alpha".parse::<Greek>(), Ok(Greek::Alpha));
-}
 
-#[test]
-fn arg_enum_trailing_comma() {
-    arg_enum!{
-        #[derive(Debug, PartialEq, Copy, Clone)]
-        pub enum Greek {
-            Alpha,
-            Bravo,
+    // Tests for each pattern
+    // meta  NO, pub  NO, trailing comma  NO
+    test_greek_no_meta!{
+        arg_enum!{
+            enum Greek {
+                Alpha,
+                Bravo
+            }
         }
-    }
-    assert_eq!("Alpha".parse::<Greek>(), Ok(Greek::Alpha));
+    };
+    // meta  NO, pub  NO, trailing comma YES
+    test_greek_no_meta!{
+        arg_enum!{
+            enum Greek {
+                Alpha,
+                Bravo,
+            }
+        }
+    };
+    // meta  NO, pub YES, trailing comma  NO
+    test_greek_no_meta!{
+        arg_enum!{
+            pub enum Greek {
+                Alpha,
+                Bravo
+            }
+        }
+    };
+    // meta  NO, pub YES, trailing comma YES
+    test_greek_no_meta!{
+        arg_enum!{
+            pub enum Greek {
+                Alpha,
+                Bravo,
+            }
+        }
+    };
+    // meta YES, pub  NO, trailing comma  NO
+    test_greek_meta!{
+        arg_enum!{
+            #[derive(Debug, PartialEq, Copy, Clone)]
+            enum Greek {
+                Alpha,
+                Bravo
+            }
+        }
+    };
+    // meta YES, pub  NO, trailing comma YES
+    test_greek_meta!{
+        arg_enum!{
+            #[derive(Debug, PartialEq, Copy, Clone)]
+            enum Greek {
+                Alpha,
+                Bravo,
+            }
+        }
+    };
+    // meta YES, pub YES, trailing comma  NO
+    test_greek_meta!{
+        arg_enum!{
+            #[derive(Debug, PartialEq, Copy, Clone)]
+            pub enum Greek {
+                Alpha,
+                Bravo
+            }
+        }
+    };
+    // meta YES, pub YES, trailing comma YES
+    test_greek_meta!{
+        arg_enum!{
+            #[derive(Debug, PartialEq, Copy, Clone)]
+            pub enum Greek {
+                Alpha,
+                Bravo,
+            }
+        }
+    };
 }