diff --git a/CHANGELOG.md b/CHANGELOG.md index df828457..ce0981c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Bug fixes * Complex indexing/dotting chains now parse correctly, for example: `a[b][c[d]].e` * `map` and `filter` for arrays are marked `pure`. Warnings are added to the documentation of pure array methods that take `this` closures. +* Syntax such as `foo.bar::baz` no longer panics, but returns a proper parse error. Version 1.12.0 diff --git a/codegen/ui_tests/rhai_fn_duplicate_attr.stderr b/codegen/ui_tests/rhai_fn_duplicate_attr.stderr index c8101565..57ce356d 100644 --- a/codegen/ui_tests/rhai_fn_duplicate_attr.stderr +++ b/codegen/ui_tests/rhai_fn_duplicate_attr.stderr @@ -4,14 +4,14 @@ error: duplicated attribute 'rhai_fn' 6 | #[rhai_fn(pure)] | ^ -error[E0433]: failed to resolve: use of undeclared crate or module `test_module` - --> ui_tests/rhai_fn_duplicate_attr.rs:13:8 - | -13 | if test_module::test_fn(n) { - | ^^^^^^^^^^^ use of undeclared crate or module `test_module` - error[E0425]: cannot find value `n` in this scope --> ui_tests/rhai_fn_duplicate_attr.rs:13:29 | 13 | if test_module::test_fn(n) { | ^ not found in this scope + +error[E0433]: failed to resolve: use of undeclared crate or module `test_module` + --> ui_tests/rhai_fn_duplicate_attr.rs:13:8 + | +13 | if test_module::test_fn(n) { + | ^^^^^^^^^^^ use of undeclared crate or module `test_module` diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index 1bc1a5a5..652134a3 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -505,14 +505,21 @@ impl Engine { event: DebuggerEvent, ) -> Result, Box> { if let Some(ref x) = self.debugger_interface { + let orig_scope_len = scope.len(); + let src = global.source_raw().cloned(); let src = src.as_ref().map(|s| s.as_str()); let context = EvalContext::new(self, global, caches, scope, this_ptr); let (.., ref on_debugger) = **x; - let command = on_debugger(context, event, node, src, node.position())?; + let command = on_debugger(context, event, node, src, node.position()); - match command { + if orig_scope_len != scope.len() { + // The scope is changed, always search from now on + global.always_search_scope = true; + } + + match command? { DebuggerCommand::Continue => { global.debugger_mut().status = DebuggerStatus::CONTINUE; Ok(None) diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 75d9c0a5..925a8132 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -174,9 +174,18 @@ impl Engine { // Check the variable resolver, if any if let Some(ref resolve_var) = self.resolve_var { + let orig_scope_len = scope.len(); + let context = EvalContext::new(self, global, caches, scope, this_ptr); let var_name = expr.get_variable_name(true).expect("`Expr::Variable`"); - match resolve_var(var_name, index, context) { + let resolved_var = resolve_var(var_name, index, context); + + if orig_scope_len != scope.len() { + // The scope is changed, always search from now on + global.always_search_scope = true; + } + + match resolved_var { Ok(Some(mut result)) => { result.set_access_mode(AccessMode::ReadOnly); return Ok(result.into()); diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index db2523ff..f9111ce6 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -728,10 +728,17 @@ impl Engine { nesting_level: global.scope_level, will_shadow, }; + let orig_scope_len = scope.len(); let context = EvalContext::new(self, global, caches, scope, this_ptr.as_deref_mut()); + let filter_result = filter(true, info, context); - if !filter(true, info, context)? { + if orig_scope_len != scope.len() { + // The scope is changed, always search from now on + global.always_search_scope = true; + } + + if !filter_result? { return Err(ERR::ErrorForbiddenVariable(var_name.to_string(), *pos).into()); } } diff --git a/src/func/call.rs b/src/func/call.rs index 2532face..837d6fb7 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -11,7 +11,7 @@ use crate::eval::{Caches, FnResolutionCacheEntry, GlobalRuntimeState}; use crate::tokenizer::{is_valid_function_name, Token}; use crate::{ calc_fn_hash, calc_fn_hash_full, Dynamic, Engine, FnArgsVec, FnPtr, ImmutableString, - OptimizationLevel, Position, RhaiError, RhaiResult, RhaiResultOf, Scope, Shared, ERR, + OptimizationLevel, Position, RhaiResult, RhaiResultOf, Scope, Shared, ERR, }; #[cfg(feature = "no_std")] use hashbrown::hash_map::Entry; @@ -1053,7 +1053,7 @@ impl Engine { let mut arg_values = curry .into_iter() .map(Ok) - .chain(a_expr.iter().map(|expr| -> Result<_, RhaiError> { + .chain(a_expr.iter().map(|expr| -> Result<_, crate::RhaiError> { let this_ptr = this_ptr.as_deref_mut(); self.get_arg_value(global, caches, scope, this_ptr, expr) .map(|(v, ..)| v) diff --git a/src/parser.rs b/src/parser.rs index 1230dbfc..590d97f3 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -65,11 +65,13 @@ pub struct ParseState<'e, 's> { /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] pub external_vars: Option>>, - /// An indicator that disables variable capturing into externals one single time - /// up until the nearest consumed Identifier token. - /// If set to false the next call to [`access_var`][ParseState::access_var] will not capture the variable. + /// An indicator that, when set to `false`, disables variable capturing into externals one + /// single time up until the nearest consumed Identifier token. + /// + /// If set to `false` the next call to [`access_var`][ParseState::access_var] will not capture + /// the variable. + /// /// All consequent calls to [`access_var`][ParseState::access_var] will not be affected. - #[cfg(not(feature = "no_closure"))] pub allow_capture: bool, /// Encapsulates a local stack with imported [module][crate::Module] names. #[cfg(not(feature = "no_module"))] @@ -118,7 +120,6 @@ impl<'e, 's> ParseState<'e, 's> { expr_filter: |_| true, #[cfg(not(feature = "no_closure"))] external_vars: None, - #[cfg(not(feature = "no_closure"))] allow_capture: true, interned_strings, external_constants, @@ -296,11 +297,8 @@ bitflags! { /// Is the construct being parsed located at global level? const GLOBAL_LEVEL = 0b0000_0001; /// Is the construct being parsed located inside a function definition? - #[cfg(not(feature = "no_function"))] const FN_SCOPE = 0b0000_0010; /// Is the construct being parsed located inside a closure definition? - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "no_closure"))] const CLOSURE_SCOPE = 0b0000_0100; /// Is the construct being parsed located inside a breakable loop? const BREAKABLE = 0b0000_1000; @@ -312,6 +310,16 @@ bitflags! { } } +bitflags! { + /// Bit-flags containing all status for parsing property/indexing/namespace chains. + struct ChainingFlags: u8 { + /// Is the construct being parsed a property? + const PROPERTY = 0b0000_0001; + /// Disallow namespaces? + const DISALLOW_NAMESPACES = 0b0000_0010; + } +} + /// A type that encapsulates all the settings for a particular parsing function. #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub struct ParseSettings { @@ -456,8 +464,8 @@ fn ensure_not_statement_expr( /// Make sure that the next expression is not a mis-typed assignment (i.e. `a = b` instead of `a == b`). fn ensure_not_assignment(input: &mut TokenStream) -> ParseResult<()> { match input.peek().expect(NEVER_ENDS) { - (Token::Equals, pos) => Err(LexError::ImproperSymbol( - "=".into(), + (token @ Token::Equals, pos) => Err(LexError::ImproperSymbol( + token.literal_syntax().into(), "Possibly a typo of '=='?".into(), ) .into_err(*pos)), @@ -1308,7 +1316,7 @@ impl Engine { state: &mut ParseState, lib: &mut FnLib, settings: ParseSettings, - is_property: bool, + options: ChainingFlags, ) -> ParseResult { let (token, token_pos) = input.peek().expect(NEVER_ENDS); @@ -1444,15 +1452,13 @@ impl Engine { #[cfg(feature = "no_closure")] let options = self.options | (settings.options & LangOptions::STRICT_VAR); - // Brand new flags, turn on function scope + // Brand new flags, turn on function scope and closure scope let flags = ParseSettingFlags::FN_SCOPE + | ParseSettingFlags::CLOSURE_SCOPE | (settings.flags & (ParseSettingFlags::DISALLOW_UNQUOTED_MAP_PROPERTIES | ParseSettingFlags::DISALLOW_STATEMENTS_IN_BLOCKS)); - #[cfg(not(feature = "no_closure"))] - let flags = flags | ParseSettingFlags::CLOSURE_SCOPE; // turn on closure scope - let new_settings = ParseSettings { flags, options, @@ -1593,14 +1599,12 @@ impl Engine { token => unreachable!("Token::Identifier expected but gets {:?}", token), }; - match input.peek().expect(NEVER_ENDS).0 { + match input.peek().expect(NEVER_ENDS) { // Function call - Token::LeftParen | Token::Bang | Token::Unit => { - #[cfg(not(feature = "no_closure"))] - { - // Once the identifier consumed we must enable next variables capturing - state.allow_capture = true; - } + (Token::LeftParen | Token::Bang | Token::Unit, _) => { + // Once the identifier consumed we must enable next variables capturing + state.allow_capture = true; + Expr::Variable( (None, ns, 0, state.get_interned_string(*s)).into(), None, @@ -1609,12 +1613,18 @@ impl Engine { } // Namespace qualification #[cfg(not(feature = "no_module"))] - Token::DoubleColon => { - #[cfg(not(feature = "no_closure"))] - { - // Once the identifier consumed we must enable next variables capturing - state.allow_capture = true; + (token @ Token::DoubleColon, pos) => { + if options.contains(ChainingFlags::DISALLOW_NAMESPACES) { + return Err(LexError::ImproperSymbol( + token.literal_syntax().into(), + String::new(), + ) + .into_err(*pos)); } + + // Once the identifier consumed we must enable next variables capturing + state.allow_capture = true; + let name = state.get_interned_string(*s); Expr::Variable((None, ns, 0, name).into(), None, settings.pos) } @@ -1622,7 +1632,7 @@ impl Engine { _ => { let (index, is_func) = state.access_var(&s, lib, settings.pos); - if !is_property + if !options.contains(ChainingFlags::PROPERTY) && !is_func && index.is_none() && settings.has_option(LangOptions::STRICT_VAR) @@ -1694,7 +1704,14 @@ impl Engine { return Ok(root_expr); } - self.parse_postfix(input, state, lib, settings, root_expr) + self.parse_postfix( + input, + state, + lib, + settings, + root_expr, + ChainingFlags::empty(), + ) } /// Tail processing of all possible postfix operators of a primary expression. @@ -1705,6 +1722,7 @@ impl Engine { lib: &mut FnLib, settings: ParseSettings, mut lhs: Expr, + options: ChainingFlags, ) -> ParseResult { let mut settings = settings; @@ -1751,6 +1769,7 @@ impl Engine { let (.., ns, _, name) = *x; settings.pos = pos; + self.parse_fn_call(input, state, lib, settings, name, no_args, true, ns)? } // Function call @@ -1758,8 +1777,20 @@ impl Engine { let (.., ns, _, name) = *x; let no_args = t == Token::Unit; settings.pos = pos; + self.parse_fn_call(input, state, lib, settings, name, no_args, false, ns)? } + // Disallowed module separator + #[cfg(not(feature = "no_module"))] + (_, token @ Token::DoubleColon) + if options.contains(ChainingFlags::DISALLOW_NAMESPACES) => + { + return Err(LexError::ImproperSymbol( + token.literal_syntax().into(), + String::new(), + ) + .into_err(tail_pos)) + } // module access #[cfg(not(feature = "no_module"))] (Expr::Variable(x, .., pos), Token::DoubleColon) => { @@ -1769,11 +1800,9 @@ impl Engine { namespace.push(var_name_def); - Expr::Variable( - (None, namespace, 0, state.get_interned_string(id2)).into(), - None, - pos2, - ) + let var_name = state.get_interned_string(id2); + + Expr::Variable((None, namespace, 0, var_name).into(), None, pos2) } // Indexing #[cfg(not(feature = "no_index"))] @@ -1792,11 +1821,8 @@ impl Engine { // Expression after dot must start with an identifier match input.peek().expect(NEVER_ENDS) { (Token::Identifier(..), ..) => { - #[cfg(not(feature = "no_closure"))] - { - // Prevents capturing of the object properties as vars: xxx. - state.allow_capture = false; - } + // Prevents capturing of the object properties as vars: xxx. + state.allow_capture = false; } (Token::Reserved(s), ..) if is_keyword_function(s).1 => (), (Token::Reserved(s), pos) => { @@ -1805,12 +1831,15 @@ impl Engine { (.., pos) => return Err(PERR::PropertyExpected.into_err(*pos)), } - let rhs = self.parse_primary(input, state, lib, settings.level_up()?, true)?; let op_flags = match op { Token::Period => ASTFlags::NONE, Token::Elvis => ASTFlags::NEGATED, _ => unreachable!("`.` or `?.`"), }; + let options = ChainingFlags::PROPERTY | ChainingFlags::DISALLOW_NAMESPACES; + let rhs = + self.parse_primary(input, state, lib, settings.level_up()?, options)?; + Self::make_dot_expr(state, expr, rhs, ASTFlags::NONE, op_flags, tail_pos)? } // Unknown postfix operator @@ -1982,7 +2011,7 @@ impl Engine { // Token::EOF => Err(PERR::UnexpectedEOF.into_err(settings.pos)), // All other tokens - _ => self.parse_primary(input, state, lib, settings, false), + _ => self.parse_primary(input, state, lib, settings, ChainingFlags::empty()), } } @@ -2081,11 +2110,13 @@ impl Engine { } } // ??? && ??? = rhs, ??? || ??? = rhs, xxx ?? xxx = rhs - Expr::And(..) | Expr::Or(..) | Expr::Coalesce(..) => Err(LexError::ImproperSymbol( - "=".into(), - "Possibly a typo of '=='?".into(), - ) - .into_err(op_pos)), + Expr::And(..) | Expr::Or(..) | Expr::Coalesce(..) if !op_info.is_op_assignment() => { + Err(LexError::ImproperSymbol( + Token::Equals.literal_syntax().into(), + "Possibly a typo of '=='?".into(), + ) + .into_err(op_pos)) + } // expr = rhs _ => Err(PERR::AssignmentToInvalidLHS(String::new()).into_err(lhs.position())), } diff --git a/tests/functions.rs b/tests/functions.rs index 8e4a7f36..af9b9ba0 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -143,6 +143,21 @@ fn test_functions_global_module() -> Result<(), Box> { 123 ); + // Other globals + let mut module = Module::new(); + module.set_var("ANSWER", 123 as INT); + engine.register_global_module(module.into()); + + assert_eq!( + engine.eval::( + " + fn foo() { global::ANSWER } + foo() + " + )?, + 123 + ); + Ok(()) } diff --git a/tests/modules.rs b/tests/modules.rs index babc862a..2c74e935 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -14,6 +14,13 @@ fn test_module() { assert_eq!(module.get_var_value::("answer").unwrap(), 42); } +#[test] +fn test_module_syntax() { + let engine = Engine::new(); + assert!(engine.compile("abc.def::xyz").is_err()); + assert!(engine.compile("abc.def::xyz()").is_err()); +} + #[test] fn test_module_sub_module() -> Result<(), Box> { let mut module = Module::new();