From b55435f4386e9a138be7275919aa784f79a2fb03 Mon Sep 17 00:00:00 2001 From: Tristan Knight Date: Sun, 8 Sep 2024 11:44:46 +0100 Subject: [PATCH] fix(lsp): handle out-of-bounds character positions #30288 Problem: str_byteindex_enc could return an error if the index was longer than the lline length. This was handled in each of the calls to it individually Solution: * Fix the call at the source level so that if the index is higher than the line length, line length is returned as per LSP specification * Remove pcalls on str_byteindex_enc calls. No longer needed now that str_byteindex_enc has a bounds check. --- runtime/lua/vim/lsp/inlay_hint.lua | 7 +- runtime/lua/vim/lsp/semantic_tokens.lua | 7 +- runtime/lua/vim/lsp/util.lua | 19 +++--- test/functional/plugin/lsp_spec.lua | 89 +++++++++++++++++-------- 4 files changed, 73 insertions(+), 49 deletions(-) diff --git a/runtime/lua/vim/lsp/inlay_hint.lua b/runtime/lua/vim/lsp/inlay_hint.lua index 1e224d1bef..4483b083de 100644 --- a/runtime/lua/vim/lsp/inlay_hint.lua +++ b/runtime/lua/vim/lsp/inlay_hint.lua @@ -77,12 +77,7 @@ function M.on_inlayhint(err, result, ctx, _) local col = position.character if col > 0 then local line = lines[position.line + 1] or '' - local ok, convert_result - ok, convert_result = pcall(util._str_byteindex_enc, line, col, client.offset_encoding) - if ok then - return convert_result - end - return math.min(#line, col) + return util._str_byteindex_enc(line, col, client.offset_encoding) end return col end diff --git a/runtime/lua/vim/lsp/semantic_tokens.lua b/runtime/lua/vim/lsp/semantic_tokens.lua index 97ea499fe7..0f9b519e80 100644 --- a/runtime/lua/vim/lsp/semantic_tokens.lua +++ b/runtime/lua/vim/lsp/semantic_tokens.lua @@ -140,12 +140,7 @@ local function tokens_to_ranges(data, bufnr, client, request) local function _get_byte_pos(col) if col > 0 then local buf_line = lines[line + 1] or '' - local ok, result - ok, result = pcall(util._str_byteindex_enc, buf_line, col, client.offset_encoding) - if ok then - return result - end - return math.min(#buf_line, col) + return util._str_byteindex_enc(buf_line, col, client.offset_encoding) end return col end diff --git a/runtime/lua/vim/lsp/util.lua b/runtime/lua/vim/lsp/util.lua index fc99f54d03..91f15d3bb9 100644 --- a/runtime/lua/vim/lsp/util.lua +++ b/runtime/lua/vim/lsp/util.lua @@ -148,6 +148,12 @@ end ---@param encoding string utf-8|utf-16|utf-32| defaults to utf-16 ---@return integer byte (utf-8) index of `encoding` index `index` in `line` function M._str_byteindex_enc(line, index, encoding) + local len = vim.fn.strlen(line) + if index > len then + -- LSP spec: if character > line length, default to the line length. + -- https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#position + return len + end if not encoding then encoding = 'utf-16' end @@ -167,7 +173,6 @@ function M._str_byteindex_enc(line, index, encoding) end local _str_utfindex_enc = M._str_utfindex_enc -local _str_byteindex_enc = M._str_byteindex_enc --- Replaces text in a range with new text. --- @@ -333,12 +338,7 @@ local function get_line_byte_from_position(bufnr, position, offset_encoding) -- character if col > 0 then local line = get_line(bufnr, position.line) or '' - local ok, result - ok, result = pcall(_str_byteindex_enc, line, col, offset_encoding) - if ok then - return result - end - return math.min(#line, col) + return M._str_byteindex_enc(line, col, offset_encoding or 'utf-16') end return col end @@ -495,14 +495,15 @@ function M.apply_text_edits(text_edits, bufnr, offset_encoding) e.end_col = last_line_len has_eol_text_edit = true else - -- If the replacement is over the end of a line (i.e. e.end_col is out of bounds and the + -- If the replacement is over the end of a line (i.e. e.end_col is equal to the line length and the -- replacement text ends with a newline We can likely assume that the replacement is assumed -- to be meant to replace the newline with another newline and we need to make sure this -- doesn't add an extra empty line. E.g. when the last line to be replaced contains a '\r' -- in the file some servers (clangd on windows) will include that character in the line -- while nvim_buf_set_text doesn't count it as part of the line. if - e.end_col > last_line_len + e.end_col >= last_line_len + and text_edit.range['end'].character > e.end_col and #text_edit.newText > 0 and string.sub(text_edit.newText, -1) == '\n' then diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua index 0cf84b50c2..0b6e8441de 100644 --- a/test/functional/plugin/lsp_spec.lua +++ b/test/functional/plugin/lsp_spec.lua @@ -3560,21 +3560,21 @@ describe('LSP', function() range = { ["end"] = { character = 8, - line = 9 + line = 3, }, start = { character = 6, - line = 9 + line = 3, } }, selectionRange = { ["end"] = { character = 8, - line = 9 + line = 3, }, start = { character = 6, - line = 9 + line = 3, } }, uri = "file:///home/jiangyinzuo/hello.cpp" @@ -3597,21 +3597,21 @@ describe('LSP', function() range = { ["end"] = { character = 8, - line = 8 + line = 2 }, start = { character = 6, - line = 8 + line = 2 } }, selectionRange = { ["end"] = { character = 8, - line = 8 + line = 2 }, start = { character = 6, - line = 8 + line = 2 } }, uri = "file:///home/jiangyinzuo/hello.cpp" @@ -3623,8 +3623,16 @@ describe('LSP', function() }, }) local client_id = vim.lsp.start({ name = 'dummy', cmd = server.cmd }) - local handler = require'vim.lsp.handlers'['typeHierarchy/subtypes'] - handler(nil, clangd_response, { client_id = client_id, bufnr = 1 }) + local handler = require 'vim.lsp.handlers'['typeHierarchy/subtypes'] + local bufnr = vim.api.nvim_get_current_buf() + vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, { + 'class B : public A{};', + 'class C : public B{};', + 'class D1 : public C{};', + 'class D2 : public C{};', + 'class E : public D1, D2 {};', + }) + handler(nil, clangd_response, { client_id = client_id, bufnr = bufnr }) return vim.fn.getqflist() ]=]) @@ -3634,7 +3642,7 @@ describe('LSP', function() col = 7, end_col = 0, end_lnum = 0, - lnum = 10, + lnum = 4, module = '', nr = 0, pattern = '', @@ -3648,7 +3656,7 @@ describe('LSP', function() col = 7, end_col = 0, end_lnum = 0, - lnum = 9, + lnum = 3, module = '', nr = 0, pattern = '', @@ -3707,8 +3715,16 @@ describe('LSP', function() }, }) local client_id = vim.lsp.start({ name = 'dummy', cmd = server.cmd }) - local handler = require'vim.lsp.handlers'['typeHierarchy/subtypes'] - handler(nil, jdtls_response, { client_id = client_id, bufnr = 1 }) + local handler = require 'vim.lsp.handlers'['typeHierarchy/subtypes'] + local bufnr = vim.api.nvim_get_current_buf() + vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, { + 'package mylist;', + '', + 'public class MyList {', + ' static class Inner extends MyList{}', + '~}', + }) + handler(nil, jdtls_response, { client_id = client_id, bufnr = bufnr }) return vim.fn.getqflist() ]=]) @@ -3778,21 +3794,21 @@ describe('LSP', function() range = { ["end"] = { character = 8, - line = 9 + line = 3, }, start = { character = 6, - line = 9 + line = 3, } }, selectionRange = { ["end"] = { character = 8, - line = 9 + line = 3, }, start = { character = 6, - line = 9 + line = 3 } }, uri = "file:///home/jiangyinzuo/hello.cpp" @@ -3815,21 +3831,21 @@ describe('LSP', function() range = { ["end"] = { character = 8, - line = 8 + line = 2 }, start = { character = 6, - line = 8 + line = 2 } }, selectionRange = { ["end"] = { character = 8, - line = 8 + line = 2 }, start = { character = 6, - line = 8 + line = 2 } }, uri = "file:///home/jiangyinzuo/hello.cpp" @@ -3841,8 +3857,17 @@ describe('LSP', function() }, }) local client_id = vim.lsp.start({ name = 'dummy', cmd = server.cmd }) - local handler = require'vim.lsp.handlers'['typeHierarchy/supertypes'] - handler(nil, clangd_response, { client_id = client_id, bufnr = 1 }) + local handler = require 'vim.lsp.handlers'['typeHierarchy/supertypes'] + local bufnr = vim.api.nvim_get_current_buf() + vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, { + 'class B : public A{};', + 'class C : public B{};', + 'class D1 : public C{};', + 'class D2 : public C{};', + 'class E : public D1, D2 {};', + }) + + handler(nil, clangd_response, { client_id = client_id, bufnr = bufnr }) return vim.fn.getqflist() ]=]) @@ -3852,7 +3877,7 @@ describe('LSP', function() col = 7, end_col = 0, end_lnum = 0, - lnum = 10, + lnum = 4, module = '', nr = 0, pattern = '', @@ -3866,7 +3891,7 @@ describe('LSP', function() col = 7, end_col = 0, end_lnum = 0, - lnum = 9, + lnum = 3, module = '', nr = 0, pattern = '', @@ -3925,8 +3950,16 @@ describe('LSP', function() }, }) local client_id = vim.lsp.start({ name = 'dummy', cmd = server.cmd }) - local handler = require'vim.lsp.handlers'['typeHierarchy/supertypes'] - handler(nil, jdtls_response, { client_id = client_id, bufnr = 1 }) + local handler = require 'vim.lsp.handlers'['typeHierarchy/supertypes'] + local bufnr = vim.api.nvim_get_current_buf() + vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, { + 'package mylist;', + '', + 'public class MyList {', + ' static class Inner extends MyList{}', + '~}', + }) + handler(nil, jdtls_response, { client_id = client_id, bufnr = bufnr }) return vim.fn.getqflist() ]=])