From 5e5f5174e3faa862a9bc353aa7da41487911140b Mon Sep 17 00:00:00 2001 From: Mathias Fussenegger Date: Sat, 21 Oct 2023 13:44:53 +0200 Subject: [PATCH] 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 --- runtime/lua/vim/lsp/_completion.lua | 104 ++++++---- .../functional/plugin/lsp/completion_spec.lua | 183 ++++++++++++++++++ test/functional/plugin/lsp_spec.lua | 47 ----- 3 files changed, 247 insertions(+), 87 deletions(-) create mode 100644 test/functional/plugin/lsp/completion_spec.lua 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()