Resolving issues for PR #129

README.md

@@ -287,6 +287,10 @@ assert_eq!(eval_with_context_mut("a = 5.5", &mut context),
            Err(EvalexprError::ExpectedInt { actual: Value::from(5.5) }));
 // Reading a variable does not require a mutable context
 assert_eq!(eval_with_context("a", &context), Ok(Value::from(5)));
+// Builtin functions are enabled by default.
+assert_eq!(eval_with_context("max(1,3)",&context),Ok(Value::from(3)));
+context.set_builtin_functions_disabled(true);
+assert_eq!(eval_with_context("max(1,3)",&context),Err(EvalexprError::FunctionIdentifierNotFound(String::from("max"))));
 
 ```
 
@@ -300,6 +304,8 @@ This means that assigning to `a` again with a different type yields an error.
 Type unsafe contexts may be implemented if requested.
 For reading `a`, it is enough to pass an immutable reference.
 
+EmptyContext have builtin functions disabled and can't be enabled.
+EmptyContextWithBuiltinFunctions have builtin functions enabled and can't be disabled.
 Contexts can also be manipulated in code.
 Take a look at the following example:
 
@@ -333,6 +339,12 @@ assert_eq!(eval_int_with_context("f 5", &context), Ok(10));
 For more information about user-defined functions, refer to the respective [section](#user-defined-functions).
 
 ### Builtin Functions
+Builtin functions are enabled by default for HashMap Context. It can be disabled by calling Context::
+set_builtin_functions_disabled
+
+It's disabled for EmptyContext. It can't be enabled for EmptyContext
+
+It's enabled for EmptyContextWithBuiltinfunctions. It can't be disabled.
 
 This crate offers a set of builtin functions.
 
@@ -395,6 +407,9 @@ Otherwise, a float is returned.
 
 The regex functions require the feature flag `regex_support`.
 
+Builtin functions are enabled by Default.
+It can be disabled by calling Context::disable_builtin_fn().
+
 ### Values
 
 Operators take values as arguments and produce values as results.

src/context/mod.rs

@@ -24,10 +24,10 @@ pub trait Context {
     fn call_function(&self, identifier: &str, argument: &Value) -> EvalexprResult<Value>;
 
     /// Checks if builtin function has been disabled.
-    fn is_builtin_fn_disabled(&self) -> bool;
+    fn are_builtin_functions_disabled(&self) -> bool;
 
     /// Disables Builtin function.
-    fn disable_builtin_fn(&mut self);
+    fn set_builtin_functions_disabled(&mut self, disabled: bool) -> EvalexprResult<()>;
 }
 
 /// A context that allows to assign to variables.
@@ -72,6 +72,7 @@ pub trait GetFunctionContext: Context {
 }*/
 
 /// A context that returns `None` for each identifier.
+/// Builtin functiions are disabled.
 #[derive(Debug, Default)]
 pub struct EmptyContext;
 
@@ -85,10 +86,12 @@ impl Context for EmptyContext {
             identifier.to_string(),
         ))
     }
-    /// Builtin functions can't be disbaled for Empty Context.
+    /// Builtin functions can't be enabled for Empty Context.
-    fn disable_builtin_fn(&mut self) {}
+    fn set_builtin_functions_disabled(&mut self, _disabled: bool) -> EvalexprResult<()> {
+        Err(EvalexprError::InvalidBuiltinFunctionsContext)
+    }
     /// Builtin functions are always disabled for Empty Context.
-    fn is_builtin_fn_disabled(&self) -> bool {
+    fn are_builtin_functions_disabled(&self) -> bool {
         true
     }
 }
@@ -106,6 +109,42 @@ impl<'a> IterateVariablesContext<'a> for EmptyContext {
     }
 }
 
+/// Same as Empty Context except Builtin functions are enabled.
+#[derive(Debug, Default)]
+pub struct EmptyContextWithBuiltinFunctions;
+
+impl Context for EmptyContextWithBuiltinFunctions {
+    fn get_value(&self, _identifier: &str) -> Option<&Value> {
+        None
+    }
+
+    fn call_function(&self, identifier: &str, _argument: &Value) -> EvalexprResult<Value> {
+        Err(EvalexprError::FunctionIdentifierNotFound(
+            identifier.to_string(),
+        ))
+    }
+    /// Builtin functions can't be disbaled for EmptyContextWithBuiltinFunctions.
+    fn set_builtin_functions_disabled(&mut self, _disabled: bool) -> EvalexprResult<()> {
+        Err(EvalexprError::InvalidBuiltinFunctionsContext)
+    }
+    /// Builtin functions are always enabled for EmptyContextWithBuiltinFunctions.
+    fn are_builtin_functions_disabled(&self) -> bool {
+        false
+    }
+}
+
+impl<'a> IterateVariablesContext<'a> for EmptyContextWithBuiltinFunctions {
+    type VariableIterator = iter::Empty<(String, Value)>;
+    type VariableNameIterator = iter::Empty<String>;
+
+    fn iter_variables(&self) -> Self::VariableIterator {
+        iter::empty()
+    }
+
+    fn iter_variable_names(&self) -> Self::VariableNameIterator {
+        iter::empty()
+    }
+}
 /// A context that stores its mappings in hash maps.
 ///
 /// *Value and function mappings are stored independently, meaning that there can be a function and a value with the same identifier.*
@@ -117,7 +156,7 @@ pub struct HashMapContext {
     variables: HashMap<String, Value>,
     #[cfg_attr(feature = "serde_support", serde(skip))]
     functions: HashMap<String, Function>,
-    without_builtin_fn: bool,
+    without_builtin_functions: bool,
 }
 
 impl HashMapContext {
@@ -142,12 +181,13 @@ impl Context for HashMapContext {
         }
     }
 
-    fn disable_builtin_fn(&mut self) {
+    fn set_builtin_functions_disabled(&mut self, disabled: bool) -> EvalexprResult<()> {
-        self.without_builtin_fn = true;
+        self.without_builtin_functions = disabled;
+        Ok(())
     }
 
-    fn is_builtin_fn_disabled(&self) -> bool {
+    fn are_builtin_functions_disabled(&self) -> bool {
-        self.without_builtin_fn
+        self.without_builtin_functions
     }
 }
 

src/error/display.rs

@@ -113,6 +113,7 @@ impl fmt::Display for EvalexprError {
                 regex, message
             ),
             ContextNotMutable => write!(f, "Cannot manipulate context"),
+            InvalidBuiltinFunctionsContext => write!(f, "Invalid Builtin Functions Context"),
             IllegalEscapeSequence(string) => write!(f, "Illegal escape sequence: {}", string),
             CustomMessage(message) => write!(f, "Error: {}", message),
         }

src/error/mod.rs

@@ -203,6 +203,9 @@ pub enum EvalexprError {
     /// An escape sequence within a string literal is illegal.
     IllegalEscapeSequence(String),
 
+    /// Empty Context can't have builtin functions enabled
+    InvalidBuiltinFunctionsContext,
+
     /// A custom error explained by its message.
     CustomMessage(String),
 }

src/lib.rs

@@ -252,8 +252,6 @@
 //! When assigning to variables, the assignment is stored in a context.
 //! When the variable is read later on, it is read from the context.
 //! Contexts can be preserved between multiple calls to eval by creating them yourself.
-//! By default, Builtin functions are diabled.
-//! Builtin functions can be disabled by calling Context::disable_builtin_fn().
 //! Here is a simple example to show the difference between preserving and not preserving context between evaluations:
 //!
 //! ```rust
