Report errors on unsupported attributes

`darling` pre-dates Rust support for non-meta attributes.
These increase flexibility for Rust code, but mean that not `darling`
does not parse all valid attributes.

`darling` previously ignored these outright, creating misleading errors.
This commit makes those explicit errors.

Fixes #96

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## Unreleased
+- Error when encountering an attribute `darling` has been asked to parse that isn't a supported shape [#113](https://github.com/TedDriggs/darling/pull/113)
+
 ## v0.11.0 (December 14, 2020)
 - Bump minor version due to unexpected breaking change [#107](https://github.com/TedDriggs/darling/issues/107)
 

core/src/codegen/attr_extractor.rs

@@ -57,13 +57,28 @@ pub trait ExtractAttribute {
             let core_loop = self.core_loop();
             quote!(
                 #(#attr_names)|* => {
-                    if let ::darling::export::Ok(::syn::Meta::List(ref __data)) = __attr.parse_meta() {
-                        let __items = &__data.nested;
-
-                        #core_loop
-                    } else {
-                        // darling currently only supports list-style
-                        continue
+                    match __attr.parse_meta() {
+                        ::darling::export::Ok(::syn::Meta::List(ref __data)) => {
+                            let __items = &__data.nested;
+
+                            #core_loop
+                        }
+                        // An attribute containing only a path is valid and empty as far as the
+                        // extractor is concerned. It's a bit strange, but it's not a problem.
+                        ::darling::export::Ok(::syn::Meta::Path(_)) => {
+                            continue;
+                        }
+                        // A name-value pair indicates the caller doesn't understand how to invoke
+                        // the proc-macro. This is worth giving the caller an error.
+                        ::darling::export::Ok(::syn::Meta::NameValue(ref __nv)) => {
+                            __errors.push(::darling::Error::name_value_attribute().with_span(&__nv));
+                        }
+                        // Failure to parse as a meta could indicate a typing error or a lack of
+                        // understanding the limits of the meta syntax. In either case, it's worth
+                        // an error message.
+                        ::darling::export::Err(__err) => {
+                            __errors.push(::darling::Error::non_meta_attribute(&__err));
+                        }
                     }
                 }
             )

core/src/error/kind.rs

@@ -15,6 +15,7 @@ pub(in error) enum ErrorKind {
     Custom(String),
     DuplicateField(FieldName),
     MissingField(FieldName),
+    UnsupportedAttribute(ErrorUnsupportedAttribute),
     UnsupportedShape(DeriveInputShape),
     UnknownField(ErrorUnknownField),
     UnexpectedFormat(MetaFormat),
@@ -39,6 +40,7 @@ impl ErrorKind {
             DuplicateField(_) => "Duplicate field",
             MissingField(_) => "Missing field",
             UnknownField(_) => "Unexpected field",
+            UnsupportedAttribute(_) => "Unsupported attribute",
             UnsupportedShape(_) => "Unsupported shape",
             UnexpectedFormat(_) => "Unexpected meta-item format",
             UnexpectedType(_) => "Unexpected literal type",
@@ -69,6 +71,7 @@ impl fmt::Display for ErrorKind {
             DuplicateField(ref field) => write!(f, "Duplicate field `{}`", field),
             MissingField(ref field) => write!(f, "Missing field `{}`", field),
             UnknownField(ref field) => field.fmt(f),
+            UnsupportedAttribute(ref error) => error.fmt(f),
             UnsupportedShape(ref shape) => write!(f, "Unsupported shape `{}`", shape),
             UnexpectedFormat(ref format) => write!(f, "Unexpected meta-item format `{}`", format),
             UnexpectedType(ref ty) => write!(f, "Unexpected literal type `{}`", ty),
@@ -102,6 +105,12 @@ impl From<ErrorUnknownField> for ErrorKind {
     }
 }
 
+impl From<ErrorUnsupportedAttribute> for ErrorKind {
+    fn from(err: ErrorUnsupportedAttribute) -> Self {
+        ErrorKind::UnsupportedAttribute(err)
+    }
+}
+
 /// An error for an unknown field, with a possible "did-you-mean" suggestion to get
 /// the user back on the right track.
 #[derive(Debug)]
@@ -199,3 +208,26 @@ where
 {
     None
 }
+
+/// Error when an attribute doesn't parse into the expected `Meta` shape, blocking further
+/// parsing.
+#[derive(Debug)]
+// Don't want to publicly commit to ErrorKind supporting equality yet, but
+// not having it makes testing very difficult.
+#[cfg_attr(test, derive(Clone, PartialEq, Eq))]
+pub(in error) enum ErrorUnsupportedAttribute {
+    /// The attribute wasn't a `Meta` at all. The value is presented as the inner error message.
+    NotMeta(String),
+    NameValue,
+}
+
+impl fmt::Display for ErrorUnsupportedAttribute {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ErrorUnsupportedAttribute::NotMeta(e) => write!(f, "Unable to parse attribute: {}", e),
+            ErrorUnsupportedAttribute::NameValue => {
+                write!(f, "Name-value attributes are not supported")
+            }
+        }
+    }
+}

core/src/error/mod.rs

@@ -17,7 +17,7 @@ use syn::{Lit, LitStr, Path};
 
 mod kind;
 
-use self::kind::{ErrorKind, ErrorUnknownField};
+use self::kind::{ErrorKind, ErrorUnknownField, ErrorUnsupportedAttribute};
 
 /// An alias of `Result` specific to attribute parsing.
 pub type Result<T> = ::std::result::Result<T, Error>;
@@ -70,6 +70,21 @@ impl Error {
         Error::duplicate_field(&path_to_string(path))
     }
 
+    /// Creates a new error for an attribute that is a single name-value pair, as opposed to the
+    /// expected `Meta::List` form.
+    pub fn name_value_attribute() -> Self {
+        Error::new(ErrorUnsupportedAttribute::NameValue.into())
+    }
+
+    /// Creates a new error for an attribute that `syn` failed to parse into a `Meta`, blocking
+    /// further parsing.
+    pub fn non_meta_attribute(error: &syn::Error) -> Self {
+        Self {
+            span: Some(error.span()),
+            ..Error::new(ErrorUnsupportedAttribute::NotMeta(error.to_string()).into())
+        }
+    }
+
     /// Creates a new error for a non-optional field that does not appear in the input.
     pub fn missing_field(name: &str) -> Self {
         Error::new(ErrorKind::MissingField(name.into()))

tests/unsupported_attributes.rs

@@ -0,0 +1,72 @@
+#[macro_use]
+extern crate darling;
+#[macro_use]
+extern crate syn;
+
+use darling::FromDeriveInput;
+use syn::{Ident, LitStr, Path};
+
+#[derive(Debug, FromDeriveInput)]
+#[darling(supports(struct_unit), attributes(bar))]
+pub struct Bar {
+    pub ident: Ident,
+    pub st: Path,
+    pub file: LitStr,
+}
+
+/// Per [#96](https://github.com/TedDriggs/darling/issues/96), make sure that an
+/// attribute which isn't a valid meta gets an error.
+#[test]
+fn non_meta_attribute_gets_own_error() {
+    let di = parse_quote! {
+        #[derive(Bar)]
+        #[bar(file = "motors/example_6.csv", st = RocketEngine)]
+        pub struct EstesC6;
+    };
+
+    let errors: darling::Error = Bar::from_derive_input(&di).unwrap_err().flatten();
+    // The number of errors here is 1 for the bad attribute + 2 for the missing fields
+    assert_eq!(3, errors.len());
+    // Make sure one of the errors propagates the syn error
+    assert!(errors
+        .into_iter()
+        .any(|e| e.to_string().contains("expected lit")));
+}
+
+/// Properties can be split across multiple attributes; this test ensures that one
+/// non-meta attribute does not interfere with the parsing of other, well-formed attributes.
+#[test]
+fn non_meta_attribute_does_not_block_others() {
+    let di = parse_quote! {
+        #[derive(Bar)]
+        #[bar(st = RocketEngine)]
+        #[bar(file = "motors/example_6.csv")]
+        pub struct EstesC6;
+    };
+
+    let errors: darling::Error = Bar::from_derive_input(&di).unwrap_err().flatten();
+    // The number of errors here is 1 for the bad attribute + 1 for the missing "st" field
+    assert_eq!(2, errors.len());
+    // Make sure one of the errors propagates the syn error
+    assert!(errors
+        .into_iter()
+        .any(|e| e.to_string().contains("expected lit")));
+}
+
+#[test]
+fn name_value_attribute_gets_own_error() {
+    let di = parse_quote! {
+        #[derive(Bar)]
+        #[bar = "motors/example_6.csv"]
+        pub struct EstesC6;
+    };
+
+    let errors: darling::Error = Bar::from_derive_input(&di).unwrap_err().flatten();
+    // The number of errors here is 1 for the bad attribute + 2 for the missing fields
+    assert_eq!(3, errors.len());
+    let expected_message = darling::Error::name_value_attribute().to_string();
+    // Make sure one of the errors propagates the syn error
+    assert!(errors
+        .into_iter()
+        .any(|e| e.to_string() == expected_message));
+}