Allow non-custom reserved symbols in custom syntax.

CHANGELOG.md

@@ -12,6 +12,11 @@ New features
 
 * Added support for integer _ranges_ via the `..` and `..=` operators.
 
+Deprecated API's
+----------------
+
+* `Expression::get_variable_name` is deprecated in favor of the new `Expression::get_string_value`.
+
 
 Version 1.3.1
 =============

src/api/deprecated.rs

@@ -1,8 +1,8 @@
 //! Module containing all deprecated API that will be removed in the next major version.
 
 use crate::{
-    Dynamic, Engine, EvalAltResult, FnPtr, ImmutableString, NativeCallContext, RhaiResult, Scope,
+    Dynamic, Engine, EvalAltResult, Expression, FnPtr, ImmutableString, NativeCallContext,
-    AST,
+    RhaiResult, Scope, AST,
 };
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
@@ -304,3 +304,19 @@ impl FnPtr {
         self.call_raw(context, this_ptr, arg_values)
     }
 }
+
+impl Expression<'_> {
+    /// If this expression is a variable name, return it.  Otherwise [`None`].
+    ///
+    /// # Deprecated
+    ///
+    /// This method is deprecated. Use [`get_string_value`][Expression::get_string_value] instead.
+    ///
+    /// This method will be removed in the next major version.
+    #[deprecated(since = "1.4.0", note = "use `get_string_value` instead")]
+    #[inline(always)]
+    #[must_use]
+    pub fn get_variable_name(&self) -> Option<&str> {
+        self.get_string_value()
+    }
+}

src/custom_syntax.rs

@@ -65,11 +65,17 @@ impl<'a> From<&'a Expr> for Expression<'a> {
 }
 
 impl Expression<'_> {
-    /// If this expression is a variable name, return it.  Otherwise [`None`].
+    /// Get the value of this expression if it is a variable name or a string constant.
+    ///
+    /// Returns [`None`] also if the constant is not of the specified type.
     #[inline(always)]
     #[must_use]
-    pub fn get_variable_name(&self) -> Option<&str> {
+    pub fn get_string_value(&self) -> Option<&str> {
-        self.0.get_variable_name(true)
+        match self.0 {
+            Expr::Variable(_, _, x) if x.1.is_none() => Some(x.2.as_str()),
+            Expr::StringConstant(x, _) => Some(x.as_str()),
+            _ => None,
+        }
     }
     /// Get the position of this expression.
     #[inline(always)]
@@ -109,6 +115,9 @@ impl Expression<'_> {
         if TypeId::of::<T>() == TypeId::of::<ImmutableString>() {
             return match self.0 {
                 Expr::StringConstant(x, _) => unsafe_try_cast(x.clone()).ok(),
+                Expr::Variable(_, _, x) => {
+                    unsafe_try_cast(Into::<ImmutableString>::into(&x.2)).ok()
+                }
                 _ => None,
             };
         }

src/parser.rs

@@ -1401,10 +1401,7 @@ fn parse_primary(
                     let msg = format!("'{}' can only be used in functions", s);
                     return Err(LexError::ImproperSymbol(s.to_string(), msg).into_err(settings.pos));
                 }
-                _ if is_valid_identifier(s.chars()) => {
+                _ => return Err(PERR::Reserved(s.to_string()).into_err(settings.pos)),
-                    return Err(PERR::Reserved(s.to_string()).into_err(settings.pos))
-                }
-                _ => return Err(LexError::UnexpectedInput(s.to_string()).into_err(settings.pos)),
             }
         }
 
@@ -1698,12 +1695,12 @@ fn make_assignment_stmt(
                 ref e => Some(e.position()),
             },
             Expr::Index(x, term, _) | Expr::Dot(x, term, _) => match x.lhs {
-                Expr::Property(_) => unreachable!("unexpected Expr::Property in indexing"),
+                Expr::Property(_) => unreachable!("unexpected `Expr::Property` in indexing"),
                 _ if !term => check_lvalue(&x.rhs, matches!(expr, Expr::Dot(_, _, _))),
                 _ => None,
             },
             Expr::Property(_) if parent_is_dot => None,
