patch 9.0.1631: passing wrong variable type to option gives multiple errors

Problem:    Passing a wrong variable type to an option gives multiple errors.
Solution:   Bail out early on failure. (closes #12504)

diff --git a/src/evalvars.c b/src/evalvars.c
index 35a1d20831b8196c76d157423462f8584deeb2f8..1033004d799cf6f67d14fb2ecdf0457d1f92cdfd 100644 (file)
--- a/src/evalvars.c
+++ b/src/evalvars.c
@@ -1639,108 +1639,116 @@ ex_let_option(
     p = find_option_end(&arg, &scope);
     if (p == NULL || (endchars != NULL
                               && vim_strchr(endchars, *skipwhite(p)) == NULL))
+    {
        emsg(_(e_unexpected_characters_in_let));
-    else
+       return NULL;
+    }
+
+    int                c1;
+    long       n = 0;
+    getoption_T        opt_type;
+    long       numval;
+    char_u     *stringval = NULL;
+    char_u     *s = NULL;
+    int                failed = FALSE;
+    int                opt_p_flags;
+    char_u     *tofree = NULL;
+    char_u     numbuf[NUMBUFLEN];
+
+    c1 = *p;
+    *p = NUL;
+
+    opt_type = get_option_value(arg, &numval, &stringval, &opt_p_flags, scope);
+    if (opt_type == gov_unknown && arg[0] != 't' && arg[1] != '_')
     {
-       int         c1;
-       long        n = 0;
-       getoption_T opt_type;
-       long        numval;
-       char_u      *stringval = NULL;
-       char_u      *s = NULL;
-       int         failed = FALSE;
-       int         opt_p_flags;
-       char_u      *tofree = NULL;
-       char_u      numbuf[NUMBUFLEN];
-
-       c1 = *p;
-       *p = NUL;
+       semsg(_(e_unknown_option_str_2), arg);
+       goto theend;
+    }
+    if (op != NULL && *op != '='
+       && (((opt_type == gov_bool || opt_type == gov_number) && *op == '.')
+           || (opt_type == gov_string && *op != '.')))
+    {
+       semsg(_(e_wrong_variable_type_for_str_equal), op);
+       goto theend;
+    }
 
-       opt_type = get_option_value(arg, &numval, &stringval, &opt_p_flags,
-                                                                       scope);
-       if ((opt_type == gov_bool
-                   || opt_type == gov_number
-                   || opt_type == gov_hidden_bool
-                   || opt_type == gov_hidden_number)
-                        && (tv->v_type != VAR_STRING || !in_vim9script()))
-       {
-           if (opt_type == gov_bool || opt_type == gov_hidden_bool)
-               // bool, possibly hidden
-               n = (long)tv_get_bool(tv);
-           else
-               // number, possibly hidden
-               n = (long)tv_get_number(tv);
-       }
+    if ((opt_type == gov_bool
+               || opt_type == gov_number
+               || opt_type == gov_hidden_bool
+               || opt_type == gov_hidden_number)
+                    && (tv->v_type != VAR_STRING || !in_vim9script()))
+    {
+       if (opt_type == gov_bool || opt_type == gov_hidden_bool)
+           // bool, possibly hidden
+           n = (long)tv_get_bool_chk(tv, &failed);
+       else
+           // number, possibly hidden
+           n = (long)tv_get_number_chk(tv, &failed);
+       if (failed)
+           goto theend;
+    }
 
-       if ((opt_p_flags & P_FUNC) && (tv->v_type == VAR_PARTIAL
-                                               || tv->v_type == VAR_FUNC))
-       {
-           // If the option can be set to a function reference or a lambda
-           // and the passed value is a function reference, then convert it to
-           // the name (string) of the function reference.
-           s = tv2string(tv, &tofree, numbuf, 0);
-       }
-       // Avoid setting a string option to the text "v:false" or similar.
-       // In Vim9 script also don't convert a number to string.
-       else if (tv->v_type != VAR_BOOL && tv->v_type != VAR_SPECIAL
-                        && (!in_vim9script() || tv->v_type != VAR_NUMBER))
-           s = tv_get_string_chk(tv);
+    if ((opt_p_flags & P_FUNC) && (tv->v_type == VAR_PARTIAL
+                                                   || tv->v_type == VAR_FUNC))
+    {
+       // If the option can be set to a function reference or a lambda
+       // and the passed value is a function reference, then convert it to
+       // the name (string) of the function reference.
+       s = tv2string(tv, &tofree, numbuf, 0);
+       if (s == NULL)
+           goto theend;
+    }
+    // Avoid setting a string option to the text "v:false" or similar.
+    // In Vim9 script also don't convert a number to string.
+    else if (tv->v_type != VAR_BOOL && tv->v_type != VAR_SPECIAL
+                    && (!in_vim9script() || tv->v_type != VAR_NUMBER))
+    {
+       s = tv_get_string_chk(tv);
+       if (s == NULL)
+           goto theend;
+    }
+    else if (opt_type == gov_string || opt_type == gov_hidden_string)
+    {
+       emsg(_(e_string_required));
+       goto theend;
+    }
 
-       if (op != NULL && *op != '=')
+    if (op != NULL && *op != '=')
+    {
+       // number, in legacy script also bool
+       if (opt_type == gov_number
+                                || (opt_type == gov_bool && !in_vim9script()))
        {
-           if (((opt_type == gov_bool || opt_type == gov_number) && *op == '.')
-                   || (opt_type == gov_string && *op != '.'))
-           {
-               semsg(_(e_wrong_variable_type_for_str_equal), op);
-               failed = TRUE;  // don't set the value
-
-           }
-           else
+           switch (*op)
            {
-               // number, in legacy script also bool
-               if (opt_type == gov_number
-                            || (opt_type == gov_bool && !in_vim9script()))
-               {
-                   switch (*op)
-                   {
-                       case '+': n = numval + n; break;
-                       case '-': n = numval - n; break;
-                       case '*': n = numval * n; break;
-                       case '/': n = (long)num_divide(numval, n,
-                                                              &failed); break;
-                       case '%': n = (long)num_modulus(numval, n,
-                                                              &failed); break;
-                   }
-                   s = NULL;
-               }
-               else if (opt_type == gov_string
-                                        && stringval != NULL && s != NULL)
-               {
-                   // string
-                   s = concat_str(stringval, s);
-                   vim_free(stringval);
-                   stringval = s;
-               }
+               case '+': n = numval + n; break;
+               case '-': n = numval - n; break;
+               case '*': n = numval * n; break;
+               case '/': n = (long)num_divide(numval, n, &failed); break;
+               case '%': n = (long)num_modulus(numval, n, &failed); break;
            }
+           s = NULL;
+           if (failed)
+               goto theend;
        }
-
-       if (!failed)
+       else if (opt_type == gov_string && stringval != NULL && s != NULL)
        {
-           if (opt_type != gov_string || s != NULL)
-           {
-               char *err = set_option_value(arg, n, s, scope);
-
-               arg_end = p;
-               if (err != NULL)
-                   emsg(_(err));
-           }
-           else
-               emsg(_(e_string_required));
+           // string
+           s = concat_str(stringval, s);
+           vim_free(stringval);
+           stringval = s;
        }
-       *p = c1;
-       vim_free(stringval);
-       vim_free(tofree);
     }
+
+    char *err = set_option_value(arg, n, s, scope);
+    arg_end = p;
+    if (err != NULL)
+       emsg(_(err));
+
+theend:
+    *p = c1;
+    vim_free(stringval);
+    vim_free(tofree);
     return arg_end;
 }
 
@@ -4303,9 +4311,17 @@ set_option_from_tv(char_u *varname, typval_T *varp)
     char_u     nbuf[NUMBUFLEN];
     int                error = FALSE;
 
+    int                opt_idx = findoption(varname);
+    if (opt_idx < 0)
+    {
+       semsg(_(e_unknown_option_str_2), varname);
+       return;
+    }
+    int                opt_p_flags = get_option_flags(opt_idx);
+
     if (varp->v_type == VAR_BOOL)
     {
-       if (is_string_option(varname))
+       if (opt_p_flags & P_STRING)
        {
            emsg(_(e_string_required));
            return;
@@ -4315,9 +4331,11 @@ set_option_from_tv(char_u *varname, typval_T *varp)
     }
     else
     {
-       if (!in_vim9script() || varp->v_type != VAR_STRING)
+       if ((opt_p_flags & (P_NUM|P_BOOL))
+               && (!in_vim9script() || varp->v_type != VAR_STRING))
            numval = (long)tv_get_number_chk(varp, &error);
-       strval = tv_get_string_buf_chk(varp, nbuf);
+       if (!error)
+           strval = tv_get_string_buf_chk(varp, nbuf);
     }
     if (!error && strval != NULL)
        set_option_value_give_err(varname, numval, strval, OPT_LOCAL);

diff --git a/src/option.c b/src/option.c
index c10687f6fd19a0242711fedf496bd1bde0b6f920..e9c6a50addd0ebfd3849b99749c789207d9b321e 100644 (file)
--- a/src/option.c
+++ b/src/option.c
@@ -5462,20 +5462,6 @@ is_option_allocated(char *name)
 }
 #endif
 
-#if defined(FEAT_EVAL) || defined(PROTO)
-/*
- * Return TRUE if "name" is a string option.
- * Returns FALSE if option "name" does not exist.
- */
-    int
-is_string_option(char_u *name)
-{
-    int idx = findoption(name);
-
-    return idx >= 0 && (options[idx].flags & P_STRING);
-}
-#endif
-
 /*
  * Translate a string like "t_xx", "<t_xx>" or "<S-Tab>" to a key number.
  * When "has_lt" is true there is a '<' before "*arg_arg".

diff --git a/src/proto/option.pro b/src/proto/option.pro
index 7ec531032bbbca918ad39441e7e1c471204e7441..871efc3b51bf09a507851098412196807b198030 100644 (file)
--- a/src/proto/option.pro
+++ b/src/proto/option.pro
@@ -105,7 +105,6 @@ char_u *get_term_code(char_u *tname);
 char_u *get_highlight_default(void);
 char_u *get_encoding_default(void);
 int is_option_allocated(char *name);
-int is_string_option(char_u *name);
 int makeset(FILE *fd, int opt_flags, int local_only);
 int makefoldset(FILE *fd);
 void clear_termoptions(void);

diff --git a/src/testdir/test_let.vim b/src/testdir/test_let.vim
index 680572f76be358048cf4aa0a3486280e09a9be8d..c99207c127ce1f6faa947511b5de01b19613603c 100644 (file)
--- a/src/testdir/test_let.vim
+++ b/src/testdir/test_let.vim
@@ -266,15 +266,35 @@ endfunc
 func Test_let_option_error()
   let _w = &tw
   let &tw = 80
-  call assert_fails('let &tw .= 1', 'E734:')
+  call assert_fails('let &tw .= 1', ['E734:', 'E734:'])
+  call assert_fails('let &tw .= []', ['E734:', 'E734:'])
+  call assert_fails('let &tw = []', ['E745:', 'E745:'])
+  call assert_fails('let &tw += []', ['E745:', 'E745:'])
   call assert_equal(80, &tw)
   let &tw = _w
 
+  let _w = &autoread
+  let &autoread = 1
+  call assert_fails('let &autoread .= 1', ['E734:', 'E734:'])
+  call assert_fails('let &autoread .= []', ['E734:', 'E734:'])
+  call assert_fails('let &autoread = []', ['E745:', 'E745:'])
+  call assert_fails('let &autoread += []', ['E745:', 'E745:'])
+  call assert_equal(1, &autoread)
+  let &autoread = _w
+
   let _w = &fillchars
   let &fillchars = "vert:|"
-  call assert_fails('let &fillchars += "diff:-"', 'E734:')
+  call assert_fails('let &fillchars += "diff:-"', ['E734:', 'E734:'])
+  call assert_fails('let &fillchars += []', ['E734:', 'E734:'])
+  call assert_fails('let &fillchars = []', ['E730:', 'E730:'])
+  call assert_fails('let &fillchars .= []', ['E730:', 'E730:'])
   call assert_equal("vert:|", &fillchars)
   let &fillchars = _w
+
+  call assert_fails('let &nosuchoption = 1', ['E355:', 'E355:'])
+  call assert_fails('let &nosuchoption = ""', ['E355:', 'E355:'])
+  call assert_fails('let &nosuchoption = []', ['E355:', 'E355:'])
+  call assert_fails('let &t_xx = []', ['E730:', 'E730:'])
 endfunc
 
 " Errors with the :let statement

diff --git a/src/testdir/test_options.vim b/src/testdir/test_options.vim
index d33389ac8db365909da2482381c822bbe873cfa3..015ecc122ddc228ce229169892b08b83f20614bf 100644 (file)
--- a/src/testdir/test_options.vim
+++ b/src/testdir/test_options.vim
@@ -376,7 +376,7 @@ func Test_set_completion()
   call assert_equal('"set filetype=' .. getcompletion('a*', 'filetype')->join(), @:)
 endfunc
 
-func Test_set_errors()
+func Test_set_option_errors()
   call assert_fails('set scroll=-1', 'E49:')
   call assert_fails('set backupcopy=', 'E474:')
   call assert_fails('set regexpengine=3', 'E474:')
@@ -478,7 +478,7 @@ func Test_set_errors()
   if has('python') || has('python3')
     call assert_fails('set pyxversion=6', 'E474:')
   endif
-  call assert_fails("let &tabstop='ab'", 'E521:')
+  call assert_fails("let &tabstop='ab'", ['E521:', 'E521:'])
   call assert_fails('set spellcapcheck=%\\(', 'E54:')
   call assert_fails('set sessionoptions=curdir,sesdir', 'E474:')
   call assert_fails('set foldmarker={{{,', 'E474:')
@@ -502,6 +502,12 @@ func Test_set_errors()
   call assert_fails('set t_#-&', 'E522:')
   call assert_fails('let &formatoptions = "?"', 'E539:')
   call assert_fails('call setbufvar("", "&formatoptions", "?")', 'E539:')
+  call assert_fails('call setwinvar(0, "&scrolloff", [])', ['E745:', 'E745:'])
+  call assert_fails('call setwinvar(0, "&list", [])', ['E745:', 'E745:'])
+  call assert_fails('call setwinvar(0, "&listchars", [])', ['E730:', 'E730:'])
+  call assert_fails('call setwinvar(0, "&nosuchoption", 0)', ['E355:', 'E355:'])
+  call assert_fails('call setwinvar(0, "&nosuchoption", "")', ['E355:', 'E355:'])
+  call assert_fails('call setwinvar(0, "&nosuchoption", [])', ['E355:', 'E355:'])
 endfunc
 
 func Test_set_encoding()

diff --git a/src/testdir/test_vim9_assign.vim b/src/testdir/test_vim9_assign.vim
index 51a6c6f8d6d3b30c0fd20f33b695df011add0956..ce728230b38e91723adf412b1586a63ae2965818 100644 (file)
--- a/src/testdir/test_vim9_assign.vim
+++ b/src/testdir/test_vim9_assign.vim
@@ -145,6 +145,12 @@ def Test_assignment()
   &ts %= 4
   assert_equal(2, &ts)
 
+  assert_fails('&ts /= 0', ['E1154:', 'E1154:'])
+  assert_fails('&ts %= 0', ['E1154:', 'E1154:'])
+  assert_fails('&ts /= []', ['E745:', 'E745:'])
+  assert_fails('&ts %= []', ['E745:', 'E745:'])
+  assert_equal(2, &ts)
+
   var f100: float = 100.0
   f100 /= 5
   assert_equal(20.0, f100)

diff --git a/src/testdir/test_vim9_builtin.vim b/src/testdir/test_vim9_builtin.vim
index a226bb2cab6643adb69a703a0da751f4778feda4..b76db7d6358fe255d8063d60f9858f6f93403ecc 100644 (file)
--- a/src/testdir/test_vim9_builtin.vim
+++ b/src/testdir/test_vim9_builtin.vim
@@ -3944,7 +3944,7 @@ def Test_setwinvar()
   v9.CheckDefAndScriptFailure(['setwinvar("a", "b", 1)'], ['E1013: Argument 1: type mismatch, expected number but got string', 'E1210: Number required for argument 1'])
   v9.CheckDefAndScriptFailure(['setwinvar(1, 2, "c")'], ['E1013: Argument 2: type mismatch, expected string but got number', 'E1174: String required for argument 2'])
   assert_fails('setwinvar(1, "", 10)', 'E461: Illegal variable name')
-  assert_fails('setwinvar(0, "&rulerformat", true)', 'E928:')
+  assert_fails('setwinvar(0, "&rulerformat", true)', ['E928:', 'E928:'])
 enddef
 
 def Test_sha256()

diff --git a/src/testdir/test_vimscript.vim b/src/testdir/test_vimscript.vim
index 2af0800bde603ff13b48603035a55a6650a57e37..002d1153e48cd0398e8023446b5ce3513825d8b8 100644 (file)
--- a/src/testdir/test_vimscript.vim
+++ b/src/testdir/test_vimscript.vim
@@ -7077,7 +7077,7 @@ func Test_compound_assignment_operators()
     call assert_equal(6, &scrolljump)
     let &scrolljump %= 5
     call assert_equal(1, &scrolljump)
-    call assert_fails('let &scrolljump .= "j"', 'E734:')
+    call assert_fails('let &scrolljump .= "j"', ['E734:', 'E734:'])
     set scrolljump&vim
 
     let &foldlevelstart = 2

diff --git a/src/version.c b/src/version.c
index 51e2e6ec529fd6a44277122dca1a3c4ef048358f..71864d9ad894a89a6bcdb44f774823810b4f8d11 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -695,6 +695,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1631,
 /**/
     1630,
 /**/