From 057e84afe5bd37fe272bf7cfafca629ef9da1bd3 Mon Sep 17 00:00:00 2001 From: Bram Moolenaar Date: Sun, 28 Feb 2021 16:55:11 +0100 Subject: 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) --- src/errors.h | 6 +++++- src/proto/vim9compile.pro | 2 +- src/testdir/test_vim9_expr.vim | 6 +++--- src/testdir/test_vim9_func.vim | 27 ++++++++++++++++++++++++++- src/testdir/test_vim9_script.vim | 6 +++--- src/userfunc.c | 27 +++++++++++++++------------ src/version.c | 2 ++ src/vim9compile.c | 31 ++++++++++++++++++++++--------- src/vim9script.c | 6 +++--- 9 files changed, 80 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/errors.h b/src/errors.h index 775c5324b..db958e054 100644 --- a/src/errors.h +++ b/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")); diff --git a/src/proto/vim9compile.pro b/src/proto/vim9compile.pro index 410910f00..4f2fffe68 100644 --- a/src/proto/vim9compile.pro +++ b/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); diff --git a/src/testdir/test_vim9_expr.vim b/src/testdir/test_vim9_expr.vim index cda1ffdb4..26a693242 100644 --- a/src/testdir/test_vim9_expr.vim +++ b/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() diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim index 05becd72c..bc65bb163 100644 --- a/src/testdir/test_vim9_func.vim +++ b/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() diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim index 629d0b836..5d1e4015a 100644 --- a/src/testdir/test_vim9_script.vim +++ b/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 diff --git a/src/userfunc.c b/src/userfunc.c index b49cef178..04c264120 100644 --- a/src/userfunc.c +++ b/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; diff --git a/src/version.c b/src/version.c index 8eead06d5..b623c4218 100644 --- a/src/version.c +++ b/src/version.c @@ -750,6 +750,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 2558, /**/ 2557, /**/ diff --git a/src/vim9compile.c b/src/vim9compile.c index 0be2b29d8..f7bdf313c 100644 --- a/src/vim9compile.c +++ b/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; } diff --git a/src/vim9script.c b/src/vim9script.c index 39d2eeb11..68d7f5e19 100644 --- a/src/vim9script.c +++ b/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 ..." -- cgit v1.2.1