patch 8.2.3179: Vim9: cannot assign to an imported variable at script level

Problem:    Vim9: cannot assign to an imported variable at script level.
Solution:   Lookup imported items when assigning.

diff --git a/src/errors.h b/src/errors.h
index 35d1487338391ab83537a8309d16f644da90ff4a..be1341adb140e90b8d9c8ddc712fce56a868de5d 100644 (file)
--- a/src/errors.h
+++ b/src/errors.h
@@ -506,3 +506,5 @@ EXTERN char e_list_required_for_argument_nr[]
        INIT(= N_("E1211: List required for argument %d"));
 EXTERN char e_bool_required_for_argument_nr[]
        INIT(= N_("E1211: Bool required for argument %d"));
+EXTERN char e_redefining_imported_item_str[]
+       INIT(= N_("E1212: Redefining imported item %s"));

diff --git a/src/eval.c b/src/eval.c
index 058b2d3bed8e0f2e3cd70eab0a169bae986be423..77774b738da15fae17acdf1d48dc1d6e44929d63 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -1358,7 +1358,8 @@ set_var_lval(
                         || (!var_check_ro(di->di_flags, lp->ll_name, FALSE)
                           && !tv_check_lock(&di->di_tv, lp->ll_name, FALSE)))
                        && tv_op(&tv, rettv, op) == OK)
-                   set_var(lp->ll_name, &tv, FALSE);
+                   set_var_const(lp->ll_name, NULL, &tv, FALSE,
+                                                           ASSIGN_NO_DECL, 0);
                clear_tv(&tv);
            }
        }

