Merge branch 'master' into plugins

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rhai"
-version = "0.19.0"
+version = "0.18.3"
 edition = "2018"
 authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung"]
 description = "Embedded scripting for Rust"

RELEASES.md

@@ -1,9 +1,25 @@
 Rhai Release Notes
 ==================
 
-Version 0.19.0
+Version 0.18.3
 ==============
 
+Bug fixes
+---------
+
+* `Engine::compile_expression`, `Engine::eval_expression` etc. no longer parse anonymous functions and closures.
+* Imported modules now work inside closures.
+
+
+Version 0.18.2
+==============
+
+Bug fixes
+---------
+
+* Fixes bug that prevents calling functions in closures.
+* Fixes bug that erroneously consumes the first argument to a module-qualified function call.
+
 New features
 ------------
 

doc/src/engine/expressions.md

@@ -11,9 +11,11 @@ In these cases, use the `Engine::compile_expression` and `Engine::eval_expressio
 let result = engine.eval_expression::<i64>("2 + (10 + 10) * 2")?;
 ```
 
-When evaluating _expressions_, no full-blown statement (e.g. `if`, `while`, `for`) - not even variable assignment -
+When evaluating _expressions_, no full-blown statement (e.g. `if`, `while`, `for`, `fn`) - not even variable assignment -
 is supported and will be considered parse errors when encountered.
 
+[Closures] and [anonymous functions] are also not supported because in the background they compile to functions.
+
 ```rust
 // The following are all syntax errors because the script is not an expression.
 

doc/src/language/json.md

@@ -3,11 +3,14 @@ Parse an Object Map from JSON
 
 {{#include ../links.md}}
 
-The syntax for an [object map] is extremely similar to JSON, with the exception of `null` values which can
+The syntax for an [object map] is extremely similar to the JSON representation of a object hash,
-technically be mapped to [`()`].  A valid JSON string does not start with a hash character `#` while a
+with the exception of `null` values which can technically be mapped to [`()`].
-Rhai [object map] does - that's the major difference!
 
