Overloaded methods do not properly return a generic bound to the self parameter

How are you using the lua-language-server?

Visual Studio Code Extension (sumneko.lua)

Which OS are you using?

Windows

What is the issue affecting?

Type Checking

Expected Behaviour

Given the following code:

---@class A
local A = {}

---@generic S
---@param self S
---@param str string
---@return S
function A:method1(str) end

---@generic S
---@param self S
---@param num1 number
---@param num2? number
---@param num3? number
---@return S
function A:method1(num1, num2, num3) end

---@class B: A
local B = {}

local x = B:method1("str")
local y = B:method1(1)
local z = B:method1(1, 2, 3)

The following should be true:

local x = B:method1("str")   --> local x: B
local y = B:method1(1)       --> local y: B
local z = B:method1(1, 2, 3) --> local z: B

All three local variables should be the B type as that is what generic S would be during this method call.

Actual Behaviour

The following is given instead by LuaLS:

local x = B:method1("str")   --> local x: unknown
local y = B:method1(1)       --> local y: unknown
local z = B:method1(1, 2, 3) --> local z: B

From my testing, it seems that given any amount of overloads and a method call the following seems to happen. Whether this is actually what happens in LuaLS is a different story as I could not figure out why this happens.

Change the method call's parameters to be all anys
B:method1("str") --> B:method1(any)
Check if that new call matches more than one overload.
B:method1(any) matches A:method1(str: string)
B:method1(any) matches A:method1(num1: number, num2?: number, num3?: number)
If multiple overloads were matched, return unknown instead.

Reproduction steps

Paste the following code into a lua file and open it:
(Alternatively, use the test cases file found in the "Additional Notes" section)

---@class A
local A = {}

---@generic S
---@param self S
---@param str string
---@return S
function A:method1(str) end

---@generic S
---@param self S
---@param num1 number
---@param num2? number
---@param num3? number
---@return S
function A:method1(num1, num2, num3) end

---@class B: A
local B = {}

local x = B:method1("str")
local y = B:method1(1)
local z = B:method1(1, 2, 3)

Hover over locals x, y, and z.
Notice that locals x and y have the wrong type while local z is fine.

Additional Notes

This method of returning self is being done because the self keyword does not actually resolve to the class being used for the method call and always seems to be class that created the method instead.

This error started appearing in 3.10.1 and has not been fixed yet as of 3.13.6.

(The testing was done before 3.13.6 was released and I have personally tested 3.13.6 and confirm that it is still the same as 3.13.5)

The following file was used to get the information for the above chart.
testcases.txt

Reverting the changes made by #2898 and #2765 appears to fix this issue (while also re-introducing the bugs those PRs fixed.)

I have searched for other issues related to this, but only found mostly unrelated issues. However, i did find one comment on issue #723 that mentions this exact bug and the PR following it (#2898) seemed to only fix half of the issue. (As shown by version 3.12.0 correcting more than half of the test cases in the test case file.)

Log File

file_d%3A__test.log

Hi, I am the author of #2898 and #2765 (though I am not author of LuaLS).

This is a very detailed bug report 👍
I will try to take a look when I have time.
But I cannot promise when / or even if possible I can come up a fix with it 🙁 .

It's been a while since I wrote those fix PRs.
I can give more details on it in case you want to do further debugging yourself.

PR #2765

This tries to fix #2758, where the infer on functions with overloads are inconsistent.

After quite some debugging, I believe the server is keeping outdated function node cache.
So the fix is to reset those nodes to allow recomputing.
This is released in 3.10.0

run time error in #2776

However the fix at that time caused a run time
sumneko (author of LuaLS) fixed it in commit 8ecec08
and this "fix" is released in 3.10.1

The regression fix

As you have already found out, that I then found a regression issue while discussing in [Feature request] generics in overload #723
I came up with the fix in fix: regression related to type narrow and generic since v3.10.1 #2898, but I was unsure how that issue occur
And quite unfortunately, this seemed to only fix half of the issue 🤦‍♂