diff --git a/src/evalvars.c b/src/evalvars.c
index 27d73287154bc375cd2ffacc8241adfa652964aa..3f209ba2166f7f23b67474b42cb087ecf0292136 100644 (file)
--- a/src/evalvars.c
+++ b/src/evalvars.c
@@ -3201,6 +3201,7 @@ set_var_const(
     typval_T   *tv = tv_arg;
     typval_T   bool_tv;
     dictitem_T *di;
+    typval_T   *dest_tv = NULL;
     char_u     *varname;
     hashtab_T  *ht;
     int                is_script_local;
@@ -3241,182 +3242,210 @@ set_var_const(
 
     di = find_var_in_ht(ht, 0, varname, TRUE);
 
-    // Search in parent scope which is possible to reference from lambda
-    if (di == NULL)
-       di = find_var_in_scoped_ht(name, TRUE);
-
-    if ((tv->v_type == VAR_FUNC || tv->v_type == VAR_PARTIAL)
-                                     && var_wrong_func_name(name, di == NULL))
-       goto failed;
-
-    if (need_convert_to_bool(type, tv))
+    if (di == NULL && var_in_vim9script)
     {
-       // Destination is a bool and the value is not, but it can be converted.
-       CLEAR_FIELD(bool_tv);
-       bool_tv.v_type = VAR_BOOL;
-       bool_tv.vval.v_number = tv2bool(tv) ? VVAL_TRUE : VVAL_FALSE;
-       tv = &bool_tv;
-    }
+       imported_T  *import = find_imported(varname, 0, NULL);
 
-    if (di != NULL)
-    {
-       // Item already exists.  Allowed to replace when reloading.
-       if ((di->di_flags & DI_FLAGS_RELOAD) == 0)
+       if (import != NULL)
        {
-           if ((flags & (ASSIGN_CONST | ASSIGN_FINAL))
-                                            && (flags & ASSIGN_FOR_LOOP) == 0)
-           {
-               emsg(_(e_cannot_mod));
-               goto failed;
-           }
+           scriptitem_T    *si = SCRIPT_ITEM(import->imp_sid);
+           svar_T          *sv;
 
-           if (is_script_local && vim9script
-                             && (flags & (ASSIGN_NO_DECL | ASSIGN_DECL)) == 0)
+           // imported variable from another script
+           if ((flags & ASSIGN_NO_DECL) == 0)
            {
-               semsg(_(e_redefining_script_item_str), name);
+               semsg(_(e_redefining_imported_item_str), name);
                goto failed;
            }
+           sv = ((svar_T *)si->sn_var_vals.ga_data)
+                                                   + import->imp_var_vals_idx;
+           // TODO: check the type
+           // TODO: check for const and locked
+           dest_tv = sv->sv_tv;
+       }
+    }
 
-           if (var_in_vim9script)
-           {
-               where_T where;
+    if (dest_tv == NULL)
+    {
+       // Search in parent scope which is possible to reference from lambda
+       if (di == NULL)
+           di = find_var_in_scoped_ht(name, TRUE);
 
-               // check the type and adjust to bool if needed
-               where.wt_index = var_idx;
-               where.wt_variable = TRUE;
-               if (check_script_var_type(&di->di_tv, tv, name, where) == FAIL)
-                   goto failed;
-           }
+       if ((tv->v_type == VAR_FUNC || tv->v_type == VAR_PARTIAL)
+                                         && var_wrong_func_name(name, di == NULL))
+           goto failed;
 
-           if (var_check_permission(di, name) == FAIL)
-               goto failed;
-       }
-       else
+       if (need_convert_to_bool(type, tv))
        {
-           // can only redefine once
-           di->di_flags &= ~DI_FLAGS_RELOAD;
-
-           // A Vim9 script-local variable is also present in sn_all_vars and
-           // sn_var_vals.  It may set "type" from "tv".
-           if (var_in_vim9script)
-               update_vim9_script_var(FALSE, di, flags, tv, &type,
-                                        (flags & ASSIGN_NO_MEMBER_TYPE) == 0);
+           // Destination is a bool and the value is not, but it can be converted.
+           CLEAR_FIELD(bool_tv);
+           bool_tv.v_type = VAR_BOOL;
+           bool_tv.vval.v_number = tv2bool(tv) ? VVAL_TRUE : VVAL_FALSE;
+           tv = &bool_tv;
        }
 
-       // existing variable, need to clear the value
-
-       // Handle setting internal di: variables separately where needed to
-       // prevent changing the type.
-       if (ht == &vimvarht)
+       if (di != NULL)
        {
-           if (di->di_tv.v_type == VAR_STRING)
+           // Item already exists.  Allowed to replace when reloading.
+           if ((di->di_flags & DI_FLAGS_RELOAD) == 0)
            {
-               VIM_CLEAR(di->di_tv.vval.v_string);
-               if (copy || tv->v_type != VAR_STRING)
+               if ((flags & (ASSIGN_CONST | ASSIGN_FINAL))
+                                                && (flags & ASSIGN_FOR_LOOP) == 0)
                {
-                   char_u *val = tv_get_string(tv);
+                   emsg(_(e_cannot_mod));
+                   goto failed;
+               }
 
-                   // Careful: when assigning to v:errmsg and tv_get_string()
-                   // causes an error message the variable will already be set.
-                   if (di->di_tv.vval.v_string == NULL)
-                       di->di_tv.vval.v_string = vim_strsave(val);
+               if (is_script_local && vim9script
+                                 && (flags & (ASSIGN_NO_DECL | ASSIGN_DECL)) == 0)
+               {
+                   semsg(_(e_redefining_script_item_str), name);
+                   goto failed;
                }
-               else
+
+               if (var_in_vim9script)
                {
-                   // Take over the string to avoid an extra alloc/free.
-                   di->di_tv.vval.v_string = tv->vval.v_string;
-                   tv->vval.v_string = NULL;
+                   where_T where;
+
+                   // check the type and adjust to bool if needed
+                   where.wt_index = var_idx;
+                   where.wt_variable = TRUE;
+                   if (check_script_var_type(&di->di_tv, tv, name, where) == FAIL)
+                       goto failed;
                }
-               goto failed;
+
+               if (var_check_permission(di, name) == FAIL)
+                   goto failed;
            }
-           else if (di->di_tv.v_type == VAR_NUMBER)
+           else
            {
-               di->di_tv.vval.v_number = tv_get_number(tv);
-               if (STRCMP(varname, "searchforward") == 0)
-                   set_search_direction(di->di_tv.vval.v_number ? '/' : '?');
-#ifdef FEAT_SEARCH_EXTRA
-               else if (STRCMP(varname, "hlsearch") == 0)
+               // can only redefine once
+               di->di_flags &= ~DI_FLAGS_RELOAD;
+
+               // A Vim9 script-local variable is also present in sn_all_vars and
+               // sn_var_vals.  It may set "type" from "tv".
+               if (var_in_vim9script)
+                   update_vim9_script_var(FALSE, di, flags, tv, &type,
+                                            (flags & ASSIGN_NO_MEMBER_TYPE) == 0);
+           }
+
+           // existing variable, need to clear the value
+
+           // Handle setting internal di: variables separately where needed to
+           // prevent changing the type.
+           if (ht == &vimvarht)
+           {
+               if (di->di_tv.v_type == VAR_STRING)
                {
-                   no_hlsearch = !di->di_tv.vval.v_number;
-                   redraw_all_later(SOME_VALID);
+                   VIM_CLEAR(di->di_tv.vval.v_string);
+                   if (copy || tv->v_type != VAR_STRING)
+                   {
+                       char_u *val = tv_get_string(tv);
+
+                       // Careful: when assigning to v:errmsg and tv_get_string()
+                       // causes an error message the variable will already be set.
+                       if (di->di_tv.vval.v_string == NULL)
+                           di->di_tv.vval.v_string = vim_strsave(val);
+                   }
+                   else
+                   {
+                       // Take over the string to avoid an extra alloc/free.
+                       di->di_tv.vval.v_string = tv->vval.v_string;
+                       tv->vval.v_string = NULL;
+                   }
+                   goto failed;
                }
+               else if (di->di_tv.v_type == VAR_NUMBER)
+               {
+                   di->di_tv.vval.v_number = tv_get_number(tv);
+                   if (STRCMP(varname, "searchforward") == 0)
+                       set_search_direction(di->di_tv.vval.v_number ? '/' : '?');
+#ifdef FEAT_SEARCH_EXTRA
+                   else if (STRCMP(varname, "hlsearch") == 0)
+                   {
+                       no_hlsearch = !di->di_tv.vval.v_number;
+                       redraw_all_later(SOME_VALID);
+                   }
 #endif
-               goto failed;
+                   goto failed;
+               }
+               else if (di->di_tv.v_type != tv->v_type)
+               {
+                   semsg(_("E963: setting %s to value with wrong type"), name);
+                   goto failed;
+               }
            }
-           else if (di->di_tv.v_type != tv->v_type)
+
+           clear_tv(&di->di_tv);
+       }
+       else
+       {
+           // Item not found, check if a function already exists.
+           if (is_script_local && (flags & (ASSIGN_NO_DECL | ASSIGN_DECL)) == 0
+                      && lookup_scriptitem(name, STRLEN(name), FALSE, NULL) == OK)
            {
-               semsg(_("E963: setting %s to value with wrong type"), name);
+               semsg(_(e_redefining_script_item_str), name);
                goto failed;
            }
-       }
 
-       clear_tv(&di->di_tv);
-    }
-    else
-    {
-       // Item not found, check if a function already exists.
-       if (is_script_local && (flags & (ASSIGN_NO_DECL | ASSIGN_DECL)) == 0
-                  && lookup_scriptitem(name, STRLEN(name), FALSE, NULL) == OK)
-       {
-           semsg(_(e_redefining_script_item_str), name);
-           goto failed;
-       }
+           // add a new variable
+           if (var_in_vim9script && (flags & ASSIGN_NO_DECL))
+           {
+               semsg(_(e_unknown_variable_str), name);
+               goto failed;
+           }
 
-       // add a new variable
-       if (var_in_vim9script && (flags & ASSIGN_NO_DECL))
-       {
-           semsg(_(e_unknown_variable_str), name);
-           goto failed;
-       }
+           // Can't add "v:" or "a:" variable.
+           if (ht == &vimvarht || ht == get_funccal_args_ht())
+           {
+               semsg(_(e_illvar), name);
+               goto failed;
+           }
 
-       // Can't add "v:" or "a:" variable.
-       if (ht == &vimvarht || ht == get_funccal_args_ht())
-       {
-           semsg(_(e_illvar), name);
-           goto failed;
-       }
+           // Make sure the variable name is valid.  In Vim9 script an autoload
+           // variable must be prefixed with "g:".
+           if (!valid_varname(varname, !vim9script
+                                                  || STRNCMP(name, "g:", 2) == 0))
+               goto failed;
 
-       // Make sure the variable name is valid.  In Vim9 script an autoload
-       // variable must be prefixed with "g:".
-       if (!valid_varname(varname, !vim9script
-                                              || STRNCMP(name, "g:", 2) == 0))
-           goto failed;
+           di = alloc(sizeof(dictitem_T) + STRLEN(varname));
+           if (di == NULL)
+               goto failed;
+           STRCPY(di->di_key, varname);
+           if (hash_add(ht, DI2HIKEY(di)) == FAIL)
+           {
+               vim_free(di);
+               goto failed;
+           }
+           di->di_flags = DI_FLAGS_ALLOC;
+           if (flags & (ASSIGN_CONST | ASSIGN_FINAL))
+               di->di_flags |= DI_FLAGS_LOCK;
 
-       di = alloc(sizeof(dictitem_T) + STRLEN(varname));
-       if (di == NULL)
-           goto failed;
-       STRCPY(di->di_key, varname);
-       if (hash_add(ht, DI2HIKEY(di)) == FAIL)
-       {
-           vim_free(di);
-           goto failed;
+           // A Vim9 script-local variable is also added to sn_all_vars and
+           // sn_var_vals. It may set "type" from "tv".
+           if (var_in_vim9script)
+               update_vim9_script_var(TRUE, di, flags, tv, &type,
+                                            (flags & ASSIGN_NO_MEMBER_TYPE) == 0);
        }
-       di->di_flags = DI_FLAGS_ALLOC;
-       if (flags & (ASSIGN_CONST | ASSIGN_FINAL))
-           di->di_flags |= DI_FLAGS_LOCK;
-
-       // A Vim9 script-local variable is also added to sn_all_vars and
-       // sn_var_vals. It may set "type" from "tv".
-       if (var_in_vim9script)
-           update_vim9_script_var(TRUE, di, flags, tv, &type,
-                                        (flags & ASSIGN_NO_MEMBER_TYPE) == 0);
+
+       dest_tv = &di->di_tv;
     }
 
     if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT)