-Use the `Engine::parse_json` method to parse a piece of _simple_ JSON into an object map:
+A valid JSON string does not start with a hash character `#` while a Rhai [object map] does - that's the major difference!
+
+Use the `Engine::parse_json` method to parse a piece of JSON into an object map.
+The JSON text must represent a single object hash (i.e. must be wrapped within "`{ .. }`")
+otherwise it returns a syntax error.
 
 ```rust
 // JSON string - notice that JSON property names are always quoted
@@ -59,6 +62,8 @@ When `Engine::parse_json` encounters JSON with sub-objects, it fails with a synt
 If it is certain that no text string in the JSON will ever contain the character '`{`',
 then it is possible to parse it by first replacing all occupance of '`{`' with "`#{`".
 
+A JSON object hash starting with `#{` is handled transparently by `Engine::parse_json`.
+
 ```rust
 // JSON with sub-object 'b'.
 let json = r#"{"a":1, "b":{"x":true, "y":false}}"#;

doc/src/rust/register-raw.md

@@ -33,7 +33,7 @@ engine.register_raw_fn(
 
         // But remember this is Rust, so you can keep only one mutable reference at any one time!
         // Therefore, get a '&mut' reference to the first argument _last_.
-        // Alternatively, use `args.split_at_mut(1)` etc. to split the slice first.
+        // Alternatively, use `args.split_first_mut()` etc. to split the slice first.
 
         let y: i64 = *args[1].read_lock::<i64>()            // get a reference to the second argument
                              .unwrap();                     // then copying it because it is a primary type
@@ -163,15 +163,15 @@ Hold Multiple References
 ------------------------
 
 In order to access a value argument that is expensive to clone _while_ holding a mutable reference
-to the first argument, either _consume_ that argument via `mem::take` as above, or use `args.split_at`
+to the first argument, either _consume_ that argument via `mem::take` as above, or use `args.split_first`
 to partition the slice:
 
 ```rust
 // Partition the slice
-let (first, rest) = args.split_at_mut(1);
+let (first, rest) = args.split_first_mut().unwrap();
 
 // Mutable reference to the first parameter
-let this_ptr: &mut A = &mut *first[0].write_lock::<A>().unwrap();
+let this_ptr: &mut A = &mut *first.write_lock::<A>().unwrap();
 
 // Immutable reference to the second value parameter
 // This can be mutable but there is no point because the parameter is passed by value

src/api.rs

@@ -18,6 +18,7 @@ use crate::engine::{FN_IDX_GET, FN_IDX_SET};
 #[cfg(not(feature = "no_object"))]
 use crate::{
     engine::{make_getter, make_setter, Map},
+    error::ParseErrorType,
     fn_register::{RegisterFn, RegisterResultFn},
     token::Token,
 };
@@ -895,6 +896,8 @@ impl Engine {
 
     /// Parse a JSON string into a map.
     ///
+    /// The JSON string must be an object hash.  It cannot be a simple JavaScript primitive.
+    ///
     /// Set `has_null` to `true` in order to map `null` values to `()`.
     /// Setting it to `false` will cause a _variable not found_ error during parsing.
     ///
@@ -934,12 +937,20 @@ impl Engine {
         let mut scope = Scope::new();
 
         // Trims the JSON string and add a '#' in front
-        let json = json.trim();
+        let json_text = json.trim_start();
-        let scripts = if json.starts_with(Token::MapStart.syntax().as_ref()) {
+        let scripts = if json_text.starts_with(Token::MapStart.syntax().as_ref()) {
-            [json, ""]
+            [json_text, ""]
+        } else if json_text.starts_with(Token::LeftBrace.syntax().as_ref()) {
+            ["#", json_text]
         } else {
-            ["#", json]
+            return Err(ParseErrorType::MissingToken(
+                Token::LeftBrace.syntax().into(),
+                "to start a JSON object hash".into(),
+            )
+            .into_err(Position::new(1, (json.len() - json_text.len() + 1) as u16))
+            .into());
         };
+
         let stream = lex(
             &scripts,
             if has_null {

src/fn_call.rs

@@ -525,13 +525,13 @@ impl Engine {
 
                 let result = if _is_method {
                     // Method call of script function - map first argument to `this`
-                    let (first, rest) = args.split_at_mut(1);
+                    let (first, rest) = args.split_first_mut().unwrap();
                     self.call_script_fn(
                         scope,
                         mods,
                         state,
                         lib,
-                        &mut Some(first[0]),
+                        &mut Some(*first),
                         fn_name,
                         func,
                         rest,
@@ -978,6 +978,7 @@ impl Engine {
     ) -> Result<Dynamic, Box<EvalAltResult>> {
         let modules = modules.as_ref().unwrap();
         let mut arg_values: StaticVec<_>;
+        let mut first_arg_value = None;
         let mut args: StaticVec<_>;
 
         if args_expr.is_empty() {
@@ -992,17 +993,28 @@ impl Engine {
                 Expr::Variable(x) if x.1.is_none() => {
                     arg_values = args_expr
                         .iter()
-                        .skip(1)
+                        .enumerate()
-                        .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level))
+                        .map(|(i, expr)| {
+                            // Skip the first argument
+                            if i == 0 {
+                                Ok(Default::default())
+                            } else {
+                                self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)
+                            }
+                        })
                         .collect::<Result<_, _>>()?;
 
+                    // Get target reference to first argument
                     let (target, _, _, pos) =
                         search_scope_only(scope, state, this_ptr, args_expr.get(0).unwrap())?;
 
                     self.inc_operations(state)
                         .map_err(|err| err.new_position(pos))?;
 
-                    args = once(target).chain(arg_values.iter_mut()).collect();
+                    let (first, rest) = arg_values.split_first_mut().unwrap();
+                    first_arg_value = Some(first);
+
+                    args = once(target).chain(rest.iter_mut()).collect();
                 }
                 // func(..., ...) or func(mod::x, ...)
                 _ => {
@@ -1041,6 +1053,13 @@ impl Engine {
         match func {
             #[cfg(not(feature = "no_function"))]
             Some(f) if f.is_script() => {
+                // Clone first argument
+                if let Some(first) = first_arg_value {
+                    let first_val = args[0].clone();
+                    args[0] = first;
+                    *args[0] = first_val;
+                }
+
                 let args = args.as_mut();
                 let func = f.get_fn_def();
 
@@ -1060,7 +1079,19 @@ impl Engine {
                 self.call_script_fn(scope, mods, state, lib, &mut None, name, func, args, level)
             }
             Some(f) if f.is_plugin_fn() => f.get_plugin_fn().call(args.as_mut(), Position::none()),
-            Some(f) => f.get_native_fn()(self, lib, args.as_mut()),
+            Some(f) if f.is_native() => {
+                if !f.is_method() {
+                    // Clone first argument
+                    if let Some(first) = first_arg_value {
+                        let first_val = args[0].clone();
+                        args[0] = first;
+                        *args[0] = first_val;
+                    }
+                }
+
+                f.get_native_fn()(self, lib, args.as_mut())
+            }
+            Some(_) => unreachable!(),
             None if def_val.is_some() => Ok(def_val.unwrap().into()),
             None => EvalAltResult::ErrorFunctionNotFound(
                 format!(

src/parser.rs

@@ -1645,11 +1645,22 @@ fn parse_primary(
             }
             Expr::Variable(Box::new(((s, settings.pos), None, 0, None)))
         }
+        // Module qualification
+        #[cfg(not(feature = "no_module"))]
+        Token::Identifier(s) if *next_token == Token::DoubleColon => {
+            // Once the identifier consumed we must enable next variables capturing
+            #[cfg(not(feature = "no_closure"))]
+            {
+                state.allow_capture = true;
+            }
+            Expr::Variable(Box::new(((s, settings.pos), None, 0, None)))
+        }
         // Normal variable access
         Token::Identifier(s) => {
             let index = state.access_var(&s, settings.pos);
             Expr::Variable(Box::new(((s, settings.pos), None, 0, index)))
         }
+
         // Function call is allowed to have reserved keyword
         Token::Reserved(s) if *next_token == Token::LeftParen || *next_token == Token::Bang => {
             if is_keyword_function(&s) {
@@ -1658,6 +1669,7 @@ fn parse_primary(
                 return Err(PERR::Reserved(s).into_err(settings.pos));
             }
         }
+
         // Access to `this` as a variable is OK
         Token::Reserved(s) if s == KEYWORD_THIS && *next_token != Token::LeftParen => {
             if !settings.is_function_scope {
@@ -1669,9 +1681,11 @@ fn parse_primary(
                 Expr::Variable(Box::new(((s, settings.pos), None, 0, None)))
             }
         }
+
         Token::Reserved(s) if is_valid_identifier(s.chars()) => {
             return Err(PERR::Reserved(s).into_err(settings.pos));
         }
+
         Token::LeftParen => parse_paren_expr(input, state, lib, settings.level_up())?,
         #[cfg(not(feature = "no_index"))]
         Token::LeftBracket => parse_array_literal(input, state, lib, settings.level_up())?,
@@ -1680,6 +1694,7 @@ fn parse_primary(
         Token::True => Expr::True(settings.pos),
         Token::False => Expr::False(settings.pos),
         Token::LexError(err) => return Err(err.into_err(settings.pos)),
+
         _ => {
             return Err(
                 PERR::BadInput(format!("Unexpected '{}'", token.syntax())).into_err(settings.pos)
@@ -1862,7 +1877,7 @@ fn parse_unary(
         }
         // | ...
         #[cfg(not(feature = "no_function"))]
-        Token::Pipe | Token::Or => {
+        Token::Pipe | Token::Or if settings.allow_anonymous_fn => {
             let mut new_state = ParseState::new(
                 state.engine,
                 #[cfg(not(feature = "unchecked"))]
@@ -3353,6 +3368,8 @@ impl Engine {
         };
         let expr = parse_expr(input, &mut state, &mut functions, settings)?;
 
+        assert!(functions.is_empty());
+
         match input.peek().unwrap() {
             (Token::EOF, _) => (),
             // Return error if the expression doesn't end

tests/call_fn.rs

@@ -120,6 +120,20 @@ fn test_fn_ptr_raw() -> Result<(), Box<EvalAltResult>> {
         42
     );
 
+    assert_eq!(
+        engine.eval::<INT>(
+            r#"
+                fn foo(x, y) { this += x + y; }
+
+                let x = 40;
+                let v = 1;
+                x.bar(Fn("foo").curry(v), 1);
+                x
+            "#
+        )?,
+        42
+    );
+
     assert!(matches!(
         *engine.eval::<INT>(
             r#"

tests/closures.rs

@@ -1,7 +1,12 @@
 #![cfg(not(feature = "no_function"))]
-use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, RegisterFn, INT};
+use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, Module, ParseErrorType, RegisterFn, Scope, INT};
 use std::any::TypeId;
+use std::cell::RefCell;
 use std::mem::take;
+use std::rc::Rc;
+
+#[cfg(not(feature = "no_object"))]
+use rhai::Map;
 
 #[test]
 fn test_fn_ptr_curry_call() -> Result<(), Box<EvalAltResult>> {
@@ -39,6 +44,14 @@ fn test_fn_ptr_curry_call() -> Result<(), Box<EvalAltResult>> {
 fn test_closures() -> Result<(), Box<EvalAltResult>> {
     let mut engine = Engine::new();
 
+    assert!(matches!(
+        *engine
+            .compile_expression("let f = |x| {};")
+            .expect_err("should error")
+            .0,
+        ParseErrorType::BadInput(_)
+    ));
+
     assert_eq!(
         engine.eval::<INT>(
             r#"
@@ -181,3 +194,59 @@ fn test_closures_data_race() -> Result<(), Box<EvalAltResult>> {
 
     Ok(())
 }
+
+type MyType = Rc<RefCell<INT>>;
+
+#[test]
+#[cfg(not(feature = "no_object"))]
+#[cfg(not(feature = "sync"))]
+fn test_closure_shared_obj() -> Result<(), Box<EvalAltResult>> {
+    let mut engine = Engine::new();
+
+    // Register API on MyType
+    engine
+        .register_type_with_name::<MyType>("MyType")
+        .register_get_set(
+            "data",
+            |p: &mut MyType| *p.borrow(),
+            |p: &mut MyType, value: INT| *p.borrow_mut() = value,
+        )
+        .register_fn("+=", |p1: &mut MyType, p2: MyType| {
+            *p1.borrow_mut() += *p2.borrow()
+        })
+        .register_fn("-=", |p1: &mut MyType, p2: MyType| {
+            *p1.borrow_mut() -= *p2.borrow()
+        });
+
+    let engine = engine; // Make engine immutable
+
+    let code = r#"
+        #{
+            name: "A",
+            description: "B",
+            cost: 1,
+            health_added: 0,
+            action: |p1, p2| { p1 += p2 }
+        }
+    "#;
+
+    let ast = engine.compile(code)?;
+    let res = engine.eval_ast::<Map>(&ast)?;
+
+    // Make closure
+    let f = move |p1: MyType, p2: MyType| -> Result<(), Box<EvalAltResult>> {
+        let action_ptr = res["action"].clone().cast::<FnPtr>();
+        let name = action_ptr.fn_name();
+        engine.call_fn::<_, ()>(&mut Scope::new(), &ast, name, (p1, p2))
+    };
+
+    // Test closure
+    let p1 = Rc::new(RefCell::new(41));
+    let p2 = Rc::new(RefCell::new(1));
+
+    f(p1.clone(), p2.clone())?;
+
+    assert_eq!(*p1.borrow(), 42);
+
+    Ok(())
+}

tests/maps.rs

@@ -1,6 +1,6 @@
 #![cfg(not(feature = "no_object"))]
 
-use rhai::{Engine, EvalAltResult, Map, Scope, INT};
+use rhai::{Engine, EvalAltResult, Map, ParseErrorType, Scope, INT};
 
 #[test]
 fn test_map_indexing() -> Result<(), Box<EvalAltResult>> {
@@ -182,6 +182,14 @@ fn test_map_json() -> Result<(), Box<EvalAltResult>> {
         );
     }
 
+    engine.parse_json(&format!("#{}", json), true)?;
+
+    assert!(matches!(
+        *engine.parse_json("   123", true).expect_err("should error"),
+        EvalAltResult::ErrorParsing(ParseErrorType::MissingToken(token, _), pos)
+            if token == "{" && pos.position() == Some(4)
+    ));
+
     Ok(())
 }
 

tests/modules.rs

@@ -94,6 +94,31 @@ fn test_module_resolver() -> Result<(), Box<EvalAltResult>> {
         42
     );
 
+    assert_eq!(
+        engine.eval::<INT>(
+            r#"
+                import "hello" as h1;
+                import "hello" as h2;
+                let x = 42;
+                h1::sum(x, -10, 3, 7)
+            "#
+        )?,
+        42
+    );
+
+    assert_eq!(
+        engine.eval::<INT>(
+            r#"
+                import "hello" as h1;
+                import "hello" as h2;
+                let x = 42;
+                h1::sum(x, 0, 0, 0);
+                x
+            "#
+        )?,
+        42
+    );
+
     assert_eq!(
         engine.eval::<INT>(
             r#"