From eeca8c90aa3bc1a04f5aded49cf2b5e695586ff5 Mon Sep 17 00:00:00 2001
From: Stephen Chung <schungx@live.com>
Date: Fri, 5 Nov 2021 23:22:05 +0800
Subject: [PATCH] Make sure the global namespace is always searched first.

---
 CHANGELOG.md                 |  1 +
 src/engine.rs                |  1 +
 src/engine_api.rs            | 11 +++++-----
 src/fn_call.rs               |  2 +-
 src/module/mod.rs            |  3 +++
 src/optimize.rs              |  9 ++++----
 src/packages/arithmetic.rs   |  2 ++
 src/packages/array_basic.rs  |  2 ++
 src/packages/fn_basic.rs     |  2 ++
 src/packages/iter_basic.rs   |  2 ++
 src/packages/lang_core.rs    |  2 ++
 src/packages/logic.rs        |  2 ++
 src/packages/map_basic.rs    |  2 ++
 src/packages/math_basic.rs   |  2 ++
 src/packages/pkg_core.rs     |  2 ++
 src/packages/pkg_std.rs      |  2 ++
 src/packages/string_basic.rs |  2 ++
 src/packages/string_more.rs  |  2 ++
 src/packages/time_basic.rs   |  2 ++
 tests/native.rs              | 40 +++++++++++++++++++++++++++++++++++-
 20 files changed, 82 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c25a151c..dfea06d4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ Bug fixes
 ---------
 
 * Reverses a regression on string `+` operations.
+* The global namespace is now searched before packages, which is the correct behavior.
 
 
 Version 1.1.2
diff --git a/src/engine.rs b/src/engine.rs
index 477e3f6e..69d8fc40 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -2970,6 +2970,7 @@ impl Engine {
                             // Create automatic global module
                             let mut global = Module::new();
                             global.internal = true;
+                            global.standard = false;
                             mods.push(KEYWORD_GLOBAL, global);
                             Some(mods.get_mut(mods.len() - 1).expect("global module exists"))
                         };
