fix(lsp): decode 'null' in server responses as vim.NIL #34849

Problem:
Previously, 'null' value in LSP responses were decoded as 'nil'.
This caused ambiguity for fields typed as '? | null' and led to
loss of explicit 'null' values, particularly in 'data' parameters.

Solution:
Decode all JSON 'null' values as 'vim.NIL' and adjust handling
where needed. This better aligns with the LSP specification,
where 'null' and absent fields are distinct, and 'null' should
not be used to represent missing values.

This also enables proper validation of response messages to
ensure that exactly one of 'result' or 'error' is present, as
required by the JSON-RPC specification.
This commit is contained in:
skewb1k
2025-08-03 17:42:44 +03:00
committed by GitHub
parent 2ef48fc65c
commit 40aef0d02e
9 changed files with 42 additions and 26 deletions

View File

@@ -86,6 +86,7 @@ LSP
arguments corresponding to a log entry instead of the individual arguments. arguments corresponding to a log entry instead of the individual arguments.
• `vim.lsp.semantic_tokens.start/stop` now renamed to • `vim.lsp.semantic_tokens.start/stop` now renamed to
`vim.lsp.semantic_tokens.enable` `vim.lsp.semantic_tokens.enable`
• Missing fields in LSP messages are now represented using |vim.NIL| instead of nil.
LUA LUA

View File

@@ -11,7 +11,7 @@ nvim -l src/gen/gen_lsp.lua --version 3.18
---@meta ---@meta
error('Cannot require a meta file') error('Cannot require a meta file')
---@alias lsp.null nil ---@alias lsp.null vim.NIL
---@alias uinteger integer ---@alias uinteger integer
---@alias decimal number ---@alias decimal number
---@alias lsp.DocumentUri string ---@alias lsp.DocumentUri string
@@ -3297,7 +3297,7 @@ error('Cannot require a meta file')
---@class lsp.FileOperationPattern ---@class lsp.FileOperationPattern
--- ---
---The glob pattern to match. Glob patterns can have the following syntax: ---The glob pattern to match. Glob patterns can have the following syntax:
---- `*` to match one or more characters in a path segment ---- `*` to match zero or more characters in a path segment
---- `?` to match on one character in a path segment ---- `?` to match on one character in a path segment
---- `**` to match any number of path segments, including none ---- `**` to match any number of path segments, including none
---- `{}` to group sub patterns into an OR expression. (e.g. `**/*.{ts,js}` matches all TypeScript and JavaScript files) ---- `{}` to group sub patterns into an OR expression. (e.g. `**/*.{ts,js}` matches all TypeScript and JavaScript files)
@@ -5699,7 +5699,7 @@ error('Cannot require a meta file')
---its resource, or a glob-pattern that is applied to the {@link TextDocument.fileName path}. ---its resource, or a glob-pattern that is applied to the {@link TextDocument.fileName path}.
--- ---
---Glob patterns can have the following syntax: ---Glob patterns can have the following syntax:
---- `*` to match one or more characters in a path segment ---- `*` to match zero or more characters in a path segment
---- `?` to match on one character in a path segment ---- `?` to match on one character in a path segment
---- `**` to match any number of path segments, including none ---- `**` to match any number of path segments, including none
---- `{}` to group sub patterns into an OR expression. (e.g. `**/*.{ts,js}` matches all TypeScript and JavaScript files) ---- `{}` to group sub patterns into an OR expression. (e.g. `**/*.{ts,js}` matches all TypeScript and JavaScript files)
@@ -5720,7 +5720,7 @@ error('Cannot require a meta file')
---@alias lsp.NotebookDocumentFilter lsp.NotebookDocumentFilterNotebookType|lsp.NotebookDocumentFilterScheme|lsp.NotebookDocumentFilterPattern ---@alias lsp.NotebookDocumentFilter lsp.NotebookDocumentFilterNotebookType|lsp.NotebookDocumentFilterScheme|lsp.NotebookDocumentFilterPattern
---The glob pattern to watch relative to the base path. Glob patterns can have the following syntax: ---The glob pattern to watch relative to the base path. Glob patterns can have the following syntax:
---- `*` to match one or more characters in a path segment ---- `*` to match zero or more characters in a path segment
---- `?` to match on one character in a path segment ---- `?` to match on one character in a path segment
---- `**` to match any number of path segments, including none ---- `**` to match any number of path segments, including none
---- `{}` to group conditions (e.g. `**/*.{ts,js}` matches all TypeScript and JavaScript files) ---- `{}` to group conditions (e.g. `**/*.{ts,js}` matches all TypeScript and JavaScript files)

View File

