]> granicus.if.org Git - vim/commitdiff
patch 8.2.1798: Vim9: trinary operator condition is too permissive v8.2.1798
authorBram Moolenaar <Bram@vim.org>
Sun, 4 Oct 2020 14:06:05 +0000 (16:06 +0200)
committerBram Moolenaar <Bram@vim.org>
Sun, 4 Oct 2020 14:06:05 +0000 (16:06 +0200)
Problem:    Vim9: trinary operator condition is too permissive.
Solution:   Use tv_get_bool_chk().

runtime/doc/vim9.txt
src/eval.c
src/testdir/test_expr.vim
src/testdir/test_vim9_cmd.vim
src/testdir/test_vim9_expr.vim
src/testdir/test_vim9_script.vim
src/testdir/vim9.vim
src/version.c
src/vim9compile.c
src/vim9execute.c

index 0c20baeb8911f097068b4d419fa6ebd61766ebce..feb889dad1305512ecf3dc7222552aefe6ef5d23 100644 (file)
@@ -164,15 +164,20 @@ the "name#" prefix is sufficient. >
 
 When using `:function` or `:def` to specify a nested function inside a `:def`
 function, this nested function is local to the code block it is defined in.
-In a `:def` function IT is not possible to define a script-local function.  it
+In a `:def` function it is not possible to define a script-local function.  it
 is possible to define a global function by using the "g:" prefix.
 
 When referring to a function and no "s:" or "g:" prefix is used, Vim will
-prefer using a local function (in the function scope, script scope or
-imported) before looking for a global function.  However, it is recommended to
-always use "g:" to refer to a local function for clarity.  In all cases the
-function must be defined before used.  That is when it is first called or when
-`:defcompile` causes the call to be compiled.
+search for the function:
+- in the function scope
+- in the script scope, possibly imported
+- in the list of global functions
+However, it is recommended to always use "g:" to refer to a global function
+for clarity.
+
+In all cases the function must be defined before used.  That is when it is
+called, when `:defcompile` causes the it to be compiled, or when code that
+calls it is being compiled (to figure out the return type).
 
 The result is that functions and variables without a namespace can usually be
 found in the script, either defined there or imported.  Global functions and
@@ -467,11 +472,21 @@ White space is not allowed:
 
 Conditions and expressions ~
 
-Conditions and expressions are mostly working like they do in JavaScript.  A
-difference is made where JavaScript does not work like most people expect.
-Specifically, an empty list is falsy.
-
-       type            TRUE when ~
+Conditions and expressions are mostly working like they do in other languages.
+Some values are different from legacy Vim script:
+       value           legacy Vim script       Vim9 script ~
+       0               falsy                   falsy
+       1               truthy                  truthy
+       99              truthy                  Error!
+       "0"             falsy                   Error!
+       "99"            truthy                  Error!
+       "text"          falsy                   Error!
+
+For the "??" operator and when using "!" then there is no error, every value
+is either falsy or truthy.  This is mostly like JavaScript, except that an
+empty list and dict is falsy:
+
+       type            truthy when ~
        bool            v:true or 1
        number          non-zero
        float           non-zero
@@ -498,13 +513,13 @@ one: >
        [] || 99     Error!
 
 When using "!" for inverting, there is no error for using any type and the
-result is a boolean: >
+result is a boolean.  "!!" can be used to turn any value into boolean: >
        !'yes'                  == false
