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)

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
     {
-	int	    c1;
+	emsg(_(e_unexpected_characters_in_let));
-	long	    n = 0;
+	return NULL;
-	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_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_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 (op != NULL && *op != '=')
-	{
-	    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
-	    {
-		// 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;
-		}
-	    }
-	}
-
-	if (!failed)
-	{
-	    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));
-	}
-	*p = c1;
-	vim_free(stringval);
-	vim_free(tofree);
     }
+
+    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] != '_')
+    {
+	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;
+    }
+
+    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);
+	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 != '=')
+    {
+	// 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;
+	    if (failed)
+		goto theend;
+	}
+	else if (opt_type == gov_string && stringval != NULL && s != NULL)
+	{
+	    // string
+	    s = concat_str(stringval, s);
+	    vim_free(stringval);
+	    stringval = s;
+	}
+    }
+
+    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);

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".

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);

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

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()

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)

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()

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

src/version.c

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