Feature: Narrow overload candidates by preceding argument types#3430
Feature: Narrow overload candidates by preceding argument types#3430lamia-zamia wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to narrow overload candidates based on the types of preceding arguments, improving autocompletion and param-type-mismatch diagnostics. The feedback identifies a critical misalignment bug in vm.isPriorArgsMatched when handling method calls (using ":"), where the implicit "self" parameter causes arguments to be matched against incorrect parameters. The reviewer suggests passing the full "call" AST node instead of just "callArgs" to detect method calls and offset the parameter index accordingly, along with the necessary updates to the calling sites.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ---@param uri uri | ||
| ---@param func parser.object -- `function` or `doc.type.function` | ||
| ---@param callArgs parser.object[] | ||
| ---@param myIndex integer | ||
| ---@param fixIndex? integer | ||
| ---@return boolean | ||
| function vm.isPriorArgsMatched(uri, func, callArgs, myIndex, fixIndex) | ||
| fixIndex = fixIndex or 0 | ||
| local params = func.args | ||
| if not params then | ||
| return true | ||
| end | ||
| for i = 1, myIndex - 1 do | ||
| local callArg = callArgs[i + fixIndex] | ||
| local param = params[i] | ||
| if not callArg or not param then | ||
| break | ||
| end | ||
| if callArg.type ~= '...' | ||
| and param.type ~= 'self' | ||
| and not isVarargParam(param) then | ||
| local defNode = vm.compileNode(param) | ||
| local refNode = vm.compileNode(callArg) | ||
| if not vm.canCastType(uri, defNode, refNode) then | ||
| return false | ||
| end | ||
| end | ||
| end | ||
| return true | ||
| end |
There was a problem hiding this comment.
There is a misalignment bug when matching prior arguments for method calls (using :).
In Lua, a method definition like function class:method(arg1, arg2) implicitly has self as its first parameter, so func.args is { self, arg1, arg2 }. However, a method call like obj:method(arg1, arg2) only has explicit arguments in call.args, i.e., { arg1, arg2 }.
When checking prior arguments for arg2 (myIndex = 2), the loop compares callArgs[1] (arg1) against params[1] (self). Since params[1] is self, it is skipped, meaning arg1 is never validated. For calls with 3 or more arguments, this misalignment causes arguments to be compared against the wrong parameters (e.g., callArgs[2] compared against params[2] which is arg1's parameter instead of arg2's), leading to incorrect overload filtering and false-positive diagnostics.
We can fix this by passing the call AST node instead of callArgs and checking if call.node.type == 'getmethod' to offset the parameter index by 1 when self is present.
---@param uri uri
---@param func parser.object -- `function` or `doc.type.function`
---@param call parser.object
---@param myIndex integer
---@param fixIndex? integer
---@return boolean
function vm.isPriorArgsMatched(uri, func, call, myIndex, fixIndex)
local callArgs = call.args
if not callArgs then
return true
end
fixIndex = fixIndex or 0
local params = func.args
if not params then
return true
end
local paramOffset = 0
if call.node and call.node.type == 'getmethod' then
if params[1] and params[1].type == 'self' then
paramOffset = 1
end
end
for i = 1, myIndex - 1 do
local callArg = callArgs[i + fixIndex]
local param = params[i + paramOffset]
if not callArg or not param then
break
end
if callArg.type ~= '...'
and param.type ~= 'self'
and not isVarargParam(param) then
local defNode = vm.compileNode(param)
local refNode = vm.compileNode(callArg)
if not vm.canCastType(uri, defNode, refNode) then
return false
end
end
end
return true
end
| ---@param uri uri | ||
| ---@param funcNode vm.node | ||
| ---@param callArgs parser.object[] | ||
| ---@param i integer | ||
| ---@param classGenericMap table<string, vm.node>? | ||
| ---@return vm.node? | ||
| local function getDefNode(funcNode, i, classGenericMap) | ||
| local defNode = vm.createNode() | ||
| ---@return parser.object[] | ||
| local function getCheckableFunctions(uri, funcNode, callArgs, i) | ||
| local funcs = {} | ||
| for src in funcNode:eachObject() do | ||
| if src.type == 'function' | ||
| or src.type == 'doc.type.function' then | ||
| funcs[#funcs+1] = src | ||
| end | ||
| end | ||
| if #funcs > 1 then | ||
| local matched = {} | ||
| for _, src in ipairs(funcs) do | ||
| if vm.isPriorArgsMatched(uri, src, callArgs, i) then | ||
| matched[#matched+1] = src | ||
| end | ||
| end | ||
| if #matched > 0 then | ||
| funcs = matched | ||
| end | ||
| end | ||
| return funcs | ||
| end |
There was a problem hiding this comment.
Update getCheckableFunctions to accept the call AST node (source) instead of callArgs to support the parameter alignment fix in vm.isPriorArgsMatched.
---@param uri uri
---@param funcNode vm.node
---@param call parser.object
---@param i integer
---@return parser.object[]
local function getCheckableFunctions(uri, funcNode, call, i)
local funcs = {}
for src in funcNode:eachObject() do
if src.type == 'function'
or src.type == 'doc.type.function' then
funcs[#funcs+1] = src
end
end
if #funcs > 1 then
local matched = {}
for _, src in ipairs(funcs) do
if vm.isPriorArgsMatched(uri, src, call, i) then
matched[#matched+1] = src
end
end
if #matched > 0 then
funcs = matched
end
end
return funcs
end
| goto CONTINUE | ||
| end | ||
| local defNode = getDefNode(funcNode, i, classGenericMap) | ||
| local funcs = getCheckableFunctions(uri, funcNode, source.args, i) |
| if #docFuncs > 1 and call.args then | ||
| local uri = guide.getUri(arg) | ||
| local matched = {} | ||
| for _, n in ipairs(docFuncs) do | ||
| if vm.isPriorArgsMatched(uri, n, call.args, myIndex, fixIndex) then | ||
| matched[#matched+1] = n | ||
| end | ||
| end | ||
| if #matched > 0 then | ||
| docFuncs = matched | ||
| end | ||
| end |
There was a problem hiding this comment.
Pass call instead of call.args to vm.isPriorArgsMatched to support the parameter alignment fix.
if #docFuncs > 1 and call.args then
local uri = guide.getUri(arg)
local matched = {}
for _, n in ipairs(docFuncs) do
if vm.isPriorArgsMatched(uri, n, call, myIndex, fixIndex) then
matched[#matched+1] = n
end
end
if #matched > 0 then
docFuncs = matched
end
end
|
bad bot |
|
maybe #2276? |
Summary
Narrow LuaDoc overload candidates using the inferred types of arguments that precede the parameter being completed or checked.
This allows later parameter types to depend on an earlier class-typed argument, consistently across completion and
param-type-mismatchdiagnostics.Problem
Given overloads whose later parameters depend on the type of an earlier argument:
LuaLS previously combined the
fieldtypes from every overload. Completion therefore suggested fields from both component types, and"mana"was accepted even though the first argument wasA.Component.Changes
vm.canCastType.param-type-mismatchdiagnostics and diagnostic messages.With the example above, completion now only suggests
"hp"and"max_hp", and passing"mana"reportsparam-type-mismatch.Testing