bevy_reflect: Fix combined field attributes (#9322)

# Objective

It seems the behavior of field attributes was accidentally broken at
some point. Take the following code:

```rust
#[derive(Reflect)]
struct Foo {
  #[reflect(ignore, default)]
  value: usize
}
```

The above code should simply mark `value` as ignored and specify a
default behavior. However, what this actually does is discard both.
That's especially a problem when we don't want the field to be be given
a `Reflect` or `FromReflect` bound (which is why we ignore it in the
first place).

This only happens when the attributes are combined into one. The
following code works properly:

```rust
#[derive(Reflect)]
struct Foo {
  #[reflect(ignore)]
  #[reflect(default)]
  value: usize
}
```

## Solution

Cleaned up the field attribute parsing logic to support combined field
attributes.

---

## Changelog

- Fixed a bug where `Reflect` derive attributes on fields are not able
to be combined into a single attribute

crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs

@@ -5,9 +5,8 @@
 //! the derive helper attribute for `Reflect`, which looks like: `#[reflect(ignore)]`.
 
 use crate::REFLECT_ATTRIBUTE_NAME;
-use quote::ToTokens;
+use syn::meta::ParseNestedMeta;
-use syn::spanned::Spanned;
+use syn::{Attribute, LitStr, Token};
-use syn::{Attribute, Expr, ExprLit, Lit, Meta};
 
 pub(crate) static IGNORE_SERIALIZATION_ATTR: &str = "skip_serializing";
 pub(crate) static IGNORE_ALL_ATTR: &str = "ignore";
@@ -78,7 +77,8 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result<ReflectFieldAttr,
         .iter()
         .filter(|a| a.path().is_ident(REFLECT_ATTRIBUTE_NAME));
     for attr in attrs {
-        if let Err(err) = parse_meta(&mut args, &attr.meta) {
+        let result = attr.parse_nested_meta(|meta| parse_meta(&mut args, meta));
+        if let Err(err) = result {
             if let Some(ref mut error) = errors {
                 error.combine(err);
             } else {
@@ -94,54 +94,53 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result<ReflectFieldAttr,
     }
 }
 
-/// Recursively parses attribute metadata for things like `#[reflect(ignore)]` and `#[reflect(default = "foo")]`
+fn parse_meta(args: &mut ReflectFieldAttr, meta: ParseNestedMeta) -> Result<(), syn::Error> {
-fn parse_meta(args: &mut ReflectFieldAttr, meta: &Meta) -> Result<(), syn::Error> {
+    if meta.path.is_ident(DEFAULT_ATTR) {
-    match meta {
+        // Allow:
-        Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => {
+        // - `#[reflect(default)]`
-            (args.ignore == ReflectIgnoreBehavior::None)
+        // - `#[reflect(default = "path::to::func")]`
-                .then(|| args.ignore = ReflectIgnoreBehavior::IgnoreSerialization)
+        if !matches!(args.default, DefaultBehavior::Required) {
-                .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed")))
+            return Err(meta.error(format!("only one of [{:?}] is allowed", [DEFAULT_ATTR])));
         }
-        Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => {
+
-            (args.ignore == ReflectIgnoreBehavior::None)
+        if meta.input.peek(Token![=]) {
-                .then(|| args.ignore = ReflectIgnoreBehavior::IgnoreAlways)
+            let lit = meta.value()?.parse::<LitStr>()?;
-                .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed")))
+            args.default = DefaultBehavior::Func(lit.parse()?);
-        }
+        } else {
-        Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => {
             args.default = DefaultBehavior::Default;
-            Ok(())
         }
-        Meta::Path(path) => Err(syn::Error::new(
+
-            path.span(),
+        Ok(())
-            format!("unknown attribute parameter: {}", path.to_token_stream()),
+    } else if meta.path.is_ident(IGNORE_ALL_ATTR) {
-        )),
+        // Allow:
-        Meta::NameValue(pair) if pair.path.is_ident(DEFAULT_ATTR) => {
+        // - `#[reflect(ignore)]`
-            if let Expr::Lit(ExprLit {lit: Lit::Str(lit_str), ..}) = &pair.value {
+        if args.ignore != ReflectIgnoreBehavior::None {
-                args.default = DefaultBehavior::Func(lit_str.parse()?);
+            return Err(meta.error(format!(
-                Ok(())
+                "only one of [{:?}] is allowed",
-            }
+                [IGNORE_ALL_ATTR, IGNORE_SERIALIZATION_ATTR]
-            else {
+            )));
-                Err(syn::Error::new(
-                    pair.span(),
-                    format!("expected a string literal containing the name of a function, but found: {}", pair.to_token_stream()),
-                ))?
-            }
         }
-        Meta::NameValue(pair) => {
+
-            let path = &pair.path;
+        args.ignore = ReflectIgnoreBehavior::IgnoreAlways;
-            Err(syn::Error::new(
+
-                path.span(),
+        Ok(())
-                format!("unknown attribute parameter: {}", path.to_token_stream()),
+    } else if meta.path.is_ident(IGNORE_SERIALIZATION_ATTR) {
-            ))
+        // Allow:
-        }
+        // - `#[reflect(skip_serializing)]`
-        Meta::List(list) if !list.path.is_ident(REFLECT_ATTRIBUTE_NAME) => {
+        if args.ignore != ReflectIgnoreBehavior::None {
-            Err(syn::Error::new(list.path.span(), "unexpected property"))
+            return Err(meta.error(format!(
-        }
+                "only one of [{:?}] is allowed",
-        Meta::List(list) => {
+                [IGNORE_ALL_ATTR, IGNORE_SERIALIZATION_ATTR]
-            if let Ok(meta) = list.parse_args() {
+            )));
-                parse_meta(args, &meta)?;
-            }
-            Ok(())
         }
+
+        args.ignore = ReflectIgnoreBehavior::IgnoreSerialization;
+
+        Ok(())
+    } else {
+        Err(meta.error(format!(
+            "unknown attribute, expected {:?}",
+            [DEFAULT_ATTR, IGNORE_ALL_ATTR, IGNORE_SERIALIZATION_ATTR]
+        )))
     }
 }

crates/bevy_reflect/bevy_reflect_derive/src/lib.rs

@@ -221,7 +221,7 @@ pub fn derive_reflect(input: TokenStream) -> TokenStream {
 ///
 /// By default, this attribute denotes that the field's type implements [`Default`].
 /// However, it can also take in a path string to a user-defined function that will return the default value.
-/// This takes the form: `#[reflect(default = "path::to::my_function)]` where `my_function` is a parameterless
+/// This takes the form: `#[reflect(default = "path::to::my_function")]` where `my_function` is a parameterless
 /// function that must return some default value for the type.
 ///
 /// Specifying a custom default can be used to give different fields their own specialized defaults,

crates/bevy_reflect/src/lib.rs

@@ -772,18 +772,26 @@ mod tests {
             foo: String,
 
             // Use `get_bar_default()`
-            #[reflect(default = "get_bar_default")]
             #[reflect(ignore)]
-            bar: usize,
+            #[reflect(default = "get_bar_default")]
+            bar: NotReflect,
+
+            // Ensure attributes can be combined
+            #[reflect(ignore, default = "get_bar_default")]
+            baz: NotReflect,
         }
 
-        fn get_bar_default() -> usize {
+        #[derive(Eq, PartialEq, Debug)]
-            123
+        struct NotReflect(usize);
+
+        fn get_bar_default() -> NotReflect {
+            NotReflect(123)
         }
 
         let expected = MyStruct {
             foo: String::default(),
-            bar: 123,
+            bar: NotReflect(123),
+            baz: NotReflect(123),
         };
 
         let dyn_struct = DynamicStruct::default();