Merge pull request #39 from jhwgh1968/rename_nonidents

codegen: rhai_fn test improvements and name restrictions

codegen/src/function.rs

@@ -18,6 +18,7 @@ pub(crate) struct ExportedFnParams {
     pub name: Option<String>,
     pub return_raw: bool,
     pub skip: bool,
+    pub span: Option<proc_macro2::Span>,
 }
 
 impl ExportedFnParams {
@@ -47,6 +48,7 @@ impl Parse for ExportedFnParams {
         let arg_list = args.call(
             syn::punctuated::Punctuated::<syn::Expr, syn::Token![,]>::parse_separated_nonempty,
         )?;
+        let span = arg_list.span();
 
         let mut attrs: HashMap<proc_macro2::Ident, Option<syn::LitStr>> = HashMap::new();
         for arg in arg_list {
@@ -113,10 +115,19 @@ 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,
             skip,
+            span: Some(span),
             ..Default::default()
         })
     }

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<ExportedFnParams> {
     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<ExportedFnParams> {
     }
 }
 
+fn check_rename_collisions(fns: &Vec<ExportedFn>) -> Result<(), syn::Error> {
+    let mut renames = HashMap::<String, proc_macro2::Span>::new();
+    let mut names = HashMap::<String, proc_macro2::Span>::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<syn::ItemMod>,
@@ -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();

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::*;

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<EvalAltResult>> {
     );
     Ok(())
 }
+
+mod duplicate_fn_rename {
+    use rhai::plugin::*;
+    #[export_module]
+    pub mod my_adds {
+        use rhai::{FLOAT, INT};
+
+        #[rhai_fn(name = "add")]
+        pub fn add_float(f1: FLOAT, f2: FLOAT) -> FLOAT {
+            f1 + f2
+        }
+
+        #[rhai_fn(name = "add")]
+        pub fn add_int(i1: INT, i2: INT) -> INT {
+            i1 + i2
+        }
+    }
+}
+
+#[test]
+fn duplicate_fn_rename_test() -> Result<(), Box<EvalAltResult>> {
+    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::<Array>(
+        r#"import "Math::Advanced" as math;
+       let fx = get_mystic_number();
+       let fy = math::add(fx, 1.0);
+       let ix = 42;
+       let iy = math::add(ix, 1);
+       [fy, iy]
+       "#,
+    )?;
+    assert_eq!(&output_array[0].as_float().unwrap(), &43.0);
+    assert_eq!(&output_array[1].as_int().unwrap(), &43);
+    Ok(())
+}

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");
+    }
+}

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`

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");
+    }
+}

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`

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");
+    }
+}

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`