From be18a8f23818c409c069d0f129de5aa9fa50a2d0 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 27 Jan 2024 17:30:58 -0700 Subject: [PATCH] Replace `custom_where` attribute with `where` --- crates/bevy_asset/src/handle.rs | 2 +- crates/bevy_asset/src/id.rs | 2 +- .../src/container_attributes.rs | 44 ++++++++++++------- .../bevy_reflect_derive/src/derive_data.rs | 12 ++--- .../bevy_reflect_derive/src/lib.rs | 8 ++-- .../bevy_reflect_derive/src/utility.rs | 14 +++--- crates/bevy_reflect/src/lib.rs | 37 +++++++--------- 7 files changed, 63 insertions(+), 56 deletions(-) diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index 60c9e7fda5d39..fa9f3ee2dc1dd 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -122,7 +122,7 @@ impl std::fmt::Debug for StrongHandle { /// [`Handle::Strong`] also provides access to useful [`Asset`] metadata, such as the [`AssetPath`] (if it exists). #[derive(Component, Reflect)] #[reflect(Component)] -#[reflect(custom_where(A: Asset))] +#[reflect(where A: Asset)] pub enum Handle { /// A "strong" reference to a live (or loading) [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept /// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata. diff --git a/crates/bevy_asset/src/id.rs b/crates/bevy_asset/src/id.rs index 6d903486767a7..a11fcad7746ff 100644 --- a/crates/bevy_asset/src/id.rs +++ b/crates/bevy_asset/src/id.rs @@ -17,7 +17,7 @@ use thiserror::Error; /// /// For an "untyped" / "generic-less" id, see [`UntypedAssetId`]. #[derive(Reflect)] -#[reflect(custom_where(A: Asset))] +#[reflect(where A: Asset)] pub enum AssetId { /// A small / efficient runtime identifier that can be used to efficiently look up an asset stored in [`Assets`]. This is /// the "default" identifier used for assets. The alternative(s) (ex: [`AssetId::Uuid`]) will only be used if assets are diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index f21d27d20a8d5..b8dc979cb7b04 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -7,13 +7,13 @@ use crate::utility; use bevy_macro_utils::fq_std::{FQAny, FQOption}; -use proc_macro2::{Ident, Span}; +use proc_macro2::{Ident, Span, TokenTree}; use quote::quote_spanned; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::token::Comma; -use syn::{Expr, LitBool, Meta, Path, Token, WherePredicate}; +use syn::{Expr, LitBool, Meta, MetaList, Path, WhereClause}; // The "special" trait idents that are used internally for reflection. // Received via attributes like `#[reflect(PartialEq, Hash, ...)]` @@ -31,9 +31,6 @@ const FROM_REFLECT_ATTR: &str = "from_reflect"; // Attributes for `TypePath` implementation const TYPE_PATH_ATTR: &str = "type_path"; -// Attributes for `Reflect` implementation -const CUSTOM_WHERE_ATTR: &str = "custom_where"; - // The error message to show when a trait/type is specified multiple times const CONFLICTING_TYPE_DATA_MESSAGE: &str = "conflicting type data registration"; @@ -214,12 +211,29 @@ pub(crate) struct ReflectTraits { partial_eq: TraitImpl, from_reflect_attrs: FromReflectAttrs, type_path_attrs: TypePathAttrs, - custom_where: Option>, + custom_where: Option, idents: Vec, } impl ReflectTraits { - pub fn from_metas( + pub fn from_meta_list( + meta: &MetaList, + is_from_reflect_derive: bool, + ) -> Result { + match meta.tokens.clone().into_iter().next() { + // Handles `#[reflect(where T: Trait, U::Assoc: Trait)]` + Some(TokenTree::Ident(ident)) if ident == "where" => Ok(Self { + custom_where: Some(meta.parse_args::()?), + ..Self::default() + }), + _ => Self::from_metas( + meta.parse_args_with(Punctuated::::parse_terminated)?, + is_from_reflect_derive, + ), + } + } + + fn from_metas( metas: Punctuated, is_from_reflect_derive: bool, ) -> Result { @@ -257,12 +271,6 @@ impl ReflectTraits { } } } - // Handles `#[reflect(custom_where(T: Trait, U::Assoc: Trait))]` - Meta::List(list) if list.path.is_ident(CUSTOM_WHERE_ATTR) => { - let predicate: Punctuated = - list.parse_args_with(Punctuated::parse_terminated)?; - traits.merge_custom_where(Some(predicate)); - } // Handles `#[reflect( Debug(custom_debug_fn) )]` Meta::List(list) if list.path.is_ident(DEBUG_ATTR) => { let ident = list.path.get_ident().unwrap(); @@ -290,7 +298,9 @@ impl ReflectTraits { Meta::List(list) => { return Err(syn::Error::new_spanned( list, - format!("expected one of [{DEBUG_ATTR:?}, {PARTIAL_EQ_ATTR:?}, {HASH_ATTR:?}, {CUSTOM_WHERE_ATTR:?}]") + format!( + "expected one of [{DEBUG_ATTR:?}, {PARTIAL_EQ_ATTR:?}, {HASH_ATTR:?}]" + ), )); } Meta::NameValue(pair) => { @@ -408,7 +418,7 @@ impl ReflectTraits { } } - pub fn custom_where(&self) -> Option<&Punctuated> { + pub fn custom_where(&self) -> Option<&WhereClause> { self.custom_where.as_ref() } @@ -430,10 +440,10 @@ impl ReflectTraits { Ok(()) } - fn merge_custom_where(&mut self, other: Option>) { + fn merge_custom_where(&mut self, other: Option) { match (&mut self.custom_where, other) { (Some(this), Some(other)) => { - this.extend(other); + this.predicates.extend(other.predicates); } (None, Some(other)) => { self.custom_where = Some(other); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 1e6328a76e38c..cfdf641714abf 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -167,10 +167,8 @@ impl<'a> ReflectDerive<'a> { } reflect_mode = Some(ReflectMode::Normal); - let new_traits = ReflectTraits::from_metas( - meta_list.parse_args_with(Punctuated::::parse_terminated)?, - is_from_reflect_derive, - )?; + let new_traits = + ReflectTraits::from_meta_list(meta_list, is_from_reflect_derive)?; traits.merge(new_traits)?; } Meta::List(meta_list) if meta_list.path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => { @@ -182,10 +180,8 @@ impl<'a> ReflectDerive<'a> { } reflect_mode = Some(ReflectMode::Value); - let new_traits = ReflectTraits::from_metas( - meta_list.parse_args_with(Punctuated::::parse_terminated)?, - is_from_reflect_derive, - )?; + let new_traits = + ReflectTraits::from_meta_list(meta_list, is_from_reflect_derive)?; traits.merge(new_traits)?; } Meta::Path(path) if path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index 99fdc654097dc..55d1fc845caa5 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -131,7 +131,7 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name"; /// This is useful for when a type can't or shouldn't implement `TypePath`, /// or if a manual implementation is desired. /// -/// ## `#[reflect(custom_where(T: Trait, U::Assoc: Trait, ...))]` +/// ## `#[reflect(where T: Trait, U::Assoc: Trait, ...)]` /// /// By default, the derive macro will automatically add certain trait bounds to all generic type parameters /// in order to make them compatible with reflection without the user needing to add them manually. @@ -147,7 +147,7 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name"; /// in general. /// /// This means that if you want to opt-out of the default bounds for _all_ type parameters, -/// you can add `#[reflect(custom_where())]` to the container item to indicate +/// you can add `#[reflect(where)]` to the container item to indicate /// that an empty `where` clause should be used. /// /// ### Example @@ -158,7 +158,7 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name"; /// } /// /// #[derive(Reflect)] -/// #[reflect(custom_where(T::Assoc: FromReflect))] +/// #[reflect(where T::Assoc: FromReflect)] /// struct Foo where T::Assoc: Default { /// value: T::Assoc, /// } @@ -192,7 +192,7 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name"; /// or allowing the use of types that do not implement `Reflect` within the container. /// /// If the field contains a generic type parameter, you will likely need to add a -/// [`#[reflect(custom_where())]`](#reflectcustom_wheret-trait-uassoc-trait-) +/// [`#[reflect(where)]`](#reflectwheret-trait-uassoc-trait-) /// attribute to the container in order to avoid the default bounds being applied to the type parameter. /// /// ## `#[reflect(skip_serializing)]` diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 51d658f3f9d16..b50b0b92eb8a7 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -112,7 +112,7 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { /// /// This will only add bounds for generic type parameters. /// - /// If the container has a `#[reflect(custom_where(...))]` attribute, + /// If the container has a `#[reflect(where)]` attribute, /// this method will extend the type parameters with the _required_ bounds. /// If the attribute is not present, it will extend the type parameters with the _additional_ bounds. /// @@ -138,7 +138,7 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { /// /// It has type parameters `T` and `U`. /// - /// Since there is no `#[reflect(custom_where(...))]` attribute, this method will extend the type parameters + /// Since there is no `#[reflect(where)]` attribute, this method will extend the type parameters /// with the additional bounds: /// /// ```ignore (bevy_reflect is not accessible from this crate) @@ -150,7 +150,7 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { /// If we had this struct: /// ```ignore (bevy_reflect is not accessible from this crate) /// #[derive(Reflect)] - /// #[reflect(custom_where(T: FromReflect + Default))] + /// #[reflect(where T: FromReflect + Default)] /// struct Foo { /// a: T, /// #[reflect(ignore)] @@ -158,7 +158,7 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { /// } /// ``` /// - /// Since there is a `#[reflect(custom_where(...))]` attribute, this method will extend the type parameters + /// Since there is a `#[reflect(where)]` attribute, this method will extend the type parameters /// with _just_ the required bounds along with the predicates specified in the attribute: /// /// ```ignore (bevy_reflect is not accessible from this crate) @@ -181,7 +181,11 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { // Add additional reflection trait bounds let types = self.type_param_idents(); - let custom_where = self.meta.traits().custom_where(); + let custom_where = self + .meta + .traits() + .custom_where() + .map(|clause| &clause.predicates); let trait_bounds = self.trait_bounds(); generic_where_clause.extend(quote! { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 4501bd35aa1f3..1d6ba6ecebe27 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -541,6 +541,7 @@ mod tests { ser::{to_string_pretty, PrettyConfig}, Deserializer, }; + use static_assertions::{assert_impl_all, assert_not_impl_all}; use std::{ any::TypeId, borrow::Cow, @@ -1866,40 +1867,36 @@ bevy_reflect::tests::Test { #[test] fn should_allow_custom_where() { #[derive(Reflect)] - #[reflect(custom_where(T: Default))] + #[reflect(where T: Default)] struct Foo(String, #[reflect(ignore)] PhantomData); #[derive(Default, TypePath)] struct Bar; - #[derive(Reflect)] - struct Baz { - a: Foo, - b: Foo, - } + #[derive(TypePath)] + struct Baz; + + assert_impl_all!(Foo: Reflect); + assert_not_impl_all!(Foo: Reflect); } #[test] fn should_allow_empty_custom_where() { #[derive(Reflect)] - #[reflect(custom_where())] + #[reflect(where)] struct Foo(String, #[reflect(ignore)] PhantomData); #[derive(TypePath)] struct Bar; - #[derive(Reflect)] - struct Baz { - a: Foo, - b: Foo, - } + assert_impl_all!(Foo: Reflect); } #[test] fn should_allow_multiple_custom_where() { #[derive(Reflect)] - #[reflect(custom_where(T: Default + FromReflect))] - #[reflect(custom_where(U: std::ops::Add + FromReflect))] + #[reflect(where T: Default + FromReflect)] + #[reflect(where U: std::ops::Add + FromReflect)] struct Foo(T, U); #[derive(Reflect)] @@ -1907,6 +1904,9 @@ bevy_reflect::tests::Test { a: Foo, b: Foo, } + + assert_impl_all!(Foo: Reflect); + assert_not_impl_all!(Foo: Reflect); } #[test] @@ -1917,7 +1917,7 @@ bevy_reflect::tests::Test { // We don't need `T` to be `Reflect` since we only care about `T::Assoc` #[derive(Reflect)] - #[reflect(custom_where(T::Assoc: FromReflect))] + #[reflect(where T::Assoc: FromReflect)] struct Foo(T::Assoc); #[derive(TypePath)] @@ -1927,10 +1927,7 @@ bevy_reflect::tests::Test { type Assoc = usize; } - #[derive(Reflect)] - struct Baz { - a: Foo, - } + assert_impl_all!(Foo: Reflect); } #[test] @@ -1969,7 +1966,7 @@ bevy_reflect::tests::Test { fn can_opt_out_type_path() { #[derive(Reflect)] #[reflect(type_path = false)] - #[reflect(custom_where())] + #[reflect(where)] struct Foo { #[reflect(ignore)] _marker: PhantomData,