@@ -623,7 +623,7 @@ function Client:_process_static_registrations()
id = self.server_capabilities[capability].id, id = self.server_capabilities[capability].id,
method = method, method = method,
registerOptions = { registerOptions = {
documentSelector = self.server_capabilities[capability].documentSelector, ---@type lsp.DocumentSelector? documentSelector = self.server_capabilities[capability].documentSelector, ---@type lsp.DocumentSelector|lsp.null
}, },
} }
end end
@@ -963,15 +963,19 @@ end
function Client:_get_registration(method, bufnr) function Client:_get_registration(method, bufnr)
bufnr = vim._resolve_bufnr(bufnr) bufnr = vim._resolve_bufnr(bufnr)
for _, reg in ipairs(self.registrations[method] or {}) do for _, reg in ipairs(self.registrations[method] or {}) do
local regoptions = reg.registerOptions --[[@as {documentSelector:lsp.TextDocumentFilter[]}]] local regoptions = reg.registerOptions --[[@as {documentSelector:lsp.DocumentSelector|lsp.null}]]
if not regoptions or not regoptions.documentSelector then if
not regoptions
or regoptions == vim.NIL
or not regoptions.documentSelector
or regoptions.documentSelector == vim.NIL
then
return reg return reg
end end
local documentSelector = regoptions.documentSelector
local language = self:_get_language_id(bufnr) local language = self:_get_language_id(bufnr)
local uri = vim.uri_from_bufnr(bufnr) local uri = vim.uri_from_bufnr(bufnr)
local fname = vim.uri_to_fname(uri) local fname = vim.uri_to_fname(uri)
for _, filter in ipairs(documentSelector) do for _, filter in ipairs(regoptions.documentSelector) do
local flang, fscheme, fpat = filter.language, filter.scheme, filter.pattern local flang, fscheme, fpat = filter.language, filter.scheme, filter.pattern
if if
not (flang and language ~= flang) not (flang and language ~= flang)
@@ -1162,7 +1166,7 @@ end
--- @param method (vim.lsp.protocol.Method.ServerToClient) LSP method name --- @param method (vim.lsp.protocol.Method.ServerToClient) LSP method name
--- @param params (table) The parameters for that method --- @param params (table) The parameters for that method
--- @return any result --- @return any result
--- @return lsp.ResponseError error code and message set in case an exception happens during the request. --- @return lsp.ResponseError? error code and message set in case an exception happens during the request.
function Client:_server_request(method, params) function Client:_server_request(method, params)
log.trace('server_request', method, params) log.trace('server_request', method, params)
local handler = self:_resolve_handler(method) local handler = self:_resolve_handler(method)

View File

@@ -323,7 +323,7 @@ end
--- @package --- @package
--- @param body string --- @param body string
function Client:handle_body(body) function Client:handle_body(body)
local ok, decoded = pcall(vim.json.decode, body, { luanil = { object = true } }) local ok, decoded = pcall(vim.json.decode, body)
if not ok then if not ok then
self:on_error(M.client_errors.INVALID_SERVER_JSON, decoded) self:on_error(M.client_errors.INVALID_SERVER_JSON, decoded)
return return
@@ -355,7 +355,6 @@ function Client:handle_body(body)
) )
end end
if err then if err then
---@cast err lsp.ResponseError
assert( assert(
type(err) == 'table', type(err) == 'table',
'err must be a table. Use rpc_response_error to help format errors.' 'err must be a table. Use rpc_response_error to help format errors.'
@@ -374,8 +373,16 @@ function Client:handle_body(body)
end end
self:send_response(decoded.id, err, result) self:send_response(decoded.id, err, result)
end)) end))
-- This works because we are expecting vim.NIL here -- Proceed only if exactly one of 'result' or 'error' is present, as required by the LSP spec:
elseif decoded.id and (decoded.result ~= vim.NIL or decoded.error ~= vim.NIL) then -- - If 'error' is nil, then 'result' must be present.
-- - If 'result' is nil, then 'error' must be present (and not vim.NIL).
elseif
decoded.id
and (
(decoded.error == nil and decoded.result ~= nil)
or (decoded.result == nil and decoded.error ~= nil and decoded.error ~= vim.NIL)
)
then
-- We sent a number, so we expect a number. -- We sent a number, so we expect a number.
local result_id = assert(tonumber(decoded.id), 'response id must be a number') --[[@as integer]] local result_id = assert(tonumber(decoded.id), 'response id must be a number') --[[@as integer]]
@@ -415,7 +422,7 @@ function Client:handle_body(body)
M.client_errors.SERVER_RESULT_CALLBACK_ERROR, M.client_errors.SERVER_RESULT_CALLBACK_ERROR,
callback, callback,
decoded.error, decoded.error,
decoded.result decoded.result ~= vim.NIL and decoded.result or nil
) )
else else
self:on_error(M.client_errors.NO_RESULT_CALLBACK_FOUND, decoded) self:on_error(M.client_errors.NO_RESULT_CALLBACK_FOUND, decoded)

View File

