Skip to content

Commit 4c7cb37

Browse files
zeertzjqbrammool
zeertzjq
authored and
brammool
committed
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 vim#12504)
1 parent 8d687a7 commit 4c7cb37

9 files changed

+151
-114
lines changed

‎src/evalvars.c

+111-93
Original file line numberDiff line numberDiff line change
@@ -1639,108 +1639,116 @@ ex_let_option(
16391639
p = find_option_end(&arg, &scope);
16401640
if (p == NULL || (endchars != NULL
16411641
&& vim_strchr(endchars, *skipwhite(p)) == NULL))
1642+
{
16421643
emsg(_(e_unexpected_characters_in_let));
1643-
else
1644+
return NULL;
1645+
}
1646+
1647+
int c1;
1648+
long n = 0;
1649+
getoption_T opt_type;
1650+
long numval;
1651+
char_u *stringval = NULL;
1652+
char_u *s = NULL;
1653+
int failed = FALSE;
1654+
int opt_p_flags;
1655+
char_u *tofree = NULL;
1656+
char_u numbuf[NUMBUFLEN];
1657+
1658+
c1 = *p;
1659+
*p = NUL;
1660+
1661+
opt_type = get_option_value(arg, &numval, &stringval, &opt_p_flags, scope);
1662+
if (opt_type == gov_unknown && arg[0] != 't' && arg[1] != '_')
16441663
{
1645-
int c1;
1646-
long n = 0;
1647-
getoption_T opt_type;
1648-
long numval;
1649-
char_u *stringval = NULL;
1650-
char_u *s = NULL;
1651-
int failed = FALSE;
1652-
int opt_p_flags;
1653-
char_u *tofree = NULL;
1654-
char_u numbuf[NUMBUFLEN];
1655-
1656-
c1 = *p;
1657-
*p = NUL;
1664+
semsg(_(e_unknown_option_str_2), arg);
1665+
goto theend;
1666+
}
1667+
if (op != NULL && *op != '='
1668+
&& (((opt_type == gov_bool || opt_type == gov_number) && *op == '.')
1669+
|| (opt_type == gov_string && *op != '.')))
1670+
{
1671+
semsg(_(e_wrong_variable_type_for_str_equal), op);
1672+
goto theend;
1673+
}
16581674

1659-
opt_type = get_option_value(arg, &numval, &stringval, &opt_p_flags,
1660-
scope);
1661-
if ((opt_type == gov_bool
1662-
|| opt_type == gov_number
1663-
|| opt_type == gov_hidden_bool
1664-
|| opt_type == gov_hidden_number)
1665-
&& (tv->v_type != VAR_STRING || !in_vim9script()))
1666-
{
1667-
if (opt_type == gov_bool || opt_type == gov_hidden_bool)
1668-
// bool, possibly hidden
1669-
n = (long)tv_get_bool(tv);
1670-
else
1671-
// number, possibly hidden
1672-
n = (long)tv_get_number(tv);
1673-
}
1675+
if ((opt_type == gov_bool
1676+
|| opt_type == gov_number
1677+
|| opt_type == gov_hidden_bool
1678+
|| opt_type == gov_hidden_number)
1679+
&& (tv->v_type != VAR_STRING || !in_vim9script()))
1680+
{
1681+
if (opt_type == gov_bool || opt_type == gov_hidden_bool)
1682+
// bool, possibly hidden
1683+
n = (long)tv_get_bool_chk(tv, &failed);
1684+
else
1685+
// number, possibly hidden
1686+
n = (long)tv_get_number_chk(tv, &failed);
1687+
if (failed)
1688+
goto theend;
1689+
}
16741690

1675-
if ((opt_p_flags & P_FUNC) && (tv->v_type == VAR_PARTIAL
1676-
|| tv->v_type == VAR_FUNC))
1677-
{
1678-
// If the option can be set to a function reference or a lambda
1679-
// and the passed value is a function reference, then convert it to
1680-
// the name (string) of the function reference.
1681-
s = tv2string(tv, &tofree, numbuf, 0);
1682-
}
1683-
// Avoid setting a string option to the text "v:false" or similar.
1684-
// In Vim9 script also don't convert a number to string.
1685-
else if (tv->v_type != VAR_BOOL && tv->v_type != VAR_SPECIAL
1686-
&& (!in_vim9script() || tv->v_type != VAR_NUMBER))
1687-
s = tv_get_string_chk(tv);
1691+
if ((opt_p_flags & P_FUNC) && (tv->v_type == VAR_PARTIAL
1692+
|| tv->v_type == VAR_FUNC))
1693+
{
1694+
// If the option can be set to a function reference or a lambda
1695+
// and the passed value is a function reference, then convert it to
1696+
// the name (string) of the function reference.
1697+
s = tv2string(tv, &tofree, numbuf, 0);
1698+
if (s == NULL)
1699+
goto theend;
1700+
}
1701+
// Avoid setting a string option to the text "v:false" or similar.
1702+
// In Vim9 script also don't convert a number to string.
1703+
else if (tv->v_type != VAR_BOOL && tv->v_type != VAR_SPECIAL
1704+
&& (!in_vim9script() || tv->v_type != VAR_NUMBER))
1705+
{
1706+
s = tv_get_string_chk(tv);
1707+
if (s == NULL)
1708+
goto theend;
1709+
}
1710+
else if (opt_type == gov_string || opt_type == gov_hidden_string)
1711+
{
1712+
emsg(_(e_string_required));
1713+
goto theend;
1714+
}
16881715

1689-
if (op != NULL && *op != '=')
1716+
if (op != NULL && *op != '=')
1717+
{
1718+
// number, in legacy script also bool
1719+
if (opt_type == gov_number
1720+
|| (opt_type == gov_bool && !in_vim9script()))
16901721
{
1691-
if (((opt_type == gov_bool || opt_type == gov_number) && *op == '.')
1692-
|| (opt_type == gov_string && *op != '.'))
1693-
{
1694-
semsg(_(e_wrong_variable_type_for_str_equal), op);
1695-
failed = TRUE; // don't set the value
1696-
1697-
}
1698-
else
1722+
switch (*op)
16991723
{
1700-
// number, in legacy script also bool
1701-
if (opt_type == gov_number
1702-
|| (opt_type == gov_bool && !in_vim9script()))
1703-
{
1704-
switch (*op)
1705-
{
1706-
case '+': n = numval + n; break;
1707-
case '-': n = numval - n; break;
1708-
case '*': n = numval * n; break;
1709-
case '/': n = (long)num_divide(numval, n,
1710-
&failed); break;
1711-
case '%': n = (long)num_modulus(numval, n,
1712-
&failed); break;
1713-
}
1714-
s = NULL;
1715-
}
1716-
else if (opt_type == gov_string
1717-
&& stringval != NULL && s != NULL)
1718-
{
1719-
// string
1720-
s = concat_str(stringval, s);
1721-
vim_free(stringval);
1722-
stringval = s;
1723-
}
1724+
case '+': n = numval + n; break;
1725+
case '-': n = numval - n; break;
1726+
case '*': n = numval * n; break;
1727+
case '/': n = (long)num_divide(numval, n, &failed); break;
1728+
case '%': n = (long)num_modulus(numval, n, &failed); break;
17241729
}
1730+
s = NULL;
1731+
if (failed)
1732+
goto theend;
17251733
}
1726-
1727-
if (!failed)
1734+
else if (opt_type == gov_string && stringval != NULL && s != NULL)
17281735
{
1729-
if (opt_type != gov_string || s != NULL)
1730-
{
1731-
char *err = set_option_value(arg, n, s, scope);
1732-
1733-
arg_end = p;
1734-
if (err != NULL)
1735-
emsg(_(err));
1736-
}
1737-
else
1738-
emsg(_(e_string_required));
1736+
// string
1737+
s = concat_str(stringval, s);
1738+
vim_free(stringval);
1739+
stringval = s;
17391740
}
1740-
*p = c1;
1741-
vim_free(stringval);
1742-
vim_free(tofree);
17431741
}
1742+
1743+
char *err = set_option_value(arg, n, s, scope);
1744+
arg_end = p;
1745+
if (err != NULL)
1746+
emsg(_(err));
1747+
1748+
theend:
1749+
*p = c1;
1750+
vim_free(stringval);
1751+
vim_free(tofree);
17441752
return arg_end;
17451753
}
17461754

