Prefer Engine::disable_symbol to disable eval.

RELEASES.md

@@ -20,6 +20,7 @@ Breaking changes
 
 * `Module::set_fn`, `Module::set_raw_fn` and `Module::set_fn_XXX_mut` all take an additional parameter of `FnNamespace`.
 * `unless` is now a reserved keyword.
+* `EvalPackage` is removed in favor of `Engine::disable_symbol`.
 
 New features
 ------------

doc/src/language/eval.md

@@ -60,7 +60,13 @@ print(x);
 --------------
 
 For those who subscribe to the (very sensible) motto of ["`eval` is evil"](http://linterrors.com/js/eval-is-evil),
-disable `eval` by overloading it, probably with something that throws.
+disable `eval` using [`Engine::disable_symbol`][disable keywords and operators]:
+
+```rust
+engine.disable_symbol("eval");  // disable usage of 'eval'
+```
+
+`eval` can also be disabled by overloading it, probably with something that throws:
 
 ```rust
 fn eval(script) { throw "eval is evil! I refuse to run " + script }
@@ -75,20 +81,3 @@ engine.register_result_fn("eval", |script: String| -> Result<(), Box<EvalAltResu
     Err(format!("eval is evil! I refuse to run {}", script).into())
 });
 ```
-
-
-`EvalPackage`
--------------
-
-There is even a package named [`EvalPackage`][packages] which implements the disabling override:
-
-```rust
-use rhai::Engine;
-use rhai::packages::Package             // load the 'Package' trait to use packages
-use rhai::packages::EvalPackage;        // the 'eval' package disables 'eval'
-
-let mut engine = Engine::new();
-let package = EvalPackage::new();       // create the package
-
-engine.load_package(package.get());     // load the package
-```

doc/src/rust/packages/builtin.md

@@ -19,7 +19,6 @@ Built-In Packages
 | `BasicArrayPackage`    | basic [array] functions (not available under `no_index`)                                               |    no     |      yes      |
 | `BasicMapPackage`      | basic [object map] functions (not available under `no_object`)                                         |    no     |      yes      |
 | `BasicFnPackage`       | basic methods for [function pointers].                                                                 |    yes    |      yes      |
-| `EvalPackage`          | disable [`eval`]                                                                                       |    no     |      no       |
 | `CorePackage`          | basic essentials                                                                                       |    yes    |      yes      |
 | `StandardPackage`      | standard library (default for `Engine::new`)                                                           |    no     |      yes      |
 

src/packages/eval.rs

@@ -1,14 +0,0 @@
-use crate::plugin::*;
-use crate::{def_package, Dynamic, EvalAltResult, ImmutableString};
-
-def_package!(crate:EvalPackage:"Disable 'eval'.", lib, {
-    combine_with_exported_module!(lib, "eval", eval_override);
-});
-
-#[export_module]
-mod eval_override {
-    #[rhai_fn(return_raw)]
-    pub fn eval(_script: ImmutableString) -> Result<Dynamic, Box<EvalAltResult>> {
-        Err("eval is evil!".into())
-    }
-}

src/packages/mod.rs

@@ -6,7 +6,6 @@ use crate::{Module, Shared, StaticVec};
 
 pub(crate) mod arithmetic;
 mod array_basic;
-mod eval;
 mod fn_basic;
 mod iter_basic;
 mod logic;
@@ -21,7 +20,6 @@ mod time_basic;
 pub use arithmetic::ArithmeticPackage;
 #[cfg(not(feature = "no_index"))]
 pub use array_basic::BasicArrayPackage;
-pub use eval::EvalPackage;
 pub use fn_basic::BasicFnPackage;
 pub use iter_basic::BasicIteratorPackage;
 pub use logic::LogicPackage;

src/parse_error.rs

@@ -32,7 +32,7 @@ pub enum LexError {
     /// An identifier is in an invalid format.
     MalformedIdentifier(String),
     /// Bad symbol encountered when tokenizing the script text.
-    ImproperSymbol(String),
+    ImproperSymbol(String, String),
 }
 
 impl Error for LexError {}
@@ -47,7 +47,10 @@ impl fmt::Display for LexError {
             Self::MalformedIdentifier(s) => write!(f, "{}: '{}'", self.desc(), s),
             Self::UnterminatedString => f.write_str(self.desc()),
             Self::StringTooLong(max) => write!(f, "{} ({})", self.desc(), max),
-            Self::ImproperSymbol(s) => f.write_str(s),
+            Self::ImproperSymbol(s, d) if d.is_empty() => {
+                write!(f, "Invalid symbol encountered: '{}'", s)
+            }
+            Self::ImproperSymbol(_, d) => f.write_str(d),
         }
     }
 }
@@ -62,7 +65,7 @@ impl LexError {
             Self::MalformedNumber(_) => "Invalid number",
             Self::MalformedChar(_) => "Invalid character",
             Self::MalformedIdentifier(_) => "Variable name is not proper",
-            Self::ImproperSymbol(_) => "Invalid symbol encountered",
+            Self::ImproperSymbol(_, _) => "Invalid symbol encountered",
         }
     }
     /// Convert a `&LexError` into a [`ParseError`].

