Repository: kbknapp/clap-rs
Pull request #1244 (2018-04-07 13:36:19)

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.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/kbknapp/clap-rs/1244)
<!-- Reviewable:end -->



Patch:
diff --git a/src/macros.rs b/src/macros.rs
index 66778ffc..384a3bf7 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


Test patch:
diff --git a/tests/macros.rs b/tests/macros.rs
old mode 100644
new mode 100755
index fcd62c6f..9823fe9c
--- 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,
+            }
+        }
+    };
 }