Report errors on unsupported attributes (#113)

`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,
such as claiming all fields were missing. This commit makes unsupported
attributes explicit errors.

To allow for detailed diagnostic information in the future without
changing the crate's public API, the parsing of `Attribute` into
`MetaList` is handled in a new function that wraps `syn`'s own
`parse_meta`. This commit uses that structure to include an attribute
path in an example for the name-value error case; future commits could
expand that function to identify places where the caller may have
missed quotation marks, for example.

Fixes #96

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+- POSSIBLY BREAKING: Derived impls of `FromDeriveInput`, `FromField`, `FromVariant`, and `FromTypeParam` will now error when encountering an attribute `darling` has been asked to parse that isn't a supported shape.
+  Any crates using `darling` that relied on those attributes being silently ignored could see new errors reported in their dependent crates. [#113](https://github.com/TedDriggs/darling/pull/113)
+- Impl `syn::spanned::Spanned` for `darling::util::SpannedValue` [#113](https://github.com/TedDriggs/darling/pull/113)
+- Add `darling::util::parse_attribute_to_meta_list` to provide useful errors during attribute parsing [#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,23 @@ 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 ::darling::util::parse_attribute_to_meta_list(__attr) {
+                        ::darling::export::Ok(__data) => {
+                            if __data.nested.is_empty() {
+                                continue;
+                            }
+
+                            let __items = &__data.nested;
+
+                            #core_loop
+                        }
+                        // darling was asked to handle this attribute name, but the actual attribute
+                        // isn't one that darling can work with. This either indicates a typing error
+                        // or some misunderstanding of the meta attribute syntax; in either case, the
+                        // caller should get a useful error.
+                        ::darling::export::Err(__err) => {
+                            __errors.push(__err);
+                        }
                     }
                 }
             )

core/src/util/mod.rs

@@ -8,13 +8,15 @@ use {FromMeta, Result};
 mod ident_string;
 mod ignored;
 mod over_ride;
+mod parse_attribute;
 mod path_list;
 mod spanned_value;
 mod with_original;
 
 pub use self::ident_string::IdentString;
 pub use self::ignored::Ignored;
 pub use self::over_ride::Override;
+pub use self::parse_attribute::parse_attribute_to_meta_list;
 pub use self::path_list::PathList;
 pub use self::spanned_value::SpannedValue;
 pub use self::with_original::WithOriginal;

core/src/util/parse_attribute.rs

@@ -0,0 +1,73 @@
+use crate::{util::SpannedValue, Error, Result};
+use std::fmt;
+use syn::{punctuated::Pair, spanned::Spanned, token, Attribute, Meta, MetaList, Path};
+
+/// Try to parse an attribute into a meta list. Path-type meta values are accepted and returned
+/// as empty lists with their passed-in path. Name-value meta values and non-meta attributes
+/// will cause errors to be returned.
+pub fn parse_attribute_to_meta_list(attr: &Attribute) -> Result<MetaList> {
+    match attr.parse_meta() {
+        Ok(Meta::List(list)) => Ok(list),
+        Ok(Meta::NameValue(nv)) => Err(Error::custom(format!(
+            "Name-value arguments are not supported. Use #[{}(...)]",
+            DisplayPath(&nv.path)
+        ))
+        .with_span(&nv)),
+        Ok(Meta::Path(path)) => Ok(MetaList {
+            path,
+            paren_token: token::Paren(attr.span()),
+            nested: Default::default(),
+        }),
+        Err(e) => Err(Error::custom(format!("Unable to parse attribute: {}", e))
+            .with_span(&SpannedValue::new((), e.span()))),
+    }
+}
+
+struct DisplayPath<'a>(&'a Path);
+
+impl fmt::Display for DisplayPath<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let path = self.0;
+        if path.leading_colon.is_some() {
+            write!(f, "::")?;
+        }
+        for segment in path.segments.pairs() {
+            match segment {
+                Pair::Punctuated(segment, _) => write!(f, "{}::", segment.ident)?,
+                Pair::End(segment) => segment.ident.fmt(f)?,
+            }
+        }
+
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::parse_attribute_to_meta_list;
+    use syn::{parse_quote, spanned::Spanned, Ident};
+
+    #[test]
+    fn parse_list() {
+        let meta = parse_attribute_to_meta_list(&parse_quote!(#[bar(baz = 4)])).unwrap();
+        assert_eq!(meta.nested.len(), 1);
+    }
+
+    #[test]
+    fn parse_path_returns_empty_list() {
+        let meta = parse_attribute_to_meta_list(&parse_quote!(#[bar])).unwrap();
+        assert!(meta.path.is_ident(&Ident::new("bar", meta.path.span())));
+        assert!(meta.nested.is_empty());
+    }
+
+    #[test]
+    fn parse_name_value_returns_error() {
+        parse_attribute_to_meta_list(&parse_quote!(#[bar = 4])).unwrap_err();
+    }
+
+    #[test]
+    fn parse_name_value_error_includes_example() {
+        let err = parse_attribute_to_meta_list(&parse_quote!(#[bar = 4])).unwrap_err();
+        assert!(err.to_string().contains("#[bar(...)]"));
+    }
+}

core/src/util/spanned_value.rs

@@ -62,6 +62,12 @@ impl<T> AsRef<T> for SpannedValue<T> {
     }
 }
 
+impl<T> Spanned for SpannedValue<T> {
+    fn span(&self) -> Span {
+        self.span
+    }
+}
+
 macro_rules! spanned {
     ($trayt:ident, $method:ident, $syn:path) => {
         impl<T: $trayt> $trayt for SpannedValue<T> {

tests/unsupported_attributes.rs

@@ -0,0 +1,54 @@
+#[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")));
+}