Merge pull request #697 from schungx/master

Fix bug in chain parsing.

CHANGELOG.md

@@ -1,6 +1,16 @@
 Rhai Release Notes
 ==================
 
+Version 1.13.0
+==============
+
+Bug fixes
+---------
+
+* Complex indexing/dotting chains now parse correctly, for example: `a[b][c[d]].e`
+* `map` and `filter` for arrays are marked `pure`. Warnings are added to the documentation of pure array methods that take `this` closures.
+
+
 Version 1.12.0
 ==============
 

Cargo.toml

@@ -3,7 +3,7 @@ members = [".", "codegen"]
 
 [package]
 name = "rhai"
-version = "1.12.0"
+version = "1.13.0"
 rust-version = "1.61.0"
 edition = "2018"
 resolver = "2"

src/api/compile.rs

@@ -202,7 +202,11 @@ impl Engine {
         scope: &Scope,
         scripts: impl AsRef<[S]>,
     ) -> ParseResult<AST> {
-        self.compile_with_scope_and_optimization_level(scope, scripts, self.optimization_level)
+        self.compile_with_scope_and_optimization_level(
+            Some(scope),
+            scripts,
+            self.optimization_level,
+        )
     }
     /// Join a list of strings and compile into an [`AST`] using own scope at a specific optimization level.
     ///
