patch 8.2.2558: no error if a lambda argument shadows a variable

Problem:    No error if a lambda argument shadows a variable.
Solution:   Check that the argument name shadows a local, argument or script
            variable. (closes #7898)

Author: brammool

src/errors.h

@@ -147,7 +147,7 @@ EXTERN char e_cannot_declare_an_option[]
 	INIT(= N_("E1052: Cannot declare an option: %s"));
 EXTERN char e_could_not_import_str[]
 	INIT(= N_("E1053: Could not import \"%s\""));
-EXTERN char e_variable_already_declared_in_script[]
+EXTERN char e_variable_already_declared_in_script_str[]
 	INIT(= N_("E1054: Variable already declared in the script: %s"));
 EXTERN char e_missing_name_after_dots[]
 	INIT(= N_("E1055: Missing name after ..."));
@@ -369,3 +369,7 @@ EXTERN char e_cannot_use_range_with_assignment_str[]
 	INIT(= N_("E1165: Cannot use a range with an assignment: %s"));
 EXTERN char e_cannot_use_range_with_dictionary[]
 	INIT(= N_("E1166: Cannot use a range with a dictionary"));
+EXTERN char e_argument_name_shadows_existing_variable_str[]
+	INIT(= N_("E1167: Argument name shadows existing variable: %s"));
+EXTERN char e_argument_already_declared_in_script_str[]
+	INIT(= N_("E1168: Argument already declared in the script: %s"));

src/proto/vim9compile.pro

@@ -1,6 +1,6 @@
 /* vim9compile.c */
 int script_var_exists(char_u *name, size_t len, int vim9script, cctx_T *cctx);
-int check_defined(char_u *p, size_t len, cctx_T *cctx);
+int check_defined(char_u *p, size_t len, cctx_T *cctx, int is_arg);
 int check_compare_types(exprtype_T type, typval_T *tv1, typval_T *tv2);
 int use_typecheck(type_T *actual, type_T *expected);
 int need_type(type_T *actual, type_T *expected, int offset, int arg_idx, cctx_T *cctx, int silent, int actual_is_const);

src/testdir/test_vim9_expr.vim

@@ -2843,7 +2843,7 @@ def Test_expr7_trailing()
 
   # lambda method call
   l = [2, 5]
-  l->((l) => add(l, 8))()
+  l->((ll) => add(ll, 8))()
   assert_equal([2, 5, 8], l)
 
   # dict member
@@ -3034,8 +3034,8 @@ def Test_expr7_subscript_linebreak()
 enddef
 
 func Test_expr7_trailing_fails()
-  call CheckDefFailure(['var l = [2]', 'l->((l) => add(l, 8))'], 'E107:', 2)
-  call CheckDefFailure(['var l = [2]', 'l->((l) => add(l, 8)) ()'], 'E274:', 2)
+  call CheckDefFailure(['var l = [2]', 'l->((ll) => add(ll, 8))'], 'E107:', 2)
+  call CheckDefFailure(['var l = [2]', 'l->((ll) => add(ll, 8)) ()'], 'E274:', 2)
 endfunc
 
 func Test_expr_fails()

src/testdir/test_vim9_func.vim

@@ -596,7 +596,7 @@ def Test_call_wrong_args()
       echo nr
     enddef
   END
-  CheckScriptFailure(lines, 'E1054:')
+  CheckScriptFailure(lines, 'E1168:')
 
   lines =<< trim END
     vim9script
@@ -699,6 +699,31 @@ def Test_call_lambda_args()
     Ref = (x, y, z) => 0
   END
   CheckDefAndScriptFailure(lines, 'E1012:')
+
+  lines =<< trim END
+      var one = 1
+      var l = [1, 2, 3]
+      echo map(l, (one) => one)
+  END
+  CheckDefFailure(lines, 'E1167:')
+  CheckScriptFailure(['vim9script'] + lines, 'E1168:')
+
+  lines =<< trim END
+      def ShadowLocal()
+        var one = 1
+        var l = [1, 2, 3]
+        echo map(l, (one) => one)
+      enddef
+  END
+  CheckDefFailure(lines, 'E1167:')
+
+  lines =<< trim END
+      def Shadowarg(one: number)
+        var l = [1, 2, 3]
+        echo map(l, (one) => one)
+      enddef
+  END
+  CheckDefFailure(lines, 'E1167:')
 enddef
 
 def Test_lambda_return_type()

src/testdir/test_vim9_script.vim

@@ -1119,7 +1119,7 @@ def Test_vim9_import_export()
     import exported from './Xexport.vim'
   END
   writefile(import_already_defined, 'Ximport.vim')
-  assert_fails('source Ximport.vim', 'E1073:', '', 3, 'Ximport.vim')
+  assert_fails('source Ximport.vim', 'E1054:', '', 3, 'Ximport.vim')
 
   # try to import something that is already defined
   import_already_defined =<< trim END
@@ -1128,7 +1128,7 @@ def Test_vim9_import_export()
     import * as exported from './Xexport.vim'
   END
   writefile(import_already_defined, 'Ximport.vim')
-  assert_fails('source Ximport.vim', 'E1073:', '', 3, 'Ximport.vim')
+  assert_fails('source Ximport.vim', 'E1054:', '', 3, 'Ximport.vim')
 
   # try to import something that is already defined
   import_already_defined =<< trim END
@@ -1137,7 +1137,7 @@ def Test_vim9_import_export()
     import {exported} from './Xexport.vim'
   END
   writefile(import_already_defined, 'Ximport.vim')
-  assert_fails('source Ximport.vim', 'E1073:', '', 3, 'Ximport.vim')
+  assert_fails('source Ximport.vim', 'E1054:', '', 3, 'Ximport.vim')
 
   # try changing an imported const
   var import_assign_to_const =<< trim END

src/userfunc.c

@@ -55,6 +55,7 @@ func_tbl_get(void)
  * Get one function argument.
  * If "argtypes" is not NULL also get the type: "arg: type".
  * If "types_optional" is TRUE a missing type is OK, use "any".
+ * If "evalarg" is not NULL use it to check for an already declared name.
  * Return a pointer to after the type.
  * When something is wrong return "arg".
  */
@@ -64,6 +65,7 @@ one_function_arg(
 	garray_T    *newargs,
 	garray_T    *argtypes,
 	int	    types_optional,
+	evalarg_T   *evalarg,
 	int	    skip)
 {
     char_u	*p = arg;
@@ -81,13 +83,11 @@ one_function_arg(
 	return arg;
     }
 
-    // Vim9 script: cannot use script var name for argument.
-    if (!skip && argtypes != NULL && script_var_exists(arg, p - arg,
-							    FALSE, NULL) == OK)
-    {
-	semsg(_(e_variable_already_declared_in_script), arg);
+    // Vim9 script: cannot use script var name for argument. In function: also
+    // check local vars and arguments.
+    if (!skip && argtypes != NULL && check_defined(arg, p - arg,
+		    evalarg == NULL ? NULL : evalarg->eval_cctx, TRUE) == FAIL)
 	return arg;
-    }
 
     if (newargs != NULL && ga_grow(newargs, 1) == FAIL)
 	return arg;
@@ -173,6 +173,7 @@ get_function_args(
     garray_T	*newargs,
     garray_T	*argtypes,	// NULL unless using :def
     int		types_optional,	// types optional if "argtypes" is not NULL
+    evalarg_T	*evalarg,	// context or NULL
     int		*varargs,
     garray_T	*default_args,
     int		skip,
@@ -247,7 +248,7 @@ get_function_args(
 
 		arg = p;
 		p = one_function_arg(p, newargs, argtypes, types_optional,
-									 skip);
+								evalarg, skip);
 		if (p == arg)
 		    break;
 		if (*skipwhite(p) == '=')
@@ -260,7 +261,8 @@ get_function_args(
 	else
 	{
 	    arg = p;
-	    p = one_function_arg(p, newargs, argtypes, types_optional, skip);
+	    p = one_function_arg(p, newargs, argtypes, types_optional,
+								evalarg, skip);
 	    if (p == arg)
 		break;
 
@@ -576,7 +578,7 @@ get_lambda_tv(
     // be found after the arguments.
     s = *arg + 1;
     ret = get_function_args(&s, equal_arrow ? ')' : '-', NULL,
-	    types_optional ? &argtypes : NULL, types_optional,
+	    types_optional ? &argtypes : NULL, types_optional, evalarg,
 						 NULL, NULL, TRUE, NULL, NULL);
     if (ret == FAIL || skip_arrow(s, equal_arrow, &ret_type, NULL) == NULL)
     {
@@ -592,7 +594,7 @@ get_lambda_tv(
 	pnewargs = NULL;
     *arg += 1;
     ret = get_function_args(arg, equal_arrow ? ')' : '-', pnewargs,
-	    types_optional ? &argtypes : NULL, types_optional,
+	    types_optional ? &argtypes : NULL, types_optional, evalarg,
 					    &varargs, NULL, FALSE, NULL, NULL);
     if (ret == FAIL
 		  || (s = skip_arrow(*arg, equal_arrow, &ret_type,
@@ -683,7 +685,6 @@ get_lambda_tv(
 
 	fp->uf_refcount = 1;
 	set_ufunc_name(fp, name);
-	hash_add(&func_hashtab, UF2HIKEY(fp));
 	fp->uf_args = newargs;
 	ga_init(&fp->uf_def_args);
 	if (types_optional)
@@ -726,6 +727,8 @@ get_lambda_tv(
 	pt->pt_refcount = 1;
 	rettv->vval.v_partial = pt;
 	rettv->v_type = VAR_PARTIAL;
+
+	hash_add(&func_hashtab, UF2HIKEY(fp));
     }
 
     eval_lavars_used = old_eval_lavars;
@@ -3278,7 +3281,7 @@ define_function(exarg_T *eap, char_u *name_arg)
     ++p;
     if (get_function_args(&p, ')', &newargs,
 			eap->cmdidx == CMD_def ? &argtypes : NULL, FALSE,
-			 &varargs, &default_args, eap->skip,
+			 NULL, &varargs, &default_args, eap->skip,
 			 eap, &line_to_free) == FAIL)
 	goto errret_2;
     whitep = p;

src/version.c

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

src/vim9compile.c

@@ -379,8 +379,9 @@ script_var_exists(char_u *name, size_t len, int vim9script, cctx_T *cctx)
     static int
 variable_exists(char_u *name, size_t len, cctx_T *cctx)
 {
-    return lookup_local(name, len, NULL, cctx) == OK
-	    || arg_exists(name, len, NULL, NULL, NULL, cctx) == OK
+    return (cctx != NULL
+		&& (lookup_local(name, len, NULL, cctx) == OK
+		    || arg_exists(name, len, NULL, NULL, NULL, cctx) == OK))
 	    || script_var_exists(name, len, FALSE, cctx) == OK
 	    || find_imported(name, len, cctx) != NULL;
 }
@@ -389,17 +390,26 @@ variable_exists(char_u *name, size_t len, cctx_T *cctx)
  * Check if "p[len]" is already defined, either in script "import_sid" or in
  * compilation context "cctx".  "cctx" is NULL at the script level.
  * Does not check the global namespace.
+ * If "is_arg" is TRUE the error message is for an argument name.
  * Return FAIL and give an error if it defined.
  */
     int
-check_defined(char_u *p, size_t len, cctx_T *cctx)
+check_defined(char_u *p, size_t len, cctx_T *cctx, int is_arg)
 {
     int		c = p[len];
     ufunc_T	*ufunc = NULL;
 
+    if (script_var_exists(p, len, FALSE, cctx) == OK)
+    {
+	if (is_arg)
+	    semsg(_(e_argument_already_declared_in_script_str), p);
+	else
+	    semsg(_(e_variable_already_declared_in_script_str), p);
+	return FAIL;
+    }
+
     p[len] = NUL;
-    if (script_var_exists(p, len, FALSE, cctx) == OK
-	    || (cctx != NULL
+    if ((cctx != NULL
 		&& (lookup_local(p, len, NULL, cctx) == OK
 		    || arg_exists(p, len, NULL, NULL, NULL, cctx) == OK))
 	    || find_imported(p, len, cctx) != NULL
@@ -409,8 +419,11 @@ check_defined(char_u *p, size_t len, cctx_T *cctx)
 	if (ufunc == NULL || !func_is_global(ufunc)
 		|| (p[0] == 'g' && p[1] == ':'))
 	{
+	    if (is_arg)
+		semsg(_(e_argument_name_shadows_existing_variable_str), p);
+	    else
+		semsg(_(e_name_already_defined_str), p);
 	    p[len] = c;
-	    semsg(_(e_name_already_defined_str), p);
 	    return FAIL;
 	}
     }
@@ -5120,7 +5133,7 @@ compile_nested_function(exarg_T *eap, cctx_T *cctx)
 	semsg(_(e_namespace_not_supported_str), name_start);
 	return NULL;
     }
-    if (check_defined(name_start, name_end - name_start, cctx) == FAIL)
+    if (check_defined(name_start, name_end - name_start, cctx, FALSE) == FAIL)
 	return NULL;
 
     eap->arg = name_end;
@@ -5686,7 +5699,7 @@ compile_lhs(
 			    semsg(_(e_cannot_declare_script_variable_in_function),
 								lhs->lhs_name);
 			else
-			    semsg(_(e_variable_already_declared_in_script),
+			    semsg(_(e_variable_already_declared_in_script_str),
 								lhs->lhs_name);
 			return FAIL;
 		    }
@@ -5723,7 +5736,7 @@ compile_lhs(
 			}
 		    }
 		}
-		else if (check_defined(var_start, lhs->lhs_varlen, cctx)
+		else if (check_defined(var_start, lhs->lhs_varlen, cctx, FALSE)
 								       == FAIL)
 		    return FAIL;
 	    }

src/vim9script.c

@@ -370,7 +370,7 @@ handle_import(
 	    if (eval_isnamec1(*arg))
 		while (eval_isnamec(*arg))
 		    ++arg;
-	    if (check_defined(p, arg - p, cctx) == FAIL)
+	    if (check_defined(p, arg - p, cctx, FALSE) == FAIL)
 		goto erret;
 	    as_name = vim_strnsave(p, arg - p);
 	    arg = skipwhite_and_linebreak(arg, evalarg);
@@ -555,7 +555,7 @@ handle_import(
 	    }
 	    else
 	    {
-		if (check_defined(name, len, cctx) == FAIL)
+		if (check_defined(name, len, cctx, FALSE) == FAIL)
 		    goto erret;
 
 		imported = new_imported(gap != NULL ? gap
@@ -567,7 +567,7 @@ handle_import(
 		{
 		    imported->imp_name = name;
 		    ((char_u **)names.ga_data)[i] = NULL;
-		} 
+		}
 		else
 		{
 		    // "import This as That ..."