-            Expr::Property(_) => unreachable!("unexpected Expr::Property in indexing"),
+            Expr::Property(_) => unreachable!("unexpected `Expr::Property` in indexing"),
             e if parent_is_dot => Some(e.position()),
             _ => None,
         }
@@ -2277,15 +2274,9 @@ fn parse_expr(
     settings.pos = input.peek().expect(NEVER_ENDS).1;
 
     // Parse expression normally.
+    let precedence = Precedence::new(1);
     let lhs = parse_unary(input, state, lib, settings.level_up())?;
-    parse_binary_op(
+    parse_binary_op(input, state, lib, precedence, lhs, settings.level_up())
-        input,
-        state,
-        lib,
-        Precedence::new(1),
-        lhs,
-        settings.level_up(),
-    )
 }
 
 /// Parse an if statement.

src/tokenizer.rs

@@ -2213,14 +2213,9 @@ impl<'a> Iterator for TokenIterator<'a> {
                 )),
                 // Reserved keyword/operator that is custom.
                 (_, true) => Token::Custom(s),
-                // Reserved operator that is not custom.
-                (token, false) if !is_valid_identifier(token.chars()) => {
-                    let msg = format!("'{}' is a reserved symbol", token);
-                    Token::LexError(LERR::ImproperSymbol(s.to_string(), msg))
-                },
                 // Reserved keyword that is not custom and disabled.
                 (token, false) if self.engine.disabled_symbols.contains(token) => {
-                    let msg = format!("reserved symbol '{}' is disabled", token);
+                    let msg = format!("reserved {} '{}' is disabled", if is_valid_identifier(token.chars()) { "keyword"} else {"symbol"}, token);
                     Token::LexError(LERR::ImproperSymbol(s.to_string(), msg))
                 },
                 // Reserved keyword/operator that is not custom.
