Merge PR #4375 'Proper type checking for set{qf,loc}list()'

src/nvim/eval.c

@@ -160,6 +160,7 @@ static char *e_listdictarg = N_(
 static char *e_emptykey = N_("E713: Cannot use empty key for Dictionary");
 static char *e_listreq = N_("E714: List required");
 static char *e_dictreq = N_("E715: Dictionary required");
+static char *e_strreq = N_("E114: String required");
 static char *e_toomanyarg = N_("E118: Too many arguments for function: %s");
 static char *e_dictkey = N_("E716: Key not present in Dictionary: %s");
 static char *e_funcexts = N_(
@@ -15291,43 +15292,51 @@ static void f_setline(typval_T *argvars, typval_T *rettv)
 ///                         replace its content or create a new one.
 /// @param[in]  title_arg  New list title. Defaults to caller function name.
 /// @param[out]  rettv  Return value: 0 in case of success, -1 otherwise.
-static void set_qf_ll_list(win_T *wp, typval_T *list_arg, typval_T *action_arg,
+static void set_qf_ll_list(win_T *wp, typval_T *args, typval_T *rettv)
-                           typval_T *title_arg, typval_T *rettv)
+  FUNC_ATTR_NONNULL_ARG(2, 3)
-  FUNC_ATTR_NONNULL_ARG(2, 3, 4, 5)
 {
-  char_u      *act;
-  int action = ' ';
   char_u *title = NULL;
-
+  int action = ' ';
   rettv->vval.v_number = -1;
 
-  if (list_arg->v_type != VAR_LIST)
+  typval_T *list_arg = &args[0];
+  if (list_arg->v_type != VAR_LIST) {
     EMSG(_(e_listreq));
-  else {
+    return;
-    list_T  *l = list_arg->vval.v_list;
+  }
 
-    if (action_arg->v_type != VAR_UNKNOWN) {
+  typval_T *action_arg = &args[1];
-      act = get_tv_string_chk(action_arg);
+  if (action_arg->v_type == VAR_UNKNOWN) {
-      if (act == NULL)
+    // Option argument was not given.
-        return;                 /* type error; errmsg already given */
+    goto skip_args;
-      if (*act == 'a' || *act == 'r')
+  } else if (action_arg->v_type != VAR_STRING) {
-        action = *act;
+    EMSG(_(e_strreq));
-    }
+    return;
+  }
+  char_u *act = get_tv_string_chk(action_arg);
+  if (*act == 'a' || *act == 'r') {
+    action = *act;
+  }
 
-    if (title_arg->v_type == VAR_STRING) {
+  typval_T *title_arg = &args[2];
-      title = get_tv_string_chk(title_arg);
+  if (title_arg->v_type == VAR_UNKNOWN) {
-      if (!title) {
+    // Option argument was not given.
-        return;  // type error; errmsg already given
+    goto skip_args;
-      }
+  }
-    }
+  title = get_tv_string_chk(title_arg);
+  if (!title) {
+    // Type error. Error already printed by get_tv_string_chk().
+    return;
+  }
 
-    if (!title) {
+skip_args:
-      title = (char_u*)(wp ? "setloclist()" : "setqflist()");
+  if (!title) {
-    }
+    title = (char_u*)(wp ? "setloclist()" : "setqflist()");
+  }
 
-    if (l && set_errorlist(wp, l, action, title) == OK) {
+  list_T *l = list_arg->vval.v_list;
-      rettv->vval.v_number = 0;
+  if (l && set_errorlist(wp, l, action, title) == OK) {
-    }
+    rettv->vval.v_number = 0;
   }
 }
 
@@ -15342,7 +15351,7 @@ static void f_setloclist(typval_T *argvars, typval_T *rettv)
 
   win = find_win_by_nr(&argvars[0], NULL);
   if (win != NULL) {
-    set_qf_ll_list(win, &argvars[1], &argvars[2], &argvars[3], rettv);
+    set_qf_ll_list(win, &argvars[1], rettv);
   }
 }
 
@@ -15479,7 +15488,7 @@ static void f_setpos(typval_T *argvars, typval_T *rettv)
  */
 static void f_setqflist(typval_T *argvars, typval_T *rettv)
 {
-  set_qf_ll_list(NULL, &argvars[0], &argvars[1], &argvars[2], rettv);
+  set_qf_ll_list(NULL, argvars, rettv);
 }
 
 /*

test/functional/viml/errorlist_spec.lua

@@ -3,32 +3,44 @@ local helpers = require('test.functional.helpers')
 local clear = helpers.clear
 local command = helpers.command
 local eq = helpers.eq
+local exc_exec = helpers.exc_exec
 local get_cur_win_var = helpers.curwinmeths.get_var
--- local exc_exec = helpers.exc_exec
 
 describe('setqflist()', function()
   local setqflist = helpers.funcs.setqflist
 
   before_each(clear)
 
+  it('requires a list for {list}', function()
+    eq('Vim(call):E714: List required', exc_exec('call setqflist("foo")'))
+    eq('Vim(call):E714: List required', exc_exec('call setqflist(5)'))
+    eq('Vim(call):E714: List required', exc_exec('call setqflist({})'))
+  end)
+
+  it('requires a string for {action}', function()
+    eq('Vim(call):E114: String required', exc_exec('call setqflist([], 5)'))
+    eq('Vim(call):E114: String required', exc_exec('call setqflist([], [])'))
+    eq('Vim(call):E114: String required', exc_exec('call setqflist([], {})'))
+  end)
+
   it('sets w:quickfix_title', function()
     setqflist({''}, 'r', 'foo')
     command('copen')
     eq(':foo', get_cur_win_var('quickfix_title'))
   end)
 
-  it('expects a proper type for {title}', function()
+  it('requires string or number for {title}', function()
     command('copen')
-    setqflist({''}, 'r', '5')
+    setqflist({}, 'r', '5')
     eq(':5', get_cur_win_var('quickfix_title'))
-    setqflist({''}, 'r', 6)
+    setqflist({}, 'r', 6)
-    eq(':setqflist()', get_cur_win_var('quickfix_title'))
+    eq(':6', get_cur_win_var('quickfix_title'))
-    -- local exc = exc_exec('call setqflist([""], "r", function("function"))')
+    local exc = exc_exec('call setqflist([], "r", function("function"))')
-    -- eq('Vim(call):E729: using Funcref as a String', exc)
+    eq('Vim(call):E729: using Funcref as a String', exc)
-    -- exc = exc_exec('call setqflist([""], "r", [])')
+    exc = exc_exec('call setqflist([], "r", [])')
-    -- eq('Vim(call):E730: using List as a String', exc)
+    eq('Vim(call):E730: using List as a String', exc)
-    -- exc = exc_exec('call setqflist([""], "r", {})')
+    exc = exc_exec('call setqflist([], "r", {})')
-    -- eq('Vim(call):E731: using Dictionary as a String', exc)
+    eq('Vim(call):E731: using Dictionary as a String', exc)
   end)
 end)
 
@@ -37,10 +49,22 @@ describe('setloclist()', function()
 
   before_each(clear)
 
+  it('requires a list for {list}', function()
+    eq('Vim(call):E714: List required', exc_exec('call setloclist(0, "foo")'))
+    eq('Vim(call):E714: List required', exc_exec('call setloclist(0, 5)'))
+    eq('Vim(call):E714: List required', exc_exec('call setloclist(0, {})'))
+  end)
+
+  it('requires a string for {action}', function()
+    eq('Vim(call):E114: String required', exc_exec('call setloclist(0, [], 5)'))
+    eq('Vim(call):E114: String required', exc_exec('call setloclist(0, [], [])'))
+    eq('Vim(call):E114: String required', exc_exec('call setloclist(0, [], {})'))
+  end)
+
   it('sets w:quickfix_title for the correct window', function()
     command('rightbelow vsplit')
-    setloclist(1, {''}, 'r', 'foo')
+    setloclist(1, {}, 'r', 'foo')
-    setloclist(2, {''}, 'r', 'bar')
+    setloclist(2, {}, 'r', 'bar')
     command('lopen')
     eq(':bar', get_cur_win_var('quickfix_title'))
     command('lclose | wincmd w | lopen')