src/parser.rs

@@ -995,11 +995,8 @@ fn parse_primary(
         // Access to `this` as a variable is OK
         Token::Reserved(s) if s == KEYWORD_THIS && *next_token != Token::LeftParen => {
             if !settings.is_function_scope {
-                return Err(PERR::BadInput(LexError::ImproperSymbol(format!(
+                let msg = format!("'{}' can only be used in functions", s);
-                    "'{}' can only be used in functions",
+                return Err(PERR::BadInput(LexError::ImproperSymbol(s, msg)).into_err(settings.pos));
-                    s
-                )))
-                .into_err(settings.pos));
             } else {
                 let var_name_def = IdentX::new(state.get_interned_string(s), settings.pos);
                 Expr::Variable(Box::new((None, None, 0, var_name_def)))
@@ -1045,6 +1042,7 @@ fn parse_primary(
                     LexError::UnexpectedInput(Token::Bang.syntax().to_string()).into_err(token_pos)
                 } else {
                     PERR::BadInput(LexError::ImproperSymbol(
+                        "!".to_string(),
                         "'!' cannot be used to call module functions".to_string(),
                     ))
                     .into_err(token_pos)
@@ -1333,6 +1331,7 @@ fn make_assignment_stmt<'a>(
         }
         // ??? && ??? = rhs, ??? || ??? = rhs
         Expr::And(_, _) | Expr::Or(_, _) => Err(PERR::BadInput(LexError::ImproperSymbol(
+            "=".to_string(),
             "Possibly a typo of '=='?".to_string(),
         ))
         .into_err(pos)),
@@ -1438,10 +1437,13 @@ fn make_dot_expr(
                 && [crate::engine::KEYWORD_FN_PTR, crate::engine::KEYWORD_EVAL]
                     .contains(&x.name.as_ref()) =>
         {
-            return Err(PERR::BadInput(LexError::ImproperSymbol(format!(
+            return Err(PERR::BadInput(LexError::ImproperSymbol(
-                "'{}' should not be called in method style. Try {}(...);",
+                x.name.to_string(),
-                x.name, x.name
+                format!(
-            )))
+                    "'{}' should not be called in method style. Try {}(...);",
+                    x.name, x.name
+                ),
+            ))
             .into_err(pos));
         }
         // lhs.func!(...)
@@ -1932,20 +1934,22 @@ fn ensure_not_statement_expr(input: &mut TokenStream, type_name: &str) -> Result
 fn ensure_not_assignment(input: &mut TokenStream) -> Result<(), ParseError> {
     match input.peek().unwrap() {
         (Token::Equals, pos) => Err(PERR::BadInput(LexError::ImproperSymbol(
+            "=".to_string(),
             "Possibly a typo of '=='?".to_string(),
         ))
         .into_err(*pos)),
-        (Token::PlusAssign, pos)
+        (token @ Token::PlusAssign, pos)
-        | (Token::MinusAssign, pos)
+        | (token @ Token::MinusAssign, pos)
-        | (Token::MultiplyAssign, pos)
+        | (token @ Token::MultiplyAssign, pos)
-        | (Token::DivideAssign, pos)
+        | (token @ Token::DivideAssign, pos)
-        | (Token::LeftShiftAssign, pos)
+        | (token @ Token::LeftShiftAssign, pos)
-        | (Token::RightShiftAssign, pos)
+        | (token @ Token::RightShiftAssign, pos)
-        | (Token::ModuloAssign, pos)
+        | (token @ Token::ModuloAssign, pos)
-        | (Token::PowerOfAssign, pos)
+        | (token @ Token::PowerOfAssign, pos)
-        | (Token::AndAssign, pos)
+        | (token @ Token::AndAssign, pos)
-        | (Token::OrAssign, pos)
+        | (token @ Token::OrAssign, pos)
-        | (Token::XOrAssign, pos) => Err(PERR::BadInput(LexError::ImproperSymbol(
+        | (token @ Token::XOrAssign, pos) => Err(PERR::BadInput(LexError::ImproperSymbol(
+            token.syntax().to_string(),
             "Expecting a boolean expression, not an assignment".to_string(),
         ))
         .into_err(*pos)),

src/syntax.rs

@@ -137,11 +137,14 @@ impl Engine {
                         .map(|v| v.is_keyword() || v.is_reserved())
                         .unwrap_or(false) =>
                 {
-                    return Err(LexError::ImproperSymbol(format!(
+                    return Err(LexError::ImproperSymbol(
-                        "Improper symbol for custom syntax at position #{}: '{}'",
+                        s.to_string(),
-                        segments.len() + 1,
+                        format!(
-                        s
+                            "Improper symbol for custom syntax at position #{}: '{}'",
-                    ))
+                            segments.len() + 1,
+                            s
+                        ),
+                    )
                     .into_err(Position::NONE)
                     .into());
                 }
@@ -154,11 +157,14 @@ impl Engine {
                 }
                 // Anything else is an error
                 _ => {
-                    return Err(LexError::ImproperSymbol(format!(
+                    return Err(LexError::ImproperSymbol(
-                        "Improper symbol for custom syntax at position #{}: '{}'",
+                        s.to_string(),
-                        segments.len() + 1,
+                        format!(
-                        s
+                            "Improper symbol for custom syntax at position #{}: '{}'",
-                    ))
+                            segments.len() + 1,
+                            s
+                        ),
+                    )
                     .into_err(Position::NONE)
                     .into());
                 }

src/token.rs

@@ -1658,39 +1658,41 @@ impl<'a> Iterator for TokenIterator<'a, '_> {
             Some((Token::Reserved(s), pos)) => Some((match
                 (s.as_str(), self.engine.custom_keywords.contains_key(&s))
             {
-                ("===", false) => Token::LexError(LERR::ImproperSymbol(
+                ("===", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'===' is not a valid operator. This is not JavaScript! Should it be '=='?".to_string(),
                 )),
-                ("!==", false) => Token::LexError(LERR::ImproperSymbol(
+                ("!==", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'!==' is not a valid operator. This is not JavaScript! Should it be '!='?".to_string(),
                 )),
-                ("->", false) => Token::LexError(LERR::ImproperSymbol(
+                ("->", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'->' is not a valid symbol. This is not C or C++!".to_string())),
-                ("<-", false) => Token::LexError(LERR::ImproperSymbol(
+                ("<-", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'<-' is not a valid symbol. This is not Go! Should it be '<='?".to_string(),
                 )),
-                (":=", false) => Token::LexError(LERR::ImproperSymbol(
+                (":=", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "':=' is not a valid assignment operator. This is not Go! Should it be simply '='?".to_string(),
                 )),
-                ("::<", false) => Token::LexError(LERR::ImproperSymbol(
+                ("::<", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'::<>' is not a valid symbol. This is not Rust! Should it be '::'?".to_string(),
                 )),
-                ("(*", false) | ("*)", false) => Token::LexError(LERR::ImproperSymbol(
+                ("(*", false) | ("*)", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'(* .. *)' is not a valid comment format. This is not Pascal! Should it be '/* .. */'?".to_string(),
                 )),
-                ("#", false) => Token::LexError(LERR::ImproperSymbol(
+                ("#", false) => Token::LexError(LERR::ImproperSymbol(s,
                     "'#' is not a valid symbol. Should it be '#{'?".to_string(),
                 )),
                 // Reserved keyword/operator that is custom.
                 (_, true) => Token::Custom(s),
                 // Reserved operator that is not custom.
-                (token, false) if !is_valid_identifier(token.chars()) => Token::LexError(LERR::ImproperSymbol(
+                (token, false) if !is_valid_identifier(token.chars()) => {
-                    format!("'{}' is a reserved symbol", token)
+                    let msg = format!("'{}' is a reserved symbol", token);
-                )),
+                    Token::LexError(LERR::ImproperSymbol(s, msg))
+                },
                 // Reserved keyword that is not custom and disabled.
-                (token, false) if self.engine.disabled_symbols.contains(token) => Token::LexError(LERR::ImproperSymbol(
+                (token, false) if self.engine.disabled_symbols.contains(token) => {
-                    format!("reserved symbol '{}' is disabled", token)
+                    let msg = format!("reserved symbol '{}' is disabled", token);
-                )),
+                    Token::LexError(LERR::ImproperSymbol(s, msg))
+                },
                 // Reserved keyword/operator that is not custom.
                 (_, false) => Token::Reserved(s),
             }, pos)),

tests/eval.rs

@@ -1,4 +1,4 @@
-use rhai::{Engine, EvalAltResult, Scope, INT};
+use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, RegisterFn, Scope, INT};
 
 #[test]
 fn test_eval() -> Result<(), Box<EvalAltResult>> {
@@ -98,10 +98,34 @@ fn test_eval_override() -> Result<(), Box<EvalAltResult>> {
                 fn eval(x) { x }    // reflect the script back
 
                 eval("40 + 2")
-    "#
+            "#
         )?,
         "40 + 2"
     );
 
+    let mut engine = Engine::new();
+
+    // Reflect the script back
+    engine.register_fn("eval", |script: &str| script.to_string());
+
+    assert_eq!(engine.eval::<String>(r#"eval("40 + 2")"#)?, "40 + 2");
+
+    Ok(())
+}
+
+#[test]
+fn test_eval_disabled() -> Result<(), Box<EvalAltResult>> {
+    let mut engine = Engine::new();
+
+    engine.disable_symbol("eval");
+
+    assert!(matches!(
+        *engine
+            .compile(r#"eval("40 + 2")"#)
+            .expect_err("should error")
+            .0,
+        ParseErrorType::BadInput(LexError::ImproperSymbol(err, _)) if err == "eval"
+    ));
+
     Ok(())
 }