@@ -4303,9 +4311,17 @@ set_option_from_tv(char_u *varname, typval_T *varp)
43034311
char_u nbuf[NUMBUFLEN];
43044312
int error = FALSE;
43054313

4314+
int opt_idx = findoption(varname);
4315+
if (opt_idx < 0)
4316+
{
4317+
semsg(_(e_unknown_option_str_2), varname);
4318+
return;
4319+
}
4320+
int opt_p_flags = get_option_flags(opt_idx);
4321+
43064322
if (varp->v_type == VAR_BOOL)
43074323
{
4308-
if (is_string_option(varname))
4324+
if (opt_p_flags & P_STRING)
43094325
{
43104326
emsg(_(e_string_required));
43114327
return;
@@ -4315,9 +4331,11 @@ set_option_from_tv(char_u *varname, typval_T *varp)
43154331
}
43164332
else
43174333
{
4318-
if (!in_vim9script() || varp->v_type != VAR_STRING)
4334+
if ((opt_p_flags & (P_NUM|P_BOOL))
4335+
&& (!in_vim9script() || varp->v_type != VAR_STRING))
43194336
numval = (long)tv_get_number_chk(varp, &error);
4320-
strval = tv_get_string_buf_chk(varp, nbuf);
4337+
if (!error)
4338+
strval = tv_get_string_buf_chk(varp, nbuf);
43214339
}
43224340
if (!error && strval != NULL)
43234341
set_option_value_give_err(varname, numval, strval, OPT_LOCAL);

‎src/option.c

-14
Original file line numberDiff line numberDiff line change
@@ -5462,20 +5462,6 @@ is_option_allocated(char *name)
54625462
}
54635463
#endif
54645464