@@ -214,7 +218,7 @@ impl Engine {
     #[inline]
     pub(crate) fn compile_with_scope_and_optimization_level<S: AsRef<str>>(
         &self,
-        scope: &Scope,
+        scope: Option<&Scope>,
         scripts: impl AsRef<[S]>,
         optimization_level: OptimizationLevel,
     ) -> ParseResult<AST> {
@@ -291,7 +295,7 @@ impl Engine {
         let scripts = [script];
         let (stream, t) = self.lex_raw(&scripts, self.token_mapper.as_deref());
         let interned_strings = &mut *locked_write(&self.interned_strings);
-        let state = &mut ParseState::new(scope, interned_strings, t);
+        let state = &mut ParseState::new(Some(scope), interned_strings, t);
         self.parse_global_expr(stream.peekable(), state, |_| {}, self.optimization_level)
     }
 }

src/api/eval.rs

@@ -69,7 +69,7 @@ impl Engine {
         script: &str,
     ) -> RhaiResultOf<T> {
         let ast = self.compile_with_scope_and_optimization_level(
-            scope,
+            Some(scope),
             [script],
             self.optimization_level,
         )?;
@@ -123,7 +123,7 @@ impl Engine {
 
             let (stream, tc) = self.lex_raw(&scripts, self.token_mapper.as_deref());
 
-            let state = &mut ParseState::new(scope, interned_strings, tc);
+            let state = &mut ParseState::new(Some(scope), interned_strings, tc);
 
             // No need to optimize a lone expression
             self.parse_global_expr(

src/api/formatting.rs

@@ -4,7 +4,7 @@ use crate::parser::{ParseResult, ParseState};
 use crate::types::StringsInterner;
 use crate::{
     Engine, ExclusiveRange, FnPtr, ImmutableString, InclusiveRange, OptimizationLevel, Position,
-    RhaiError, Scope, SmartString, ERR,
+    RhaiError, SmartString, ERR,
 };
 use std::any::type_name;
 #[cfg(feature = "no_std")]
@@ -282,9 +282,8 @@ impl Engine {
         let (mut stream, tc) = self.lex_raw(&scripts, self.token_mapper.as_deref());
         tc.borrow_mut().compressed = Some(String::new());
         stream.state.last_token = Some(SmartString::new_const());
-        let scope = Scope::new();
         let mut interner = StringsInterner::new();
-        let mut state = ParseState::new(&scope, &mut interner, tc);
+        let mut state = ParseState::new(None, &mut interner, tc);
         let mut _ast = self.parse(
             stream.peekable(),
             &mut state,

src/api/json.rs

@@ -4,7 +4,7 @@
 use crate::func::native::locked_write;
 use crate::parser::{ParseSettingFlags, ParseState};
 use crate::tokenizer::Token;
-use crate::{Engine, LexError, Map, OptimizationLevel, RhaiResultOf, Scope};
+use crate::{Engine, LexError, Map, OptimizationLevel, RhaiResultOf};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 
@@ -115,9 +115,8 @@ impl Engine {
         );
 
         let ast = {
-            let scope = Scope::new();
             let interned_strings = &mut *locked_write(&self.interned_strings);
-            let state = &mut ParseState::new(&scope, interned_strings, tokenizer_control);
+            let state = &mut ParseState::new(None, interned_strings, tokenizer_control);
 
             self.parse_global_expr(
                 stream.peekable(),

src/api/optimize.rs

@@ -52,7 +52,7 @@ impl Engine {
         let mut ast = ast;
 
         let mut _new_ast = self.optimize_into_ast(
-            scope,
+            Some(scope),
             ast.take_statements(),
             #[cfg(not(feature = "no_function"))]
             ast.shared_lib()

src/api/run.rs

@@ -60,7 +60,7 @@ impl Engine {
         let ast = {
             let (stream, tc) = self.lex_raw(&scripts, self.token_mapper.as_deref());
             let interned_strings = &mut *locked_write(&self.interned_strings);
-            let state = &mut ParseState::new(scope, interned_strings, tc);
+            let state = &mut ParseState::new(Some(scope), interned_strings, tc);
             self.parse(stream.peekable(), state, self.optimization_level)?
         };
         self.run_ast_with_scope(scope, &ast)

src/eval/chaining.rs

@@ -389,6 +389,11 @@ impl Engine {
             )?,
         }
 
+        #[cfg(feature = "debugging")]
+        let scope2 = &mut Scope::new();
+        #[cfg(not(feature = "debugging"))]
+        let scope2 = ();
+
         match lhs {
             // id.??? or id[???]
             Expr::Variable(.., var_pos) => {
@@ -399,7 +404,7 @@ impl Engine {
                 let target = &mut self.search_namespace(global, caches, scope, this_ptr, lhs)?;
 
                 self.eval_dot_index_chain_raw(
-                    global, caches, None, lhs, expr, target, rhs, idx_values, new_val,
+                    global, caches, scope2, None, lhs, expr, target, rhs, idx_values, new_val,
                 )
             }
             // {expr}.??? = ??? or {expr}[???] = ???
@@ -412,7 +417,8 @@ impl Engine {
                 let obj_ptr = &mut value.into();
 
                 self.eval_dot_index_chain_raw(
-                    global, caches, this_ptr, lhs_expr, expr, obj_ptr, rhs, idx_values, new_val,
+                    global, caches, scope2, this_ptr, lhs_expr, expr, obj_ptr, rhs, idx_values,
+                    new_val,
                 )
             }
         }
@@ -523,6 +529,8 @@ impl Engine {
         &self,
         global: &mut GlobalRuntimeState,
         caches: &mut Caches,
+        #[cfg(feature = "debugging")] scope: &mut Scope,
+        #[cfg(not(feature = "debugging"))] scope: (),
         this_ptr: Option<&mut Dynamic>,
         root: &Expr,
         parent: &Expr,
@@ -537,9 +545,6 @@ impl Engine {
         #[cfg(feature = "debugging")]
         let mut this_ptr = this_ptr;
 
-        #[cfg(feature = "debugging")]
-        let scope = &mut Scope::new();
-
         match ChainType::from(parent) {
             #[cfg(not(feature = "no_index"))]
             ChainType::Indexing => {
@@ -570,8 +575,8 @@ impl Engine {
                             let obj_ptr = &mut obj;
 
                             match self.eval_dot_index_chain_raw(
-                                global, caches, this_ptr, root, rhs, obj_ptr, &x.rhs, idx_values,
+                                global, caches, scope, this_ptr, root, rhs, obj_ptr, &x.rhs,
-                                new_val,
+                                idx_values, new_val,
                             ) {
                                 Ok((result, true)) if is_obj_temp_val => {
                                     (Some(obj.take_or_clone()), (result, true))
@@ -880,8 +885,8 @@ impl Engine {
                         };
 
                         self.eval_dot_index_chain_raw(
-                            global, caches, _this_ptr, root, rhs, val_target, &x.rhs, idx_values,
+                            global, caches, scope, _this_ptr, root, rhs, val_target, &x.rhs,
-                            new_val,
+                            idx_values, new_val,
                         )
                     }
                     // xxx.sub_lhs[expr] | xxx.sub_lhs.expr
@@ -926,8 +931,8 @@ impl Engine {
                                 let val = &mut (&mut val).into();
 
                                 let (result, may_be_changed) = self.eval_dot_index_chain_raw(
-                                    global, caches, _this_ptr, root, rhs, val, &x.rhs, idx_values,
+                                    global, caches, scope, _this_ptr, root, rhs, val, &x.rhs,
-                                    new_val,
+                                    idx_values, new_val,
                                 )?;
 
                                 // Feed the value back via a setter just in case it has been updated
@@ -994,8 +999,8 @@ impl Engine {
                                 let val = &mut val.into();
 
                                 self.eval_dot_index_chain_raw(
-                                    global, caches, _this_ptr, root, rhs, val, &x.rhs, idx_values,
+                                    global, caches, scope, _this_ptr, root, rhs, val, &x.rhs,
-                                    new_val,
+                                    idx_values, new_val,
                                 )
                             }
                             // xxx.module::fn_name(...) - syntax error

src/eval/expr.rs

@@ -298,7 +298,7 @@ impl Engine {
                         let source = global.source();
                         let context = &(self, FUNC_TO_STRING, source, &*global, pos).into();
                         let display = print_with_func(FUNC_TO_STRING, context, item);
-                        write!(concat, "{}", display).unwrap();
+                        write!(concat, "{display}").unwrap();
                     }
 
                     #[cfg(not(feature = "unchecked"))]

src/func/call.rs

@@ -1518,7 +1518,7 @@ impl Engine {
         // Compile the script text
         // No optimizations because we only run it once
         let ast = self.compile_with_scope_and_optimization_level(
-            &Scope::new(),
+            None,
             [script],
             #[cfg(not(feature = "no_optimize"))]
             OptimizationLevel::None,

src/lib.rs

@@ -100,8 +100,6 @@ use std::prelude::v1::*;
 mod reify;
 #[macro_use]
 mod restore;
-#[macro_use]
-mod types;
 
 mod api;
 mod ast;
@@ -117,6 +115,7 @@ mod parser;
 pub mod serde;
 mod tests;
 mod tokenizer;
+mod types;
 
 /// Error encountered when parsing a script.
 type PERR = ParseErrorType;

src/optimizer.rs

@@ -1288,7 +1288,7 @@ impl Engine {
     fn optimize_top_level(
         &self,
         statements: StmtBlockContainer,
-        scope: &Scope,
+        scope: Option<&Scope>,
         lib: &[crate::SharedModule],
         optimization_level: OptimizationLevel,
     ) -> StmtBlockContainer {
@@ -1309,11 +1309,13 @@ impl Engine {
         }
 
         // Add constants and variables from the scope
-        for (name, constant, value) in scope.iter() {
+        if let Some(scope) = scope {
-            if constant {
+            for (name, constant, value) in scope.iter() {
-                state.push_var(name, AccessMode::ReadOnly, Some(value));
+                if constant {
-            } else {
+                    state.push_var(name, AccessMode::ReadOnly, Some(value));
-                state.push_var(name, AccessMode::ReadWrite, None);
+                } else {
+                    state.push_var(name, AccessMode::ReadWrite, None);
+                }
             }
         }
 
@@ -1323,7 +1325,7 @@ impl Engine {
     /// Optimize a collection of statements and functions into an [`AST`].
     pub(crate) fn optimize_into_ast(
         &self,
-        scope: &Scope,
+        scope: Option<&Scope>,
         statements: StmtBlockContainer,
         #[cfg(not(feature = "no_function"))] functions: StaticVec<
             crate::Shared<crate::ast::ScriptFnDef>,

src/packages/array_basic.rs

@@ -651,6 +651,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `mapper` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -669,7 +671,7 @@ pub mod array_functions {
     ///
     /// print(y);       // prints "[0, 2, 6, 12, 20]"
     /// ```
-    #[rhai_fn(return_raw)]
+    #[rhai_fn(return_raw, pure)]
     pub fn map(ctx: NativeCallContext, array: &mut Array, map: FnPtr) -> RhaiResultOf<Array> {
         if array.is_empty() {
             return Ok(Array::new());
@@ -692,6 +694,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `filter` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -710,7 +714,7 @@ pub mod array_functions {
     ///
     /// print(y);       // prints "[12, 20]"
     /// ```
-    #[rhai_fn(return_raw)]
+    #[rhai_fn(return_raw, pure)]
     pub fn filter(ctx: NativeCallContext, array: &mut Array, filter: FnPtr) -> RhaiResultOf<Array> {
         if array.is_empty() {
             return Ok(Array::new());
@@ -882,6 +886,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `filter` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -922,6 +928,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `filter` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -1011,6 +1019,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `filter` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -1055,6 +1065,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `mapper` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -1087,6 +1099,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `mapper` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -1143,6 +1157,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `filter` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -1185,6 +1201,8 @@ pub mod array_functions {
     ///
     /// Array element (mutable) is bound to `this`.
     ///
+    /// This method is marked _pure_; the `filter` function should not mutate array elements.
+    ///
     /// # Function Parameters
     ///
     /// * `element`: copy of array element
@@ -1281,9 +1299,11 @@ pub mod array_functions {
     /// # Function Parameters
     ///
     /// * `result`: accumulated result, initially `()`
-    /// * `element`: copy of array element
+    /// * `element`: copy of array element, or bound to `this` if omitted
     /// * `index` _(optional)_: current index in the array
     ///
+    /// This method is marked _pure_; the `reducer` function should not mutate array elements.
+    ///
     /// # Example
     ///
     /// ```rhai
@@ -1306,9 +1326,11 @@ pub mod array_functions {
     /// # Function Parameters
     ///
     /// * `result`: accumulated result, starting with the value of `initial`
-    /// * `element`: copy of array element
+    /// * `element`: copy of array element, or bound to `this` if omitted
     /// * `index` _(optional)_: current index in the array
     ///
+    /// This method is marked _pure_; the `reducer` function should not mutate array elements.
+    ///
     /// # Example
     ///
     /// ```rhai
@@ -1347,9 +1369,11 @@ pub mod array_functions {
     /// # Function Parameters
     ///
     /// * `result`: accumulated result, initially `()`
-    /// * `element`: copy of array element
+    /// * `element`: copy of array element, or bound to `this` if omitted
     /// * `index` _(optional)_: current index in the array
     ///
+    /// This method is marked _pure_; the `reducer` function should not mutate array elements.
+    ///
     /// # Example
     ///
     /// ```rhai
@@ -1373,9 +1397,11 @@ pub mod array_functions {
     /// # Function Parameters
     ///
     /// * `result`: accumulated result, starting with the value of `initial`
-    /// * `element`: copy of array element
+    /// * `element`: copy of array element, or bound to `this` if omitted
     /// * `index` _(optional)_: current index in the array
     ///
+    /// This method is marked _pure_; the `reducer` function should not mutate array elements.
+    ///
     /// # Example
     ///
     /// ```rhai

src/parser.rs

@@ -55,7 +55,7 @@ pub struct ParseState<'e, 's> {
     /// Strings interner.
     pub interned_strings: &'s mut StringsInterner,
     /// External [scope][Scope] with constants.
-    pub scope: &'e Scope<'e>,
+    pub external_constants: Option<&'e Scope<'e>>,
     /// Global runtime state.
     pub global: Option<Box<GlobalRuntimeState>>,
     /// Encapsulates a local stack with variable names to simulate an actual runtime scope.
@@ -87,7 +87,7 @@ impl fmt::Debug for ParseState<'_, '_> {
 
         f.field("tokenizer_control", &self.tokenizer_control)
             .field("interned_strings", &self.interned_strings)
-            .field("scope", &self.scope)
+            .field("external_constants_scope", &self.external_constants)
             .field("global", &self.global)
             .field("stack", &self.stack)
             .field("block_stack_len", &self.block_stack_len);
@@ -109,7 +109,7 @@ impl<'e, 's> ParseState<'e, 's> {
     #[inline]
     #[must_use]
     pub fn new(
-        scope: &'e Scope,
+        external_constants: Option<&'e Scope>,
         interned_strings: &'s mut StringsInterner,
         tokenizer_control: TokenizerControl,
     ) -> Self {
@@ -121,7 +121,7 @@ impl<'e, 's> ParseState<'e, 's> {
             #[cfg(not(feature = "no_closure"))]
             allow_capture: true,
             interned_strings,
-            scope,
+            external_constants,
             global: None,
             stack: None,
             block_stack_len: 0,
@@ -1416,7 +1416,7 @@ impl Engine {
                 // Build new parse state
                 let new_interner = &mut StringsInterner::new();
                 let new_state = &mut ParseState::new(
-                    state.scope,
+                    state.external_constants,
                     new_interner,
                     state.tokenizer_control.clone(),
                 );
@@ -1476,7 +1476,9 @@ impl Engine {
                         && index.is_none()
                         && !settings.has_flag(ParseSettingFlags::CLOSURE_SCOPE)
                         && settings.has_option(LangOptions::STRICT_VAR)
-                        && !state.scope.contains(name)
+                        && !state
+                            .external_constants
+                            .map_or(false, |scope| scope.contains(name))
                     {
                         // If the parent scope is not inside another capturing closure
                         // then we can conclude that the captured variable doesn't exist.
@@ -1624,7 +1626,9 @@ impl Engine {
                             && !is_func
                             && index.is_none()
                             && settings.has_option(LangOptions::STRICT_VAR)
-                            && !state.scope.contains(&s)
+                            && !state
+                                .external_constants
+                                .map_or(false, |scope| scope.contains(&s))
                         {
                             return Err(
                                 PERR::VariableUndefined(s.to_string()).into_err(settings.pos)
@@ -2098,8 +2102,10 @@ impl Engine {
         op_pos: Position,
     ) -> ParseResult<Expr> {
         match (lhs, rhs) {
-            // lhs[idx_expr].rhs
+            // lhs[...][...].rhs
-            (Expr::Index(mut x, options, pos), rhs) => {
+            (Expr::Index(mut x, options, pos), rhs)
+                if !parent_options.contains(ASTFlags::BREAK) =>
+            {
                 let options = options | parent_options;
                 x.rhs = Self::make_dot_expr(state, x.rhs, rhs, options, op_flags, op_pos)?;
                 Ok(Expr::Index(x, ASTFlags::NONE, pos))
@@ -3298,7 +3304,7 @@ impl Engine {
                     (Token::Fn, pos) => {
                         // Build new parse state
                         let new_state = &mut ParseState::new(
-                            state.scope,
+                            state.external_constants,
                             state.interned_strings,
                             state.tokenizer_control.clone(),
                         );
@@ -3848,7 +3854,7 @@ impl Engine {
 
         #[cfg(not(feature = "no_optimize"))]
         return Ok(self.optimize_into_ast(
-            state.scope,
+            state.external_constants,
             statements,
             #[cfg(not(feature = "no_function"))]
             functions.into_iter().map(|(.., v)| v).collect(),
@@ -3934,7 +3940,7 @@ impl Engine {
 
         #[cfg(not(feature = "no_optimize"))]
         return Ok(self.optimize_into_ast(
-            state.scope,
+            state.external_constants,
             statements,
             #[cfg(not(feature = "no_function"))]
             _lib,

src/tokenizer.rs

@@ -276,8 +276,6 @@ pub enum Token {
     /// End of the input stream.
     /// Used as a placeholder for the end of input.
     EOF,
-    /// Placeholder to indicate the lack of a token.
-    NONE,
 }
 
 impl fmt::Display for Token {
@@ -303,7 +301,6 @@ impl fmt::Display for Token {
             Comment(s) => f.write_str(s),
 
             EOF => f.write_str("{EOF}"),
-            NONE => f.write_str("{NONE}"),
 
             token => f.write_str(token.literal_syntax()),
         }
@@ -332,7 +329,7 @@ impl Token {
             Custom(..) => false,
             LexError(..) | Comment(..) => false,
 
-            EOF | NONE => false,
+            EOF => false,
 
             _ => true,
         }

tests/arrays.rs

@@ -133,6 +133,20 @@ fn test_arrays() -> Result<(), Box<EvalAltResult>> {
         );
     }
 
+    #[cfg(not(feature = "no_object"))]
+    assert_eq!(
+        engine.eval::<INT>(
+            "
+                let x = #{ foo: 42 };
+                let n = 0;
+                let a = [[x]];
+                let i = [n];
+                a[n][i[n]].foo
+            "
+        )?,
+        42
+    );
+
     assert_eq!(
         engine
             .eval::<Dynamic>(