From da2d4aced258bdc443c90d8b3de6b127912e2af0 Mon Sep 17 00:00:00 2001
From: Tom Kirchner <git@halffull.org>
Date: Sun, 7 Jul 2019 18:05:43 -0700
Subject: [PATCH] Add error checks for duplicate attributes

---
 .../tests/ui/attribute-duplication.rs         | 28 +++++++++++
 .../tests/ui/attribute-duplication.stderr     | 47 +++++++++++++++++++
 snafu-derive/src/lib.rs                       | 39 +++++++++++++--
 3 files changed, 110 insertions(+), 4 deletions(-)
 create mode 100644 compatibility-tests/compile-fail/tests/ui/attribute-duplication.rs
 create mode 100644 compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr

diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-duplication.rs b/compatibility-tests/compile-fail/tests/ui/attribute-duplication.rs
new file mode 100644
index 00000000..3ea85be8
--- /dev/null
+++ b/compatibility-tests/compile-fail/tests/ui/attribute-duplication.rs
@@ -0,0 +1,28 @@
+mod enum_duplication {
+    use snafu::Snafu;
+
+    #[derive(Debug, Snafu)]
+    enum EnumError {
+        #[snafu(display("an error variant"))]
+        AVariant {
+            #[snafu(source(from(EnumError, Box::new)))]
+            #[snafu(backtrace(delegate))]
+            source: Box<EnumError>,
+            #[snafu(source)]
+            #[snafu(backtrace(delegate))]
+            source2: String,
+            #[snafu(source)]
+            #[snafu(backtrace(delegate))]
+            source3: String,
+
+            #[snafu(backtrace)]
+            backtrace1: String,
+            #[snafu(backtrace)]
+            backtrace2: String,
+            #[snafu(backtrace)]
+            backtrace3: String,
+        },
+    }
+}
+
+fn main() {}
diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr b/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr
new file mode 100644
index 00000000..dcc1f1a3
--- /dev/null
+++ b/compatibility-tests/compile-fail/tests/ui/attribute-duplication.stderr
@@ -0,0 +1,47 @@
+error: Only one field can be designated as 'source'; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:16:13
+   |
+16 |             source3: String,
+   |             ^^^^^^^
+
+error: Only one field can be designated as 'source'; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:13:13
+   |
+13 |             source2: String,
+   |             ^^^^^^^
+
+error: Only one field can be designated as 'backtrace'; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:23:13
+   |
+23 |             backtrace3: String,
+   |             ^^^^^^^^^^
+
+error: Only one field can be designated as 'backtrace'; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:21:13
+   |
+21 |             backtrace2: String,
+   |             ^^^^^^^^^^
+
+error: Only one field can be designated as 'backtrace(delegate)'; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:16:13
+   |
+16 |             source3: String,
+   |             ^^^^^^^
+
+error: Only one field can be designated as 'backtrace(delegate)'; found a duplicate here:
+  --> $DIR/attribute-duplication.rs:13:13
+   |
+13 |             source2: String,
+   |             ^^^^^^^
+
+error: Cannot have 'backtrace' and 'backtrace(delegate)' fields in the same enum
+  --> $DIR/attribute-duplication.rs:19:13
+   |
+19 |             backtrace1: String,
+   |             ^^^^^^^^^^
+
+error: Cannot have 'backtrace' and 'backtrace(delegate)' fields in the same enum
+  --> $DIR/attribute-duplication.rs:10:13
+   |
+10 |             source: Box<EnumError>,
+   |             ^^^^^^
diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs
index 3afd8013..b6801e18 100644
--- a/snafu-derive/src/lib.rs
+++ b/snafu-derive/src/lib.rs
@@ -284,15 +284,46 @@ fn parse_snafu_enum(
                 }
             }
 
+            // Reverse the lists of fields where we may find duplicates so that we can call the
+            // second (and later) items the "duplicates" without uglier indexing.
+            source_fields.reverse();
+            backtrace_fields.reverse();
+            backtrace_delegates.reverse();
+
             let source_field = source_fields.pop();
-            // Report a warning if there are multiple?
+            for extra_field in source_fields.into_iter() {
+                errors.push(syn::Error::new(
+                    extra_field.name.span(),
+                    "Only one field can be designated as 'source'; found a duplicate here:",
+                ));
+            }
 
             let backtrace_field = backtrace_fields.pop();
-            // Report a warning if there are multiple?
+            for extra_field in backtrace_fields.into_iter() {
+                errors.push(syn::Error::new(
+                    extra_field.name.span(),
+                    "Only one field can be designated as 'backtrace'; found a duplicate here:",
+                ));
+            }
 
             let backtrace_delegate = backtrace_delegates.pop();
-            // Report a warning if there are multiple?
-            // Report a warning if delegating and our own?
+            for extra_field in backtrace_delegates.into_iter() {
+                errors.push(syn::Error::new(
+                    extra_field.name.span(),
+                    "Only one field can be designated as 'backtrace(delegate)'; found a duplicate here:",
+                ));
+            }
+
+            if backtrace_field.is_some() && backtrace_delegate.is_some() {
+                errors.push(syn::Error::new(
+                    backtrace_field.as_ref().unwrap().name.span(),
+                    "Cannot have 'backtrace' and 'backtrace(delegate)' fields in the same enum",
+                ));
+                errors.push(syn::Error::new(
+                    backtrace_delegate.as_ref().unwrap().name.span(),
+                    "Cannot have 'backtrace' and 'backtrace(delegate)' fields in the same enum",
+                ));
+            }
 
             Ok(VariantInfo {
                 name,