bevy_reflect: Fix trailing comma breaking derives (#8014)

# Objective

Fixes #7989

Based on #7991 by @CoffeeVampir3

## Solution

There were three parts to this issue:
1. `extend_where_clause` did not account for the optionality of a where
clause's trailing comma
    ```rust
    // OKAY
    struct Foo<T> where T: Asset, {/* ... */}
    // ERROR
    struct Foo<T> where T: Asset {/* ... */}
    ```
2. `FromReflect` derive logic was not actively using
`extend_where_clause` which led to some inconsistencies (enums weren't
adding _any_ additional bounds even)
3. Using `extend_where_clause` in the `FromReflect` derive logic meant
we had to optionally add `Default` bounds to ignored fields iff the
entire item itself was not already `Default` (otherwise the definition
for `Handle<T>` wouldn't compile since `HandleType` doesn't impl
`Default` but `Handle<T>` itself does)

---

## Changelog

- Fixed issue where a missing trailing comma could break the reflection
derives

crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs

@@ -3,7 +3,7 @@ use crate::derive_data::ReflectEnum;
 use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors};
 use crate::field_attributes::DefaultBehavior;
 use crate::fq_std::{FQAny, FQClone, FQDefault, FQOption};
-use crate::utility::ident_or_index;
+use crate::utility::{extend_where_clause, ident_or_index, WhereClauseOptions};
 use crate::{ReflectMeta, ReflectStruct};
 use proc_macro::TokenStream;
 use proc_macro2::Span;
@@ -49,8 +49,20 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
 
     let (impl_generics, ty_generics, where_clause) =
         reflect_enum.meta().generics().split_for_impl();
