Refine what can be called in method style.

CHANGELOG.md

@@ -11,6 +11,7 @@ Bug fixes
 * Parsing deeply-nested closures (e.g. `||{||{||{||{||{||{||{...}}}}}}}`) no longer panics but will be confined to the nesting limit.
 * Closures containing a single expression are now allowed in `Engine::eval_expression` etc.
 * Strings interpolation now works under `Engine::new_raw` without any standard package.
+* `Fn` now throws an error if the name is a reserved keyword as it cannot possibly map to such a function. This also disallows creating function pointers to custom operators which are defined as disabled keywords (a mouthful), but such custom operators are designed primarily to be used as operators.
 
 Breaking API changes
 --------------------
@@ -60,6 +61,7 @@ Net features
 
 * Array methods that take a function pointer, usually a closure (e.g. `map`, `filter`, `index_of`, `reduce` etc.), can now bind the array element to `this` when calling a closure.
 * This vastly improves performance when working with arrays of large types (e.g. object maps) by avoiding unnecessary cloning.
+* `find` and `find_map` are added for arrays.
 * `for_each` is also added for arrays, allowing a closure to mutate array elements (bound to `this`) in turn.
 
 Enhancements
@@ -79,7 +81,6 @@ Enhancements
 * `FnPtr::iter_curry` and `FnPtr::iter_curry_mut` are added.
 * `Dynamic::deep_scan` is added to recursively scan for `Dynamic` values.
 * `>>` and `<<` operators on integers no longer throw errors when the number of bits to shift is out of bounds.  Shifting by a negative number of bits simply reverses the shift direction.
-* `find` and `find_map` are added for arrays.
 
 
 Version 1.11.0

src/api/custom_syntax.rs

@@ -4,7 +4,7 @@
 use crate::ast::Expr;
 use crate::func::SendSync;
 use crate::parser::ParseResult;