-       copy_tv(tv, &di->di_tv);
+       copy_tv(tv, dest_tv);
     else
     {
-       di->di_tv = *tv;
-       di->di_tv.v_lock = 0;
+       *dest_tv = *tv;
+       dest_tv->v_lock = 0;
        init_tv(tv);
     }
 
     if (vim9script && type != NULL)
     {
-       if (type->tt_type == VAR_DICT && di->di_tv.vval.v_dict != NULL)
-           di->di_tv.vval.v_dict->dv_type = alloc_type(type);
-       else if (type->tt_type == VAR_LIST && di->di_tv.vval.v_list != NULL)
-           di->di_tv.vval.v_list->lv_type = alloc_type(type);
+       if (type->tt_type == VAR_DICT && dest_tv->vval.v_dict != NULL)
+           dest_tv->vval.v_dict->dv_type = alloc_type(type);
+       else if (type->tt_type == VAR_LIST && dest_tv->vval.v_list != NULL)
+           dest_tv->vval.v_list->lv_type = alloc_type(type);
     }
 
     // ":const var = value" locks the value
@@ -3425,8 +3454,9 @@ set_var_const(
        // Like :lockvar! name: lock the value and what it contains, but only
        // if the reference count is up to one.  That locks only literal
        // values.
-       item_lock(&di->di_tv, DICT_MAXNEST, TRUE, TRUE);
+       item_lock(dest_tv, DICT_MAXNEST, TRUE, TRUE);
     return;
+
 failed:
     if (!copy)
        clear_tv(tv_arg);

diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index 9baa1ff5aa7d1dd00a6e1e647725d73adb07a744..203c5c905a445de587cf86624a54752ae0f2fb24 100644 (file)
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -1062,6 +1062,12 @@ let s:export_script_lines =<< trim END
   export def Exported(): string
     return 'Exported'
   enddef
+  export def ExportedValue(): number
+    return exported
+  enddef
+  export def ExportedInc()
+    exported += 5
+  enddef
   export final theList = [1]
 END
 
@@ -1073,10 +1079,21 @@ enddef
 def Test_vim9_import_export()
   var import_script_lines =<< trim END
     vim9script
-    import {exported, Exported} from './Xexport.vim'
-    g:imported = exported
+    import {exported, Exported, ExportedValue} from './Xexport.vim'
+    g:exported1 = exported
     exported += 3
-    g:imported_added = exported
+    g:exported2 = exported
+    g:exported3 = ExportedValue()
+
+    import ExportedInc from './Xexport.vim'
+    ExportedInc()
+    g:exported_i1 = exported
+    g:exported_i2 = ExportedValue()
+
+    exported = 11
+    g:exported_s1 = exported
+    g:exported_s2 = ExportedValue()
+
     g:imported_func = Exported()
 
     def GetExported(): string
@@ -1091,7 +1108,7 @@ def Test_vim9_import_export()
     g:imported_name = exp_name
     exp_name ..= ' Doe'
     g:imported_name_appended = exp_name
-    g:imported_later = exported
+    g:exported_later = exported
 
     import theList from './Xexport.vim'
     theList->add(2)
@@ -1105,9 +1122,17 @@ def Test_vim9_import_export()
 
   assert_equal('bobbie', g:result)
   assert_equal('bob', g:localname)
-  assert_equal(9876, g:imported)
-  assert_equal(9879, g:imported_added)
-  assert_equal(9879, g:imported_later)
+  assert_equal(9876, g:exported1)
+  assert_equal(9879, g:exported2)
+  assert_equal(9879, g:exported3)
+
+  assert_equal(9884, g:exported_i1)
+  assert_equal(9884, g:exported_i2)
+
+  assert_equal(11, g:exported_s1)
+  assert_equal(11, g:exported_s2)
+  assert_equal(11, g:exported_later)
+
   assert_equal('Exported', g:imported_func)
   assert_equal('Exported', g:funcref_result)
   assert_equal('John', g:imported_name)
@@ -1115,9 +1140,12 @@ def Test_vim9_import_export()
   assert_false(exists('g:name'))
 
   Undo_export_script_lines()
-  unlet g:imported
-  unlet g:imported_added
-  unlet g:imported_later
+  unlet g:exported1
+  unlet g:exported2
+  unlet g:exported3
+  unlet g:exported_i1
+  unlet g:exported_i2
+  unlet g:exported_later
   unlet g:imported_func
   unlet g:imported_name g:imported_name_appended
   delete('Ximport.vim')
@@ -1131,22 +1159,22 @@ def Test_vim9_import_export()
         }
         from
         './Xexport.vim'