Seems we need to find out why the run time error fix in 8ecec08 (3.10.1) is problematic 🤔

@sumneko

known facts

3.10.0 is correct
PR#2765 is released in 3.10.0
- => PR#2765 itself has no problem
- i.e. unrelated to PR#2765
from the provided additional note, this issue is introduced in 3.10.1
the only major logic change in 3.10.1 is the run time error fix in 8ecec08
diff: 3.10.0...3.10.1
- => the change there must be the culprit

the overly complicated fix in `8ecec08`

8ecec08 is the runtime error fix to #2776.
To be honest, now I don't know why @sumneko comes up with such a complicated fix 🙈
(I don't have enough knowledge in LuaLS's codebase at that time)

I reinvestigated #2776, the root cause seems to be that:

if an arg node in call.args is already compiled by the time of removing unmatched function signature
- this compiled arg node must exist afterwards
- i.e. simply clearing it by vm.removeNode() will lead to possible index nil value error in that callstack / callframe
So the simplest fix would be just recompiling it immediately after removing 🤔

the fix

replace the logic here:

lua-language-server/script/vm/compiler.lua

Lines 643 to 660 in 64c7084

    
           if call.args then 
        
               -- clear existing node caches of args to allow recomputation with the type narrowed call 
        
               for _, arg in ipairs(call.args) do 
        
                   if vm.getNode(arg) then 
        
                       vm.setNode(arg, vm.createNode(), true) 
        
                   end 
        
               end 
        
               for n in newNode:eachObject() do 
        
                   if n.type == 'function' 
        
                   or n.type == 'doc.type.function' then 
        
                       for i, arg in ipairs(call.args) do 
        
                           if vm.getNode(arg) and n.args[i] then 
        
                               vm.setNode(arg, vm.compileNode(n.args[i])) 
        
                           end 
        
                       end 
        
                   end 
        
               end 
        
           end

with:

        if call.args then
            -- recompile existing node caches of args to allow recomputation with the type narrowed call
            for _, arg in ipairs(call.args) do
                if vm.getNode(arg) then
                    vm.removeNode(arg)
                    vm.compileNode(arg)
                end
            end
        end

This seems passed all existing tests, as well as the reproduction snippet in your example 😄
I will try to open a PR soon.

Gonna be honest, I might have confused 2765 with another PR, that's my bad. I did actually look into what was causing this but LuaLS is a complex beast and I couldn't find anything that would lead to the solution, so I'm glad you found something. Thanks for looking into this so quickly.

Navigation Menu

Repository navigation

Overloaded methods do not properly return a generic bound to the self parameter #3087

How are you using the lua-language-server?

Which OS are you using?

What is the issue affecting?

Expected Behaviour

Actual Behaviour

Reproduction steps

Additional Notes

Log File

PR #2765

run time error in #2776

The regression fix

known facts

the overly complicated fix in `8ecec08`

the fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if call.args then
	-- clear existing node caches of args to allow recomputation with the type narrowed call
	for _, arg in ipairs(call.args) do
	if vm.getNode(arg) then
	vm.setNode(arg, vm.createNode(), true)
	end
	end
	for n in newNode:eachObject() do
	if n.type == 'function'
	or n.type == 'doc.type.function' then
	for i, arg in ipairs(call.args) do
	if vm.getNode(arg) and n.args[i] then
	vm.setNode(arg, vm.compileNode(n.args[i]))
	end
	end
	end
	end
	end

Overloaded methods do not properly return a generic bound to the self parameter #3087

Description

How are you using the lua-language-server?

Which OS are you using?

What is the issue affecting?

Expected Behaviour

Actual Behaviour

Reproduction steps

Additional Notes

Log File

Activity

tomlau10 commented on Feb 23, 2025

PR #2765

run time error in #2776

The regression fix

tomlau10 commented on Feb 23, 2025

known facts

the overly complicated fix in 8ecec08

the fix

GrandpaScout commented on Feb 23, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

the overly complicated fix in `8ecec08`