5465-
#if defined(FEAT_EVAL) || defined(PROTO)
5466-
/*
5467-
* Return TRUE if "name" is a string option.
5468-
* Returns FALSE if option "name" does not exist.
5469-
*/
5470-
int
5471-
is_string_option(char_u *name)
5472-
{
5473-
int idx = findoption(name);
5474-
5475-
return idx >= 0 && (options[idx].flags & P_STRING);
5476-
}
5477-
#endif
5478-
54795465
/*
54805466
* Translate a string like "t_xx", "<t_xx>" or "<S-Tab>" to a key number.
54815467
* When "has_lt" is true there is a '<' before "*arg_arg".

‎src/proto/option.pro

-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ char_u *get_term_code(char_u *tname);
105105
char_u *get_highlight_default(void);
106106
char_u *get_encoding_default(void);
107107
int is_option_allocated(char *name);
108-
int is_string_option(char_u *name);
109108
int makeset(FILE *fd, int opt_flags, int local_only);
110109
int makefoldset(FILE *fd);
111110
void clear_termoptions(void);

‎src/testdir/test_let.vim

+22-2
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,35 @@ endfunc
266266
func Test_let_option_error()
267267
let _w = &tw
268268
let &tw = 80
269-
call assert_fails('let &tw .= 1', 'E734:')
269+
call assert_fails('let &tw .= 1', ['E734:', 'E734:'])
270+
call assert_fails('let &tw .= []', ['E734:', 'E734:'])
271+
call assert_fails('let &tw = []', ['E745:', 'E745:'])
272+
call assert_fails('let &tw += []', ['E745:', 'E745:'])
270273
call assert_equal(80, &tw)
271274
let &tw = _w
272275

276+
let _w = &autoread
277+
let &autoread = 1
278+
call assert_fails('let &autoread .= 1', ['E734:', 'E734:'])
279+
call assert_fails('let &autoread .= []', ['E734:', 'E734:'])
280+
call assert_fails('let &autoread = []', ['E745:', 'E745:'])
281+
call assert_fails('let &autoread += []', ['E745:', 'E745:'])
282+
call assert_equal(1, &autoread)
283+
let &autoread = _w
284+
273285
let _w = &fillchars
274286
let &fillchars = "vert:|"
275-
call assert_fails('let &fillchars += "diff:-"', 'E734:')
287+
call assert_fails('let &fillchars += "diff:-"', ['E734:', 'E734:'])
288+
call assert_fails('let &fillchars += []', ['E734:', 'E734:'])
289+
call assert_fails('let &fillchars = []', ['E730:', 'E730:'])
290+
call assert_fails('let &fillchars .= []', ['E730:', 'E730:'])
276291
call assert_equal("vert:|", &fillchars)
277292
let &fillchars = _w
293+
294+
call assert_fails('let &nosuchoption = 1', ['E355:', 'E355:'])
295+
call assert_fails('let &nosuchoption = ""', ['E355:', 'E355:'])
296+
call assert_fails('let &nosuchoption = []', ['E355:', 'E355:'])
297+
call assert_fails('let &t_xx = []', ['E730:', 'E730:'])
278298
endfunc
279299

280300
" Errors with the :let statement

‎src/testdir/test_options.vim

+8-2
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func Test_set_completion()
376376
call assert_equal('"set filetype=' .. getcompletion('a*', 'filetype')->join(), @:)
377377
endfunc
378378

379-
func Test_set_errors()
379+
func Test_set_option_errors()
380380
call assert_fails('set scroll=-1', 'E49:')
381381
call assert_fails('set backupcopy=', 'E474:')
382382
call assert_fails('set regexpengine=3', 'E474:')
@@ -478,7 +478,7 @@ func Test_set_errors()
478478
if has('python') || has('python3')
479479
call assert_fails('set pyxversion=6', 'E474:')
480480
endif
481-
call assert_fails("let &tabstop='ab'", 'E521:')
481+
call assert_fails("let &tabstop='ab'", ['E521:', 'E521:'])
482482
call assert_fails('set spellcapcheck=%\\(', 'E54:')
483483
call assert_fails('set sessionoptions=curdir,sesdir', 'E474:')
484484
call assert_fails('set foldmarker={{{,', 'E474:')
@@ -502,6 +502,12 @@ func Test_set_errors()
502502
call assert_fails('set t_#-&', 'E522:')
503503
call assert_fails('let &formatoptions = "?"', 'E539:')
504504
call assert_fails('call setbufvar("", "&formatoptions", "?")', 'E539:')
505+
call assert_fails('call setwinvar(0, "&scrolloff", [])', ['E745:', 'E745:'])
506+
call assert_fails('call setwinvar(0, "&list", [])', ['E745:', 'E745:'])
507+
call assert_fails('call setwinvar(0, "&listchars", [])', ['E730:', 'E730:'])
508+
call assert_fails('call setwinvar(0, "&nosuchoption", 0)', ['E355:', 'E355:'])
509+
call assert_fails('call setwinvar(0, "&nosuchoption", "")', ['E355:', 'E355:'])
510+
call assert_fails('call setwinvar(0, "&nosuchoption", [])', ['E355:', 'E355:'])
505511
endfunc
506512

507513
func Test_set_encoding()

‎src/testdir/test_vim9_assign.vim

+6
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,12 @@ def Test_assignment()
145145
&ts %= 4
146146
assert_equal(2, &ts)
147147

148+
assert_fails('&ts /= 0', ['E1154:', 'E1154:'])
149+
assert_fails('&ts %= 0', ['E1154:', 'E1154:'])
150+
assert_fails('&ts /= []', ['E745:', 'E745:'])
151+
assert_fails('&ts %= []', ['E745:', 'E745:'])
152+
assert_equal(2, &ts)
153+
148154
var f100: float = 100.0
149155
f100 /= 5
150156
assert_equal(20.0, f100)

‎src/testdir/test_vim9_builtin.vim

+1-1
Original file line numberDiff line numberDiff line change
@@ -3944,7 +3944,7 @@ def Test_setwinvar()
39443944
v9.CheckDefAndScriptFailure(['setwinvar("a", "b", 1)'], ['E1013: Argument 1: type mismatch, expected number but got string', 'E1210: Number required for argument 1'])
39453945
v9.CheckDefAndScriptFailure(['setwinvar(1, 2, "c")'], ['E1013: Argument 2: type mismatch, expected string but got number', 'E1174: String required for argument 2'])
39463946
assert_fails('setwinvar(1, "", 10)', 'E461: Illegal variable name')
3947-
assert_fails('setwinvar(0, "&rulerformat", true)', 'E928:')
3947+
assert_fails('setwinvar(0, "&rulerformat", true)', ['E928:', 'E928:'])
39483948
enddef
39493949

39503950
def Test_sha256()

‎src/testdir/test_vimscript.vim

+1-1
Original file line numberDiff line numberDiff line change
@@ -7077,7 +7077,7 @@ func Test_compound_assignment_operators()
70777077
call assert_equal(6, &scrolljump)
70787078
let &scrolljump %= 5
70797079
call assert_equal(1, &scrolljump)
7080-
call assert_fails('let &scrolljump .= "j"', 'E734:')
7080+
call assert_fails('let &scrolljump .= "j"', ['E734:', 'E734:'])
70817081
set scrolljump&vim
70827082

70837083
let &foldlevelstart = 2

‎src/version.c

+2
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,8 @@ static char *(features[]) =
695695

696696
static int included_patches[] =
697697
{ /* Add new patch number below this line */
698+
/**/
699+
1631,
698700
/**/
699701
1630,
700702
/**/

0 commit comments

Comments
 (0)