From 2f16731581285f19566280d305136cb80a91a9a8 Mon Sep 17 00:00:00 2001 From: Tom Kirchner 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 | 65 +++++++++++++++++-- 3 files changed, 135 insertions(+), 5 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, + #[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, + | ^^^^^^ diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs index d2aa4579..56271081 100644 --- a/snafu-derive/src/lib.rs +++ b/snafu-derive/src/lib.rs @@ -113,7 +113,7 @@ impl SyntaxErrors { /// Adds a new syntax error. The given description will be used in the compile error pointing /// to the given span. Helper structs are available to format common descriptions, e.g. - /// OnlyValidOn. + /// OnlyValidOn and DuplicateAttribute. fn add(&mut self, span: proc_macro2::Span, description: D) where D: fmt::Display, @@ -153,6 +153,22 @@ impl fmt::Display for OnlyValidOn { } } +/// Helper structure to simplify parameters to SyntaxErrors.add, handling cases where an attribute +/// was incorrectly used multiple times on the same element. +struct DuplicateAttribute { + attribute: &'static str, +} + +impl fmt::Display for DuplicateAttribute { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "Only one field can be designated as '{}'; found a duplicate here:", + self.attribute + ) + } +} + fn impl_snafu_macro(ty: syn::DeriveInput) -> TokenStream { match parse_snafu_information(ty) { Ok(info) => info.into(), @@ -406,15 +422,54 @@ 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.add( + extra_field.name.span(), + DuplicateAttribute { + attribute: "source", + }, + ); + } let backtrace_field = backtrace_fields.pop(); - // Report a warning if there are multiple? + for extra_field in backtrace_fields.into_iter() { + errors.add( + extra_field.name.span(), + DuplicateAttribute { + attribute: "backtrace", + }, + ); + } 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.add( + extra_field.name.span(), + DuplicateAttribute { + attribute: "backtrace(delegate)", + }, + ); + } + + if let (Some(backtrace_field), Some(backtrace_delegate)) = + (backtrace_field.as_ref(), backtrace_delegate.as_ref()) + { + errors.add( + backtrace_field.name.span(), + "Cannot have 'backtrace' and 'backtrace(delegate)' fields in the same enum", + ); + errors.add( + backtrace_delegate.name.span(), + "Cannot have 'backtrace' and 'backtrace(delegate)' fields in the same enum", + ); + } Ok(VariantInfo { name,