Skip to content

fix: generic type resolution for class methods and iterators #3392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sumneko merged 9 commits into from
Apr 3, 2026
Merged

fix: generic type resolution for class methods and iterators #3392

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
46d0faf
Fix type inference for and/or idioms and circular dependency in tracer
RomanSpector Mar 27, 2026
eb1135e
Add changelog entries for type inference fixes
RomanSpector Mar 27, 2026
2276dc2
Fix generic class method resolution for @param self list<T> pattern
RomanSpector Mar 27, 2026
c3c783e
Fix nested generic resolution in cloneObject for doc.type.sign
RomanSpector Mar 27, 2026
a72090f
Fix ipairs(self) type resolution in generic class methods
RomanSpector Mar 28, 2026
3e19eb2
Fix double angle brackets in generic sign display (list<<T>> -> list<T>)
RomanSpector Mar 28, 2026
d31f0c8
Fix nil crash in getParentClass for doc.field without class
RomanSpector Mar 28, 2026
5c78df1
Refactor: extract shared helpers and reduce nesting
RomanSpector Mar 28, 2026
513cedd
Merge branch 'master' into fix/generic-class-resolution
sumneko Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
<!-- Add all new changes here. They will be moved under a version at release -->
* `CHG` Modified the `ResolveRequire` function to pass the source URI as a third argument.
* `CHG` Improved the output of test failures during development
* `FIX` Fix type inference for `x == nil and "default" or x` idiom [#2236](https://github.com/LuaLS/lua-language-server/issues/2236)
* `FIX` Fix type loss for assignments inside `if`/`for` blocks due to circular dependency in tracer [#2374](https://github.com/LuaLS/lua-language-server/issues/2374) [#2494](https://github.com/LuaLS/lua-language-server/issues/2494)
Comment on lines +8 to +9
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The changelog entries are for secondary fixes in this PR. Please also add entries for the main fixes related to generic type resolution as described in the PR summary. For example:

  • FIX Resolve generic class method return types.
  • FIX Correctly resolve nested generics in doc.type.sign.
  • FIX Resolve loop variables for ipairs(self) in generic methods.
  • FIX Fix display of generic types with double angle brackets (e.g., list<<T>>).
  • FIX Prevent nil crash in getParentClass for doc.field without a class.

Copy link
Copy Markdown
Contributor Author

@RomanSpector RomanSpector Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added changelog entries for all main generic resolution fixes.


## 3.17.1
`2026-01-20`
Expand Down
28 changes: 23 additions & 5 deletions script/core/diagnostics/no-unknown.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,29 @@ return function (uri, callback)
guide.eachSourceTypes(ast.ast, types, function (source)
await.delay()
if vm.getInfer(source):view(uri) == 'unknown' then
callback {
start = source.start,
finish = source.finish,
message = lang.script('DIAG_UNKNOWN'),
}
-- When a node only contains a 'variable' object whose base
-- declaration has a known type, this is a false positive caused
-- by circular dependency during compilation, not a true unknown.
local dominated = false
local node = vm.getNode(source)
if node then
for n in node:eachObject() do
if n.type == 'variable' and n.base and n.base.value then
local baseView = vm.getInfer(n.base):view(uri)
if baseView ~= 'unknown' then
dominated = true
break
end
end
end
end
if not dominated then
callback {
start = source.start,
finish = source.finish,
message = lang.script('DIAG_UNKNOWN'),
}
end
end
end)
end
46 changes: 44 additions & 2 deletions script/vm/compiler.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1671,7 +1671,22 @@ local function bindReturnOfFunction(source, mfunc, index, args)
else
local clonedObject = vm.cloneObject(nd, resolved)
if clonedObject then
result:merge(vm.compileNode(clonedObject))
if clonedObject.type == 'doc.generic.name' and clonedObject._resolved then
local allGeneric = true
for rn in clonedObject._resolved:eachObject() do
if rn.type ~= 'doc.generic.name' then
allGeneric = false
break
end
end
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic to check if a resolved node contains only generic names is duplicated in this file (lines 1796-1801) and also in script/vm/generic.lua (lines 203-209). To improve maintainability and reduce duplication, consider extracting this into a helper function in a shared module, for example on the vm object.

For example, you could define a function in vm.vm:

function vm.isPurelyGenericNode(node)
    if not node then return false end
    for rn in node:eachObject() do
        if rn.type ~= 'doc.generic.name' then
            return false
        end
    end
    return true
end

Then you could simplify this block and the other occurrences by calling vm.isPurelyGenericNode(...).

Copy link
Copy Markdown
Contributor Author

@RomanSpector RomanSpector Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Extracted vm.isResolvedToGeneric() helper in generic.lua to replace all 3 duplicated checks.

if allGeneric then
result:merge(clonedObject)
else
result:merge(vm.compileNode(clonedObject))
end
else
result:merge(vm.compileNode(clonedObject))
end
end
end
end
Expand All @@ -1686,13 +1701,25 @@ local function bindReturnOfFunction(source, mfunc, index, args)
end
end

if mfunc.type == 'function' then
if mfunc.type == 'function' or mfunc.type == 'doc.type.function' then
local hasUnresolvedGeneric = false
for rnode in returnNode:eachObject() do
if vm.isGenericUnsolved(rnode) then
hasUnresolvedGeneric = true
break
end
-- Also check inside doc.type.sign for unresolved generics
-- (e.g. list<T> where T is not yet resolved)
if rnode.type == 'doc.type.sign' and rnode.signs then
guide.eachSourceType(rnode, 'doc.generic.name', function (src)
if not src._resolved then
hasUnresolvedGeneric = true
end
end)
if hasUnresolvedGeneric then
break
end
end
end
if hasUnresolvedGeneric then
local sign = vm.getSign(mfunc)
Expand Down Expand Up @@ -1760,6 +1787,21 @@ local function bindReturnOfFunction(source, mfunc, index, args)
for rnode in returnNode:eachObject() do
if rnode.type ~= 'doc.generic.name' then
vm.setNode(source, rnode)
elseif rnode._resolved then
-- Allow generics that resolved to another generic type
-- parameter (e.g. V -> T in generic method's ipairs(self)).
-- Only allow when resolved purely to other generics, not
-- to concrete types like string/boolean.
local allGeneric = true
for rn in rnode._resolved:eachObject() do
if rn.type ~= 'doc.generic.name' then
allGeneric = false
break
end
end
if allGeneric then
vm.setNode(source, rnode)
end
end
end
if returnNode:isOptional() then
Expand Down
57 changes: 48 additions & 9 deletions script/vm/generic.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---@class vm
local vm = require 'vm.vm'
local guide = require 'parser.guide'

---@class parser.object
---@field package _generic vm.generic
Expand Down Expand Up @@ -130,16 +131,34 @@ local function cloneObject(source, resolved)
end
if source.type == 'doc.type.sign' and source.signs then
local needsClone = false
-- Check if any sign parameter has a resolvable name with a concrete
-- (non-generic) resolved type. Skip cloning when the resolved value
-- is just another doc.generic.name (e.g. T -> T inside a method body),
-- which would cause double-wrapping in display (list<<T>>).
local function hasConcreteResolution(name)
local rnode = resolved[name]
if not rnode then
return false
end
for rn in rnode:eachObject() do
if rn.type ~= 'doc.generic.name' and rn.type ~= 'generic' then
return true
end
end
return false
end
for _, sign in ipairs(source.signs) do
if sign.type == 'doc.type' then
for _, tp in ipairs(sign.types) do
if tp.type == 'doc.type.name' and resolved[tp[1]] then
guide.eachSourceType(sign, 'doc.type.name', function (src)
if hasConcreteResolution(src[1]) then
needsClone = true
end
end)
if not needsClone then
guide.eachSourceType(sign, 'doc.generic.name', function (src)
if hasConcreteResolution(src[1]) then
needsClone = true
break
end
end
elseif sign.type == 'doc.type.name' and resolved[sign[1]] then
needsClone = true
end)
end
if needsClone then break end
end
Expand Down Expand Up @@ -176,8 +195,28 @@ function mt:resolve(uri, args)
---@cast nd -vm.global, -vm.variable
local clonedObject = cloneObject(nd, resolved)
if clonedObject then
local clonedNode = vm.compileNode(clonedObject)
result:merge(clonedNode)
-- When a generic resolves to another generic (e.g. V -> T
-- inside a generic method), keep the resolved wrapper so
-- the resolution chain is preserved and downstream filters
-- can distinguish "resolved to generic T" from "unresolved".
if clonedObject.type == 'doc.generic.name' and clonedObject._resolved then
local allGeneric = true
for rn in clonedObject._resolved:eachObject() do
if rn.type ~= 'doc.generic.name' then
allGeneric = false
break
end
end
if allGeneric then
result:merge(clonedObject)
else
local clonedNode = vm.compileNode(clonedObject)
result:merge(clonedNode)
end
else
local clonedNode = vm.compileNode(clonedObject)
result:merge(clonedNode)
end
end
end
end
Expand Down
8 changes: 7 additions & 1 deletion script/vm/infer.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,13 @@ local viewNodeSwitch;viewNodeSwitch = util.switch()
infer._hasClass = true
local buf = {}
for i, sign in ipairs(source.signs) do
buf[i] = vm.getInfer(sign):view(uri)
local view = vm.getInfer(sign):view(uri)
-- Strip outer <> from generic names since the sign
-- already wraps parameters in <>, avoiding list<<T>>
if view and view:sub(1, 1) == '<' and view:sub(-1) == '>' then
view = view:sub(2, -2)
end
buf[i] = view
end
local node = vm.compileNode(source)
for c in node:eachObject() do
Expand Down
114 changes: 93 additions & 21 deletions script/vm/sign.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,28 +83,83 @@ function mt:resolve(uri, args)
return
end
if object.type == 'doc.type.array' then
-- If the argument contains a doc.type.sign (generic class like
-- list<T> extending { [integer]: V }), resolve element type
-- exclusively through class generic map. This directly maps
-- the array element generic (V) to the sign parameter, even
-- when it's another generic name (T inside a method body).
local handled = false
for n in node:eachObject() do
if n.type == 'doc.type.array' then
-- number[] -> T[]
resolve(object.node, vm.compileNode(n.node))
end
if n.type == 'doc.type.table' then
-- { [integer]: number } -> T[]
local tvalueNode = vm.getTableValue(uri, node, 'integer', true)
if tvalueNode then
resolve(object.node, tvalueNode)
if n.type == 'doc.type.sign' and n.signs and n.node and n.node[1] then
local classGlobal = vm.getGlobal('type', n.node[1])
if classGlobal then
local genericMap = vm.getClassGenericMap(uri, classGlobal, n.signs)
if genericMap and object.node and object.node.type == 'doc.generic.name' then
-- V[] matching list<T>: look up [integer] field,
-- find which class generic it references, then
-- map V directly to the sign's concrete parameter
local vKey = object.node[1]
-- First try @field annotations
vm.getClassFields(uri, classGlobal, vm.declareGlobal('type', 'integer'), function (field)
if field.extends then
guide.eachSourceType(field.extends, 'doc.generic.name', function (src)
if genericMap[src[1]] then
resolved[vKey] = genericMap[src[1]]
handled = true
end
end)
end
end)
-- Also search extends tables (for @class list<T>: {[integer]:T})
if not handled then
for _, set in ipairs(classGlobal:getSets(uri)) do
if set.type == 'doc.class' and set.extends then
for _, ext in ipairs(set.extends) do
if ext.type == 'doc.type.table' and ext.fields then
for _, field in ipairs(ext.fields) do
if field.extends then
guide.eachSourceType(field.extends, 'doc.generic.name', function (src)
if genericMap[src[1]] then
resolved[vKey] = genericMap[src[1]]
handled = true
end
end)
end
if handled then break end
end
end
if handled then break end
end
end
if handled then break end
end
end
Comment on lines +150 to +167
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block of code has a couple of areas for improvement:

  1. Deep Nesting: The logic to search extends tables (lines 114-136) is nested up to 6 levels deep. This reduces readability and makes the code harder to maintain. Consider refactoring this into smaller helper functions to reduce the nesting depth.

  2. Inefficient Loops: The callbacks to guide.eachSourceType (lines 105-110 and 121-126) set handled = true, but the loops continue to iterate. If guide.eachSourceType can be made to break early (e.g., by returning a value from the callback), it would improve efficiency.

Copy link
Copy Markdown
Contributor Author

@RomanSpector RomanSpector Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Extracted findGenericInTableFields() and simplified findGenericInExtendsTable() — max nesting reduced from 6 to 3 levels.

end
end
if handled then break end
end
if n.type == 'global' and n.cate == 'type' then
-- ---@field [integer]: number -> T[]
---@cast n vm.global
vm.getClassFields(uri, n, vm.declareGlobal('type', 'integer'), function (field)
resolve(object.node, vm.compileNode(field.extends))
end)
end
if n.type == 'table' and #n >= 1 then
-- { x } / { ... } -> T[]
resolve(object.node, vm.compileNode(n[1]))
end
if not handled then
for n in node:eachObject() do
if n.type == 'doc.type.array' then
-- number[] -> T[]
resolve(object.node, vm.compileNode(n.node))
elseif n.type == 'doc.type.table' then
-- { [integer]: number } -> T[]
local tvalueNode = vm.getTableValue(uri, node, 'integer', true)
if tvalueNode then
resolve(object.node, tvalueNode)
end
elseif n.type == 'global' and n.cate == 'type' then
-- ---@field [integer]: number -> T[]
---@cast n vm.global
vm.getClassFields(uri, n, vm.declareGlobal('type', 'integer'), function (field)
resolve(object.node, vm.compileNode(field.extends))
end)
elseif n.type == 'table' and #n >= 1 then
-- { x } / { ... } -> T[]
resolve(object.node, vm.compileNode(n[1]))
end
end
end
return
Expand Down Expand Up @@ -176,6 +231,21 @@ function mt:resolve(uri, args)
end
return
end
if object.type == 'doc.type.sign' and object.signs then
-- list<T> -> list<string>: match sign parameters positionally
for n in node:eachObject() do
if n.type == 'doc.type.sign' and n.signs
and n.node and object.node
and n.node[1] == object.node[1] then
for i, signParam in ipairs(object.signs) do
if n.signs[i] then
resolve(vm.compileNode(signParam), vm.compileNode(n.signs[i]))
end
end
end
end
return
end
end

---@param sign vm.node
Expand All @@ -191,7 +261,8 @@ function mt:resolve(uri, args)
end
if obj.type == 'doc.type.table'
or obj.type == 'doc.type.function'
or obj.type == 'doc.type.array' then
or obj.type == 'doc.type.array'
or obj.type == 'doc.type.sign' then
---@cast obj parser.object
local hasGeneric
guide.eachSourceType(obj, 'doc.generic.name', function (src)
Expand All @@ -203,7 +274,8 @@ function mt:resolve(uri, args)
end
end
if obj.type == 'variable'
or obj.type == 'local' then
or obj.type == 'local'
or obj.type == 'self' then
goto CONTINUE
end
local view = vm.getInfer(obj):view(uri)
Expand Down
Loading
Loading