fix(lsp): fix off-by-one error for omnifunc word boundary

Fixes https://github.com/neovim/neovim/issues/25177

I initially wanted to split this into a refactor commit to make it more
testable, but it appears that already accidentally fixed the issue by
normalizing lnum/col to 0-indexing

3 files changed, 247 insertions, 87 deletions

diff --git a/runtime/lua/vim/lsp/_completion.lua b/runtime/lua/vim/lsp/_completion.lua
index efd1aaacf7..f0e3af7f03 100644
--- a/runtime/lua/vim/lsp/_completion.lua
+++ b/runtime/lua/vim/lsp/_completion.lua
@@ -106,11 +106,12 @@ function M._lsp_to_complete_items(result, prefix)
   return matches
 end
 
+---@param lnum integer 0-indexed
 ---@param items lsp.CompletionItem[]
 local function adjust_start_col(lnum, line, items, encoding)
   local min_start_char = nil
   for _, item in pairs(items) do
-    if item.textEdit and item.textEdit.range.start.line == lnum - 1 then
+    if item.textEdit and item.textEdit.range.start.line == lnum then
       if min_start_char and min_start_char ~= item.textEdit.range.start.character then
         return nil
       end
@@ -124,12 +125,57 @@ local function adjust_start_col(lnum, line, items, encoding)
   end
 end
 
