Disallow registering indexers for arrays, maps and strings.

RELEASES.md

@@ -17,6 +17,7 @@ Breaking changes
 ----------------
 
 * `Engine::register_set_result` and `Engine::register_indexer_set_result` now take a function that returns `Result<(), Box<EvalAltResult>>`.
+* `Engine::register_indexer_XXX` and `Module::set_indexer_XXX` panic when the type is `Arrary`, `Map` or `String`.
 
 New features
 ------------

doc/src/rust/getters-setters.md

@@ -18,6 +18,15 @@ Getters and setters are disabled when the [`no_object`] feature is used.
 | `register_set_result` | Register a setter                                 |   `Result<(), Box<EvalAltResult>>`    |
 
 
+Cannot Override Object Maps
+--------------------------
+
+Getters and setters are only intended for [custom types].
+
+Any getter or setter function registered for [object maps] is simply ignored because
+the get/set calls will be interpreted as properties on the [object maps].
+
+
 Examples
 --------
 
@@ -28,15 +37,13 @@ struct TestStruct {
 }
 
 impl TestStruct {
-    // Returning a 'String' is OK - Rhai converts it into 'ImmutableString'
+    // Remember &mut must be used even for getters
     fn get_field(&mut self) -> String {
         self.field.clone()
     }
 
-    // Remember Rhai uses 'ImmutableString' or '&str' instead of 'String'
+    fn set_field(&mut self, new_val: &str) {
-    fn set_field(&mut self, new_val: ImmutableString) {
+        self.field = new_val.to_string();
-        // Get a 'String' from an 'ImmutableString'
-        self.field = (*new_val).clone();
     }
 
     fn new() -> Self {
@@ -51,7 +58,6 @@ let mut engine = Engine::new();
         .register_get_set("xyz", TestStruct::get_field, TestStruct::set_field)
         .register_fn("new_ts", TestStruct::new);
 
-// Return result can be 'String' - Rhai will automatically convert it from 'ImmutableString'
 let result = engine.eval::<String>(r#"let a = new_ts(); a.xyz = "42"; a.xyz"#)?;
 
 println!("Answer: {}", result);                     // prints 42

doc/src/rust/indexers.md

@@ -13,9 +13,6 @@ Like getters and setters, indexers take a `&mut` reference to the first paramete
 
 Indexers are disabled when the [`no_index`] feature is used.
 
-For efficiency reasons, indexers **cannot** be used to overload (i.e. override) built-in indexing operations for
-[arrays] and [object maps].
-
 | `Engine` API                  | Description                                              |       Return Value of Function        |
 | ----------------------------- | -------------------------------------------------------- | :-----------------------------------: |
 | `register_indexer_get`        | Register an index getter                                 |                 _Any_                 |
@@ -25,6 +22,15 @@ For efficiency reasons, indexers **cannot** be used to overload (i.e. override)
 | `register_indexer_set_result` | Register an index setter                                 |   `Result<(), Box<EvalAltResult>>`    |
 
 
+Cannot Override Arrays, Object Maps and Strings
+----------------------------------------------
+
+For efficiency reasons, indexers **cannot** be used to overload (i.e. override)
+built-in indexing operations for [arrays], [object maps] and [strings].
+
+Attempting to register indexers for an [array], [object map] or [string] panics.
+
+
 Examples
 --------
 
@@ -35,6 +41,7 @@ struct TestStruct {
 }
 
 impl TestStruct {
+    // Remember &mut must be used even for getters
     fn get_field(&mut self, index: i64) -> i64 {
         self.fields[index as usize]
     }
@@ -60,3 +67,5 @@ let result = engine.eval::<i64>("let a = new_ts(); a[2] = 42; a[2]")?;
 
 println!("Answer: {}", result);                     // prints 42
 ```
+
+**IMPORTANT: Rhai does NOT support normal references (i.e. `&T`) as parameters.**

src/api.rs

@@ -12,7 +12,10 @@ use crate::scope::Scope;
 use crate::token::{lex, Position};
 
 #[cfg(not(feature = "no_index"))]
-use crate::engine::{FN_IDX_GET, FN_IDX_SET};
+use crate::{
+    engine::{Array, FN_IDX_GET, FN_IDX_SET},
+    utils::ImmutableString,
+};
 
 #[cfg(not(feature = "no_object"))]
 use crate::{
@@ -433,6 +436,11 @@ impl Engine {
     ///
     /// The function signature must start with `&mut self` and not `&self`.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Example
     ///
     /// ```
@@ -475,6 +483,20 @@ impl Engine {
         U: Variant + Clone,
         X: Variant + Clone,
     {
+        if TypeId::of::<T>() == TypeId::of::<Array>() {
+            panic!("Cannot register indexer for arrays.");
+        }
+        #[cfg(not(feature = "no_object"))]
+        if TypeId::of::<T>() == TypeId::of::<Map>() {
+            panic!("Cannot register indexer for object maps.");
+        }
+        if TypeId::of::<T>() == TypeId::of::<String>()
+            || TypeId::of::<T>() == TypeId::of::<&str>()
+            || TypeId::of::<T>() == TypeId::of::<ImmutableString>()
+        {
+            panic!("Cannot register indexer for strings.");
+        }
+
         self.register_fn(FN_IDX_GET, callback)
     }
 
@@ -483,6 +505,11 @@ impl Engine {
     ///
     /// The function signature must start with `&mut self` and not `&self`.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Example
     ///
     /// ```
@@ -526,11 +553,30 @@ impl Engine {
         T: Variant + Clone,
         X: Variant + Clone,
     {
+        if TypeId::of::<T>() == TypeId::of::<Array>() {
+            panic!("Cannot register indexer for arrays.");
+        }
+        #[cfg(not(feature = "no_object"))]
+        if TypeId::of::<T>() == TypeId::of::<Map>() {
+            panic!("Cannot register indexer for object maps.");
+        }
+        if TypeId::of::<T>() == TypeId::of::<String>()
+            || TypeId::of::<T>() == TypeId::of::<&str>()
+            || TypeId::of::<T>() == TypeId::of::<ImmutableString>()
+        {
+            panic!("Cannot register indexer for strings.");
+        }
+
         self.register_result_fn(FN_IDX_GET, callback)
     }
 
     /// Register an index setter for a custom type with the `Engine`.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Example
     ///
     /// ```
@@ -575,12 +621,31 @@ impl Engine {
         U: Variant + Clone,
         X: Variant + Clone,
     {
+        if TypeId::of::<T>() == TypeId::of::<Array>() {
+            panic!("Cannot register indexer for arrays.");
+        }
+        #[cfg(not(feature = "no_object"))]
+        if TypeId::of::<T>() == TypeId::of::<Map>() {
+            panic!("Cannot register indexer for object maps.");
+        }
+        if TypeId::of::<T>() == TypeId::of::<String>()
+            || TypeId::of::<T>() == TypeId::of::<&str>()
+            || TypeId::of::<T>() == TypeId::of::<ImmutableString>()
+        {
+            panic!("Cannot register indexer for strings.");
+        }
+
         self.register_fn(FN_IDX_SET, callback)
     }
 
     /// Register an index setter for a custom type with the `Engine`.
     /// Returns `Result<(), Box<EvalAltResult>>`.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Example
     ///
     /// ```
@@ -628,6 +693,20 @@ impl Engine {
         U: Variant + Clone,
         X: Variant + Clone,
     {
+        if TypeId::of::<T>() == TypeId::of::<Array>() {
+            panic!("Cannot register indexer for arrays.");
+        }
+        #[cfg(not(feature = "no_object"))]
+        if TypeId::of::<T>() == TypeId::of::<Map>() {
+            panic!("Cannot register indexer for object maps.");
+        }
+        if TypeId::of::<T>() == TypeId::of::<String>()
+            || TypeId::of::<T>() == TypeId::of::<&str>()
+            || TypeId::of::<T>() == TypeId::of::<ImmutableString>()
+        {
+            panic!("Cannot register indexer for strings.");
+        }
+
         self.register_result_fn(FN_IDX_SET, move |obj: &mut T, index: X, value: U| {
             callback(obj, index, value)?;
             Ok(().into())
@@ -636,6 +715,11 @@ impl Engine {
 
     /// Short-hand for register both index getter and setter functions for a custom type with the `Engine`.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Example
     ///
     /// ```

src/engine.rs

@@ -756,10 +756,7 @@ impl Engine {
                             Err(err) => match *err {
                                 // No index getter - try to call an index setter
                                 #[cfg(not(feature = "no_index"))]
-                                EvalAltResult::ErrorIndexingType(_, _) => {
+                                EvalAltResult::ErrorIndexingType(_, _) => Some(new_val.unwrap()),
-                                    // Raise error if there is no index getter nor setter
-                                    Some(new_val.unwrap())
-                                }
                                 // Any other error - return
                                 err => return Err(Box::new(err)),
                             },

src/module.rs

@@ -21,11 +21,10 @@ use crate::{
 };
 
 #[cfg(not(feature = "no_index"))]
-#[cfg(not(feature = "no_object"))]
+use crate::engine::{Array, FN_IDX_GET, FN_IDX_SET};
-use crate::engine::{FN_IDX_GET, FN_IDX_SET};
 
 #[cfg(not(feature = "no_object"))]
-use crate::engine::{make_getter, make_setter};
+use crate::engine::{make_getter, make_setter, Map};
 
 use crate::stdlib::{
     any::TypeId,
@@ -675,6 +674,11 @@ impl Module {
     ///
     /// If there is a similar existing setter Rust function, it is replaced.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Examples
     ///
     /// ```
@@ -686,12 +690,25 @@ impl Module {
     /// });
     /// assert!(module.contains_fn(hash, true));
     /// ```
-    #[cfg(not(feature = "no_object"))]
     #[cfg(not(feature = "no_index"))]
     pub fn set_indexer_get_fn<A: Variant + Clone, B: Variant + Clone, T: Variant + Clone>(
         &mut self,
         func: impl Fn(&mut A, B) -> FuncReturn<T> + SendSync + 'static,
     ) -> u64 {
+        if TypeId::of::<A>() == TypeId::of::<Array>() {
+            panic!("Cannot register indexer for arrays.");
+        }
+        #[cfg(not(feature = "no_object"))]
+        if TypeId::of::<A>() == TypeId::of::<Map>() {
+            panic!("Cannot register indexer for object maps.");
+        }
+        if TypeId::of::<A>() == TypeId::of::<String>()
+            || TypeId::of::<A>() == TypeId::of::<&str>()
+            || TypeId::of::<A>() == TypeId::of::<ImmutableString>()
+        {
+            panic!("Cannot register indexer for strings.");
+        }
+
         self.set_fn_2_mut(FN_IDX_GET, func)
     }
 
@@ -773,6 +790,11 @@ impl Module {
     ///
     /// If there is a similar existing Rust function, it is replaced.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Examples
     ///
     /// ```
@@ -785,12 +807,25 @@ impl Module {
     /// });
     /// assert!(module.contains_fn(hash, true));
     /// ```
-    #[cfg(not(feature = "no_object"))]
     #[cfg(not(feature = "no_index"))]
     pub fn set_indexer_set_fn<A: Variant + Clone, B: Variant + Clone, C: Variant + Clone>(
         &mut self,
         func: impl Fn(&mut A, B, C) -> FuncReturn<()> + SendSync + 'static,
     ) -> u64 {
+        if TypeId::of::<A>() == TypeId::of::<Array>() {
+            panic!("Cannot register indexer for arrays.");
+        }
+        #[cfg(not(feature = "no_object"))]
+        if TypeId::of::<A>() == TypeId::of::<Map>() {
+            panic!("Cannot register indexer for object maps.");
+        }
+        if TypeId::of::<A>() == TypeId::of::<String>()
+            || TypeId::of::<A>() == TypeId::of::<&str>()
+            || TypeId::of::<A>() == TypeId::of::<ImmutableString>()
+        {
+            panic!("Cannot register indexer for strings.");
+        }
+
         let f = move |_: &Engine, _: &Module, args: &mut FnCallArgs| {
             let b = cast_arg::<B>(&mut args[1]);
             let c = cast_arg::<C>(&mut args[2]);
@@ -812,6 +847,11 @@ impl Module {
     ///
     /// If there are similar existing Rust functions, they are replaced.
     ///
+    /// # Panics
+    ///
+    /// Panics if the type is `Array` or `Map`.
+    /// Indexers for arrays, object maps and strings cannot be registered.
+    ///
     /// # Examples
     ///
     /// ```
@@ -830,7 +870,6 @@ impl Module {
     /// assert!(module.contains_fn(hash_get, true));
     /// assert!(module.contains_fn(hash_set, true));
     /// ```
-    #[cfg(not(feature = "no_object"))]
     #[cfg(not(feature = "no_index"))]
     pub fn set_indexer_get_set_fn<A: Variant + Clone, B: Variant + Clone, T: Variant + Clone>(
         &mut self,

src/packages/map_basic.rs

@@ -18,7 +18,6 @@ mod map_functions {
     pub fn has(map: &mut Map, prop: ImmutableString) -> bool {
         map.contains_key(&prop)
     }
-    #[rhai_fn(name = "len", get = "len")]
     #[inline(always)]
     pub fn len(map: &mut Map) -> INT {
         map.len() as INT