diff --git a/runtime/doc/api.txt b/runtime/doc/api.txt index f643bab7a0..13b4c0a6f1 100644 --- a/runtime/doc/api.txt +++ b/runtime/doc/api.txt @@ -196,6 +196,7 @@ About the `functions` map: - Container types may be decorated with type/size constraints, e.g. ArrayOf(Buffer) or ArrayOf(Integer, 2). + - Each function `parameters` item is a `[type, name, optional]` tuple. - Functions considered to be methods that operate on instances of Nvim special types (msgpack EXT) have the "method=true" flag. The receiver type is that of the first argument. Method names are prefixed with `nvim_` plus @@ -244,6 +245,9 @@ As Nvim evolves the API may change in compliance with this CONTRACT: new return value. - An optional `opts` parameter may be ADDED. - Optional parameters may be ADDED following an `opts` parameter. + - The `opts` parameter and any following parameters MAY BE OMITTED by the + client (equivalent to passing an empty dict). Explicitly passing a nil + value is an error. - Event parameters will not be removed or reordered (after release). - Events may be EXTENDED: new parameters may be added. - Deprecated functions will not be removed until Nvim 2.0. diff --git a/runtime/doc/lsp.txt b/runtime/doc/lsp.txt index 328dec84ea..0fc7410478 100644 --- a/runtime/doc/lsp.txt +++ b/runtime/doc/lsp.txt @@ -279,7 +279,7 @@ LspAttach handler. Example: Enable auto-completion and auto-formatting ("linting"): >lua vim.api.nvim_create_autocmd('LspAttach', { - group = vim.api.nvim_create_augroup('my.lsp', {}), + group = vim.api.nvim_create_augroup('my.lsp'), callback = function(ev) local client = assert(vim.lsp.get_client_by_id(ev.data.client_id)) if client:supports_method('textDocument/implementation') then diff --git a/runtime/doc/lua.txt b/runtime/doc/lua.txt index 5aca77d2e3..b3a3088d6f 100644 --- a/runtime/doc/lua.txt +++ b/runtime/doc/lua.txt @@ -4596,7 +4596,7 @@ mark({buf}, {lnum}, {col}) *vim.pos.mark()* Creates a new |vim.Pos| from mark position (see |api-indexing|). Example: >lua - local mark_info = vim.api.nvim_get_mark('M', {}) + local mark_info = vim.api.nvim_get_mark('M') local lnum, col, buf, name = unpack(mark_info) if lnum == 0 and col == 0 and buf == 0 then @@ -4688,7 +4688,7 @@ to_mark({pos}) *vim.pos.to_mark()* -- Convert to mark position, you can call it in a method style. local lnum, col = pos:to_mark() - vim.api.nvim_buf_set_mark(0, 'M', lnum, col, {}) + vim.api.nvim_buf_set_mark(0, 'M', lnum, col) < Parameters: ~ diff --git a/runtime/doc/news.txt b/runtime/doc/news.txt index f1d2aa01ab..85338c5eaa 100644 --- a/runtime/doc/news.txt +++ b/runtime/doc/news.txt @@ -141,8 +141,15 @@ API • |nvim_win_resize()| resizes a window with a specified anchor edge. |nvim_win_set_height()| and |nvim_win_set_width()| are now deprecated. -• |api-contract| allows existing functions to add an optional trailing `opts` - parameter, which clients may omit. +• |api-contract| treats `opts` and any parameters thereafter, as optional + (equivalent to passing an empty dict). Clients may omit such optional + parameters. + • The Lua |vim.api| bridge is also a "client" and thus may omit `opts` and + any following parameters: >lua + -- Before: + vim.print(vim.api.nvim_get_mark('A', {})) + -- After: + vim.print(vim.api.nvim_get_mark('A')) • |nvim_buf_call()| and |nvim_win_call()| now preserve multiple return values. • |nvim_set_hl()| supports "font" key. • |nvim_open_win()| `zindex` controls whether the UI will use a dimmed cursor diff --git a/runtime/lua/vim/pos.lua b/runtime/lua/vim/pos.lua index 9f1f61b446..df55e5e431 100644 --- a/runtime/lua/vim/pos.lua +++ b/runtime/lua/vim/pos.lua @@ -210,7 +210,7 @@ end --- --- -- Convert to mark position, you can call it in a method style. --- local lnum, col = pos:to_mark() ---- vim.api.nvim_buf_set_mark(0, 'M', lnum, col, {}) +--- vim.api.nvim_buf_set_mark(0, 'M', lnum, col) --- ``` ---@param pos vim.Pos ---@return integer lnum, integer col @@ -223,7 +223,7 @@ end --- --- Example: --- ```lua ---- local mark_info = vim.api.nvim_get_mark('M', {}) +--- local mark_info = vim.api.nvim_get_mark('M') --- local lnum, col, buf, name = unpack(mark_info) --- --- if lnum == 0 and col == 0 and buf == 0 then diff --git a/src/gen/gen_api_dispatch.lua b/src/gen/gen_api_dispatch.lua index 9a224d3077..990f6f0a01 100644 --- a/src/gen/gen_api_dispatch.lua +++ b/src/gen/gen_api_dispatch.lua @@ -59,7 +59,7 @@ end --- @field can_fail? true --- @field has_lua_imp? true --- @field receives_arena? true ---- @field has_opts? true +--- @field opts_idx? integer Index of the `opts` param: starts optional params. #31903 --- @field impl_name? string --- @field remote? boolean --- @field lua? boolean @@ -117,12 +117,22 @@ local function add_function(fn) fn.receives_arena = true fn.parameters[#fn.parameters] = nil end - if - #fn.parameters ~= 0 - and fn.parameters[#fn.parameters][2] == 'opts' - and real_type(fn.parameters[#fn.parameters][1]):match('^KeyDict_') - then - fn.has_opts = true + -- `opts` (a KeyDict) and every parameter following it are optional: clients may omit them. + for k = 1, #fn.parameters do + if fn.parameters[k][2] == 'opts' and real_type(fn.parameters[k][1]):match('^KeyDict_') then + fn.opts_idx = k + break + end + end + -- Omitted optional param is zero-initialized, which assumes Dict/Array. + if fn.opts_idx then + for k = fn.opts_idx + 1, #fn.parameters do + local rt = real_type(fn.parameters[k][1]) + if not (rt == 'Dict' or rt == 'Dictionary' or rt == 'Array' or rt:match('^KeyDict_')) then + local msg = '%s: optional param "%s" has type "%s" but (currently) we assume Dict/Array' + error(msg:format(fn.name, fn.parameters[k][2], fn.parameters[k][1])) + end + end end end end @@ -280,7 +290,7 @@ end --- don't expose internal attributes like "impl_name" in public metadata --- @class gen_api_dispatch.Function.Exported --- @field name string ---- @field parameters [string, string][] +--- @field parameters [string, string, boolean][] each param is `[ type, name, optional ]` --- @field return_type string --- @field method boolean --- @field since integer @@ -301,7 +311,9 @@ for _, f in ipairs(functions) do return_type = real_type(f.return_type, true), } for i, param in ipairs(f.parameters) do - f_exported.parameters[i] = { real_type(param[1], true), param[2] } + -- `opts` and every param after it are optional. + local optional = f.opts_idx ~= nil and i >= f.opts_idx + f_exported.parameters[i] = { real_type(param[1], true), param[2], optional } end exported_functions[#exported_functions + 1] = f_exported end @@ -439,7 +451,8 @@ for i = 1, #functions do local param = fn.parameters[j] local rt = real_type(param[1]) local converted = 'arg_' .. j - if fn.has_opts and j == #fn.parameters then + -- Optional params (`opts` + rest) are currently assumed to be Dict/Array types (enforced in add_function). + if fn.opts_idx and j >= fn.opts_idx then output:write('\n ' .. rt .. ' ' .. converted .. ' = { 0 };') else output:write('\n ' .. rt .. ' ' .. converted .. ';') @@ -447,42 +460,23 @@ for i = 1, #functions do end output:write('\n') if not fn.receives_array_args then - if fn.has_opts then - local minargs = #fn.parameters - 1 - if minargs > 0 then - output:write( - '\n if (args.size < ' .. minargs .. ' || args.size > ' .. #fn.parameters .. ') {' - ) - output:write( - '\n api_set_error(error, kErrorTypeException, \ - "Wrong number of arguments: expecting ' - .. minargs - .. ' to ' - .. #fn.parameters - .. ' but got %zu", args.size);' - ) - else - output:write('\n if (args.size > ' .. #fn.parameters .. ') {') - output:write( - '\n api_set_error(error, kErrorTypeException, \ - "Wrong number of arguments: expecting at most ' - .. #fn.parameters - .. ' but got %zu", args.size);' - ) - end - output:write('\n goto cleanup;') - output:write('\n }\n') + -- `opts` and following params are optional: accept minargs..maxargs; else require maxargs. + local maxargs = #fn.parameters + local minargs = fn.opts_idx and fn.opts_idx - 1 or maxargs + local cond, expect + if minargs == maxargs then + cond, expect = 'args.size != ' .. maxargs, maxargs + elseif minargs > 0 then + cond, expect = + 'args.size < ' .. minargs .. ' || args.size > ' .. maxargs, minargs .. ' to ' .. maxargs else - output:write('\n if (args.size != ' .. #fn.parameters .. ') {') - output:write( - '\n api_set_error(error, kErrorTypeException, \ - "Wrong number of arguments: expecting ' - .. #fn.parameters - .. ' but got %zu", args.size);' - ) - output:write('\n goto cleanup;') - output:write('\n }\n') + cond, expect = 'args.size > ' .. maxargs, 'at most ' .. maxargs end + output:write('\n if (' .. cond .. ') {') + output:write('\n api_set_error(error, kErrorTypeException, \ + "Wrong number of arguments: expecting ' .. expect .. ' but got %zu", args.size);') + output:write('\n goto cleanup;') + output:write('\n }\n') end -- Validation/conversion for each argument @@ -491,14 +485,15 @@ for i = 1, #functions do param = fn.parameters[j] converted = 'arg_' .. j local rt = real_type(param[1]) + -- `opts` + rest are optional: ensure omitted arg keeps its zero-initialized default. + local optional = fn.opts_idx and j >= fn.opts_idx + if optional then + output:write('\n if (args.size >= ' .. j .. ') {') + end if rt == 'Object' then output:write('\n ' .. converted .. ' = args.items[' .. (j - 1) .. '];\n') elseif rt:match('^KeyDict_') then converted = '&' .. converted - local optional = fn.has_opts and j == #fn.parameters - if optional then - output:write('\n if (args.size == ' .. j .. ') {') - end output:write('\n if (args.items[' .. (j - 1) .. '].type == kObjectTypeDict) {') --luacheck: ignore 631 output:write('\n memset(' .. converted .. ', 0, sizeof(*' .. converted .. '));') -- TODO: neeeee output:write( @@ -534,9 +529,6 @@ for i = 1, #functions do ) output:write('\n goto cleanup;') output:write('\n }\n') - if optional then - output:write('\n }\n') - end else if rt:match('^Buffer$') or rt:match('^Window$') or rt:match('^Tabpage$') then -- Buffer, Window, and Tabpage have a specific type, but are stored in integer @@ -614,6 +606,9 @@ for i = 1, #functions do output:write('\n goto cleanup;') output:write('\n }\n') end + if optional then + output:write('\n }\n') + end args[#args + 1] = converted end @@ -779,45 +774,30 @@ local function process_function(fn) ]], lua_c_function_name ) - if fn.has_opts then - local minargs = #fn.parameters - 1 - if minargs > 0 then - write_shifted_output( - [[ - if (lua_gettop(lstate) < %i || lua_gettop(lstate) > %i) { - api_set_error(&err, kErrorTypeValidation, "Expected %i to %i arguments"); - goto exit_0; - } - ]], - minargs, - #fn.parameters, - minargs, - #fn.parameters - ) + do + -- `opts` + rest are optional: accept minargs..maxargs; else require maxargs. + local maxargs = #fn.parameters + local minargs = fn.opts_idx and fn.opts_idx - 1 or maxargs + local cond, msg + if minargs == maxargs then + cond = ('lua_gettop(lstate) != %d'):format(maxargs) + msg = ('Expected %d argument%s'):format(maxargs, maxargs == 1 and '' or 's') + elseif minargs > 0 then + cond = ('lua_gettop(lstate) < %d || lua_gettop(lstate) > %d'):format(minargs, maxargs) + msg = ('Expected %d to %d arguments'):format(minargs, maxargs) else - write_shifted_output( - [[ - if (lua_gettop(lstate) > %i) { - api_set_error(&err, kErrorTypeValidation, "Expected at most %i argument%s"); - goto exit_0; - } - ]], - #fn.parameters, - #fn.parameters, - (#fn.parameters == 1) and '' or 's' - ) + cond = ('lua_gettop(lstate) > %d'):format(maxargs) + msg = ('Expected at most %d argument%s'):format(maxargs, maxargs == 1 and '' or 's') end - else write_shifted_output( [[ - if (lua_gettop(lstate) != %i) { - api_set_error(&err, kErrorTypeValidation, "Expected %i argument%s"); + if (%s) { + api_set_error(&err, kErrorTypeValidation, "%s"); goto exit_0; } ]], - #fn.parameters, - #fn.parameters, - (#fn.parameters == 1) and '' or 's' + cond, + msg ) end lua_c_functions[#lua_c_functions + 1] = { @@ -864,7 +844,9 @@ local function process_function(fn) end local errshift = 0 local seterr = '' - local optional = fn.has_opts and j == #fn.parameters + -- `opts` + rest are optional. Params pop in reverse, so the stack top holds arg `j` exactly + -- when `lua_gettop(lstate) == j`; else it was omitted. + local optional = fn.opts_idx and j >= fn.opts_idx if param_type:match('^KeyDict_') then if optional then write_shifted_output( @@ -875,7 +857,7 @@ local function process_function(fn) ]], param_type, cparam, - #fn.parameters, + j, cparam, param_type ) @@ -898,6 +880,21 @@ local function process_function(fn) .. ', ' .. param_type:sub(9) .. '_table);' + elseif optional then + -- Conditionally pop; keep a zero-init `{ 0 }` default when the arg was omitted. + write_shifted_output( + [[ + %s %s = { 0 }; + if (lua_gettop(lstate) == %d) { + %s = nlua_pop_%s(lstate, %s&arena, &err);]], + param[1], + cparam, + j, + cparam, + param_type, + extra + ) + seterr = '\n err_param = "' .. param[2] .. '";' else write_shifted_output( [[ diff --git a/test/functional/api/version_spec.lua b/test/functional/api/version_spec.lua index ba3bb7a0f2..c77683b28d 100644 --- a/test/functional/api/version_spec.lua +++ b/test/functional/api/version_spec.lua @@ -7,15 +7,10 @@ local matches = t.matches local pcall_err = t.pcall_err local function read_mpack_file(fname) - local fd = io.open(fname, 'rb') - if fd == nil then + if vim.fn.filereadable(fname) == 0 then return nil end - - local data = fd:read('*a') - fd:close() - local unpack = vim.mpack.Unpacker() - return unpack(data) + return vim.mpack.Unpacker()(vim.fn.readblob(fname)) end describe("api_info()['version']", function() @@ -74,6 +69,9 @@ describe('api metadata', function() f.parameters[idx][1] = f.parameters[idx][1]:gsub('ArrayOf%(.*', 'Array') f.parameters[idx][2] = '' -- Remove parameter name. + -- Strip the `optional` flag: `assert_func_backcompat` checks it asymmetrically (relaxing a + -- param to optional is allowed, the reverse is breaking). + f.parameters[idx][3] = nil end if string.sub(f.name, 1, 4) ~= 'nvim' then @@ -87,6 +85,13 @@ describe('api metadata', function() --- @param old_fn gen_api_dispatch.Function.Exported --- @param new_fn gen_api_dispatch.Function.Exported local function assert_func_backcompat(old_fn, new_fn) + -- Optional param must stay optional. + for idx, old_param in ipairs(old_fn.parameters) do + local new_param = new_fn.parameters[idx] + if old_param[3] and new_param and not new_param[3] then + error(('"%s": parameter %d was optional, now required'):format(old_fn.name, idx)) + end + end old_fn = normalize_func_metadata(old_fn) new_fn = normalize_func_metadata(new_fn) if old_fn.return_type == 'void' then @@ -160,6 +165,14 @@ describe('api metadata', function() eq('ArrayOf(String)', funcs.nvim_buf_set_lines.parameters[5][1]) end) + it('function parameters', function() + local funcs = name_table(api_info.functions) + eq({ 'String', 'src', false }, funcs.nvim_exec2.parameters[1]) + eq({ 'Dict', 'opts', true }, funcs.nvim_exec2.parameters[2]) + eq({ 'Dict', 'opts', true }, funcs.nvim_get_context.parameters[1]) + eq({ 'String', 'name', false }, funcs.nvim_get_var.parameters[1]) + end) + it('functions are compatible with old metadata or have new level', function() local funcs_new = name_table(api_info.functions) local funcs_compat = {} @@ -215,6 +228,25 @@ describe('api metadata', function() end end) + it('param may not change from optional to required', function() + local function example_fn(opts_optional) + return { + name = 'nvim_example', + method = false, + since = 1, + return_type = 'void', + parameters = { { 'String', 'src', false }, { 'Dict', 'opts', opts_optional } }, + } + end + -- "Required -> Optional" is allowed. + assert_func_backcompat(example_fn(false), example_fn(true)) + -- "Optional -> Required" is breaking. + matches( + '"nvim_example": parameter 2 was optional, now required', + pcall_err(assert_func_backcompat, example_fn(true), example_fn(false)) + ) + end) + it('UI events are compatible with old metadata or have new level', function() local ui_events_new = name_table(api_info.ui_events) local ui_events_compat = {} @@ -283,20 +315,32 @@ describe('api metadata', function() end) end) -describe('api: optional trailing opts parameter', function() +describe('api: optional parameters', function() before_each(clear) it('may be omitted', function() + eq('table', type(api.nvim_get_context({}))) + eq('table', type(api.nvim_get_context())) + api.nvim_exec2('let g:x = 41') eq(41, api.nvim_get_var('x')) eq({ output = 'hi' }, api.nvim_exec2('echo "hi"', { output = true })) + + -- Lua bridge: vim.api + n.exec_lua([[vim.api.nvim_exec2('let g:y = 7')]]) + eq(7, api.nvim_get_var('y')) + eq('table', n.exec_lua('return type(vim.api.nvim_get_context())')) end) - it('may be omitted when it is the only parameter', function() - eq('table', type(api.nvim_get_context())) + it('omitted is equivalent to empty dict', function() + eq(api.nvim_get_context({}), api.nvim_get_context()) -- RPC path + eq( + n.exec_lua('return vim.api.nvim_get_context({})'), + n.exec_lua('return vim.api.nvim_get_context()') + ) -- Lua-binding path end) - it('errors on too many arguments', function() + it('validation', function() matches( 'Wrong number of arguments: expecting 1 to 2 but got 3', pcall_err(api.nvim_exec2, 'echo 1', {}, 'surplus') @@ -305,9 +349,17 @@ describe('api: optional trailing opts parameter', function() 'Wrong number of arguments: expecting at most 1 but got 2', pcall_err(api.nvim_get_context, {}, 'surplus') ) - end) - - it('errors on too few arguments', function() matches('Wrong number of arguments: expecting 1 to 2 but got 0', pcall_err(api.nvim_exec2)) + + -- Lua bridge: vim.api + matches( + 'Expected 1 to 2 arguments', + pcall_err(n.exec_lua, [[vim.api.nvim_exec2('echo 1', {}, 'surplus')]]) + ) + matches('Expected 1 to 2 arguments', pcall_err(n.exec_lua, [[vim.api.nvim_exec2()]])) + matches( + 'Expected at most 1 argument', + pcall_err(n.exec_lua, [[vim.api.nvim_get_context({}, 'surplus')]]) + ) end) end)