From f1dc2cbf1817a0df9f45368c92628ca176678470 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Sun, 16 Aug 2020 16:53:25 -0500 Subject: [PATCH 1/3] codegen: update tests for new name attribute behavior --- codegen/tests/test_functions.rs | 2 +- codegen/tests/test_modules.rs | 43 ++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/codegen/tests/test_functions.rs b/codegen/tests/test_functions.rs index 4a179376..2f8a881e 100644 --- a/codegen/tests/test_functions.rs +++ b/codegen/tests/test_functions.rs @@ -1,6 +1,6 @@ use rhai::module_resolvers::*; use rhai::plugin::*; -use rhai::{Array, Engine, EvalAltResult, Module, RegisterFn, FLOAT}; +use rhai::{Engine, EvalAltResult, Module, RegisterFn, FLOAT}; pub mod raw_fn { use rhai::plugin::*; diff --git a/codegen/tests/test_modules.rs b/codegen/tests/test_modules.rs index 56ffb4a5..b9690a5c 100644 --- a/codegen/tests/test_modules.rs +++ b/codegen/tests/test_modules.rs @@ -1,5 +1,5 @@ use rhai::module_resolvers::*; -use rhai::{Engine, EvalAltResult, RegisterFn, FLOAT, INT}; +use rhai::{Array, Engine, EvalAltResult, RegisterFn, FLOAT, INT}; pub mod empty_module { use rhai::plugin::*; @@ -180,3 +180,44 @@ fn mut_opaque_ref_test() -> Result<(), Box> { ); Ok(()) } + +mod duplicate_fn_rename { + use rhai::plugin::*; + #[export_module] + pub mod my_adds { + use rhai::{FLOAT, INT}; + + #[rhai_fn(name = "add_f")] + pub fn add_float(f1: FLOAT, f2: FLOAT) -> FLOAT { + f1 + f2 + } + + #[rhai_fn(name = "add_i")] + pub fn add_int(i1: INT, i2: INT) -> INT { + i1 + i2 + } + } +} + +#[test] +fn duplicate_fn_rename_test() -> Result<(), Box> { + let mut engine = Engine::new(); + engine.register_fn("get_mystic_number", || 42 as FLOAT); + let m = rhai::exported_module!(crate::duplicate_fn_rename::my_adds); + let mut r = StaticModuleResolver::new(); + r.insert("Math::Advanced".to_string(), m); + engine.set_module_resolver(Some(r)); + + let output_array = engine.eval::( + r#"import "Math::Advanced" as math; + let fx = get_mystic_number(); + let fy = math::add_f(fx, 1.0); + let ix = 42; + let iy = math::add_i(ix, 1); + [fy, iy] + "#, + )?; + assert_eq!(&output_array[0].as_float().unwrap(), &43.0); + assert_eq!(&output_array[1].as_int().unwrap(), &43); + Ok(()) +} From 8efde3c7cec4cf4cff63291b63fed226a1ae6d99 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Wed, 19 Aug 2020 22:15:48 -0500 Subject: [PATCH 2/3] codegen: Rhai names cannot contain dot --- codegen/src/function.rs | 8 +++++++ codegen/ui_tests/rhai_fn_rename_dot.rs | 28 ++++++++++++++++++++++ codegen/ui_tests/rhai_fn_rename_dot.stderr | 11 +++++++++ 3 files changed, 47 insertions(+) create mode 100644 codegen/ui_tests/rhai_fn_rename_dot.rs create mode 100644 codegen/ui_tests/rhai_fn_rename_dot.stderr diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 4e5bf139..41b3aac1 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -113,6 +113,14 @@ impl Parse for ExportedFnParams { } } + // Check validity of name, if present. + if name.as_ref().filter(|n| n.contains('.')).is_some() { + return Err(syn::Error::new( + span, + "Rhai function names may not contain dot" + )) + } + Ok(ExportedFnParams { name, return_raw, diff --git a/codegen/ui_tests/rhai_fn_rename_dot.rs b/codegen/ui_tests/rhai_fn_rename_dot.rs new file mode 100644 index 00000000..9e2180d9 --- /dev/null +++ b/codegen/ui_tests/rhai_fn_rename_dot.rs @@ -0,0 +1,28 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[export_module] +pub mod test_module { + pub use super::Point; + #[rhai_fn(name = "foo.bar")] + pub fn test_fn(input: Point) -> bool { + input.x > input.y + } +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_module::test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/rhai_fn_rename_dot.stderr b/codegen/ui_tests/rhai_fn_rename_dot.stderr new file mode 100644 index 00000000..f650a081 --- /dev/null +++ b/codegen/ui_tests/rhai_fn_rename_dot.stderr @@ -0,0 +1,11 @@ +error: Rhai function names may not contain dot + --> $DIR/rhai_fn_rename_dot.rs:12:15 + | +12 | #[rhai_fn(name = "foo.bar")] + | ^^^^^^^^^^^^^^^^ + +error[E0433]: failed to resolve: use of undeclared type or module `test_module` + --> $DIR/rhai_fn_rename_dot.rs:23:8 + | +23 | if test_module::test_fn(n) { + | ^^^^^^^^^^^ use of undeclared type or module `test_module` From c87da31328e6398981e910e84589170205345d28 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Wed, 19 Aug 2020 23:12:39 -0500 Subject: [PATCH 3/3] codegen: prevent name duplication in Rust or Rhai --- codegen/src/function.rs | 3 ++ codegen/src/module.rs | 52 +++++++++++++++++++ codegen/tests/test_modules.rs | 8 +-- codegen/ui_tests/rhai_fn_rename_collision.rs | 33 ++++++++++++ .../ui_tests/rhai_fn_rename_collision.stderr | 17 ++++++ .../rhai_fn_rename_collision_oneattr.rs | 32 ++++++++++++ .../rhai_fn_rename_collision_oneattr.stderr | 17 ++++++ 7 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 codegen/ui_tests/rhai_fn_rename_collision.rs create mode 100644 codegen/ui_tests/rhai_fn_rename_collision.stderr create mode 100644 codegen/ui_tests/rhai_fn_rename_collision_oneattr.rs create mode 100644 codegen/ui_tests/rhai_fn_rename_collision_oneattr.stderr diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 41b3aac1..0dba5afa 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -18,6 +18,7 @@ pub(crate) struct ExportedFnParams { pub name: Option, pub return_raw: bool, pub skip: bool, + pub span: Option, } impl ExportedFnParams { @@ -47,6 +48,7 @@ impl Parse for ExportedFnParams { let arg_list = args.call( syn::punctuated::Punctuated::::parse_separated_nonempty, )?; + let span = arg_list.span(); let mut attrs: HashMap> = HashMap::new(); for arg in arg_list { @@ -125,6 +127,7 @@ impl Parse for ExportedFnParams { name, return_raw, skip, + span: Some(span), ..Default::default() }) } diff --git a/codegen/src/module.rs b/codegen/src/module.rs index 337e38e9..bba97c2a 100644 --- a/codegen/src/module.rs +++ b/codegen/src/module.rs @@ -12,6 +12,8 @@ use std::vec as new_vec; #[cfg(no_std)] use core::mem; +use std::collections::HashMap; + fn inner_fn_attributes(f: &mut syn::ItemFn) -> syn::Result { if let Some(rhai_fn_idx) = f.attrs.iter().position(|a| { a.path @@ -28,6 +30,48 @@ fn inner_fn_attributes(f: &mut syn::ItemFn) -> syn::Result { } } +fn check_rename_collisions(fns: &Vec) -> Result<(), syn::Error> { + let mut renames = HashMap::::new(); + let mut names = HashMap::::new(); + for itemfn in fns.iter() { + if let Some(ref name) = itemfn.params.name { + let current_span = itemfn.params.span.as_ref().unwrap(); + let key = itemfn.arg_list().fold(name.clone(), |mut argstr, fnarg| { + let type_string: String = match fnarg { + syn::FnArg::Receiver(_) => unimplemented!("receiver rhai_fns not implemented"), + syn::FnArg::Typed(syn::PatType { ref ty, .. }) => + ty.as_ref().to_token_stream().to_string(), + }; + argstr.push('.'); + argstr.extend(type_string.chars()); + argstr + }); + if let Some(other_span) = renames.insert(key, + current_span.clone()) { + let mut err = syn::Error::new(current_span.clone(), + format!("duplicate Rhai signature for '{}'", &name)); + err.combine(syn::Error::new(other_span, + format!("duplicated function renamed '{}'", &name))); + return Err(err); + } + } else { + let ident = itemfn.name(); + names.insert(ident.to_string(), ident.span()); + } + } + for (new_name, attr_span) in renames.drain() { + let new_name = new_name.split('.').next().unwrap(); + if let Some(fn_span) = names.get(new_name) { + let mut err = syn::Error::new(attr_span, + format!("duplicate Rhai signature for '{}'", &new_name)); + err.combine(syn::Error::new(fn_span.clone(), + format!("duplicated function '{}'", &new_name))); + return Err(err); + } + } + Ok(()) +} + #[derive(Debug)] pub(crate) struct Module { mod_all: Option, @@ -92,7 +136,15 @@ impl Parse for Module { impl Module { pub fn generate(self) -> proc_macro2::TokenStream { + // Check for collisions if the "name" attribute was used on inner functions. + if let Err(e) = check_rename_collisions(&self.fns) { + return e.to_compile_error(); + } + + // Perform the generation of new module items. let mod_gen = crate::rhai_module::generate_body(&self.fns, &self.consts); + + // Rebuild the structure of the module, with the new content added. let Module { mod_all, .. } = self; let mut mod_all = mod_all.unwrap(); let mod_name = mod_all.ident.clone(); diff --git a/codegen/tests/test_modules.rs b/codegen/tests/test_modules.rs index b9690a5c..dd5acd05 100644 --- a/codegen/tests/test_modules.rs +++ b/codegen/tests/test_modules.rs @@ -187,12 +187,12 @@ mod duplicate_fn_rename { pub mod my_adds { use rhai::{FLOAT, INT}; - #[rhai_fn(name = "add_f")] + #[rhai_fn(name = "add")] pub fn add_float(f1: FLOAT, f2: FLOAT) -> FLOAT { f1 + f2 } - #[rhai_fn(name = "add_i")] + #[rhai_fn(name = "add")] pub fn add_int(i1: INT, i2: INT) -> INT { i1 + i2 } @@ -211,9 +211,9 @@ fn duplicate_fn_rename_test() -> Result<(), Box> { let output_array = engine.eval::( r#"import "Math::Advanced" as math; let fx = get_mystic_number(); - let fy = math::add_f(fx, 1.0); + let fy = math::add(fx, 1.0); let ix = 42; - let iy = math::add_i(ix, 1); + let iy = math::add(ix, 1); [fy, iy] "#, )?; diff --git a/codegen/ui_tests/rhai_fn_rename_collision.rs b/codegen/ui_tests/rhai_fn_rename_collision.rs new file mode 100644 index 00000000..38814f3d --- /dev/null +++ b/codegen/ui_tests/rhai_fn_rename_collision.rs @@ -0,0 +1,33 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[export_module] +pub mod test_module { + pub use super::Point; + #[rhai_fn(name = "foo")] + pub fn test_fn(input: Point) -> bool { + input.x > input.y + } + + #[rhai_fn(name = "foo")] + pub fn test_fn_2(input: Point) -> bool { + input.x < input.y + } +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_module::test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/rhai_fn_rename_collision.stderr b/codegen/ui_tests/rhai_fn_rename_collision.stderr new file mode 100644 index 00000000..19ddfd35 --- /dev/null +++ b/codegen/ui_tests/rhai_fn_rename_collision.stderr @@ -0,0 +1,17 @@ +error: duplicate Rhai signature for 'foo' + --> $DIR/rhai_fn_rename_collision.rs:17:15 + | +17 | #[rhai_fn(name = "foo")] + | ^^^^^^^^^^^^ + +error: duplicated function renamed 'foo' + --> $DIR/rhai_fn_rename_collision.rs:12:15 + | +12 | #[rhai_fn(name = "foo")] + | ^^^^^^^^^^^^ + +error[E0433]: failed to resolve: use of undeclared type or module `test_module` + --> $DIR/rhai_fn_rename_collision.rs:28:8 + | +28 | if test_module::test_fn(n) { + | ^^^^^^^^^^^ use of undeclared type or module `test_module` diff --git a/codegen/ui_tests/rhai_fn_rename_collision_oneattr.rs b/codegen/ui_tests/rhai_fn_rename_collision_oneattr.rs new file mode 100644 index 00000000..373eab0d --- /dev/null +++ b/codegen/ui_tests/rhai_fn_rename_collision_oneattr.rs @@ -0,0 +1,32 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[export_module] +pub mod test_module { + pub use super::Point; + #[rhai_fn(name = "foo")] + pub fn test_fn(input: Point) -> bool { + input.x > input.y + } + + pub fn foo(input: Point) -> bool { + input.x < input.y + } +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_module::test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/rhai_fn_rename_collision_oneattr.stderr b/codegen/ui_tests/rhai_fn_rename_collision_oneattr.stderr new file mode 100644 index 00000000..6702b43a --- /dev/null +++ b/codegen/ui_tests/rhai_fn_rename_collision_oneattr.stderr @@ -0,0 +1,17 @@ +error: duplicate Rhai signature for 'foo' + --> $DIR/rhai_fn_rename_collision_oneattr.rs:12:15 + | +12 | #[rhai_fn(name = "foo")] + | ^^^^^^^^^^^^ + +error: duplicated function 'foo' + --> $DIR/rhai_fn_rename_collision_oneattr.rs:17:12 + | +17 | pub fn foo(input: Point) -> bool { + | ^^^ + +error[E0433]: failed to resolve: use of undeclared type or module `test_module` + --> $DIR/rhai_fn_rename_collision_oneattr.rs:27:8 + | +27 | if test_module::test_fn(n) { + | ^^^^^^^^^^^ use of undeclared type or module `test_module`