fix(api, lua): handle setting v: variables properly (#25325)

3 files changed, 114 insertions, 62 deletions

diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 8fcabb3605..5463578b56 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -21,8 +21,10 @@
 #include "nvim/buffer_defs.h"
 #include "nvim/eval/typval.h"
 #include "nvim/eval/typval_defs.h"
+#include "nvim/eval/vars.h"
 #include "nvim/ex_eval.h"
 #include "nvim/garray.h"
+#include "nvim/globals.h"
 #include "nvim/highlight_group.h"
 #include "nvim/lua/executor.h"
 #include "nvim/map.h"
@@ -235,8 +237,7 @@ Object dict_set_var(dict_T *dict, String key, Object value, bool del, bool retva
     // Delete the key
     if (di == NULL) {
       // Doesn't exist, fail
-      api_set_error(err, kErrorTypeValidation, "Key not found: %s",
-                    key.data);
+      api_set_error(err, kErrorTypeValidation, "Key not found: %s", key.data);
     } else {
       // Notify watchers
       if (watched) {
@@ -265,13 +266,23 @@ Object dict_set_var(dict_T *dict, String key, Object value, bool del, bool retva
       di = tv_dict_item_alloc_len(key.data, key.size);
       tv_dict_add(dict, di);
     } else {
-      if (watched) {
-        tv_copy(&di->di_tv, &oldtv);
-      }
       // Return the old value
       if (retval) {
         rv = vim_to_object(&di->di_tv);
       }
+      bool type_error = false;
+      if (dict == &vimvardict
+          && !before_set_vvar(key.data, di, &tv, true, watched, &type_error)) {
+        tv_clear(&tv);
+        if (type_error) {
+          api_set_error(err, kErrorTypeValidation,
+                        "Setting v:%s to value with wrong type", key.data);
+        }
+        return rv;
+      }
+      if (watched) {
+        tv_copy(&di->di_tv, &oldtv);
+      }
       tv_clear(&di->di_tv);
     }
 
diff --git a/src/nvim/eval/vars.c b/src/nvim/eval/vars.c
index e5b1b88eef..5d1da956ee 100644
--- a/src/nvim/eval/vars.c
+++ b/src/nvim/eval/vars.c
@@ -52,8 +52,8 @@
 
 static const char *e_letunexp = N_("E18: Unexpected characters in :let");
 static const char *e_lock_unlock = N_("E940: Cannot lock or unlock variable %s");
-static const char e_setting_str_to_value_with_wrong_type[]
-  = N_("E963: Setting %s to value with wrong type");
+static const char e_setting_v_str_to_value_with_wrong_type[]
+  = N_("E963: Setting v:%s to value with wrong type");
 static const char e_cannot_use_heredoc_here[]
   = N_("E991: Cannot use =<< here");
 
@@ -1389,6 +1389,62 @@ static void list_one_var_a(const char *prefix, const char *name, const ptrdiff_t
   }
 }
 
+/// Additional handling for setting a v: variable.
+///
+/// @return  true if the variable should be set normally,
+///          false if nothing else needs to be done.
+bool before_set_vvar(const char *const varname, dictitem_T *const di, typval_T *const tv,
+                     const bool copy, const bool watched, bool *const type_error)
+{
+  if (di->di_tv.v_type == VAR_STRING) {
+    typval_T oldtv = TV_INITIAL_VALUE;
+    if (watched) {
+      tv_copy(&di->di_tv, &oldtv);
+    }
+    XFREE_CLEAR(di->di_tv.vval.v_string);
+    if (copy || tv->v_type != VAR_STRING) {
+      const char *const 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 = xstrdup(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;
+    }
+    // Notify watchers
+    if (watched) {
+      tv_dict_watcher_notify(&vimvardict, varname, &di->di_tv, &oldtv);
+      tv_clear(&oldtv);
+    }
+    return false;
+  } else if (di->di_tv.v_type == VAR_NUMBER) {
+    typval_T oldtv = TV_INITIAL_VALUE;
+    if (watched) {
+      tv_copy(&di->di_tv, &oldtv);
+    }
+    di->di_tv.vval.v_number = tv_get_number(tv);
+    if (strcmp(varname, "searchforward") == 0) {
+      set_search_direction(di->di_tv.vval.v_number ? '/' : '?');
+    } else if (strcmp(varname, "hlsearch") == 0) {
+      no_hlsearch = !di->di_tv.vval.v_number;
+      redraw_all_later(UPD_SOME_VALID);
+    }
+    // Notify watchers
+    if (watched) {
+      tv_dict_watcher_notify(&vimvardict, varname, &di->di_tv, &oldtv);
+      tv_clear(&oldtv);
+    }
+    return false;
+  } else if (di->di_tv.v_type != tv->v_type) {
+    *type_error = true;
+    return false;
+  }
+  return true;
+}
+
 /// Set variable to the given value
 ///
 /// If the variable already exists, the value is updated. Otherwise the variable
@@ -1418,31 +1474,29 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
                    const bool is_const)
   FUNC_ATTR_NONNULL_ALL
 {
-  dictitem_T *v;
-  hashtab_T *ht;
-  dict_T *dict;
-
   const char *varname;
-  ht = find_var_ht_dict(name, name_len, &varname, &dict);
+  dict_T *dict;
+  hashtab_T *ht = find_var_ht_dict(name, name_len, &varname, &dict);
   const bool watched = tv_dict_is_watched(dict);
 
   if (ht == NULL || *varname == NUL) {
     semsg(_(e_illvar), name);
     return;
   }
-  v = find_var_in_ht(ht, 0, varname, name_len - (size_t)(varname - name), true);
+  const size_t varname_len = name_len - (size_t)(varname - name);
+  dictitem_T *di = find_var_in_ht(ht, 0, varname, varname_len, true);
 
   // Search in parent scope which is possible to reference from lambda
-  if (v == NULL) {
-    v = find_var_in_scoped_ht(name, name_len, true);
+  if (di == NULL) {
+    di = find_var_in_scoped_ht(name, name_len, true);
   }
 
-  if (tv_is_func(*tv) && var_wrong_func_name(name, v == NULL)) {
+  if (tv_is_func(*tv) && var_wrong_func_name(name, di == NULL)) {
     return;
   }
 
   typval_T oldtv = TV_INITIAL_VALUE;
-  if (v != NULL) {
+  if (di != NULL) {
     if (is_const) {
       emsg(_(e_cannot_mod));
       return;
@@ -1452,9 +1506,9 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
     // - Whether the variable is read-only
     // - Whether the variable value is locked
     // - Whether the variable is locked
-    if (var_check_ro(v->di_flags, name, name_len)
-        || value_check_lock(v->di_tv.v_lock, name, name_len)
-        || var_check_lock(v->di_flags, name, name_len)) {
+    if (var_check_ro(di->di_flags, name, name_len)
+        || value_check_lock(di->di_tv.v_lock, name, name_len)
+        || var_check_lock(di->di_flags, name, name_len)) {
       return;
     }
 
@@ -1462,42 +1516,19 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
 
     // Handle setting internal v: variables separately where needed to
     // prevent changing the type.
-    if (is_vimvarht(ht)) {
-      if (v->di_tv.v_type == VAR_STRING) {
-        XFREE_CLEAR(v->di_tv.vval.v_string);
-        if (copy || tv->v_type != VAR_STRING) {
-          const char *const 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 (v->di_tv.vval.v_string == NULL) {
-            v->di_tv.vval.v_string = xstrdup(val);
-          }
-        } else {
-          // Take over the string to avoid an extra alloc/free.
-          v->di_tv.vval.v_string = tv->vval.v_string;
-          tv->vval.v_string = NULL;
-        }
-        return;
-      } else if (v->di_tv.v_type == VAR_NUMBER) {
-        v->di_tv.vval.v_number = tv_get_number(tv);
-        if (strcmp(varname, "searchforward") == 0) {
-          set_search_direction(v->di_tv.vval.v_number ? '/' : '?');
-        } else if (strcmp(varname, "hlsearch") == 0) {
-          no_hlsearch = !v->di_tv.vval.v_number;
-          redraw_all_later(UPD_SOME_VALID);
-        }
-        return;
-      } else if (v->di_tv.v_type != tv->v_type) {
-        semsg(_(e_setting_str_to_value_with_wrong_type), name);
-        return;
+    bool type_error = false;
+    if (is_vimvarht(ht)
+        && !before_set_vvar(varname, di, tv, copy, watched, &type_error)) {
+      if (type_error) {
+        semsg(_(e_setting_v_str_to_value_with_wrong_type), varname);
       }
+      return;
     }
 
     if (watched) {
-      tv_copy(&v->di_tv, &oldtv);
+      tv_copy(&di->di_tv, &oldtv);
     }
-    tv_clear(&v->di_tv);
+    tv_clear(&di->di_tv);
   } else {  // Add a new variable.
     // Can't add "v:" or "a:" variable.
     if (is_vimvarht(ht) || ht == get_funccal_args_ht()) {
@@ -1513,28 +1544,28 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
     // Make sure dict is valid
     assert(dict != NULL);
 
-    v = xmalloc(offsetof(dictitem_T, di_key) + strlen(varname) + 1);
-    STRCPY(v->di_key, varname);
-    if (hash_add(ht, v->di_key) == FAIL) {
-      xfree(v);
+    di = xmalloc(offsetof(dictitem_T, di_key) + varname_len + 1);
+    memcpy(di->di_key, varname, varname_len + 1);
+    if (hash_add(ht, di->di_key) == FAIL) {
+      xfree(di);
       return;
     }
-    v->di_flags = DI_FLAGS_ALLOC;
+    di->di_flags = DI_FLAGS_ALLOC;
     if (is_const) {
-      v->di_flags |= DI_FLAGS_LOCK;
+      di->di_flags |= DI_FLAGS_LOCK;
     }
   }
 
   if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT) {
-    tv_copy(tv, &v->di_tv);
+    tv_copy(tv, &di->di_tv);
   } else {
-    v->di_tv = *tv;
-    v->di_tv.v_lock = VAR_UNLOCKED;
+    di->di_tv = *tv;
+    di->di_tv.v_lock = VAR_UNLOCKED;
     tv_init(tv);
   }
 
   if (watched) {
-    tv_dict_watcher_notify(dict, v->di_key, &v->di_tv, &oldtv);
+    tv_dict_watcher_notify(dict, di->di_key, &di->di_tv, &oldtv);
     tv_clear(&oldtv);
   }
 
@@ -1542,7 +1573,7 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
     // 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.
-    tv_item_lock(&v->di_tv, DICT_MAXNEST, true, true);
+    tv_item_lock(&di->di_tv, DICT_MAXNEST, true, true);
   }
 }
 
diff --git a/src/nvim/lua/stdlib.c b/src/nvim/lua/stdlib.c
index 8f1b5c7b86..f175a7abb0 100644
--- a/src/nvim/lua/stdlib.c
+++ b/src/nvim/lua/stdlib.c
@@ -24,6 +24,7 @@
 #include "nvim/buffer_defs.h"
 #include "nvim/eval/typval.h"
 #include "nvim/eval/typval_defs.h"
+#include "nvim/eval/vars.h"
 #include "nvim/ex_eval.h"
 #include "nvim/fold.h"
 #include "nvim/globals.h"
@@ -397,6 +398,15 @@ int nlua_setvar(lua_State *lstate)
       di = tv_dict_item_alloc_len(key.data, key.size);
       tv_dict_add(dict, di);
     } else {
+      bool type_error = false;
+      if (dict == &vimvardict
+          && !before_set_vvar(key.data, di, &tv, true, watched, &type_error)) {
+        tv_clear(&tv);
+        if (type_error) {
+          return luaL_error(lstate, "Setting v:%s to value with wrong type", key.data);
+        }
+        return 0;
+      }
       if (watched) {
         tv_copy(&di->di_tv, &oldtv);
       }