@@ -520,7 +520,7 @@ function M.apply_text_document_edit(
-- do not check the version after the first edit. -- do not check the version after the first edit.
not (index and index > 1) not (index and index > 1)
and ( and (
text_document.version text_document.version ~= vim.NIL
and text_document.version > 0 and text_document.version > 0
and M.buf_versions[bufnr] > text_document.version and M.buf_versions[bufnr] > text_document.version
) )
@@ -827,8 +827,12 @@ function M.convert_signature_help_to_markdown_lines(signature_help, ft, triggers
if signature.parameters and #signature.parameters > 0 then if signature.parameters and #signature.parameters > 0 then
-- First check if the signature has an activeParameter. If it doesn't check if the response -- First check if the signature has an activeParameter. If it doesn't check if the response
-- had that property instead. Else just default to 0. -- had that property instead. Else just default to 0.
local active_parameter = --
math.max(signature.activeParameter or signature_help.activeParameter or 0, 0) -- NOTE: Using tonumber() as a temporary workaround to handle `vim.NIL` until #34838 is merged
local active_parameter = math.max(
tonumber(signature.activeParameter) or tonumber(signature_help.activeParameter) or 0,
0
)
-- If the activeParameter is > #parameters, then set it to the last -- If the activeParameter is > #parameters, then set it to the last
-- NOTE: this is not fully according to the spec, but a client-side interpretation -- NOTE: this is not fully according to the spec, but a client-side interpretation

View File

@@ -370,7 +370,7 @@ local function write_to_meta_protocol(protocol, version, output_file)
'---@meta', '---@meta',
"error('Cannot require a meta file')", "error('Cannot require a meta file')",
'', '',
'---@alias lsp.null nil', '---@alias lsp.null vim.NIL',
'---@alias uinteger integer', '---@alias uinteger integer',
'---@alias decimal number', '---@alias decimal number',
'---@alias lsp.DocumentUri string', '---@alias lsp.DocumentUri string',

View File

@@ -172,7 +172,7 @@ function tests.prepare_rename_nil()
body = function() body = function()
notify('start') notify('start')
expect_request('textDocument/prepareRename', function() expect_request('textDocument/prepareRename', function()
return nil, nil return {}, nil
end) end)
notify('shutdown') notify('shutdown')
end, end,
@@ -197,7 +197,7 @@ function tests.prepare_rename_placeholder()
end) end)
expect_request('textDocument/rename', function(params) expect_request('textDocument/rename', function(params)
assert_eq(params.newName, 'renameto') assert_eq(params.newName, 'renameto')
return nil, nil return {}, nil
end) end)
notify('shutdown') notify('shutdown')
end, end,
@@ -226,7 +226,7 @@ function tests.prepare_rename_range()
end) end)
expect_request('textDocument/rename', function(params) expect_request('textDocument/rename', function(params)
assert_eq(params.newName, 'renameto') assert_eq(params.newName, 'renameto')
return nil, nil return {}, nil
end) end)
notify('shutdown') notify('shutdown')
end, end,

View File

@@ -129,7 +129,7 @@ body {
exec_lua(function() exec_lua(function()
_G.server2 = _G._create_server({ _G.server2 = _G._create_server({
colorProvider = { colorProvider = {
documentSelector = nil, documentSelector = vim.NIL,
}, },
handlers = { handlers = {
['textDocument/documentColor'] = function(_, _, callback) ['textDocument/documentColor'] = function(_, _, callback)

View File

@@ -1976,7 +1976,7 @@ describe('LSP', function()
{ {
NIL, NIL,
{ {
arguments = { 'EXTRACT_METHOD', { metadata = {} }, 3, 0, 6123, NIL }, arguments = { 'EXTRACT_METHOD', { metadata = { field = vim.NIL } }, 3, 0, 6123, NIL },
command = 'refactor.perform', command = 'refactor.perform',
title = 'EXTRACT_METHOD', title = 'EXTRACT_METHOD',
}, },
@@ -4498,7 +4498,7 @@ describe('LSP', function()
name = 'prepare_rename_placeholder', name = 'prepare_rename_placeholder',
expected_handlers = { expected_handlers = {
{ NIL, {}, { method = 'shutdown', client_id = 1 } }, { NIL, {}, { method = 'shutdown', client_id = 1 } },
{ NIL, NIL, { method = 'textDocument/rename', client_id = 1, bufnr = 1 } }, { {}, NIL, { method = 'textDocument/rename', client_id = 1, bufnr = 1 } },
{ NIL, {}, { method = 'start', client_id = 1 } }, { NIL, {}, { method = 'start', client_id = 1 } },
}, },
expected_text = 'placeholder', -- see fake lsp response expected_text = 'placeholder', -- see fake lsp response
@@ -4508,7 +4508,7 @@ describe('LSP', function()
name = 'prepare_rename_range', name = 'prepare_rename_range',
expected_handlers = { expected_handlers = {
{ NIL, {}, { method = 'shutdown', client_id = 1 } }, { NIL, {}, { method = 'shutdown', client_id = 1 } },
{ NIL, NIL, { method = 'textDocument/rename', client_id = 1, bufnr = 1 } }, { {}, NIL, { method = 'textDocument/rename', client_id = 1, bufnr = 1 } },
{ NIL, {}, { method = 'start', client_id = 1 } }, { NIL, {}, { method = 'start', client_id = 1 } },
}, },
expected_text = 'line', -- see test case and fake lsp response expected_text = 'line', -- see test case and fake lsp response