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
This commit is contained in:
Mathias Fussenegger 2023-10-21 13:44:53 +02:00 committed by Mathias Fußenegger
parent bc850ba2a0
commit 5e5f5174e3
3 changed files with 247 additions and 87 deletions

View File

@ -106,11 +106,12 @@ function M._lsp_to_complete_items(result, prefix)
return matches return matches
end end
---@param lnum integer 0-indexed
---@param items lsp.CompletionItem[] ---@param items lsp.CompletionItem[]
local function adjust_start_col(lnum, line, items, encoding) local function adjust_start_col(lnum, line, items, encoding)
local min_start_char = nil local min_start_char = nil
for _, item in pairs(items) do 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 if min_start_char and min_start_char ~= item.textEdit.range.start.character then
return nil return nil
end end
@ -124,12 +125,57 @@ local function adjust_start_col(lnum, line, items, encoding)
end end
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 findstart integer 0 or 1, decides behavior
---@param base integer findstart=0, text to match against ---@param base integer findstart=0, text to match against
---@return integer|table Decided by {findstart}: ---@return integer|table Decided by {findstart}:
--- - findstart=0: column where the completion starts, or -2 or -3 --- - findstart=0: column where the completion starts, or -2 or -3
--- - findstart=1: list of matches (actually just calls |complete()|) --- - findstart=1: list of matches (actually just calls |complete()|)
function M.omnifunc(findstart, base) function M.omnifunc(findstart, base)
assert(base) -- silence luals
local bufnr = api.nvim_get_current_buf() local bufnr = api.nvim_get_current_buf()
local clients = lsp.get_clients({ bufnr = bufnr, method = ms.textDocument_completion }) local clients = lsp.get_clients({ bufnr = bufnr, method = ms.textDocument_completion })
local remaining = #clients local remaining = #clients
@ -137,26 +183,20 @@ function M.omnifunc(findstart, base)
return findstart == 1 and -1 or {} return findstart == 1 and -1 or {}
end 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 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 = api.nvim_get_current_line()
local line_to_cursor = line:sub(1, pos[2]) local line_to_cursor = line:sub(1, cursor_col)
log.trace('omnifunc.line', pos, line) local client_start_boundary = vim.fn.match(line_to_cursor, '\\k*$') --[[@as integer]]
local server_start_boundary = nil
local word_boundary = vim.fn.match(line_to_cursor, '\\k*$') + 1 --[[@as integer]]
local items = {} local items = {}
local startbyte = nil
local function on_done() local function on_done()
local mode = api.nvim_get_mode()['mode'] local mode = api.nvim_get_mode()['mode']
if mode == 'i' or mode == 'ic' then 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
end end
@ -165,34 +205,18 @@ function M.omnifunc(findstart, base)
local params = util.make_position_params(win, client.offset_encoding) local params = util.make_position_params(win, client.offset_encoding)
client.request(ms.textDocument_completion, params, function(err, result) client.request(ms.textDocument_completion, params, function(err, result)
if err then if err then
log.warn(err.message) require('vim.lsp.log').warn(err.message)
end end
if result and vim.fn.mode() == 'i' then if result and vim.fn.mode() == 'i' then
-- Completion response items may be relative to a position different than `textMatch`. local matches
-- Concrete example, with sumneko/lua-language-server: matches, server_start_boundary = M._convert_results(
-- line,
-- require('plenary.asy| lnum,
-- ▲ ▲ ▲ client_start_boundary,
-- │ │ └── cursor_pos: 20 server_start_boundary,
-- │ └────── textMatch: 17 result,
-- └────────────── textEdit.range.start.character: 9 client.offset_encoding
-- .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)
vim.list_extend(items, matches) vim.list_extend(items, matches)
end end
remaining = remaining - 1 remaining = remaining - 1

View File

@ -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)

View File

@ -2281,53 +2281,6 @@ describe('LSP', function()
end) end)
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() describe('lsp.util.rename', function()
local pathsep = helpers.get_pathsep() local pathsep = helpers.get_pathsep()