patch 9.2.0265: unnecessary restrictions for defining dictionary function names

Problem:  unnecessary restrictions for defining dictionary function
          names
Solution: Allow defining dict function with bracket key that is not a
          valid identifier (thinca)

In Vim script, "function obj.func()" and "function obj['func']()" both
define a dictionary function.  However, the bracket form required the
key to match function naming rules (eval_isnamec), so
"function obj['foo-bar']()" failed with E475.

Assigning and calling already work: "let obj['foo-bar'] = obj.func"
and "call obj['foo-bar']()" are valid.  Only the definition was
incorrectly restricted.

Skip the identifier check when the name comes from fd_newkey (i.e. the
key was given in bracket notation).  Dictionary keys may be any string.

Supported by AI

closes: #19833

Signed-off-by: thinca <[email protected]>
Signed-off-by: Yegappan Lakshmanan <[email protected]>
Signed-off-by: Christian Brabandt <[email protected]>

Author: thinca

src/testdir/test_user_func.vim

@@ -584,6 +584,18 @@ func Test_func_dict()
   call assert_fails('call mydict.nonexist()', 'E716:')
 endfunc
 
+func Test_func_dict_bracket_key()
+  " Dictionary function can be defined with bracket notation using a key
+  " that does not follow function naming rules (e.g. containing a hyphen).
+  let obj = {}
+  function obj['foo-bar']() dict
+    return self.value
+  endfunction
+  let obj.value = 42
+  call assert_equal(42, obj['foo-bar']())
+  call assert_equal(42, call(obj['foo-bar'], []))
+endfunc
+
 func Test_func_range()
   new
   call setline(1, range(1, 8))

src/userfunc.c

@@ -5253,33 +5253,40 @@ define_function(
 	    char_u  *name_base = arg;
 	    int	    i;
 
-	    if (*arg == K_SPECIAL)
+	    // When defining a dictionary function with bracket notation
+	    // (e.g. obj['foo-bar']()), the key is a dictionary key and is not
+	    // required to follow function naming rules.  Skip the identifier
+	    // check in that case.
+	    if (arg != fudi.fd_newkey)
 	    {
-		name_base = vim_strchr(arg, '_');
-		if (name_base == NULL)
-		    name_base = arg + 3;
-		else
-		    ++name_base;
-	    }
-	    for (i = 0; name_base[i] != NUL && (i == 0
-					? eval_isnamec1(name_base[i])
-					: eval_isnamec(name_base[i])); ++i)
-		;
-	    if (name_base[i] != NUL)
-	    {
-		emsg_funcname(e_invalid_argument_str, arg);
-		goto ret_free;
-	    }
+		if (*arg == K_SPECIAL)
+		{
+		    name_base = vim_strchr(arg, '_');
+		    if (name_base == NULL)
+			name_base = arg + 3;
+		    else
+			++name_base;
+		}
+		for (i = 0; name_base[i] != NUL && (i == 0
+					    ? eval_isnamec1(name_base[i])
+					    : eval_isnamec(name_base[i])); ++i)
+		    ;
+		if (name_base[i] != NUL)
+		{
+		    emsg_funcname(e_invalid_argument_str, arg);
+		    goto ret_free;
+		}
 
-	    // In Vim9 script a function cannot have the same name as a
-	    // variable.
-	    if (vim9script && *arg == K_SPECIAL
-		&& eval_variable(name_base, i, 0, NULL,
-		    NULL, EVAL_VAR_NOAUTOLOAD + EVAL_VAR_IMPORT
+		// In Vim9 script a function cannot have the same name as a
+		// variable.
+		if (vim9script && *arg == K_SPECIAL
+		    && eval_variable(name_base, i, 0, NULL,
+			NULL, EVAL_VAR_NOAUTOLOAD + EVAL_VAR_IMPORT
 						     + EVAL_VAR_NO_FUNC) == OK)
-	    {
-		semsg(_(e_redefining_script_item_str), name_base);
-		goto ret_free;
+		{
+		    semsg(_(e_redefining_script_item_str), name_base);
+		    goto ret_free;
+		}
 	    }
 	}
 	// Disallow using the g: dict.

src/version.c

@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    265,
 /**/
     264,
 /**/