forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bevy_reflect: Automatic arg count validation (#15145)

Browse Source 
# Objective

Functions created into `DynamicFunction[Mut]` do not currently validate
the number of arguments they are given before calling the function.

I originally did this because I felt users would want to validate this
themselves in the function rather than have it be done
behind-the-scenes. I'm now realizing, however, that we could remove this
boilerplate and if users wanted to check again then they would still be
free to do so (it'd be more of a sanity check at that point).

## Solution

Automatically validate the number of arguments passed to
`DynamicFunction::call` and `DynamicFunctionMut::call[_once]`.

This is a pretty trivial change since we just need to compare the length
of the `ArgList` to the length of the `[ArgInfo]` in the function's
`FunctionInfo`.

I also ran the benchmarks just in case and saw no regression by doing
this.

## Testing

You can test locally by running:

```
cargo test --package bevy_reflect --all-features
```
This commit is contained in:
Gino Valente 2024-09-23 10:03:14 -07:00 committed by GitHub
parent e312da8c52
commit 6e95f297ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 117 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
crates/bevy_reflect/src/func/dynamic_function.rs
View File

@ -1,9 +1,9 @@
use crate as bevy_reflect;
use crate::__macro_exports::RegisterForReflection;
use crate::func::args::{ArgInfo, ArgList};
use crate::func::args::ArgList;
use crate::func::info::FunctionInfo;
use crate::func::{
DynamicFunctionMut, Function, FunctionResult, IntoFunction, IntoFunctionMut, ReturnInfo,
DynamicFunctionMut, Function, FunctionError, FunctionResult, IntoFunction, IntoFunctionMut,
};
use crate::serde::Serializable;
use crate::{
@ -64,8 +64,10 @@ impl<'env> DynamicFunction<'env> {
/// The given function can be used to call out to any other callable,
/// including functions, closures, or methods.
///
/// It's important that the function signature matches the provided [`FunctionInfo`].
/// This info may be used by consumers of this function for validation and debugging.
/// It's important that the function signature matches the provided [`FunctionInfo`]
/// as this will be used to validate arguments when [calling] the function.
///
/// [calling]: DynamicFunction::call
pub fn new<F: for<'a> Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'env>(
func: F,
info: FunctionInfo,
@ -89,21 +91,6 @@ impl<'env> DynamicFunction<'env> {
self
}
/// Set the argument information of the function.
///
/// It's important that the arguments match the intended function signature,
/// as this can be used by consumers of this function for validation and debugging.
pub fn with_args(mut self, args: Vec<ArgInfo>) -> Self {
self.info = self.info.with_args(args);
self
}
/// Set the return information of the function.
pub fn with_return_info(mut self, return_info: ReturnInfo) -> Self {
self.info = self.info.with_return_info(return_info);
self
}
/// Call the function with the given arguments.
///
/// # Example
@ -120,8 +107,25 @@ impl<'env> DynamicFunction<'env> {
/// let result = func.call(args).unwrap().unwrap_owned();
/// assert_eq!(result.try_take::<i32>().unwrap(), 123);
/// ```
///
/// # Errors
///
/// This method will return an error if the number of arguments provided does not match
/// the number of arguments expected by the function's [`FunctionInfo`].
///
/// The function itself may also return any errors it needs to.
pub fn call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a> {
(self.func)(args)
let expected_arg_count = self.info.arg_count();
let received_arg_count = args.len();
if expected_arg_count != received_arg_count {
Err(FunctionError::ArgCountMismatch {
expected: expected_arg_count,
received: received_arg_count,
})
} else {
(self.func)(args)
}
}
/// Returns the function info.
@ -321,6 +325,21 @@ mod tests {
let _: DynamicFunction = make_closure(function);
}
#[test]
fn should_return_error_on_arg_count_mismatch() {
let func = (|a: i32, b: i32| a + b).into_function();
let args = ArgList::default().push_owned(25_i32);
let error = func.call(args).unwrap_err();
assert!(matches!(
error,
FunctionError::ArgCountMismatch {
expected: 2,
received: 1
}
));
}
#[test]
fn should_clone_dynamic_function() {
let hello = String::from("Hello");
@ -379,7 +398,10 @@ mod tests {
}
},
// The `FunctionInfo` doesn't really matter for this test
FunctionInfo::anonymous(),
// so we can just give it dummy information.
FunctionInfo::anonymous()
.with_arg::<i32>("curr")
.with_arg::<()>("this"),
);
let args = ArgList::new().push_ref(&factorial).push_owned(5_i32);

89
crates/bevy_reflect/src/func/dynamic_function_mut.rs
View File

@ -1,9 +1,9 @@
use alloc::borrow::Cow;
use core::fmt::{Debug, Formatter};
use crate::func::args::{ArgInfo, ArgList};
use crate::func::args::ArgList;
use crate::func::info::FunctionInfo;
use crate::func::{DynamicFunction, FunctionResult, IntoFunctionMut, ReturnInfo};
use crate::func::{DynamicFunction, FunctionError, FunctionResult, IntoFunctionMut};
/// A dynamic representation of a function.
///
@ -72,8 +72,10 @@ impl<'env> DynamicFunctionMut<'env> {
/// The given function can be used to call out to any other callable,
/// including functions, closures, or methods.
///
/// It's important that the function signature matches the provided [`FunctionInfo`].
/// This info may be used by consumers of this function for validation and debugging.
/// It's important that the function signature matches the provided [`FunctionInfo`]
/// as this will be used to validate arguments when [calling] the function.
///
/// [calling]: DynamicFunctionMut::call
pub fn new<F: for<'a> FnMut(ArgList<'a>) -> FunctionResult<'a> + 'env>(
func: F,
info: FunctionInfo,
@ -97,21 +99,6 @@ impl<'env> DynamicFunctionMut<'env> {
self
}
/// Set the argument information of the function.
///
/// It's important that the arguments match the intended function signature,
/// as this can be used by consumers of this function for validation and debugging.
pub fn with_args(mut self, args: Vec<ArgInfo>) -> Self {
self.info = self.info.with_args(args);
self
}
/// Set the return information of the function.
pub fn with_return_info(mut self, return_info: ReturnInfo) -> Self {
self.info = self.info.with_return_info(return_info);
self
}
/// Call the function with the given arguments.
///
/// Variables that are captured mutably by this function
@ -135,9 +122,26 @@ impl<'env> DynamicFunctionMut<'env> {
/// assert_eq!(result.try_take::<i32>().unwrap(), 100);
/// ```
///
/// # Errors
///
/// This method will return an error if the number of arguments provided does not match
/// the number of arguments expected by the function's [`FunctionInfo`].
///
/// The function itself may also return any errors it needs to.
///
/// [`call_once`]: DynamicFunctionMut::call_once
pub fn call<'a>(&mut self, args: ArgList<'a>) -> FunctionResult<'a> {
(self.func)(args)
let expected_arg_count = self.info.arg_count();
let received_arg_count = args.len();
if expected_arg_count != received_arg_count {
Err(FunctionError::ArgCountMismatch {
expected: expected_arg_count,
received: received_arg_count,
})
} else {
(self.func)(args)
}
}
/// Call the function with the given arguments and consume it.
@ -161,8 +165,25 @@ impl<'env> DynamicFunctionMut<'env> {
/// increment_function.call_once(args).unwrap();
/// assert_eq!(count, 5);
/// ```
///
/// # Errors
///
/// This method will return an error if the number of arguments provided does not match
/// the number of arguments expected by the function's [`FunctionInfo`].
///
/// The function itself may also return any errors it needs to.
pub fn call_once(mut self, args: ArgList) -> FunctionResult {
(self.func)(args)
let expected_arg_count = self.info.arg_count();
let received_arg_count = args.len();
if expected_arg_count != received_arg_count {
Err(FunctionError::ArgCountMismatch {
expected: expected_arg_count,
received: received_arg_count,
})
} else {
(self.func)(args)
}
}
/// Returns the function info.
@ -252,4 +273,30 @@ mod tests {
let closure: DynamicFunctionMut = make_closure(|a: i32, b: i32| total = a + b);
let _: DynamicFunctionMut = make_closure(closure);
}
#[test]
fn should_return_error_on_arg_count_mismatch() {
let mut total = 0;
let mut func = (|a: i32, b: i32| total = a + b).into_function_mut();
let args = ArgList::default().push_owned(25_i32);
let error = func.call(args).unwrap_err();
assert!(matches!(
error,
FunctionError::ArgCountMismatch {
expected: 2,
received: 1
}
));
let args = ArgList::default().push_owned(25_i32);
let error = func.call_once(args).unwrap_err();
assert!(matches!(
error,
FunctionError::ArgCountMismatch {
expected: 2,
received: 1
}
));
}
}

18
examples/reflection/function_reflection.rs
View File

@ -7,7 +7,7 @@
//! processing deserialized reflection data, or even just storing type-erased versions of your functions.
use bevy::reflect::func::{
ArgList, DynamicFunction, DynamicFunctionMut, FunctionError, FunctionInfo, IntoFunction,
ArgList, DynamicFunction, DynamicFunctionMut, FunctionInfo, FunctionResult, IntoFunction,
IntoFunctionMut, Return,
};
use bevy::reflect::{PartialReflect, Reflect};
@ -129,16 +129,10 @@ fn main() {
}
let get_or_insert_function = dbg!(DynamicFunction::new(
|mut args| {
// We can optionally add a check to ensure we were given the correct number of arguments.
if args.len() != 2 {
return Err(FunctionError::ArgCountMismatch {
expected: 2,
received: args.len(),
});
}
|mut args: ArgList| -> FunctionResult {
// The `ArgList` contains the arguments in the order they were pushed.
// The `DynamicFunction` will validate that the list contains
// exactly the number of arguments we expect.
// We can retrieve them out in order (note that this modifies the `ArgList`):
let value = args.take::<i32>()?;
let container = args.take::<&mut Option<i32>>()?;
@ -160,8 +154,8 @@ fn main() {
// such as by using its type name or by prefixing it with your crate name.
.with_name("my_crate::get_or_insert")
// Since our function takes arguments, we should provide that argument information.
// This helps ensure that consumers of the function can validate the arguments they
// pass into the function and helps for debugging.
// This is used to validate arguments when calling the function.
// And it aids consumers of the function with their own validation and debugging.
// Arguments should be provided in the order they are defined in the function.
.with_arg::<i32>("value")
.with_arg::<&mut Option<i32>>("container")