From 3ae7cf4018a8017ecb0da2a750771766107e5054 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 17 Jul 2020 14:50:23 +0800 Subject: [PATCH] Improve treatment of disabled symbols and custom symbols. --- doc/src/engine/custom-syntax.md | 8 +-- doc/src/engine/disable.md | 3 +- src/parser.rs | 3 -- src/settings.rs | 23 +++++++-- src/syntax.rs | 5 ++ src/token.rs | 86 ++++++++++++++++++--------------- tests/modules.rs | 37 ++++++-------- tests/syntax.rs | 15 ++++-- tests/tokens.rs | 7 ++- 9 files changed, 110 insertions(+), 77 deletions(-) diff --git a/doc/src/engine/custom-syntax.md b/doc/src/engine/custom-syntax.md index e0529841..0bd9fb30 100644 --- a/doc/src/engine/custom-syntax.md +++ b/doc/src/engine/custom-syntax.md @@ -68,11 +68,13 @@ These symbol types can be used: ### The First Symbol Must be a Keyword There is no specific limit on the combination and sequencing of each symbol type, -except the _first_ symbol which must be a [custom keyword]. +except the _first_ symbol which must be a custom keyword that follows the naming rules +of [variables]. -It _cannot_ be a [built-in keyword]({{rootUrl}}/appendix/keywords.md). +The first symbol also cannot be a reserved [keyword], unless that keyword +has been [disabled][disable keywords and operators]. -However, it _may_ be a built-in keyword that has been [disabled][disable keywords and operators]. +In other words, any valid identifier that is not an active [keyword] will work fine. ### The First Symbol Must be Unique diff --git a/doc/src/engine/disable.md b/doc/src/engine/disable.md index 3810fa5e..28e35f70 100644 --- a/doc/src/engine/disable.md +++ b/doc/src/engine/disable.md @@ -20,8 +20,7 @@ engine // The following all return parse errors. engine.compile("let x = if true { 42 } else { 0 };")?; -// ^ missing ';' after statement end -// ^ 'if' is parsed as a variable name +// ^ 'if' is rejected as a reserved keyword engine.compile("let x = 40 + 2; x += 1;")?; // ^ '+=' is not recognized as an operator diff --git a/src/parser.rs b/src/parser.rs index e2f0c3f2..3f79d089 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2151,9 +2151,6 @@ fn parse_expr( exprs.push(Expr::Stmt(Box::new((stmt, pos)))) } s => match input.peek().unwrap() { - (Token::Custom(custom), _) if custom == s => { - input.next().unwrap(); - } (t, _) if t.syntax().as_ref() == s => { input.next().unwrap(); } diff --git a/src/settings.rs b/src/settings.rs index f1967cdf..e7e4fd16 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -2,7 +2,7 @@ use crate::engine::Engine; use crate::module::ModuleResolver; use crate::optimize::OptimizationLevel; use crate::packages::PackageLibrary; -use crate::token::is_valid_identifier; +use crate::token::{is_valid_identifier, Token}; use crate::stdlib::{boxed::Box, format, string::String}; @@ -183,8 +183,7 @@ impl Engine { /// engine.disable_symbol("if"); // disable the 'if' keyword /// /// engine.compile("let x = if true { 42 } else { 0 };")?; - /// // ^ 'if' is parsed as a variable name - /// // ^ missing ';' after statement end + /// // ^ 'if' is rejected as a reserved keyword /// # Ok(()) /// # } /// ``` @@ -252,6 +251,24 @@ impl Engine { return Err(format!("not a valid identifier: '{}'", keyword).into()); } + match Token::lookup_from_syntax(keyword) { + // Standard identifiers, reserved keywords and custom keywords are OK + None | Some(Token::Reserved(_)) | Some(Token::Custom(_)) => (), + // Disabled keywords are also OK + Some(token) + if !self + .disabled_symbols + .as_ref() + .map(|d| d.contains(token.syntax().as_ref())) + .unwrap_or(false) => + { + () + } + // Active standard keywords cannot be made custom + Some(_) => return Err(format!("'{}' is a reserved keyword", keyword).into()), + } + + // Add to custom keywords if self.custom_keywords.is_none() { self.custom_keywords = Some(Default::default()); } diff --git a/src/syntax.rs b/src/syntax.rs index 3b646d25..9bd5fad1 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -88,11 +88,16 @@ impl Engine { MARKER_EXPR | MARKER_BLOCK | MARKER_IDENT if !segments.is_empty() => s.to_string(), // Standard symbols not in first position s if !segments.is_empty() && Token::lookup_from_syntax(s).is_some() => { + // Make it a custom keyword/operator if it is a disabled standard keyword/operator + // or a reserved keyword/operator. if self .disabled_symbols .as_ref() .map(|d| d.contains(s)) .unwrap_or(false) + || Token::lookup_from_syntax(s) + .map(|token| token.is_reserved()) + .unwrap_or(false) { // If symbol is disabled, make it a custom keyword if self.custom_keywords.is_none() { diff --git a/src/token.rs b/src/token.rs index f3579317..dae43624 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1334,73 +1334,81 @@ impl<'a> Iterator for TokenIterator<'a, '_> { self.engine.disabled_symbols.as_ref(), self.engine.custom_keywords.as_ref(), ) { + // {EOF} (None, _, _) => None, - (Some((Token::Reserved(s), pos)), None, None) => return Some((match s.as_str() { - "===" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'===' is not a valid operator. This is not JavaScript! Should it be '=='?" - .to_string(), + // Reserved keyword/symbol + (Some((Token::Reserved(s), pos)), disabled, custom) => Some((match + (s.as_str(), custom.map(|c| c.contains_key(&s)).unwrap_or(false)) + { + ("===", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'===' is not a valid operator. This is not JavaScript! Should it be '=='?".to_string(), ))), - "!==" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'!==' is not a valid operator. This is not JavaScript! Should it be '!='?" - .to_string(), + ("!==", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'!==' is not a valid operator. This is not JavaScript! Should it be '!='?".to_string(), ))), - "->" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'->' is not a valid symbol. This is not C or C++!".to_string(), - ))), - "<-" => Token::LexError(Box::new(LERR::ImproperSymbol( + ("->", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'->' is not a valid symbol. This is not C or C++!".to_string()))), + ("<-", false) => Token::LexError(Box::new(LERR::ImproperSymbol( "'<-' is not a valid symbol. This is not Go! Should it be '<='?".to_string(), ))), - "=>" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'=>' is not a valid symbol. This is not Rust! Should it be '>='?" - .to_string(), + ("=>", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'=>' is not a valid symbol. This is not Rust! Should it be '>='?".to_string(), ))), - ":=" => Token::LexError(Box::new(LERR::ImproperSymbol( - "':=' is not a valid assignment operator. This is not Go! Should it be simply '='?" - .to_string(), + (":=", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "':=' is not a valid assignment operator. This is not Go! Should it be simply '='?".to_string(), ))), - "::<" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'::<>' is not a valid symbol. This is not Rust! Should it be '::'?" - .to_string(), + ("::<", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'::<>' is not a valid symbol. This is not Rust! Should it be '::'?".to_string(), ))), - "(*" | "*)" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'(* .. *)' is not a valid comment format. This is not Pascal! Should it be '/* .. */'?" - .to_string(), + ("(*", false) | ("*)", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'(* .. *)' is not a valid comment format. This is not Pascal! Should it be '/* .. */'?".to_string(), ))), - "#" => Token::LexError(Box::new(LERR::ImproperSymbol( - "'#' is not a valid symbol. Should it be '#{'?" - .to_string(), + ("#", false) => Token::LexError(Box::new(LERR::ImproperSymbol( + "'#' is not a valid symbol. Should it be '#{'?".to_string(), ))), - token if !is_valid_identifier(token.chars()) => Token::LexError(Box::new(LERR::ImproperSymbol( - format!("'{}' is a reserved symbol.", token) + // 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(Box::new(LERR::ImproperSymbol( + format!("'{}' is a reserved symbol", token) ))), - _ => Token::Reserved(s) + // Reserved keyword that is not custom and disabled. + (token, false) if disabled.map(|d| d.contains(token)).unwrap_or(false) => Token::LexError(Box::new(LERR::ImproperSymbol( + format!("reserved symbol '{}' is disabled", token) + ))), + // Reserved keyword/operator that is not custom. + (_, false) => Token::Reserved(s), }, pos)), - (r @ Some(_), None, None) => r, + // Custom keyword (Some((Token::Identifier(s), pos)), _, Some(custom)) if custom.contains_key(&s) => { - // Convert custom keywords Some((Token::Custom(s), pos)) } - (Some((token, pos)), _, Some(custom)) - if (token.is_keyword() || token.is_operator() || token.is_reserved()) - && custom.contains_key(token.syntax().as_ref()) => + // Custom standard keyword - must be disabled + (Some((token, pos)), Some(disabled), Some(custom)) + if token.is_keyword() && custom.contains_key(token.syntax().as_ref()) => { - // Convert into custom keywords - Some((Token::Custom(token.syntax().into()), pos)) + if disabled.contains(token.syntax().as_ref()) { + // Disabled standard keyword + Some((Token::Custom(token.syntax().into()), pos)) + } else { + // Active standard keyword - should never be a custom keyword! + unreachable!() + } } + // Disabled operator (Some((token, pos)), Some(disabled), _) if token.is_operator() && disabled.contains(token.syntax().as_ref()) => { - // Convert disallowed operators into lex errors Some(( Token::LexError(Box::new(LexError::UnexpectedInput(token.syntax().into()))), pos, )) } + // Disabled standard keyword (Some((token, pos)), Some(disabled), _) if token.is_keyword() && disabled.contains(token.syntax().as_ref()) => { - // Convert disallowed keywords into identifiers - Some((Token::Identifier(token.syntax().into()), pos)) + Some((Token::Reserved(token.syntax().into()), pos)) } (r, _, _) => r, } diff --git a/tests/modules.rs b/tests/modules.rs index 3d0fc48d..b7393f14 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -1,6 +1,6 @@ #![cfg(not(feature = "no_module"))] use rhai::{ - module_resolvers::StaticModuleResolver, Engine, EvalAltResult, Module, ParseError, + module_resolvers::StaticModuleResolver, Dynamic, Engine, EvalAltResult, Module, ParseError, ParseErrorType, Scope, INT, }; @@ -26,6 +26,7 @@ fn test_module_sub_module() -> Result<(), Box> { sub_module.set_sub_module("universe", sub_module2); module.set_sub_module("life", sub_module); + module.set_var("MYSTIC_NUMBER", Dynamic::from(42 as INT)); assert!(module.contains_sub_module("life")); let m = module.get_sub_module("life").unwrap(); @@ -44,18 +45,16 @@ fn test_module_sub_module() -> Result<(), Box> { let mut engine = Engine::new(); engine.set_module_resolver(Some(resolver)); - let mut scope = Scope::new(); - assert_eq!( - engine.eval_with_scope::( - &mut scope, - r#"import "question" as q; q::life::universe::answer + 1"# - )?, + engine.eval::(r#"import "question" as q; q::MYSTIC_NUMBER"#)?, 42 ); assert_eq!( - engine.eval_with_scope::( - &mut scope, + engine.eval::(r#"import "question" as q; q::life::universe::answer + 1"#)?, + 42 + ); + assert_eq!( + engine.eval::( r#"import "question" as q; q::life::universe::inc(q::life::universe::answer)"# )?, 42 @@ -221,36 +220,30 @@ fn test_module_from_ast() -> Result<(), Box> { resolver2.insert("testing", module); engine.set_module_resolver(Some(resolver2)); - let mut scope = Scope::new(); - assert_eq!( - engine.eval_with_scope::(&mut scope, r#"import "testing" as ttt; ttt::abc"#)?, + engine.eval::(r#"import "testing" as ttt; ttt::abc"#)?, 123 ); assert_eq!( - engine.eval_with_scope::(&mut scope, r#"import "testing" as ttt; ttt::foo"#)?, + engine.eval::(r#"import "testing" as ttt; ttt::foo"#)?, 42 ); - assert!(engine - .eval_with_scope::(&mut scope, r#"import "testing" as ttt; ttt::extra::foo"#)?); + assert!(engine.eval::(r#"import "testing" as ttt; ttt::extra::foo"#)?); assert_eq!( - engine.eval_with_scope::(&mut scope, r#"import "testing" as ttt; ttt::hello"#)?, + engine.eval::(r#"import "testing" as ttt; ttt::hello"#)?, "hello, 42 worlds!" ); assert_eq!( - engine.eval_with_scope::(&mut scope, r#"import "testing" as ttt; ttt::calc(999)"#)?, + engine.eval::(r#"import "testing" as ttt; ttt::calc(999)"#)?, 1000 ); assert_eq!( - engine.eval_with_scope::( - &mut scope, - r#"import "testing" as ttt; ttt::add_len(ttt::foo, ttt::hello)"# - )?, + engine.eval::(r#"import "testing" as ttt; ttt::add_len(ttt::foo, ttt::hello)"#)?, 59 ); assert!(matches!( *engine - .eval_with_scope::<()>(&mut scope, r#"import "testing" as ttt; ttt::hidden()"#) + .consume(r#"import "testing" as ttt; ttt::hidden()"#) .expect_err("should error"), EvalAltResult::ErrorFunctionNotFound(fn_name, _) if fn_name == "ttt::hidden" )); diff --git a/tests/syntax.rs b/tests/syntax.rs index 0fdf3c07..d4737b71 100644 --- a/tests/syntax.rs +++ b/tests/syntax.rs @@ -1,16 +1,25 @@ #![cfg(feature = "internals")] use rhai::{ - Dynamic, Engine, EvalAltResult, EvalState, Expression, Imports, LexError, Module, Scope, INT, + Dynamic, Engine, EvalAltResult, EvalState, Expression, Imports, LexError, Module, ParseError, + ParseErrorType, Scope, INT, }; #[test] fn test_custom_syntax() -> Result<(), Box> { let mut engine = Engine::new(); + engine.consume("while false {}")?; + // Disable 'while' and make sure it still works with custom syntax engine.disable_symbol("while"); - engine.consume("while false {}").expect_err("should error"); - engine.consume("let while = 0")?; + assert!(matches!( + *engine.compile("while false {}").expect_err("should error").0, + ParseErrorType::Reserved(err) if err == "while" + )); + assert!(matches!( + *engine.compile("let while = 0").expect_err("should error").0, + ParseErrorType::Reserved(err) if err == "while" + )); engine .register_custom_syntax( diff --git a/tests/tokens.rs b/tests/tokens.rs index f54afa84..523beab7 100644 --- a/tests/tokens.rs +++ b/tests/tokens.rs @@ -7,8 +7,11 @@ fn test_tokens_disabled() { engine.disable_symbol("if"); // disable the 'if' keyword assert!(matches!( - *engine.compile("let x = if true { 42 } else { 0 };").expect_err("should error").0, - ParseErrorType::MissingToken(ref token, _) if token == ";" + *engine + .compile("let x = if true { 42 } else { 0 };") + .expect_err("should error") + .0, + ParseErrorType::Reserved(err) if err == "if" )); engine.disable_symbol("+="); // disable the '+=' operator