-use crate::tokenizer::{is_valid_identifier, Token, NO_TOKEN};
+use crate::tokenizer::{is_reserved_keyword_or_symbol, is_valid_identifier, Token, NO_TOKEN};
 use crate::types::dynamic::Variant;
 use crate::{
     Dynamic, Engine, EvalContext, Identifier, ImmutableString, LexError, Position, RhaiResult,
@@ -232,7 +232,7 @@ impl Engine {
             }
 
             let token = Token::lookup_symbol_from_syntax(s).unwrap_or_else(|| {
-                if Token::is_reserved_keyword(s) {
+                if is_reserved_keyword_or_symbol(s) {
                     Token::Reserved(Box::new(s.into()))
                 } else {
                     NO_TOKEN

src/func/call.rs

@@ -575,14 +575,6 @@ impl Engine {
         _is_method_call: bool,
         pos: Position,
     ) -> RhaiResultOf<(Dynamic, bool)> {
-        fn no_method_err(name: &str, pos: Position) -> RhaiResultOf<(Dynamic, bool)> {
-            Err(ERR::ErrorRuntime(
-                format!("'{name}' should not be called this way. Try {name}(...);").into(),
-                pos,
-            )
-            .into())
-        }
-
         // Check for data race.
         #[cfg(not(feature = "no_closure"))]
         ensure_no_data_race(fn_name, _args, is_ref_mut)?;
@@ -622,16 +614,13 @@ impl Engine {
 
                 // Handle is_shared()
                 #[cfg(not(feature = "no_closure"))]
-                crate::engine::KEYWORD_IS_SHARED if _args.len() == 1 => {
+                crate::engine::KEYWORD_IS_SHARED => {
-                    return no_method_err(fn_name, pos)
+                    unreachable!("{} called as method", fn_name)
                 }
 
-                KEYWORD_FN_PTR | KEYWORD_EVAL | KEYWORD_IS_DEF_VAR if _args.len() == 1 => {
+                KEYWORD_FN_PTR | KEYWORD_EVAL | KEYWORD_IS_DEF_VAR | KEYWORD_FN_PTR_CALL
-                    return no_method_err(fn_name, pos)
+                | KEYWORD_FN_PTR_CURRY => {
-                }
+                    unreachable!("{} called as method", fn_name)
-
-                KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY if !_args.is_empty() => {
-                    return no_method_err(fn_name, pos)
                 }
 
                 _ => (),

src/parser.rs

@@ -1657,7 +1657,7 @@ impl Engine {
 
                 match input.peek().expect(NEVER_ENDS).0 {
                     // Function call is allowed to have reserved keyword
-                    Token::LeftParen | Token::Bang | Token::Unit if is_keyword_function(&s) => {
+                    Token::LeftParen | Token::Bang | Token::Unit if is_keyword_function(&s).0 => {
                         Expr::Variable(
                             (None, ns, 0, state.get_interned_string(*s)).into(),
                             None,
@@ -1800,7 +1800,10 @@ impl Engine {
                                 state.allow_capture = false;
                             }
                         }
-                        (Token::Reserved(s), ..) if is_keyword_function(s) => (),
+                        (Token::Reserved(s), ..) if is_keyword_function(s).1 => (),
+                        (Token::Reserved(s), pos) => {
+                            return Err(PERR::Reserved(s.to_string()).into_err(*pos))
+                        }
                         (.., pos) => return Err(PERR::PropertyExpected.into_err(*pos)),
                     }
 
@@ -2109,9 +2112,7 @@ impl Engine {
             }
             // lhs.module::id - syntax error
             #[cfg(not(feature = "no_module"))]
-            (.., Expr::Variable(x, ..)) if !x.1.is_empty() => {
+            (.., Expr::Variable(x, ..)) if !x.1.is_empty() => unreachable!("lhs.ns::id"),
-                Err(PERR::PropertyExpected.into_err(x.1.position()))
-            }
             // lhs.id
             (lhs, var_expr @ Expr::Variable(..)) => {
                 let rhs = var_expr.into_property(state);
@@ -2125,9 +2126,7 @@ impl Engine {
             )),
             // lhs.nnn::func(...) - syntax error
             #[cfg(not(feature = "no_module"))]
-            (.., Expr::FnCall(f, ..)) if f.is_qualified() => {
+            (.., Expr::FnCall(f, ..)) if f.is_qualified() => unreachable!("lhs.ns::func()"),
-                Err(PERR::PropertyExpected.into_err(f.namespace.position()))
-            }
             // lhs.Fn() or lhs.eval()
             (.., Expr::FnCall(f, func_pos))
                 if f.args.is_empty()
@@ -2174,13 +2173,11 @@ impl Engine {
                 match x.lhs {
                     // lhs.module::id.dot_rhs or lhs.module::id[idx_rhs] - syntax error
                     #[cfg(not(feature = "no_module"))]
-                    Expr::Variable(x, ..) if !x.1.is_empty() => {
+                    Expr::Variable(x, ..) if !x.1.is_empty() => unreachable!("lhs.ns::id..."),
-                        Err(PERR::PropertyExpected.into_err(x.1.position()))
-                    }
                     // lhs.module::func().dot_rhs or lhs.module::func()[idx_rhs] - syntax error
                     #[cfg(not(feature = "no_module"))]
                     Expr::FnCall(f, ..) if f.is_qualified() => {
-                        Err(PERR::PropertyExpected.into_err(f.namespace.position()))
+                        unreachable!("lhs.ns::func()...")
                     }
                     // lhs.id.dot_rhs or lhs.id[idx_rhs]
                     Expr::Variable(..) | Expr::Property(..) => {

src/tokenizer.rs

@@ -631,41 +631,6 @@ impl Token {
         })
     }
 
-    /// Is a piece of syntax a reserved keyword?
-    #[must_use]
-    pub fn is_reserved_keyword(syntax: &str) -> bool {
-        match syntax {
-            #[cfg(feature = "no_object")]
-            "?." => true,
-            #[cfg(feature = "no_index")]
-            "?[" => true,
-            #[cfg(feature = "no_function")]
-            "fn" | "private" => true,
-            #[cfg(feature = "no_module")]
-            "import" | "export" | "as" => true,
-
-            // List of reserved operators
-            "===" | "!==" | "->" | "<-" | "?" | ":=" | ":;" | "~" | "!." | "::<" | "(*" | "*)"
-            | "#" | "#!" | "@" | "$" | "++" | "--" | "..." | "<|" | "|>" => true,
-
-            // List of reserved keywords
-            "public" | "protected" | "super" | "new" | "use" | "module" | "package" | "var"
-            | "static" | "shared" | "with" | "is" | "goto" | "exit" | "match" | "case"
-            | "default" | "void" | "null" | "nil" | "spawn" | "thread" | "go" | "sync"
-            | "async" | "await" | "yield" => true,
-
-            KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR
-            | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_THIS | KEYWORD_IS_DEF_VAR => {
-                true
-            }
-
-            #[cfg(not(feature = "no_function"))]
-            crate::engine::KEYWORD_IS_DEF_FN => true,
-
-            _ => false,
-        }
-    }
-
     /// If another operator is after these, it's probably a unary operator
     /// (not sure about `fn` name).
     #[must_use]
@@ -1994,7 +1959,8 @@ fn parse_identifier_token(
     if let Some(token) = Token::lookup_symbol_from_syntax(&identifier) {
         return (token, start_pos);
     }
-    if Token::is_reserved_keyword(&identifier) {
+
+    if is_reserved_keyword_or_symbol(&identifier) {
         return (Token::Reserved(Box::new(identifier)), start_pos);
     }
 
@@ -2008,18 +1974,27 @@ fn parse_identifier_token(
     (Token::Identifier(identifier.into()), start_pos)
 }
 
-/// Is a keyword allowed as a function?
+/// Can a keyword be called like a function?
+///
+/// # Return values
+///
+/// The first `bool` indicates whether the keyword can be called normally as a function.
+///
+/// The second `bool` indicates whether the keyword can be called in method-call style.
 #[inline]
 #[must_use]
-pub fn is_keyword_function(name: &str) -> bool {
+pub fn is_keyword_function(name: &str) -> (bool, bool) {
     match name {
-        KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR
+        KEYWORD_TYPE_OF | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY => (true, true),
-        | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_IS_DEF_VAR => true,
+
+        KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_EVAL | KEYWORD_FN_PTR | KEYWORD_IS_DEF_VAR => {
+            (true, false)
+        }
 
         #[cfg(not(feature = "no_function"))]
-        crate::engine::KEYWORD_IS_DEF_FN => true,
+        crate::engine::KEYWORD_IS_DEF_FN => (true, false),
 
-        _ => false,
+        _ => (false, false),
     }
 }
 
@@ -2047,7 +2022,9 @@ pub fn is_valid_identifier(name: &str) -> bool {
 #[inline(always)]
 #[must_use]
 pub fn is_valid_function_name(name: &str) -> bool {
-    is_valid_identifier(name) && !is_keyword_function(name)
+    is_valid_identifier(name)
+        && !is_reserved_keyword_or_symbol(name)
+        && Token::lookup_symbol_from_syntax(name).is_none()
 }
 
 /// Is a character valid to start an identifier?
@@ -2082,6 +2059,39 @@ pub const fn is_id_continue(x: char) -> bool {
     x.is_ascii_alphanumeric() || x == '_'
 }
 
+/// Is a piece of syntax a reserved keyword or symbol?
+#[must_use]
+pub fn is_reserved_keyword_or_symbol(syntax: &str) -> bool {
+    match syntax {
+        #[cfg(feature = "no_object")]
+        "?." => true,
+        #[cfg(feature = "no_index")]
+        "?[" => true,
+        #[cfg(feature = "no_function")]
+        "fn" | "private" => true,
+        #[cfg(feature = "no_module")]
+        "import" | "export" | "as" => true,
+
+        // List of reserved operators
+        "===" | "!==" | "->" | "<-" | "?" | ":=" | ":;" | "~" | "!." | "::<" | "(*" | "*)"
+        | "#" | "#!" | "@" | "$" | "++" | "--" | "..." | "<|" | "|>" => true,
+
+        // List of reserved keywords
+        "public" | "protected" | "super" | "new" | "use" | "module" | "package" | "var"
+        | "static" | "shared" | "with" | "is" | "goto" | "exit" | "match" | "case" | "default"
+        | "void" | "null" | "nil" | "spawn" | "thread" | "go" | "sync" | "async" | "await"
+        | "yield" => true,
+
+        KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR
+        | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_THIS | KEYWORD_IS_DEF_VAR => true,
+
+        #[cfg(not(feature = "no_function"))]
+        crate::engine::KEYWORD_IS_DEF_FN => true,
+
+        _ => false,
+    }
+}
+
 /// _(internals)_ A type that implements the [`InputStream`] trait.
 /// Exported under the `internals` feature only.
 ///

src/types/fn_ptr.rs

@@ -2,11 +2,11 @@
 
 use crate::eval::GlobalRuntimeState;
 use crate::func::EncapsulatedEnviron;
-use crate::tokenizer::is_valid_function_name;
+use crate::tokenizer::{is_reserved_keyword_or_symbol, is_valid_function_name, Token};
 use crate::types::dynamic::Variant;
 use crate::{
-    Dynamic, Engine, FnArgsVec, FuncArgs, ImmutableString, NativeCallContext, Position, RhaiError,
+    Dynamic, Engine, FnArgsVec, FuncArgs, ImmutableString, NativeCallContext, ParseErrorType,
-    RhaiResult, RhaiResultOf, Shared, StaticVec, AST, ERR,
+    Position, RhaiError, RhaiResult, RhaiResultOf, Shared, StaticVec, AST, ERR,
 };
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
@@ -536,6 +536,13 @@ impl TryFrom<ImmutableString> for FnPtr {
                 #[cfg(not(feature = "no_function"))]
                 fn_def: None,
             })
+        } else if is_reserved_keyword_or_symbol(&value)
+            || Token::lookup_symbol_from_syntax(&value).is_some()
+        {
+            Err(
+                ERR::ErrorParsing(ParseErrorType::Reserved(value.to_string()), Position::NONE)
+                    .into(),
+            )
         } else {
             Err(ERR::ErrorFunctionNotFound(value.to_string(), Position::NONE).into())
         }