From 7fbfd3a4f85121581eaaa321fd31aa3f3c0d3fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Thu, 27 Jun 2024 19:39:49 +0200 Subject: [PATCH 1/2] Validate that optional params appear only at the end --- Cargo.lock | 2 + crates/bevy_plugin/Cargo.toml | 1 + .../src/commands/command_wrapping.rs | 15 ++++ crates/core/Cargo.toml | 3 + crates/core/src/yarn_fn.rs | 1 + crates/core/src/yarn_fn/function_wrapping.rs | 33 +++++++++ crates/core/src/yarn_fn/optionality.rs | 70 +++++++++++++++++++ crates/core/src/yarn_fn/parameter_wrapping.rs | 20 +++++- 8 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 crates/core/src/yarn_fn/optionality.rs diff --git a/Cargo.lock b/Cargo.lock index 9f767593..0c9e067b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1184,6 +1184,7 @@ dependencies = [ "rand", "serde", "sha2", + "static_assertions", "tempfile", "yarnspinner", ] @@ -5020,6 +5021,7 @@ dependencies = [ "bevy", "prost", "serde", + "static_assertions", "yarnspinner_macros", ] diff --git a/crates/bevy_plugin/Cargo.toml b/crates/bevy_plugin/Cargo.toml index a0355b5a..fe90b294 100644 --- a/crates/bevy_plugin/Cargo.toml +++ b/crates/bevy_plugin/Cargo.toml @@ -37,6 +37,7 @@ features = [ [dev-dependencies] tempfile = "3" +static_assertions = "1.1.0" [dev-dependencies.bevy] version = "0.14.0-rc.2" diff --git a/crates/bevy_plugin/src/commands/command_wrapping.rs b/crates/bevy_plugin/src/commands/command_wrapping.rs index 5d8381dd..7c2d619a 100644 --- a/crates/bevy_plugin/src/commands/command_wrapping.rs +++ b/crates/bevy_plugin/src/commands/command_wrapping.rs @@ -334,5 +334,20 @@ pub mod tests { accepts_yarn_command(f); } + macro_rules! assert_is_yarn_command { + (($($param:ty),*) -> $ret:ty) => { + static_assertions::assert_impl_all!(fn($($param),*) -> $ret: YarnCommand $ret>); + }; + } + + macro_rules! assert_is_not_yarn_command { + (($($param:ty),*) -> $ret:ty) => { + static_assertions::assert_not_impl_any!(fn($($param),*) -> $ret: YarnCommand $ret>); + }; + } + + assert_is_yarn_command! { (In<((), Option<()>)>) -> bool } + assert_is_not_yarn_command! { (In<(Option<()>, ())>) -> bool } + fn accepts_yarn_command(_: impl YarnCommand) {} } diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index f98f94ca..4f84ffcf 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -19,3 +19,6 @@ yarnspinner_macros = { path = "../macros", version = "0.1" } prost = "0.12" serde = { version = "1", features = ["derive"], optional = true } bevy = { version = "0.14.0-rc.2", default-features = false, optional = true } + +[dev-dependencies] +static_assertions = "1.1.0" diff --git a/crates/core/src/yarn_fn.rs b/crates/core/src/yarn_fn.rs index 7a44a5b7..51e4d176 100644 --- a/crates/core/src/yarn_fn.rs +++ b/crates/core/src/yarn_fn.rs @@ -4,6 +4,7 @@ mod function_registry; mod function_wrapping; +mod optionality; mod parameter_wrapping; pub(crate) use function_registry::*; diff --git a/crates/core/src/yarn_fn/function_wrapping.rs b/crates/core/src/yarn_fn/function_wrapping.rs index 54c93910..d3db6fab 100644 --- a/crates/core/src/yarn_fn/function_wrapping.rs +++ b/crates/core/src/yarn_fn/function_wrapping.rs @@ -1,3 +1,4 @@ +use super::optionality::AllowedOptionalityChain; use crate::prelude::*; use std::any::TypeId; use std::fmt::{Debug, Display, Formatter}; @@ -179,6 +180,7 @@ macro_rules! impl_yarn_fn_tuple { Fn($(<$param as YarnFnParam>::Item<'a>,)*) -> O, O: IntoYarnValueFromNonYarnValue + 'static, $($param: YarnFnParam + 'static,)* + ($(<$param as YarnFnParam>::Optionality,)*): AllowedOptionalityChain, { type Out = O; #[allow(non_snake_case)] @@ -362,4 +364,35 @@ mod tests { { f.call(input) } + + mod optionality { + use super::*; + + macro_rules! assert_is_yarn_fn { + (($($param:ty),*) -> $ret:ty) => { + static_assertions::assert_impl_all!(fn($($param),*) -> $ret: YarnFn $ret>); + }; + } + + macro_rules! assert_is_not_yarn_fn { + (($($param:ty),*) -> $ret:ty) => { + static_assertions::assert_not_impl_any!(fn($($param),*) -> $ret: YarnFn $ret>); + }; + } + + assert_is_yarn_fn! { (()) -> bool } + assert_is_yarn_fn! { (Option<()>) -> bool } + + assert_is_yarn_fn! { ((), ()) -> bool } + assert_is_yarn_fn! { ((), Option<()>) -> bool } + assert_is_yarn_fn! { (Option<()>, Option<()>) -> bool } + assert_is_not_yarn_fn! { (Option<()>, ()) -> bool } + + assert_is_yarn_fn! { (Option<()>, Option<()>, Option<()>, Option<()>) -> bool } + assert_is_not_yarn_fn! { (Option<()>, Option<()>, Option<()>, ()) -> bool } + + assert_is_yarn_fn! { (((), (), ()), ((), Option<()>), (Option<()>, Option<()>)) -> bool } + assert_is_yarn_fn! { ((), ((), ((), ((), Option<()>)))) -> bool } + assert_is_not_yarn_fn! { ((), ((), ((), ((), Option<()>))), ()) -> bool } + } } diff --git a/crates/core/src/yarn_fn/optionality.rs b/crates/core/src/yarn_fn/optionality.rs new file mode 100644 index 00000000..981986f5 --- /dev/null +++ b/crates/core/src/yarn_fn/optionality.rs @@ -0,0 +1,70 @@ +#![allow(missing_debug_implementations)] + +use yarnspinner_macros::all_tuples; + +/// Marker trait for valid optionality hints. +pub trait Optionality {} + +/// An optional parameter or a tuple where +/// the last element is optional. +pub struct Optional; + +impl Optionality for Optional {} + +/// A parameter that is required. +pub struct Required; + +impl Optionality for Required {} + +/// A valid chain of optionality hints +/// i.e. a chain where no optional element follows +/// a required element. +pub trait AllowedOptionalityChain { + /// The optionality hint of the last element in the chain. + type Last: Optionality; +} + +impl AllowedOptionalityChain for () { + type Last = Required; +} + +impl AllowedOptionalityChain for (O,) { + type Last = O; +} + +impl AllowedOptionalityChain for (Required, O) { + type Last = O; +} + +impl AllowedOptionalityChain for (Optional, Optional) { + type Last = Optional; +} + +// `impl AllowedOptionalityChain for (Optional, Required) {}` +// is intentionally missing (that's the whole point of this trait). + +macro_rules! impl_chain { + // Implementations for zero, one and two-element tuples covered manually. + () => {}; + ($p1:ident) => {}; + ($p1:ident, $p2:ident) => {}; + ($($param:ident),*) => { + // A tuple of three or more elements is valid + // if all two-pairs from left to right are valid. + // example: (A, B, C) is valid if (A, B) and (B, C) are. + impl_chain!(@pairwise [$($param),*] [] $($param,)*); + }; + (@pairwise [$($param:ident),*] [$($tt:tt)*] $a:ident, $b:ident,) => { + impl_chain!(@emit [$($param),*] [$($tt)* ($a, $b): AllowedOptionalityChain,] $b,); + }; + (@pairwise [$($param:ident),*] [$($tt:tt)*] $a:ident, $b:ident, $($tail:ident,)*) => { + impl_chain!(@pairwise [$($param),*] [$($tt)* ($a, $b): AllowedOptionalityChain,] $b, $($tail,)*); + }; + (@emit [$($param: ident),*] [$($tt:tt)*] $last:ident,) => { + impl<$($param: Optionality),*> AllowedOptionalityChain for ($($param),*) where $($tt)* { + type Last = $last; + } + }; +} + +all_tuples!(impl_chain, 0, 16, O); diff --git a/crates/core/src/yarn_fn/parameter_wrapping.rs b/crates/core/src/yarn_fn/parameter_wrapping.rs index 7d47b950..6ced55b3 100644 --- a/crates/core/src/yarn_fn/parameter_wrapping.rs +++ b/crates/core/src/yarn_fn/parameter_wrapping.rs @@ -2,6 +2,7 @@ //! //! Inspired by +use super::optionality::{AllowedOptionalityChain, Optional, Optionality, Required}; use crate::prelude::*; use std::any::Any; use std::borrow::Borrow; @@ -54,7 +55,10 @@ impl YarnValueWrapper { pub trait YarnFnParam { /// The item type returned when constructing this [`YarnFn`] param. The value of this associated type should be `Self`, instantiated with a new lifetime. /// You could think of `YarnFnParam::Item<'new>` as being an operation that changes the lifetime bound to `Self`. - type Item<'new>: YarnFnParam; + type Item<'new>; + + #[doc(hidden)] + type Optionality: Optionality; #[doc(hidden)] fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a>; @@ -65,6 +69,7 @@ pub type YarnFnParamItem<'a, P> =

::Item<'a>; impl YarnFnParam for Option { type Item<'new> = Option>; + type Optionality = Optional; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { if iter.peek().is_some() { @@ -79,8 +84,11 @@ macro_rules! impl_yarn_fn_param_tuple { ($($param: ident),*) => { #[allow(non_snake_case)] impl<$($param,)*> YarnFnParam for ($($param,)*) - where $($param: YarnFnParam,)* { + where $($param: YarnFnParam,)* + ($(<$param as YarnFnParam>::Optionality,)*): AllowedOptionalityChain + { type Item<'new> = ($($param::Item<'new>,)*); + type Optionality = <($(<$param as YarnFnParam>::Optionality,)*) as AllowedOptionalityChain>::Last; #[allow(unused_variables, clippy::unused_unit)] // for n = 0 tuples fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { @@ -107,6 +115,7 @@ where >::Error: Display, { type Item<'new> = ResRef<'new, T>; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { let value = iter.next().expect("Passed too few arguments to YarnFn"); @@ -141,6 +150,7 @@ where U: ?Sized + 'static, { type Item<'new> = ResRefBorrow<'new, T, U>; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { let value = iter.next().expect("Passed too few arguments to YarnFn"); @@ -168,6 +178,7 @@ where >::Error: Display, { type Item<'new> = ResOwned; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { let value = iter.next().expect("Passed too few arguments to YarnFn"); @@ -192,6 +203,7 @@ macro_rules! impl_yarn_fn_param_inner { ($referenced:ty: YarnFnParam) => { impl YarnFnParam for &$referenced { type Item<'new> = &'new $referenced; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { ResRef::<$referenced>::retrieve(iter).value @@ -200,6 +212,7 @@ macro_rules! impl_yarn_fn_param_inner { impl YarnFnParam for $referenced { type Item<'new> = $referenced; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { ResOwned::<$referenced>::retrieve(iter).value @@ -209,6 +222,7 @@ macro_rules! impl_yarn_fn_param_inner { ($referenced:ty => $owned:ty: YarnFnParam) => { impl YarnFnParam for &$referenced { type Item<'new> = &'new $referenced; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { ResRefBorrow::<$owned, $referenced>::retrieve(iter).value @@ -217,6 +231,7 @@ macro_rules! impl_yarn_fn_param_inner { impl YarnFnParam for &$owned { type Item<'new> = &'new $owned; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { ResRef::<$owned>::retrieve(iter).value @@ -225,6 +240,7 @@ macro_rules! impl_yarn_fn_param_inner { impl YarnFnParam for $owned { type Item<'new> = $owned; + type Optionality = Required; fn retrieve<'a>(iter: &mut YarnValueWrapperIter<'a>) -> Self::Item<'a> { ResOwned::<$owned>::retrieve(iter).value From 9d98acc13c12bc1cf897d528c084f0f6a00a3916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Thu, 27 Jun 2024 20:12:58 +0200 Subject: [PATCH 2/2] Add comment --- crates/core/src/yarn_fn/parameter_wrapping.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/core/src/yarn_fn/parameter_wrapping.rs b/crates/core/src/yarn_fn/parameter_wrapping.rs index 6ced55b3..6c443fee 100644 --- a/crates/core/src/yarn_fn/parameter_wrapping.rs +++ b/crates/core/src/yarn_fn/parameter_wrapping.rs @@ -57,6 +57,9 @@ pub trait YarnFnParam { /// You could think of `YarnFnParam::Item<'new>` as being an operation that changes the lifetime bound to `Self`. type Item<'new>; + /// Tracks if this parameter is optional or required. + /// This information is used to disallow required parameters to follow optional ones. + /// See the [`AllowedOptionalityChain`] trait for details. #[doc(hidden)] type Optionality: Optionality;