@@ -2230,14 +2225,14 @@ impl<'a> Iterator for TokenIterator<'a> {
             Some((Token::Identifier(s), pos)) if self.engine.custom_keywords.contains_key(&*s) => {
                 (Token::Custom(s), pos)
             }
-            // Custom standard keyword/symbol - must be disabled
+            // Custom keyword/symbol - must be disabled
             Some((token, pos)) if self.engine.custom_keywords.contains_key(&*token.syntax()) => {
                 if self.engine.disabled_symbols.contains(&*token.syntax()) {
                     // Disabled standard keyword/symbol
                     (Token::Custom(token.syntax().into()), pos)
                 } else {
                     // Active standard keyword - should never be a custom keyword!
-                    unreachable!("{:?} is an active keyword", token)
+                    unreachable!("`{:?}` is an active keyword", token)
                 }
             }
             // Disabled symbol

src/types/parse_error.rs

@@ -1,5 +1,6 @@
 //! Module containing error definitions for the parsing process.
 
+use crate::tokenizer::is_valid_identifier;
 use crate::{EvalAltResult, Position};
 #[cfg(feature = "no_std")]
 use core_error::Error;
@@ -120,7 +121,7 @@ pub enum ParseErrorType {
     PropertyExpected,
     /// Missing a variable name after the `let`, `const`, `for` or `catch` keywords.
     VariableExpected,
-    /// An identifier is a reserved keyword.
+    /// An identifier is a reserved symbol.
     Reserved(String),
     /// An expression is of the wrong type.
     /// Wrapped values are the type requested and type of the actual result.
@@ -260,7 +261,8 @@ impl fmt::Display for ParseErrorType {
             },
 
             Self::LiteralTooLarge(typ, max) => write!(f, "{} exceeds the maximum limit ({})", typ, max),
-            Self::Reserved(s) => write!(f, "'{}' is a reserved keyword", s),
+            Self::Reserved(s) if is_valid_identifier(s.chars()) => write!(f, "'{}' is a reserved keyword", s),
+            Self::Reserved(s) => write!(f, "'{}' is a reserved symbol", s),
             Self::UnexpectedEOF => f.write_str("Script is incomplete"),
             Self::WrongSwitchIntegerCase => f.write_str("Integer switch case cannot follow a range case"),
             Self::WrongSwitchDefaultCase => f.write_str("Default switch case must be the last"),

tests/custom_syntax.rs

@@ -25,13 +25,13 @@ fn test_custom_syntax() -> Result<(), Box<EvalAltResult>> {
         ],
         true,
         |context, inputs| {
-            let var_name = inputs[0].get_variable_name().unwrap().to_string();
+            let var_name = inputs[0].get_string_value().unwrap();
             let op = inputs[1].get_literal_value::<ImmutableString>().unwrap();
             let max = inputs[2].get_literal_value::<INT>().unwrap();
             let stmt = &inputs[3];
             let condition = &inputs[4];
 
-            context.scope_mut().push(var_name.clone(), 0 as INT);
+            context.scope_mut().push(var_name.to_string(), 0 as INT);
 
             let mut count: INT = 0;
 
@@ -151,14 +151,14 @@ fn test_custom_syntax() -> Result<(), Box<EvalAltResult>> {
         &["var", "$ident$", "=", "$expr$"],
         true,
         |context, inputs| {
-            let var_name = inputs[0].get_variable_name().unwrap().to_string();
+            let var_name = inputs[0].get_string_value().unwrap();
             let expr = &inputs[1];
 
             // Evaluate the expression
             let value = context.eval_expression_tree(expr)?;
 
-            if !context.scope().is_constant(&var_name).unwrap_or(false) {
+            if !context.scope().is_constant(var_name).unwrap_or(false) {
-                context.scope_mut().set_value(var_name, value);
+                context.scope_mut().set_value(var_name.to_string(), value);
                 Ok(Dynamic::UNIT)
             } else {
                 Err(format!("variable {} is constant", var_name).into())
@@ -206,7 +206,7 @@ fn test_custom_syntax_raw() -> Result<(), Box<EvalAltResult>> {
         |context, inputs| {
             context.scope_mut().push("foo", 999 as INT);
 
-            Ok(match inputs[0].get_variable_name().unwrap() {
+            Ok(match inputs[0].get_string_value().unwrap() {
                 "world"
                     if inputs
                         .last()
@@ -244,9 +244,7 @@ fn test_custom_syntax_raw() -> Result<(), Box<EvalAltResult>> {
 fn test_custom_syntax_raw2() -> Result<(), Box<EvalAltResult>> {
     let mut engine = Engine::new();
 
-    engine
+    engine.register_custom_syntax_raw(
-        .register_custom_operator("#", 255)?
-        .register_custom_syntax_raw(
         "#",
         |symbols, lookahead| match symbols.len() {
             1 if lookahead == "-" => Ok(Some("$symbol$".into())),
@@ -267,9 +265,10 @@ fn test_custom_syntax_raw2() -> Result<(), Box<EvalAltResult>> {
         },
     );
 
+    assert_eq!(engine.eval::<INT>("#-1")?, -1);
     assert_eq!(engine.eval::<INT>("let x = 41; x + #1")?, 42);
     assert_eq!(engine.eval::<INT>("#42/2")?, 21);
-    assert_eq!(engine.eval::<INT>("#-1")?, -1);
+    assert_eq!(engine.eval::<INT>("sign(#1)")?, 1);
 
     Ok(())
 }

tests/tokens.rs

@@ -1,4 +1,4 @@
-use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, INT};
+use rhai::{Engine, EvalAltResult, ParseErrorType, INT};
 
 #[test]
 fn test_tokens_disabled() {
@@ -26,7 +26,7 @@ fn test_tokens_disabled() {
 
     assert!(matches!(
         *engine.compile("let x = += 0;").expect_err("should error").0,
-        ParseErrorType::BadInput(LexError::UnexpectedInput(err)) if err == "+="
+        ParseErrorType::Reserved(err) if err == "+="
     ));
 }