-       var myList = [1, 2, 3]
-       !!myList                == true
+       !![]                    == false
+       !![1, 2, 3]             == true
 
 When using "`.."` for string concatenation arguments of simple types are
-always converted to string. >
+always converted to string: >
        'hello ' .. 123  == 'hello 123'
        'hello ' .. v:true  == 'hello v:true'
 
index a678d32e6471345c18f41e158f38d7bdd1fefce3..285558df8fbfac728966606c625f0d28dc232eed 100644 (file)
@@ -191,7 +191,7 @@ eval_to_bool(
        if (!skip)
        {
            if (in_vim9script())
-               retval = tv2bool(&tv);
+               retval = tv_get_bool_chk(&tv, error);
            else
                retval = (tv_get_number_chk(&tv, error) != 0);
            clear_tv(&tv);
@@ -2143,6 +2143,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg)
        evalarg_T       local_evalarg;
        int             orig_flags;
        int             evaluate;
+       int             vim9script = in_vim9script();
 
        if (evalarg == NULL)
        {
@@ -2156,7 +2157,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg)
            *arg = eval_next_line(evalarg_used);
        else
        {
-           if (evaluate && in_vim9script() && !VIM_ISWHITE(p[-1]))
+           if (evaluate && vim9script && !VIM_ISWHITE(p[-1]))
            {
                error_white_both(p, 1);
                clear_tv(rettv);
@@ -2170,8 +2171,10 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg)
        {
            int         error = FALSE;
 
-           if (in_vim9script() || op_falsy)
+           if (op_falsy)
                result = tv2bool(rettv);
+           else if (vim9script)
+               result = tv_get_bool_chk(rettv, &error);
            else if (tv_get_number_chk(rettv, &error) != 0)
                result = TRUE;
            if (error || !op_falsy || !result)
@@ -2185,7 +2188,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg)
         */
        if (op_falsy)
            ++*arg;
-       if (evaluate && in_vim9script() && !IS_WHITE_OR_NUL((*arg)[1]))
+       if (evaluate && vim9script && !IS_WHITE_OR_NUL((*arg)[1]))
        {
            error_white_both(p, 1);
            clear_tv(rettv);
@@ -2220,7 +2223,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg)
                *arg = eval_next_line(evalarg_used);
            else
            {
-               if (evaluate && in_vim9script() && !VIM_ISWHITE(p[-1]))
+               if (evaluate && vim9script && !VIM_ISWHITE(p[-1]))
                {
                    error_white_both(p, 1);
                    clear_tv(rettv);
@@ -2233,7 +2236,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg)
            /*
             * Get the third variable.  Recursive!
             */
-           if (evaluate && in_vim9script() && !IS_WHITE_OR_NUL((*arg)[1]))
+           if (evaluate && vim9script && !IS_WHITE_OR_NUL((*arg)[1]))
            {
                error_white_both(p, 1);
                clear_tv(rettv);
index 1086534dcae0803ac972737f859eba0edce3bb6a..754f856c2271059557ec8b303f236edcd110d2a7 100644 (file)
@@ -42,6 +42,16 @@ func Test_version()
   call assert_false(has('patch-9.9.1'))
 endfunc
 
+func Test_op_trinary()
+  call assert_equal('yes', 1 ? 'yes' : 'no')
+  call assert_equal('no', 0 ? 'yes' : 'no')
+  call assert_equal('no', 'x' ? 'yes' : 'no')
+  call assert_equal('yes', '1x' ? 'yes' : 'no')
+
+  call assert_fails('echo [1] ? "yes" : "no"', 'E745:')
+  call assert_fails('echo {} ? "yes" : "no"', 'E728:')
+endfunc
+
 func Test_op_falsy()
   call assert_equal(v:true, v:true ?? 456)
   call assert_equal(123, 123 ?? 456)
index 82d5f325e47c8a979b214528f4f0947bc217bbc1..599c287e238e39aaa31ba1a413cc632d0ecdac9c 100644 (file)
@@ -68,6 +68,74 @@ def Test_echo_linebreak()
   CheckScriptSuccess(lines)
 enddef
 
+def Test_condition_types()
+  var lines =<< trim END
+      if 'text'
+      endif
+  END
+  CheckDefAndScriptFailure(lines, 'E1030:', 1)
+
+  lines =<< trim END
+      if [1]
+      endif
+  END
+  CheckDefFailure(lines, 'E1012:', 1)
+  CheckScriptFailure(['vim9script'] + lines, 'E745:', 2)
+
+  lines =<< trim END
+      g:cond = 'text'
+      if g:cond
+      endif
+  END
+  CheckDefExecAndScriptFailure(lines, 'E1030:', 2)
+
+  lines =<< trim END
+      g:cond = 0
+      if g:cond
+      elseif 'text'
+      endif
+  END
+  CheckDefFailure(lines, 'E1012:', 3)
+  CheckScriptFailure(['vim9script'] + lines, 'E1030:', 4)
+
+  lines =<< trim END
+      if g:cond
+      elseif [1]
+      endif
+  END
+  CheckDefFailure(lines, 'E1012:', 2)
+  CheckScriptFailure(['vim9script'] + lines, 'E745:', 3)
+
+  lines =<< trim END
+      g:cond = 'text'
+      if 0
+      elseif g:cond
+      endif
+  END
+  CheckDefExecAndScriptFailure(lines, 'E1030:', 3)
+
+  lines =<< trim END
+      while 'text'
+      endwhile
+  END
+  CheckDefFailure(lines, 'E1012:', 1)
+  CheckScriptFailure(['vim9script'] + lines, 'E1030:', 2)
+
+  lines =<< trim END
+      while [1]
+      endwhile
+  END
+  CheckDefFailure(lines, 'E1012:', 1)
+  CheckScriptFailure(['vim9script'] + lines, 'E745:', 2)
+
+  lines =<< trim END
+      g:cond = 'text'
+      while g:cond
+      endwhile
+  END
+  CheckDefExecAndScriptFailure(lines, 'E1030:', 2)
+enddef
+
 def Test_if_linebreak()
   var lines =<< trim END
       vim9script
index 5158e1093fb6420616b961f61fa662c8a34cd54e..2cbb2854a69832248dd462765cc1db38cd2a5225 100644 (file)
@@ -18,27 +18,27 @@ def Test_expr1_trinary()
                        'one' :
                        'two')
   if has('float')
-    assert_equal('one', 0.1 ? 'one' : 'two')
+    assert_equal('one', !!0.1 ? 'one' : 'two')
   endif
-  assert_equal('one', 'x' ? 'one' : 'two')
-  assert_equal('one', 'x'
+  assert_equal('one', !!'x' ? 'one' : 'two')
+  assert_equal('one', !!'x'
                        ? 'one'
                        : 'two')
-  assert_equal('one', 0z1234 ? 'one' : 'two')
-  assert_equal('one', [0] ? 'one' : 'two')
-  assert_equal('one', #{x: 0} ? 'one' : 'two')
+  assert_equal('one', !!0z1234 ? 'one' : 'two')
+  assert_equal('one', !![0] ? 'one' : 'two')
+  assert_equal('one', !!#{x: 0} ? 'one' : 'two')
   var name = 1
   assert_equal('one', name ? 'one' : 'two')
 
   assert_equal('two', false ? 'one' : 'two')
   assert_equal('two', 0 ? 'one' : 'two')
   if has('float')
-    assert_equal('two', 0.0 ? 'one' : 'two')
+    assert_equal('two', !!0.0 ? 'one' : 'two')
   endif
-  assert_equal('two', '' ? 'one' : 'two')
-  assert_equal('two', 0z ? 'one' : 'two')
-  assert_equal('two', [] ? 'one' : 'two')
-  assert_equal('two', {} ? 'one' : 'two')
+  assert_equal('two', !!'' ? 'one' : 'two')
+  assert_equal('two', !!0z ? 'one' : 'two')
+  assert_equal('two', !![] ? 'one' : 'two')
+  assert_equal('two', !!{} ? 'one' : 'two')
   name = 0
   assert_equal('two', name ? 'one' : 'two')
 
@@ -117,6 +117,24 @@ def Test_expr1_trinary_vimscript()
   END
   CheckScriptFailure(lines, 'E1004:', 2)
 
+  lines =<< trim END
+      vim9script
+      var name = 'x' ? 1 : 2
+  END
+  CheckScriptFailure(lines, 'E1030:', 2)
+
+  lines =<< trim END
+      vim9script
+      var name = [] ? 1 : 2
+  END
+  CheckScriptFailure(lines, 'E745:', 2)
+
+  lines =<< trim END
+      vim9script
+      var name = {} ? 1 : 2
+  END
+  CheckScriptFailure(lines, 'E728:', 2)
+
   # check after failure eval_flags is reset
   lines =<< trim END
       vim9script
@@ -152,6 +170,15 @@ func Test_expr1_trinary_fails()
   call CheckDefFailure(["var x = 1 ? 'one' :'two'"], msg, 1)
   call CheckDefFailure(["var x = 1 ? 'one':'two'"], msg, 1)
 
+  call CheckDefFailure(["var x = 'x' ? 'one' : 'two'"], 'E1030:', 1)
+  call CheckDefFailure(["var x = 0z1234 ? 'one' : 'two'"], 'E974:', 1)
+  call CheckDefExecFailure(["var x = [] ? 'one' : 'two'"], 'E745:', 1)
+  call CheckDefExecFailure(["var x = {} ? 'one' : 'two'"], 'E728:', 1)
+
+  if has('float')
+    call CheckDefFailure(["var x = 0.1 ? 'one' : 'two'"], 'E805:', 1)
+  endif
+
   " missing argument detected even when common type is used
   call CheckDefFailure([
        \ 'var X = FuncOne',
index 6dbabe38929c1ecf3901423e56383efde8e9a348..7fc522362dbd371ca8f618a34717c9d63d07f1a1 100644 (file)
@@ -543,7 +543,7 @@ def Test_try_catch_fails()
   CheckDefFailure(['endtry'], 'E602:')
   CheckDefFailure(['while 1', 'endtry'], 'E170:')
   CheckDefFailure(['for i in range(5)', 'endtry'], 'E170:')
-  CheckDefFailure(['if 2', 'endtry'], 'E171:')
+  CheckDefFailure(['if 1', 'endtry'], 'E171:')
   CheckDefFailure(['try', 'echo 1', 'endtry'], 'E1032:')
 
   CheckDefFailure(['throw'], 'E1015:')
index 2e4b03f495d6b3a091ea1fd803ef2ca4c949507e..aeeb5a349ba0f163bde7d4e1b2a9bc515b494ace 100644 (file)
@@ -52,3 +52,10 @@ def CheckDefAndScriptFailure(lines: list<string>, error: string, lnum = -3)
   CheckDefFailure(lines, error, lnum)
   CheckScriptFailure(['vim9script'] + lines, error, lnum + 1)
 enddef
+
+" Check that a command fails both when executed in a :def function and when
+" used in Vim9 script.
+def CheckDefExecAndScriptFailure(lines: list<string>, error: string, lnum = -3)
+  CheckDefExecFailure(lines, error, lnum)
+  CheckScriptFailure(['vim9script'] + lines, error, lnum + 1)
+enddef
index 914e4f0844abf47a614735fb075183ae8271fd41..58eb3d6b60d8ab4996c12226a8de2be22ec6ef77 100644 (file)
@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1798,
 /**/
     1797,
 /**/
index f7dc9df2030941f34986fe7f6a53eaaf4b9ca806..8d362ab1d09872ec89b70606ea187f51e2899ad2 100644 (file)
@@ -4212,7 +4212,17 @@ compile_expr1(char_u **arg,  cctx_T *cctx, ppconst_T *ppconst)
            // the condition is a constant, we know whether the ? or the :
            // expression is to be evaluated.
            has_const_expr = TRUE;
-           const_value = tv2bool(&ppconst->pp_tv[ppconst_used]);
+           if (op_falsy)
+               const_value = tv2bool(&ppconst->pp_tv[ppconst_used]);
+           else
+           {
+               int error = FALSE;
+
+               const_value = tv_get_bool_chk(&ppconst->pp_tv[ppconst_used],
+                                                                      &error);
+               if (error)
+                   return FAIL;
+           }
            cctx->ctx_skip = save_skip == SKIP_YES ||
                 (op_falsy ? const_value : !const_value) ? SKIP_YES : SKIP_NOT;
 
@@ -5637,6 +5647,23 @@ drop_scope(cctx_T *cctx)
     vim_free(scope);
 }
 
+/*
+ * Check that the top of the type stack has a type that can be used as a
+ * condition.  Give an error and return FAIL if not.
+ */
+    static int
+bool_on_stack(cctx_T *cctx)
+{
+    garray_T   *stack = &cctx->ctx_type_stack;
+    type_T     *type;
+
+    type = ((type_T **)stack->ga_data)[stack->ga_len - 1];
+    if (type != &t_bool && type != &t_number && type != &t_any
+           && need_type(type, &t_bool, -1, cctx, FALSE) == FAIL)
+       return FAIL;
+    return OK;
+}
+
 /*
  * compile "if expr"
  *
@@ -5689,8 +5716,14 @@ compile_if(char_u *arg, cctx_T *cctx)
        clear_ppconst(&ppconst);
     else if (instr->ga_len == instr_count && ppconst.pp_used == 1)
     {
+       int error = FALSE;
+       int v;
+
        // The expression results in a constant.
-       cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES;
+       v = tv_get_bool_chk(&ppconst.pp_tv[0], &error);
+       if (error)
+           return NULL;
+       cctx->ctx_skip = v ? SKIP_NOT : SKIP_YES;
        clear_ppconst(&ppconst);
     }
     else
@@ -5699,6 +5732,8 @@ compile_if(char_u *arg, cctx_T *cctx)
        cctx->ctx_skip = SKIP_UNKNOWN;
        if (generate_ppconst(cctx, &ppconst) == FAIL)
            return NULL;
+       if (bool_on_stack(cctx) == FAIL)
+           return NULL;
     }
 
     scope = new_scope(cctx, IF_SCOPE);
@@ -5764,9 +5799,15 @@ compile_elseif(char_u *arg, cctx_T *cctx)
        clear_ppconst(&ppconst);
     else if (instr->ga_len == instr_count && ppconst.pp_used == 1)
     {
+       int error = FALSE;
+       int v;
+
        // The expression results in a constant.
        // TODO: how about nesting?
-       cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES;
+       v = tv_get_bool_chk(&ppconst.pp_tv[0], &error);
+       if (error)
+           return NULL;
+       cctx->ctx_skip = v ? SKIP_NOT : SKIP_YES;
        clear_ppconst(&ppconst);
        scope->se_u.se_if.is_if_label = -1;
     }
@@ -5776,6 +5817,8 @@ compile_elseif(char_u *arg, cctx_T *cctx)
        cctx->ctx_skip = SKIP_UNKNOWN;
        if (generate_ppconst(cctx, &ppconst) == FAIL)
            return NULL;
+       if (bool_on_stack(cctx) == FAIL)
+           return NULL;
 
        // "where" is set when ":elseif", "else" or ":endif" is found
        scope->se_u.se_if.is_if_label = instr->ga_len;
@@ -6037,6 +6080,9 @@ compile_while(char_u *arg, cctx_T *cctx)
     if (compile_expr0(&p, cctx) == FAIL)
        return NULL;
 
+    if (bool_on_stack(cctx) == FAIL)
+       return FAIL;
+
     // "while_end" is set when ":endwhile" is found
     if (compile_jump_to_end(&scope->se_u.se_while.ws_end_label,
                                                  JUMP_IF_FALSE, cctx) == FAIL)
index b1065ca8e8fd1ceb0630df3b170009159a1f6828..7afc5c27cdec53d5cd73e69374601191f68ed026 100644 (file)
@@ -1908,6 +1908,7 @@ call_def_function(
                    {
                        tv = STACK_TV_BOT(-1);
                        if (when == JUMP_IF_COND_FALSE
+                               || when == JUMP_IF_FALSE
                                || when == JUMP_IF_COND_TRUE)
                        {
                            SOURCING_LNUM = iptr->isn_lnum;
@@ -3403,7 +3404,7 @@ ex_disassemble(exarg_T *eap)
 }
 
 /*
- * Return TRUE when "tv" is not falsey: non-zero, non-empty string, non-empty
+ * Return TRUE when "tv" is not falsy: non-zero, non-empty string, non-empty
  * list, etc.  Mostly like what JavaScript does, except that empty list and
  * empty dictionary are FALSE.
  */