window: Fix matchaddpos() and enhance error reporting

runtime/doc/eval.txt

@@ -5493,7 +5493,7 @@ matchaddpos({group}, {pos} [, {priority} [, {id} [, {dict}]]])
 		sets buffer line boundaries to redraw screen. It is supposed
 		to be used when fast match additions and deletions are
 		required, for example to highlight matching parentheses.
-
+							*E5030* *E5031*
 		The list {pos} can contain one of these items:
 		- A number.  This whole line will be highlighted.  The first
 		  line has number 1.
@@ -5507,6 +5507,10 @@ matchaddpos({group}, {pos} [, {priority} [, {id} [, {dict}]]])
 		- A list with three numbers, e.g., [23, 11, 3]. As above, but
 		  the third number gives the length of the highlight in bytes.
 
+		Entries with zero and negative line numbers are silently 
+		ignored, as well as entries with negative column numbers and 
+		lengths.
+
 		The maximum number of positions is 8.
 
 		Example: >

src/nvim/eval.c

@@ -12440,56 +12440,56 @@ static void f_matchadd(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 
 static void f_matchaddpos(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 {
-    rettv->vval.v_number = -1;
+  rettv->vval.v_number = -1;
 
-    char buf[NUMBUFLEN];
+  char buf[NUMBUFLEN];
-    const char *const group = tv_get_string_buf_chk(&argvars[0], buf);
+  const char *const group = tv_get_string_buf_chk(&argvars[0], buf);
-    if (group == NULL) {
+  if (group == NULL) {
-        return;
+    return;
-    }
+  }
 
-    if (argvars[1].v_type != VAR_LIST) {
+  if (argvars[1].v_type != VAR_LIST) {
-        EMSG2(_(e_listarg), "matchaddpos()");
+    EMSG2(_(e_listarg), "matchaddpos()");
-        return;
+    return;
-    }
+  }
 
-    list_T *l;
+  list_T *l;
-    l = argvars[1].vval.v_list;
+  l = argvars[1].vval.v_list;
-    if (l == NULL) {
+  if (l == NULL) {
-        return;
+    return;
-    }
+  }
 
-    bool error = false;
+  bool error = false;
-    int prio = 10;
+  int prio = 10;
-    int id = -1;
+  int id = -1;
-    const char *conceal_char = NULL;
+  const char *conceal_char = NULL;
 
-    if (argvars[2].v_type != VAR_UNKNOWN) {
+  if (argvars[2].v_type != VAR_UNKNOWN) {
-      prio = tv_get_number_chk(&argvars[2], &error);
+    prio = tv_get_number_chk(&argvars[2], &error);
-      if (argvars[3].v_type != VAR_UNKNOWN) {
+    if (argvars[3].v_type != VAR_UNKNOWN) {
-        id = tv_get_number_chk(&argvars[3], &error);
+      id = tv_get_number_chk(&argvars[3], &error);
-        if (argvars[4].v_type != VAR_UNKNOWN) {
+      if (argvars[4].v_type != VAR_UNKNOWN) {
-          if (argvars[4].v_type != VAR_DICT) {
+        if (argvars[4].v_type != VAR_DICT) {
-            EMSG(_(e_dictreq));
+          EMSG(_(e_dictreq));
-            return;
+          return;
-          }
+        }
-          dictitem_T *di;
+        dictitem_T *di;
-          if ((di = tv_dict_find(argvars[4].vval.v_dict, S_LEN("conceal")))
+        if ((di = tv_dict_find(argvars[4].vval.v_dict, S_LEN("conceal")))
-              != NULL) {
+            != NULL) {
-            conceal_char = tv_get_string(&di->di_tv);
+          conceal_char = tv_get_string(&di->di_tv);
-          }
         }
       }
     }
-    if (error == true) {
+  }
-        return;
+  if (error == true) {
-    }
+    return;
+  }
 
-    // id == 3 is ok because matchaddpos() is supposed to substitute :3match 
+  // id == 3 is ok because matchaddpos() is supposed to substitute :3match 
-    if (id == 1 || id == 2) {
+  if (id == 1 || id == 2) {
-        EMSGN(_("E798: ID is reserved for \"match\": %" PRId64), id);
+    EMSGN(_("E798: ID is reserved for \"match\": %" PRId64), id);
-        return;
+    return;
-    }
+  }
 
   rettv->vval.v_number = match_add(curwin, group, NULL, prio, id, l,
                                    conceal_char);

src/nvim/window.c

@@ -5619,19 +5619,17 @@ int match_add(win_T *wp, const char *const grp, const char *const pat,
 
       if (TV_LIST_ITEM_TV(li)->v_type == VAR_LIST) {
         const list_T *const subl = TV_LIST_ITEM_TV(li)->vval.v_list;
-        if (subl == NULL) {
-          goto fail;
-        }
         const listitem_T *subli = tv_list_first(subl);
         if (subli == NULL) {
+          emsgf(_("E5030: Empty list at position %d"),
+                (int)tv_list_idx_of_item(pos_list, li));
           goto fail;
         }
         lnum = tv_get_number_chk(TV_LIST_ITEM_TV(subli), &error);
         if (error) {
           goto fail;
         }
-        if (lnum == 0) {
+        if (lnum <= 0) {
-          --i;
           continue;
         }
         m->pos.pos[i].lnum = lnum;
@@ -5641,9 +5639,15 @@ int match_add(win_T *wp, const char *const grp, const char *const pat,
           if (error) {
             goto fail;
           }
+          if (col < 0) {
+            continue;
+          }
           subli = TV_LIST_ITEM_NEXT(subl, subli);
           if (subli != NULL) {
             len = tv_get_number_chk(TV_LIST_ITEM_TV(subli), &error);
+            if (len < 0) {
+              continue;
+            }
             if (error) {
               goto fail;
             }
@@ -5652,15 +5656,15 @@ int match_add(win_T *wp, const char *const grp, const char *const pat,
         m->pos.pos[i].col = col;
         m->pos.pos[i].len = len;
       } else if (TV_LIST_ITEM_TV(li)->v_type == VAR_NUMBER) {
-        if (TV_LIST_ITEM_TV(li)->vval.v_number == 0) {
+        if (TV_LIST_ITEM_TV(li)->vval.v_number <= 0) {
-          i--;
           continue;
         }
         m->pos.pos[i].lnum = TV_LIST_ITEM_TV(li)->vval.v_number;
         m->pos.pos[i].col = 0;
         m->pos.pos[i].len = 0;
       } else {
-        EMSG(_("List or number required"));
+        emsgf(_("E5031: List or number required at position %d"),
+              (int)tv_list_idx_of_item(pos_list, li));
         goto fail;
       }
       if (toplnum == 0 || lnum < toplnum) {

test/functional/eval/match_functions_spec.lua

@@ -1,9 +1,12 @@
 local helpers = require('test.functional.helpers')(after_each)
+local Screen = require('test.functional.ui.screen')
 
 local eq = helpers.eq
 local clear = helpers.clear
 local funcs = helpers.funcs
+local meths = helpers.meths
 local command = helpers.command
+local exc_exec = helpers.exc_exec
 
 before_each(clear)
 
@@ -59,3 +62,95 @@ describe('matchadd()', function()
     }}, funcs.getmatches())
   end)
 end)
+
+describe('matchaddpos()', function()
+  it('errors out on invalid input', function()
+    command('hi clear PreProc')
+    eq('Vim(let):E5030: Empty list at position 0',
+       exc_exec('let val = matchaddpos("PreProc", [[]])'))
+    eq('Vim(let):E5030: Empty list at position 1',
+       exc_exec('let val = matchaddpos("PreProc", [1, v:_null_list])'))
+    eq('Vim(let):E5031: List or number required at position 1',
+       exc_exec('let val = matchaddpos("PreProc", [1, v:_null_dict])'))
+  end)
+  it('works with 0 lnum', function()
+    command('hi clear PreProc')
+    eq(4, funcs.matchaddpos('PreProc', {1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+    funcs.matchdelete(4)
+    eq(4, funcs.matchaddpos('PreProc', {{0}, 1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+    funcs.matchdelete(4)
+    eq(4, funcs.matchaddpos('PreProc', {0, 1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+  end)
+  it('works with negative numbers', function()
+    command('hi clear PreProc')
+    eq(4, funcs.matchaddpos('PreProc', {-10, 1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+    funcs.matchdelete(4)
+    eq(4, funcs.matchaddpos('PreProc', {{-10}, 1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+    funcs.matchdelete(4)
+    eq(4, funcs.matchaddpos('PreProc', {{2, -1}, 1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+    funcs.matchdelete(4)
+    eq(4, funcs.matchaddpos('PreProc', {{2, 0, -1}, 1}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+  end)
+  it('works with zero length', function()
+    local screen = Screen.new(40, 5)
+    screen:attach()
+    funcs.setline(1, 'abcdef')
+    command('hi PreProc guifg=Red')
+    eq(4, funcs.matchaddpos('PreProc', {{1, 2, 0}}, 3, 4))
+    eq({{
+      group='PreProc',
+      pos1 = {1, 2, 0},
+      priority=3,
+      id=4,
+    }}, funcs.getmatches())
+    screen:expect([[
+      ^a{1:b}cdef                                  |
+      {2:~                                       }|
+      {2:~                                       }|
+      {2:~                                       }|
+                                              |
+    ]], {[1] = {foreground = Screen.colors.Red}, [2] = {bold = true, foreground = Screen.colors.Blue1}})
+  end)
+end)