diff --git a/src/engine_api.rs b/src/engine_api.rs
index 8ba01d01..8998f01e 100644
--- a/src/engine_api.rs
+++ b/src/engine_api.rs
@@ -27,16 +27,16 @@ impl Engine {
     #[allow(dead_code)]
     pub(crate) fn global_namespace(&self) -> &Module {
         self.global_modules
-            .last()
+            .first()
             .expect("global_modules contains at least one module")
     }
     /// Get a mutable reference to the global namespace module
-    /// (which is the last module in `global_modules`).
+    /// (which is the first module in `global_modules`).
     #[inline(always)]
     pub(crate) fn global_namespace_mut(&mut self) -> &mut Module {
         Shared::get_mut(
             self.global_modules
-                .last_mut()
+                .first_mut()
                 .expect("global_modules contains at least one module"),
         )
         .expect("global namespace module is never shared")
@@ -895,8 +895,9 @@ impl Engine {
     /// modules are searched in reverse order.
     #[inline(always)]
     pub fn register_global_module(&mut self, module: Shared<Module>) -> &mut Self {
-        // Insert the module into the front
-        self.global_modules.insert(0, module);
+        // Insert the module into the front.
+        // The first module is always the global namespace.
+        self.global_modules.insert(1, module);
         self
     }
     /// Register a shared [`Module`] as a static module namespace with the [`Engine`].
diff --git a/src/fn_call.rs b/src/fn_call.rs
index 2c317c14..51666df4 100644
--- a/src/fn_call.rs
+++ b/src/fn_call.rs
@@ -613,7 +613,7 @@ impl Engine {
 
         // First check script-defined functions
         let result = lib.iter().any(|&m| m.contains_fn(hash_script))
-            // Then check packages
+            // Then check the global namespace and packages
             || self.global_modules.iter().any(|m| m.contains_fn(hash_script))
             // Then check imported modules
             || mods.map_or(false, |m| m.contains_fn(hash_script))
diff --git a/src/module/mod.rs b/src/module/mod.rs
index baa9c7fb..f9a7c2bc 100644
--- a/src/module/mod.rs
+++ b/src/module/mod.rs
@@ -131,6 +131,8 @@ pub struct Module {
     id: Option<Identifier>,
     /// Is this module internal?
     pub(crate) internal: bool,
+    /// Is this module part of a standard library?
+    pub(crate) standard: bool,
     /// Sub-modules.
     modules: BTreeMap<Identifier, Shared<Module>>,
     /// [`Module`] variables.
@@ -246,6 +248,7 @@ impl Module {
         Self {
             id: None,
             internal: false,
+            standard: false,
             modules: Default::default(),
             variables: Default::default(),
             all_variables: Default::default(),
diff --git a/src/optimize.rs b/src/optimize.rs
index 4eee9bc1..788fd82b 100644
--- a/src/optimize.rs
+++ b/src/optimize.rs
@@ -145,12 +145,13 @@ impl<'a> OptimizerState<'a> {
             .map(|(v, _)| v)
     }
     // Has a system function a Rust-native override?
-    pub fn has_native_fn(&self, hash_script: u64, arg_types: &[TypeId]) -> bool {
+    pub fn has_native_fn_override(&self, hash_script: u64, arg_types: &[TypeId]) -> bool {
         let hash_params = calc_fn_params_hash(arg_types.iter().cloned());
         let hash = combine_hashes(hash_script, hash_params);
 
-        // First  check packages
-        self.engine.global_modules.iter().any(|m| m.contains_fn(hash))
+        // First check the global namespace and packages, but skip modules that are standard because
+        // they should never conflict with system functions.
+        self.engine.global_modules.iter().filter(|m| !m.standard).any(|m| m.contains_fn(hash))
             // Then check sub-modules
             || self.engine.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash))
     }
@@ -997,7 +998,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
                     return;
                 }
                 // Overloaded operators can override built-in.
-                _ if x.args.len() == 2 && !state.has_native_fn(x.hashes.native, arg_types.as_ref()) => {
+                _ if x.args.len() == 2 && !state.has_native_fn_override(x.hashes.native, arg_types.as_ref()) => {
                     if let Some(result) = get_builtin_binary_op_fn(x.name.as_ref(), &arg_values[0], &arg_values[1])
                         .and_then(|f| {
                             let ctx = (state.engine, x.name.as_ref(), state.lib).into();
diff --git a/src/packages/arithmetic.rs b/src/packages/arithmetic.rs
index 66c05978..898117bc 100644
--- a/src/packages/arithmetic.rs
+++ b/src/packages/arithmetic.rs
@@ -177,6 +177,8 @@ macro_rules! reg_functions {
 }
 
 def_package!(crate:ArithmeticPackage:"Basic arithmetic", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "int", int_functions);
     reg_functions!(lib += signed_basic; INT);
 
diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs
index f119bc0e..10c0316a 100644
--- a/src/packages/array_basic.rs
+++ b/src/packages/array_basic.rs
@@ -9,6 +9,8 @@ use std::prelude::v1::*;
 use std::{any::TypeId, cmp::Ordering, mem};
 
 def_package!(crate:BasicArrayPackage:"Basic array utilities.", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "array", array_functions);
 
     // Register array iterator
diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs
index 4a92e1cc..48835a57 100644
--- a/src/packages/fn_basic.rs
+++ b/src/packages/fn_basic.rs
@@ -4,6 +4,8 @@ use crate::{def_package, FnPtr, ImmutableString, NativeCallContext};
 use std::prelude::v1::*;
 
 def_package!(crate:BasicFnPackage:"Basic Fn functions.", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "FnPtr", fn_ptr_functions);
 });
 
diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs
index 65fb6484..d06c823f 100644
--- a/src/packages/iter_basic.rs
+++ b/src/packages/iter_basic.rs
@@ -292,6 +292,8 @@ macro_rules! reg_range {
 }
 
 def_package!(crate:BasicIteratorPackage:"Basic range iterators.", lib, {
+    lib.standard = true;
+
     reg_range!(lib | "range" => INT);
 
     #[cfg(not(feature = "only_i32"))]
diff --git a/src/packages/lang_core.rs b/src/packages/lang_core.rs
index 26d4c5b0..1c88adbe 100644
--- a/src/packages/lang_core.rs
+++ b/src/packages/lang_core.rs
@@ -43,5 +43,7 @@ mod core_functions {
 }
 
 def_package!(crate:LanguageCorePackage:"Language core functions.", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "language_core", core_functions);
 });
diff --git a/src/packages/logic.rs b/src/packages/logic.rs
index a1fd8d8f..4bcfdca6 100644
--- a/src/packages/logic.rs
+++ b/src/packages/logic.rs
@@ -38,6 +38,8 @@ macro_rules! reg_functions {
 }
 
 def_package!(crate:LogicPackage:"Logical operators.", lib, {
+    lib.standard = true;
+
     #[cfg(not(feature = "only_i32"))]
     #[cfg(not(feature = "only_i64"))]
     {
diff --git a/src/packages/map_basic.rs b/src/packages/map_basic.rs
index b042d9e1..c8cf5eb2 100644
--- a/src/packages/map_basic.rs
+++ b/src/packages/map_basic.rs
@@ -10,6 +10,8 @@ use std::prelude::v1::*;
 use crate::Array;
 
 def_package!(crate:BasicMapPackage:"Basic object map utilities.", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "map", map_functions);
 });
 
diff --git a/src/packages/math_basic.rs b/src/packages/math_basic.rs
index 0f9f6a88..8caf5d72 100644
--- a/src/packages/math_basic.rs
+++ b/src/packages/math_basic.rs
@@ -62,6 +62,8 @@ macro_rules! reg_functions {
 }
 
 def_package!(crate:BasicMathPackage:"Basic mathematic functions.", lib, {
+    lib.standard = true;
+
     // Integer functions
     combine_with_exported_module!(lib, "int", int_functions);
 
diff --git a/src/packages/pkg_core.rs b/src/packages/pkg_core.rs
index 7f79b3f4..e82e7fc2 100644
--- a/src/packages/pkg_core.rs
+++ b/src/packages/pkg_core.rs
@@ -10,6 +10,8 @@ use std::prelude::v1::*;
 use crate::def_package;
 
 def_package!(crate:CorePackage:"_Core_ package containing basic facilities.", lib, {
+    lib.standard = true;
+
     LanguageCorePackage::init(lib);
     ArithmeticPackage::init(lib);
     LogicPackage::init(lib);
diff --git a/src/packages/pkg_std.rs b/src/packages/pkg_std.rs
index 3c18d603..afdece41 100644
--- a/src/packages/pkg_std.rs
+++ b/src/packages/pkg_std.rs
@@ -13,6 +13,8 @@ use std::prelude::v1::*;
 use crate::def_package;
 
 def_package!(crate:StandardPackage:"_Standard_ package containing all built-in features.", lib, {
+    lib.standard = true;
+
     CorePackage::init(lib);
     BasicMathPackage::init(lib);
     #[cfg(not(feature = "no_index"))]
diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs
index 71a326a3..500cd30f 100644
--- a/src/packages/string_basic.rs
+++ b/src/packages/string_basic.rs
@@ -16,6 +16,8 @@ pub const FUNC_TO_STRING: &str = "to_string";
 pub const FUNC_TO_DEBUG: &str = "to_debug";
 
 def_package!(crate:BasicStringPackage:"Basic string utilities, including printing.", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "print_debug", print_debug_functions);
     combine_with_exported_module!(lib, "number_formatting", number_formatting);
 });
diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs
index 240d0adb..7cb4f027 100644
--- a/src/packages/string_more.rs
+++ b/src/packages/string_more.rs
@@ -9,6 +9,8 @@ use std::{any::TypeId, mem};
 use super::string_basic::{print_with_func, FUNC_TO_STRING};
 
 def_package!(crate:MoreStringPackage:"Additional string utilities, including string building.", lib, {
+    lib.standard = true;
+
     combine_with_exported_module!(lib, "string", string_functions);
 });
 
diff --git a/src/packages/time_basic.rs b/src/packages/time_basic.rs
index eeff7a50..7b5bbbcd 100644
--- a/src/packages/time_basic.rs
+++ b/src/packages/time_basic.rs
@@ -16,6 +16,8 @@ use std::time::{Duration, Instant};
 use instant::{Duration, Instant};
 
 def_package!(crate:BasicTimePackage:"Basic timing utilities.", lib, {
+    lib.standard = true;
+
     // Register date/time functions
     combine_with_exported_module!(lib, "time", time_functions);
 });
diff --git a/tests/native.rs b/tests/native.rs
index 349cebf8..28b00680 100644
--- a/tests/native.rs
+++ b/tests/native.rs
@@ -1,4 +1,4 @@
-use rhai::{Dynamic, Engine, EvalAltResult, NativeCallContext, INT};
+use rhai::{Dynamic, Engine, EvalAltResult, ImmutableString, NativeCallContext, INT};
 use std::any::TypeId;
 
 #[cfg(not(feature = "no_module"))]
@@ -49,3 +49,41 @@ fn test_native_context_fn_name() -> Result<(), Box<EvalAltResult>> {
 
     Ok(())
 }
+
+#[test]
+fn test_native_overload() -> Result<(), Box<EvalAltResult>> {
+    let mut engine = Engine::new();
+
+    assert_eq!(
+        engine.eval::<String>(r#"let x = "hello, "; let y = "world"; x + y"#)?,
+        "hello, world"
+    );
+    assert_eq!(
+        engine.eval::<String>(r#"let x = "hello"; let y = (); x + y"#)?,
+        "hello"
+    );
+
+    // Overload the `+` operator for strings
+
+    engine
+        .register_fn(
+            "+",
+            |s1: ImmutableString, s2: ImmutableString| -> ImmutableString {
+                format!("{}***{}", s1, s2).into()
+            },
+        )
+        .register_fn("+", |s1: ImmutableString, _: ()| -> ImmutableString {
+            format!("{} Foo!", s1).into()
+        });
+
+    assert_eq!(
+        engine.eval::<String>(r#"let x = "hello"; let y = "world"; x + y"#)?,
+        "hello***world"
+    );
+    assert_eq!(
+        engine.eval::<String>(r#"let x = "hello"; let y = (); x + y"#)?,
+        "hello Foo!"
+    );
+
+    Ok(())
+}