@@ -274,9 +272,7 @@
 //! assert_eq!(eval_with_context("a", &context), Ok(Value::from(5)));
 //! // Builtin functions are enabled by default.
 //! assert_eq!(eval_with_context("max(1,3)",&context),Ok(Value::from(3)));
-//! //Disabling builtin function in Context.
+//! context.set_builtin_functions_disabled(true);
-//! context.disable_builtin_fn();
-//! // Builtin functions are disabled and using them returns Error.
 //! assert_eq!(eval_with_context("max(1,3)",&context),Err(EvalexprError::FunctionIdentifierNotFound(String::from("max"))));
 //!
 //! ```
@@ -291,6 +287,8 @@
 //! Type unsafe contexts may be implemented if requested.
 //! For reading `a`, it is enough to pass an immutable reference.
 //!
+//! EmptyContext have builtin functions disabled and can't be enabled.
+//! EmptyContextWithBuiltinFunctions have builtin functions enabled and can't be disabled.
 //! Contexts can also be manipulated in code.
 //! Take a look at the following example:
 //!
@@ -324,6 +322,12 @@
 //! For more information about user-defined functions, refer to the respective [section](#user-defined-functions).
 //!
 //! ### Builtin Functions
+//! Builtin functions are enabled by default for HashMap Context. It can be disabled by calling Context::
+//! set_builtin_functions_disabled
+//!
+//! It's disabled for EmptyContext. It can't be enabled for EmptyContext
+//!
+//! It's enabled for EmptyContextWithBuiltinfunctions. It can't be disabled.
 //!
 //! This crate offers a set of builtin functions.
 //!