-    g:imported = exported
-    exported += 5
-    g:imported_added = exported
+    g:exported = exported
+    exported += 7
+    g:exported_added = exported
     g:imported_func = Exported()
   END
   writefile(import_line_break_script_lines, 'Ximport_lbr.vim')
   source Ximport_lbr.vim
 
-  assert_equal(9876, g:imported)
-  assert_equal(9881, g:imported_added)
+  assert_equal(11, g:exported)
+  assert_equal(18, g:exported_added)
   assert_equal('Exported', g:imported_func)
 
   # exported script not sourced again
   assert_false(exists('g:result'))
-  unlet g:imported
-  unlet g:imported_added
+  unlet g:exported
+  unlet g:exported_added
   unlet g:imported_func
   delete('Ximport_lbr.vim')
 
@@ -1154,18 +1182,20 @@ def Test_vim9_import_export()
     vim9script
     import * as Export from './Xexport.vim'
     def UseExport()
-      g:imported_def = Export.exported
+      g:exported_def = Export.exported
     enddef
-    g:imported_script = Export.exported
+    g:exported_script = Export.exported
     assert_equal(1, exists('Export.exported'))
     assert_equal(0, exists('Export.notexported'))
     UseExport()
   END
   writefile(import_star_as_lines, 'Ximport.vim')
   source Ximport.vim
-  # FIXME: this should be 9881
-  assert_equal(9876, g:imported_def)
-  assert_equal(9876, g:imported_script)
+
+  assert_equal(18, g:exported_def)
+  assert_equal(18, g:exported_script)
+  unlet g:exported_def
+  unlet g:exported_script
 
   var import_star_as_lines_no_dot =<< trim END
     vim9script
@@ -1234,13 +1264,14 @@ def Test_vim9_import_export()
         from
         './Xexport.vim'
     def UseExport()
-      g:imported = Export.exported
+      g:exported = Export.exported
     enddef
     UseExport()
   END
   writefile(import_star_as_lbr_lines, 'Ximport.vim')
   source Ximport.vim
-  assert_equal(9876, g:imported)
+  assert_equal(18, g:exported)
+  unlet g:exported
 
   var import_star_lines =<< trim END
     vim9script

diff --git a/src/version.c b/src/version.c
index 6f7864a6aa000627490c6773a7a004f2c9be3660..48e6748aeb647949610c60c68df9ef483618c7e9 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -755,6 +755,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    3179,
 /**/
     3178,
 /**/