+---@private
+---@param line string line content
+---@param lnum integer 0-indexed line number
+---@param client_start_boundary integer 0-indexed word boundary
+---@param server_start_boundary? integer 0-indexed word boundary, based on textEdit.range.start.character
+---@param result lsp.CompletionList|lsp.CompletionItem[]
+---@param encoding string
+---@return table[] matches
+---@return integer? server_start_boundary
+function M._convert_results(
+  line,
+  lnum,
+  client_start_boundary,
+  server_start_boundary,
+  result,
+  encoding
+)
+  -- Completion response items may be relative to a position different than `client_start_boundary`.
+  -- Concrete example, with lua-language-server:
+  --
+  -- require('plenary.asy|
+  --         ▲       ▲   ▲
+  --         │       │   └── cursor_pos:                     20
+  --         │       └────── client_start_boundary:          17
+  --         └────────────── textEdit.range.start.character: 9
+  --                                 .newText = 'plenary.async'
+  --                  ^^^
+  --                  prefix (We'd remove everything not starting with `asy`,
+  --                  so we'd eliminate the `plenary.async` result
+  --
+  -- `adjust_start_col` is used to prefer the language server boundary.
+  --
+  local candidates = get_items(result)
+  local curstartbyte = adjust_start_col(lnum, line, candidates, encoding)
+  if server_start_boundary == nil then
+    server_start_boundary = curstartbyte
+  elseif curstartbyte ~= nil and curstartbyte ~= server_start_boundary then
+    server_start_boundary = client_start_boundary
+  end
+  local prefix = line:sub((server_start_boundary or client_start_boundary) + 1)
+  local matches = M._lsp_to_complete_items(result, prefix)
+  return matches, server_start_boundary
+end
+
 ---@param findstart integer 0 or 1, decides behavior
 ---@param base integer findstart=0, text to match against
 ---@return integer|table Decided by {findstart}:
 --- - findstart=0: column where the completion starts, or -2 or -3
 --- - findstart=1: list of matches (actually just calls |complete()|)
 function M.omnifunc(findstart, base)
+  assert(base) -- silence luals
   local bufnr = api.nvim_get_current_buf()
   local clients = lsp.get_clients({ bufnr = bufnr, method = ms.textDocument_completion })
   local remaining = #clients
@@ -137,26 +183,20 @@ function M.omnifunc(findstart, base)
     return findstart == 1 and -1 or {}
   end
 
-  local log = require('vim.lsp.log')
-  -- Then, perform standard completion request
-  if log.info() then
-    log.info('base ', base)
-  end
-
   local win = api.nvim_get_current_win()
-  local pos = api.nvim_win_get_cursor(win)
+  local cursor = api.nvim_win_get_cursor(win)
+  local lnum = cursor[1] - 1
+  local cursor_col = cursor[2]
   local line = api.nvim_get_current_line()
-  local line_to_cursor = line:sub(1, pos[2])
-  log.trace('omnifunc.line', pos, line)
-
-  local word_boundary = vim.fn.match(line_to_cursor, '\\k*$') + 1 --[[@as integer]]
+  local line_to_cursor = line:sub(1, cursor_col)
+  local client_start_boundary = vim.fn.match(line_to_cursor, '\\k*$') --[[@as integer]]
+  local server_start_boundary = nil
   local items = {}
-  local startbyte = nil
 
   local function on_done()
     local mode = api.nvim_get_mode()['mode']
     if mode == 'i' or mode == 'ic' then
-      vim.fn.complete(startbyte or word_boundary, items)
+      vim.fn.complete((server_start_boundary or client_start_boundary) + 1, items)
     end
   end
 
@@ -165,34 +205,18 @@ function M.omnifunc(findstart, base)
     local params = util.make_position_params(win, client.offset_encoding)
     client.request(ms.textDocument_completion, params, function(err, result)
       if err then
-        log.warn(err.message)
+        require('vim.lsp.log').warn(err.message)
       end
       if result and vim.fn.mode() == 'i' then
-        -- Completion response items may be relative to a position different than `textMatch`.
-        -- Concrete example, with sumneko/lua-language-server:
-        --
-        -- require('plenary.asy|
-        --         ▲       ▲   ▲
-        --         │       │   └── cursor_pos: 20
-        --         │       └────── textMatch: 17
-        --         └────────────── textEdit.range.start.character: 9
-        --                                 .newText = 'plenary.async'
-        --                  ^^^
-        --                  prefix (We'd remove everything not starting with `asy`,
-        --                  so we'd eliminate the `plenary.async` result
-        --
-        -- `adjust_start_col` is used to prefer the language server boundary.
-        --
-        local encoding = client.offset_encoding
-        local candidates = get_items(result)
-        local curstartbyte = adjust_start_col(pos[1], line, candidates, encoding)
-        if startbyte == nil then
-          startbyte = curstartbyte
-        elseif curstartbyte ~= nil and curstartbyte ~= startbyte then
-          startbyte = word_boundary
-        end
-        local prefix = startbyte and line:sub(startbyte + 1) or line_to_cursor:sub(word_boundary)
-        local matches = M._lsp_to_complete_items(result, prefix)
+        local matches
+        matches, server_start_boundary = M._convert_results(
+          line,
+          lnum,
+          client_start_boundary,
+          server_start_boundary,
+          result,
+          client.offset_encoding
+        )
         vim.list_extend(items, matches)
       end
       remaining = remaining - 1
diff --git a/test/functional/plugin/lsp/completion_spec.lua b/test/functional/plugin/lsp/completion_spec.lua
new file mode 100644
index 0000000000..aa13d6f4b1
--- /dev/null
+++ b/test/functional/plugin/lsp/completion_spec.lua
@@ -0,0 +1,183 @@
+---@diagnostic disable: no-unknown
+local helpers = require('test.functional.helpers')(after_each)
+local eq = helpers.eq
+local exec_lua = helpers.exec_lua
+
+
+--- Convert completion results.
+---
+---@param line string line contents. Mark cursor position with `|`
+---@param candidates lsp.CompletionList|lsp.CompletionItem[]
+---@param lnum? integer 0-based, defaults to 0
+---@return {items: table[], server_start_boundary: integer?}
+local function complete(line, candidates, lnum)
+  lnum = lnum or 0
+  local cursor_col = line:find("|")
+  line = line:gsub("|", "")
+  return exec_lua([[
+    local line, cursor_col, lnum, result = ...
+    local line_to_cursor = line:sub(1, cursor_col)
+    local client_start_boundary = vim.fn.match(line_to_cursor, '\\k*$')
+    local items, server_start_boundary = require("vim.lsp._completion")._convert_results(
+      line,
+      lnum,
+      client_start_boundary,
+      nil,
+      result,
+      "utf-16"
+    )
+    return {
+      items = items,
+      server_start_boundary = server_start_boundary
+    }
+  ]], line, cursor_col, lnum, candidates)
+end
+
+
+describe("vim.lsp._completion", function()
+  before_each(helpers.clear)
+
+  -- https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_completion
+  it('prefers textEdit over label as word', function()
+    local range0 = {
+      start = { line = 0, character = 0 },
+      ["end"] = { line = 0, character = 0 },
+    }
+    local completion_list = {
+      -- resolves into label
+      { label = 'foobar', sortText = 'a', documentation = 'documentation' },
+      {
+        label = 'foobar',
+        sortText = 'b',
+        documentation = { value = 'documentation' },
+      },
+      -- resolves into insertText
+      { label = 'foocar', sortText = 'c', insertText = 'foobar' },
+      { label = 'foocar', sortText = 'd', insertText = 'foobar' },
+      -- resolves into textEdit.newText
+      { label = 'foocar', sortText = 'e', insertText = 'foodar', textEdit = { newText = 'foobar', range = range0 } },
+      { label = 'foocar', sortText = 'f', textEdit = { newText = 'foobar', range = range0 } },
+      -- real-world snippet text
+      {
+        label = 'foocar',
+        sortText = 'g',
+        insertText = 'foodar',
+        insertTextFormat = 2,
+        textEdit = { newText = 'foobar(${1:place holder}, ${2:more ...holder{\\}})', range = range0 },
+      },
+      {
+        label = 'foocar',
+        sortText = 'h',
+        insertText = 'foodar(${1:var1} typ1, ${2:var2} *typ2) {$0\\}',
+        insertTextFormat = 2,
+      },
+      -- nested snippet tokens
+      {
+        label = 'foocar',
+        sortText = 'i',
+        insertText = 'foodar(${1:${2|typ1,typ2|}}) {$0\\}',
+        insertTextFormat = 2,
+      },
+      -- braced tabstop
+      { label = 'foocar', sortText = 'j', insertText = 'foodar()${0}', insertTextFormat = 2},
+      -- plain text
+      {
+        label = 'foocar',
+        sortText = 'k',
+        insertText = 'foodar(${1:var1})',
+        insertTextFormat = 1,
+      },
+    }
+    local expected = {
+      {
+        abbr = 'foobar',
+        word = 'foobar',
+      },
+      {
+        abbr = 'foobar',
+        word = 'foobar',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foobar',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foobar',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foobar',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foobar',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foobar(place holder, more ...holder{})',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foodar(var1 typ1, var2 *typ2) {}',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foodar(typ1) {}',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foodar()',
+      },
+      {
+        abbr = 'foocar',
+        word = 'foodar(${1:var1})',
+      },
+    }
+    local result = complete('|', completion_list)
+    result = vim.tbl_map(function(x)
+      return {
+        abbr = x.abbr,
+        word = x.word
+      }
+    end, result.items)
+    eq(expected, result)
+  end)
+  it("uses correct start boundary", function()
+    local completion_list = {
+      isIncomplete = false,
+      items = {
+        {
+          filterText = "this_thread",
+          insertText = "this_thread",
+          insertTextFormat = 1,
+          kind = 9,
+          label = " this_thread",
+          score = 1.3205767869949,
+          sortText = "4056f757this_thread",
+          textEdit = {
+            newText = "this_thread",
+            range = {
+              start = { line = 0, character = 7 },
+              ["end"] = { line = 0, character = 11 },
+            },
+          }
+        },
+      }
+    }
+    local expected = {
+      abbr = ' this_thread',
+      dup = 1,
+      empty = 1,
+      icase = 1,
+      kind = 'Module',
+      menu = '',
+      word = 'this_thread',
+    }
+    local result = complete("  std::this|", completion_list)
+    eq(7, result.server_start_boundary)
+    local item = result.items[1]
+    item.user_data = nil
+    eq(expected, item)
+  end)
+end)
diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua
index 3bd8db815d..d56e5b4afa 100644
--- a/test/functional/plugin/lsp_spec.lua
+++ b/test/functional/plugin/lsp_spec.lua
@@ -2281,53 +2281,6 @@ describe('LSP', function()
     end)
   end)
 
-  describe('completion_list_to_complete_items', function()
-    -- Completion option precedence:
-    -- textEdit.newText > insertText > label
-    -- https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_completion
-    it('should choose right completion option', function ()
-      local prefix = 'foo'
-      local completion_list = {
-        -- resolves into label
-        { label = 'foobar', sortText = 'a', documentation = 'documentation' },
-        { label = 'foobar', sortText = 'b', documentation = { value = 'documentation' }, textEdit = {} },
-        -- resolves into insertText
-        { label='foocar', sortText="c", insertText='foobar' },
-        { label='foocar', sortText="d", insertText='foobar', textEdit={} },
-        -- resolves into textEdit.newText
-        { label='foocar', sortText="e", insertText='foodar', textEdit={newText='foobar'} },
-        { label='foocar', sortText="f", textEdit={newText='foobar'} },
-        -- real-world snippet text
-        { label='foocar', sortText="g", insertText='foodar', insertTextFormat=2, textEdit={newText='foobar(${1:place holder}, ${2:more ...holder{\\}})'} },
-        { label='foocar', sortText="h", insertText='foodar(${1:var1} typ1, ${2:var2} *typ2) {$0\\}', insertTextFormat=2, textEdit={} },
-        -- nested snippet tokens
-        { label='foocar', sortText="i", insertText='foodar(${1:${2|typ1,typ2|}}) {$0\\}', insertTextFormat=2, textEdit={} },
-        -- braced tabstop
-        { label='foocar', sortText="j", insertText='foodar()${0}', insertTextFormat=2, textEdit={} },
-        -- plain text
-        { label='foocar', sortText="k", insertText='foodar(${1:var1})', insertTextFormat=1, textEdit={} },
-      }
-      local completion_list_items = {items=completion_list}
-      local expected = {
-        { abbr = 'foobar', dup = 1, empty = 1, icase = 1, info = 'documentation', kind = 'Unknown', menu = '', word = 'foobar', user_data = { nvim = { lsp = { completion_item = { label = 'foobar', sortText="a",  documentation = 'documentation' } } } } },
-        { abbr = 'foobar', dup = 1, empty = 1, icase = 1, info = 'documentation', kind = 'Unknown', menu = '', word = 'foobar', user_data = { nvim = { lsp = { completion_item = { label='foobar', sortText="b", textEdit={},documentation = { value = 'documentation' } } } }  } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foobar', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="c", insertText='foobar' } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foobar', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="d", insertText='foobar', textEdit={} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foobar', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="e", insertText='foodar', textEdit={newText='foobar'} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foobar', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="f", textEdit={newText='foobar'} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foobar(place holder, more ...holder{})', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="g", insertText='foodar', insertTextFormat=2, textEdit={newText='foobar(${1:place holder}, ${2:more ...holder{\\}})'} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foodar(var1 typ1, var2 *typ2) {}', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="h", insertText='foodar(${1:var1} typ1, ${2:var2} *typ2) {$0\\}', insertTextFormat=2, textEdit={} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foodar(typ1) {}', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="i", insertText='foodar(${1:${2|typ1,typ2|}}) {$0\\}', insertTextFormat=2, textEdit={} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foodar()', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="j", insertText='foodar()${0}', insertTextFormat=2, textEdit={} } } } } },
-        { abbr = 'foocar', dup = 1, empty = 1, icase = 1, kind = 'Unknown', menu = '', word = 'foodar(${1:var1})', user_data = { nvim = { lsp = { completion_item = { label='foocar', sortText="k", insertText='foodar(${1:var1})', insertTextFormat=1, textEdit={} } } } } },
-      }
-
-      eq(expected, exec_lua([[return vim.lsp.util.text_document_completion_list_to_complete_items(...)]], completion_list, prefix))
-      eq(expected, exec_lua([[return vim.lsp.util.text_document_completion_list_to_complete_items(...)]], completion_list_items, prefix))
-      eq({}, exec_lua([[return vim.lsp.util.text_document_completion_list_to_complete_items(...)]], {}, prefix))
-    end)
-  end)
-
   describe('lsp.util.rename', function()
     local pathsep = helpers.get_pathsep()