@@ -549,7 +553,7 @@ extern crate serde_derive;
 pub use crate::{
     context::{
         Context, ContextWithMutableFunctions, ContextWithMutableVariables, EmptyContext,
-        HashMapContext, IterateVariablesContext,
+        EmptyContextWithBuiltinFunctions, HashMapContext, IterateVariablesContext,
     },
     error::{EvalexprError, EvalexprResult},
     function::Function,

src/operator/mod.rs

@@ -460,7 +460,7 @@ impl Operator {
 
                 match context.call_function(identifier, arguments) {
                     Err(EvalexprError::FunctionIdentifierNotFound(_))
-                        if !context.is_builtin_fn_disabled() =>
+                        if !context.are_builtin_functions_disabled() =>
                     {
                         if let Some(builtin_function) = builtin_function(identifier) {
                             builtin_function.call(arguments)

tests/integration.rs

@@ -1515,12 +1515,32 @@ fn test_type_errors_in_binary_operators() {
 
 #[test]
 fn test_empty_context() {
-    let context = EmptyContext;
+    let mut context = EmptyContext;
     assert_eq!(context.get_value("abc"), None);
     assert_eq!(
         context.call_function("abc", &Value::Empty),
         Err(EvalexprError::FunctionIdentifierNotFound("abc".to_owned()))
     );
+    assert_eq!(
+        eval_with_context("max(1,3)", &context),
+        Err(EvalexprError::FunctionIdentifierNotFound(String::from(
+            "max"
+        )))
+    );
+    assert_eq!(
+        context.set_builtin_functions_disabled(false),
+        Err(EvalexprError::InvalidBuiltinFunctionsContext)
+    )
+}
+
+#[test]
+fn test_empty_context_with_builtin_functions() {
+    let mut context = EmptyContextWithBuiltinFunctions;
+    assert_eq!(eval_with_context("max(1,3)", &context), Ok(Value::Int(3)));
+    assert_eq!(
+        context.set_builtin_functions_disabled(true),
+        Err(EvalexprError::InvalidBuiltinFunctionsContext)
+    );
 }
 
 #[test]
@@ -2245,12 +2265,17 @@ fn test_negative_power() {
 }
 
 #[test]
-fn test_disabling_builtin_fn() {
+fn test_builtin_functions_context() {
     let mut context = HashMapContext::new();
-    // Built in functions are enabled by default.
+    // Builtin functions are enabled by default for HashMapContext.
-    assert_eq!(eval_with_context("max(1,3)",&context),Ok(Value::from(3)));
+    assert_eq!(eval_with_context("max(1,3)", &context), Ok(Value::from(3)));
     // Disabling builtin function in Context.
-    context.disable_builtin_fn();
+    context.set_builtin_functions_disabled(true);
     // Builting functions are disabled and using them returns Error.
-    assert_eq!(eval_with_context("max(1,3)",&context),Err(EvalexprError::FunctionIdentifierNotFound(String::from("max"))));
+    assert_eq!(
+        eval_with_context("max(1,3)", &context),
+        Err(EvalexprError::FunctionIdentifierNotFound(String::from(
+            "max"
+        )))
+    );
 }