+
+    // Add FromReflect bound for each active field
+    let where_from_reflect_clause = extend_where_clause(
+        where_clause,
+        &WhereClauseOptions {
+            active_types: reflect_enum.active_types().into_boxed_slice(),
+            ignored_types: reflect_enum.ignored_types().into_boxed_slice(),
+            active_trait_bounds: quote!(#bevy_reflect_path::FromReflect),
+            ignored_trait_bounds: quote!(#FQDefault),
+        },
+    );
+
     TokenStream::from(quote! {
-        impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause  {
+        impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_from_reflect_clause  {
             fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
                 if let #bevy_reflect_path::ReflectRef::Enum(#ref_value) = #bevy_reflect_path::Reflect::reflect_ref(#ref_value) {
                     match #bevy_reflect_path::Enum::variant_name(#ref_value) {
@@ -89,11 +101,11 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
         Ident::new("Struct", Span::call_site())
     };
 
-    let field_types = reflect_struct.active_types();
     let MemberValuePair(active_members, active_values) =
         get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple);
 
-    let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) {
+    let is_defaultable = reflect_struct.meta().traits().contains(REFLECT_DEFAULT);
+    let constructor = if is_defaultable {
         quote!(
             let mut __this: Self = #FQDefault::default();
             #(
@@ -120,16 +132,19 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
     let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
 
     // Add FromReflect bound for each active field
-    let mut where_from_reflect_clause = if where_clause.is_some() {
-        quote! {#where_clause}
-    } else if !active_members.is_empty() {
-        quote! {where}
-    } else {
-        quote! {}
-    };
-    where_from_reflect_clause.extend(quote! {
-        #(#field_types: #bevy_reflect_path::FromReflect,)*
-    });
+    let where_from_reflect_clause = extend_where_clause(
+        where_clause,
+        &WhereClauseOptions {
+            active_types: reflect_struct.active_types().into_boxed_slice(),
+            ignored_types: reflect_struct.ignored_types().into_boxed_slice(),
+            active_trait_bounds: quote!(#bevy_reflect_path::FromReflect),
+            ignored_trait_bounds: if is_defaultable {
+                quote!()
+            } else {
+                quote!(#FQDefault)
+            },
+        },
+    );
 
     TokenStream::from(quote! {
         impl #impl_generics #bevy_reflect_path::FromReflect for #struct_name #ty_generics #where_from_reflect_clause

crates/bevy_reflect/bevy_reflect_derive/src/utility.rs

@@ -122,8 +122,9 @@ pub(crate) fn extend_where_clause(
     let active_trait_bounds = &where_clause_options.active_trait_bounds;
     let ignored_trait_bounds = &where_clause_options.ignored_trait_bounds;
 
-    let mut generic_where_clause = if where_clause.is_some() {
-        quote! {#where_clause}
+    let mut generic_where_clause = if let Some(where_clause) = where_clause {
+        let predicates = where_clause.predicates.iter();
+        quote! {where #(#predicates,)*}
     } else if !(active_types.is_empty() && ignored_types.is_empty()) {
         quote! {where}
     } else {

crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/bounds.pass.rs

@@ -0,0 +1,282 @@
+use bevy_reflect::prelude::*;
+
+fn main() {}
+
+#[derive(Default)]
+struct NonReflect;
+
+struct NonReflectNonDefault;
+
+mod structs {
+    use super::*;
+
+    #[derive(Reflect)]
+    struct ReflectGeneric<T> {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    struct FromReflectGeneric<T> {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    struct DefaultGeneric<T> {
+        foo: Option<T>,
+        #[reflect(ignore)]
+        _ignored: NonReflectNonDefault,
+    }
+
+    impl<T> Default for DefaultGeneric<T> {
+        fn default() -> Self {
+            Self {
+                foo: None,
+                _ignored: NonReflectNonDefault,
+            }
+        }
+    }
+
+    #[derive(Reflect)]
+    struct ReflectBoundGeneric<T: Clone> {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    struct FromReflectBoundGeneric<T: Clone> {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    struct DefaultBoundGeneric<T: Clone> {
+        foo: Option<T>,
+        #[reflect(ignore)]
+        _ignored: NonReflectNonDefault,
+    }
+
+    impl<T: Clone> Default for DefaultBoundGeneric<T> {
+        fn default() -> Self {
+            Self {
+                foo: None,
+                _ignored: NonReflectNonDefault,
+            }
+        }
+    }
+
+    #[derive(Reflect)]
+    struct ReflectGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    struct FromReflectGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    struct DefaultGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        foo: Option<T>,
+        #[reflect(ignore)]
+        _ignored: NonReflectNonDefault,
+    }
+
+    impl<T> Default for DefaultGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        fn default() -> Self {
+            Self {
+                foo: None,
+                _ignored: NonReflectNonDefault,
+            }
+        }
+    }
+
+    #[derive(Reflect)]
+    #[rustfmt::skip]
+    struct ReflectGenericWithWhereNoTrailingComma<T>
+        where
+            T: Clone
+    {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    #[rustfmt::skip]
+    struct FromReflectGenericWithWhereNoTrailingComma<T>
+        where
+            T: Clone
+    {
+        foo: T,
+        #[reflect(ignore)]
+        _ignored: NonReflect,
+    }
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    #[rustfmt::skip]
+    struct DefaultGenericWithWhereNoTrailingComma<T>
+        where
+            T: Clone
+    {
+        foo: Option<T>,
+        #[reflect(ignore)]
+        _ignored: NonReflectNonDefault,
+    }
+
+    impl<T> Default for DefaultGenericWithWhereNoTrailingComma<T>
+    where
+        T: Clone,
+    {
+        fn default() -> Self {
+            Self {
+                foo: None,
+                _ignored: NonReflectNonDefault,
+            }
+        }
+    }
+}
+
+mod tuple_structs {
+    use super::*;
+
+    #[derive(Reflect)]
+    struct ReflectGeneric<T>(T, #[reflect(ignore)] NonReflect);
+
+    #[derive(Reflect, FromReflect)]
+    struct FromReflectGeneric<T>(T, #[reflect(ignore)] NonReflect);
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    struct DefaultGeneric<T>(Option<T>, #[reflect(ignore)] NonReflectNonDefault);
+
+    impl<T> Default for DefaultGeneric<T> {
+        fn default() -> Self {
+            Self(None, NonReflectNonDefault)
+        }
+    }
+
+    #[derive(Reflect)]
+    struct ReflectBoundGeneric<T: Clone>(T, #[reflect(ignore)] NonReflect);
+
+    #[derive(Reflect, FromReflect)]
+    struct FromReflectBoundGeneric<T: Clone>(T, #[reflect(ignore)] NonReflect);
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    struct DefaultBoundGeneric<T: Clone>(Option<T>, #[reflect(ignore)] NonReflectNonDefault);
+
+    impl<T: Clone> Default for DefaultBoundGeneric<T> {
+        fn default() -> Self {
+            Self(None, NonReflectNonDefault)
+        }
+    }
+
+    #[derive(Reflect)]
+    struct ReflectGenericWithWhere<T>(T, #[reflect(ignore)] NonReflect)
+    where
+        T: Clone;
+
+    #[derive(Reflect, FromReflect)]
+    struct FromReflectGenericWithWhere<T>(T, #[reflect(ignore)] NonReflect)
+    where
+        T: Clone;
+
+    #[derive(Reflect, FromReflect)]
+    #[reflect(Default)]
+    struct DefaultGenericWithWhere<T>(Option<T>, #[reflect(ignore)] NonReflectNonDefault)
+    where
+        T: Clone;
+
+    impl<T> Default for DefaultGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        fn default() -> Self {
+            Self(None, NonReflectNonDefault)
+        }
+    }
+}
+
+mod enums {
+    use super::*;
+
+    #[derive(Reflect)]
+    enum ReflectGeneric<T> {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect, FromReflect)]
+    enum FromReflectGeneric<T> {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect)]
+    enum ReflectBoundGeneric<T: Clone> {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect, FromReflect)]
+    enum FromReflectBoundGeneric<T: Clone> {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect)]
+    enum ReflectGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect, FromReflect)]
+    enum FromReflectGenericWithWhere<T>
+    where
+        T: Clone,
+    {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect)]
+    #[rustfmt::skip]
+    enum ReflectGenericWithWhereNoTrailingComma<T>
+        where
+            T: Clone
+    {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+
+    #[derive(Reflect, FromReflect)]
+    #[rustfmt::skip]
+    enum FromReflectGenericWithWhereNoTrailingComma<T>
+        where
+            T: Clone
+    {
+        Foo(T, #[reflect(ignore)] NonReflect),
+    }
+}